From 61ce38d63d6bf9efb9a3eb10e2335dda05171f97 Mon Sep 17 00:00:00 2001 From: Aditya Sanjeev Date: Fri, 13 Nov 2026 14:27:55 -0800 Subject: [PATCH] Say when the model is busy on Refine, and free the model slot when the tab closes Refine with model failed with a generic "Refining failed. Try again." when the model server was full: the request layer words that as "Server is busy (too many requests)..." (HTTP 429) or "Server error (5xx)...", neither of which the Refine flyout recognised, and the "model is busy" text it did list is produced nowhere. It now says the model is busy, or unavailable, so a person knows to wait. The viewer released its model-server lease only when the component unmounted or the case, structure or resolution changed. Closing the tab or leaving the site never runs React cleanup, so the lease stayed held until the idle timeout and every other prompt or Refine was refused as busy. It now also releases on pagehide (the release call already uses keepalive for exactly this). --- .../components/segmentation/RefineFlyout.tsx | 13 ++++- .../viewer/useInteractivePromptTool.test.tsx | 20 ++++++++ .../viewer/useInteractivePromptTool.ts | 12 ++++- .../src/test/refineFlyoutErrors.test.tsx | 51 +++++++++++++++++++ 4 files changed, 94 insertions(+), 2 deletions(-) create mode 100644 PanTS-Demo/src/test/refineFlyoutErrors.test.tsx 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."); + }); +});