diff --git a/PanTS-Demo/src/components/segmentation/RefineFlyout.tsx b/PanTS-Demo/src/components/segmentation/RefineFlyout.tsx index 48944ea8..2b505d39 100644 --- a/PanTS-Demo/src/components/segmentation/RefineFlyout.tsx +++ b/PanTS-Demo/src/components/segmentation/RefineFlyout.tsx @@ -21,9 +21,20 @@ const FRIENDLY_ERRORS = new Set([ "The interactive model is busy. Wait a moment and try again.", ]); +const BUSY_MESSAGE = "The interactive model is busy. Wait a moment and try again."; +const UNAVAILABLE_MESSAGE = "The interactive model is unavailable right now. Try again in a moment."; + +// The request layer words a full model server as "Server is busy (too many +// requests)..." (HTTP 429) and a stalled or down one as "Server error (5xx)...", +// neither of which is in FRIENDLY_ERRORS above, so both used to surface as a +// generic "Refining failed" with no hint that waiting would help. function refineErrorMessage(error: unknown): string { if (error instanceof TypeError) return "Could not reach the model server. Check your connection and try again."; - if (error instanceof Error && FRIENDLY_ERRORS.has(error.message)) return error.message; + if (error instanceof Error) { + if (FRIENDLY_ERRORS.has(error.message)) return error.message; + if (/\b(busy|at capacity|too many requests)\b/i.test(error.message)) return BUSY_MESSAGE; + if (/^Server error \(5\d\d\)/.test(error.message)) return UNAVAILABLE_MESSAGE; + } return "Refining failed. Try again."; } diff --git a/PanTS-Demo/src/helpers/viewer/useInteractivePromptTool.test.tsx b/PanTS-Demo/src/helpers/viewer/useInteractivePromptTool.test.tsx index 2d972e54..578e4bfd 100644 --- a/PanTS-Demo/src/helpers/viewer/useInteractivePromptTool.test.tsx +++ b/PanTS-Demo/src/helpers/viewer/useInteractivePromptTool.test.tsx @@ -203,6 +203,26 @@ describe("interactive prompt resource changes", () => { ); }); + it("releases the model session when the page is closed, and only once when it unmounts afterwards", async () => { + promptApi.releaseInteractivePromptSession.mockClear(); + const { unmount } = renderHook(() => useInteractivePromptTool({ + enabled: true, mode: "point", apiBase: "http://api.test", caseId: "case-a", + activeSegmentIndex: 12, res: "full", + })); + + // Closing a tab never unmounts the component, so React cleanup alone would leave + // the model server's only lease held until the idle timeout. + window.dispatchEvent(new Event("pagehide")); + expect(promptApi.releaseInteractivePromptSession).toHaveBeenCalledTimes(1); + expect(promptApi.releaseInteractivePromptSession).toHaveBeenCalledWith("http://api.test", "case-a", 12, "full"); + + unmount(); + // The unmount still releases (the real helper finds no token the second time and does nothing). + expect(promptApi.releaseInteractivePromptSession).toHaveBeenCalledTimes(2); + window.dispatchEvent(new Event("pagehide")); + expect(promptApi.releaseInteractivePromptSession).toHaveBeenCalledTimes(2); + }); + it("identifies a region-grow fallback as not produced by nnInteractive", async () => { promptApi.submitInteractiveSegmentPrompt.mockResolvedValue({ changed: 8, modelFallback: true, sessionActive: false, diff --git a/PanTS-Demo/src/helpers/viewer/useInteractivePromptTool.ts b/PanTS-Demo/src/helpers/viewer/useInteractivePromptTool.ts index 844904e5..94bf6410 100644 --- a/PanTS-Demo/src/helpers/viewer/useInteractivePromptTool.ts +++ b/PanTS-Demo/src/helpers/viewer/useInteractivePromptTool.ts @@ -141,11 +141,21 @@ export function useInteractivePromptTool({ // The four prompt-mode hooks are mounted together. Their shared session is // released only when its case, target label, or resolution is abandoned; // changing between point, box, lasso, and scribble keeps the same context. + // + // Closing the tab or leaving the site never runs this cleanup, and the model + // server may allow only one lease at a time, so an abandoned tab would keep + // every other user's prompts and Refine "busy" until the idle timeout. Release + // on `pagehide` too (releasing twice is harmless: the second call finds no token). useEffect(() => { if (caseId == null || activeSegmentIndex == null) return; - return () => { + const release = () => { void releaseInteractivePromptSession(apiBase, caseId, activeSegmentIndex, res); }; + window.addEventListener("pagehide", release); + return () => { + window.removeEventListener("pagehide", release); + release(); + }; }, [apiBase, caseId, activeSegmentIndex, res]); const executeSubmit = useCallback(async (_pane: CinePane, payload: any) => { diff --git a/PanTS-Demo/src/test/refineFlyoutErrors.test.tsx b/PanTS-Demo/src/test/refineFlyoutErrors.test.tsx new file mode 100644 index 00000000..a57bf468 --- /dev/null +++ b/PanTS-Demo/src/test/refineFlyoutErrors.test.tsx @@ -0,0 +1,51 @@ +/** + * What a person sees when "Refine with model" fails. The request layer words a full + * model server as HTTP 429 ("Server is busy...") and a down or stalled one as + * "Server error (5xx)...", and neither used to match the flyout's known messages, so + * every one of them read as a generic "Refining failed" with no hint to wait. + */ +import { fireEvent, render, screen } from "@testing-library/react"; +import { describe, expect, it, vi } from "vitest"; +import RefineFlyout from "../components/segmentation/RefineFlyout"; + +const BUSY = "The interactive model is busy. Wait a moment and try again."; +const UNAVAILABLE = "The interactive model is unavailable right now. Try again in a moment."; + +async function failWith(error: unknown) { + const refine = vi.fn().mockRejectedValue(error); + render(); + fireEvent.click(screen.getByRole("button", { name: /Refine Liver/ })); + return (await screen.findByRole("alert")).textContent; +} + +describe("RefineFlyout failure messages", () => { + it.each([ + ["a 429 from the request layer", "Server is busy (too many requests). Please wait a moment and try again."], + ["the backend's capacity wording", "Interactive segmentation is at capacity - please retry in a few seconds."], + ["a stalled model server", "Interactive segmentation timed out - the server is busy. Please retry."], + ])("says the model is busy for %s", async (_name, message) => { + expect(await failWith(new Error(message))).toBe(BUSY); + }); + + it.each([ + ["503", "Server error (503). Please try again later. If this keeps happening, try reloading or contact support."], + ["502", "Server error (502). Please try again later. If this keeps happening, try reloading or contact support."], + ])("says the model is unavailable for a %s", async (_code, message) => { + expect(await failWith(new Error(message))).toBe(UNAVAILABLE); + }); + + it("keeps the specific messages it already knew", async () => { + const known = "This class has no voxels to refine yet. Draw or segment it first."; + expect(await failWith(new Error(known))).toBe(known); + }); + + it("keeps the network wording for a failed fetch", async () => { + expect(await failWith(new TypeError("Failed to fetch"))).toBe( + "Could not reach the model server. Check your connection and try again.", + ); + }); + + it("still falls back to the generic message for anything else", async () => { + expect(await failWith(new Error("Some unexpected failure"))).toBe("Refining failed. Try again."); + }); +});