From f3a5e626f8233d500efd48b7a4f8afe30fa7dd33 Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya <1445792+JArmandoAnaya@users.noreply.github.com> Date: Wed, 16 Sep 2026 23:59:46 -0700 Subject: [PATCH 01/42] feat(ui-core): add browser asset-source, acquisition and active-target types --- frontend/ui-core/src/index.ts | 3 ++ .../ui-core/src/inference/browserPort.test.ts | 23 ++++++++++ frontend/ui-core/src/inference/browserPort.ts | 43 +++++++++++++++++++ 3 files changed, 69 insertions(+) create mode 100644 frontend/ui-core/src/inference/browserPort.test.ts diff --git a/frontend/ui-core/src/index.ts b/frontend/ui-core/src/index.ts index 277ffa0d..abc11b12 100644 --- a/frontend/ui-core/src/index.ts +++ b/frontend/ui-core/src/index.ts @@ -123,6 +123,9 @@ export { type VisionSetBrowserInferenceProviderProps, } from "./inference/VisionSetBrowserInferenceProvider.js"; export type { + ActiveSuggestionTarget, + BrowserModelAcquisition, + BrowserSuggestionAssetSource, BrowserSuggestionTarget, VisionSetBrowserInferenceRuntime, } from "./inference/browserPort.js"; diff --git a/frontend/ui-core/src/inference/browserPort.test.ts b/frontend/ui-core/src/inference/browserPort.test.ts new file mode 100644 index 00000000..f53095c1 --- /dev/null +++ b/frontend/ui-core/src/inference/browserPort.test.ts @@ -0,0 +1,23 @@ +import { describe, expect, it } from "vitest"; +import type { VisionSetBrowserInferenceRuntime } from "./browserPort.js"; + +describe("VisionSetBrowserInferenceRuntime", () => { + it("is satisfied by a runtime that implements only the two original members", () => { + const runtime: VisionSetBrowserInferenceRuntime = { + listTargets: async () => [], + executorFor: () => ({ suggest: async () => { throw new Error("unused"); } }), + }; + expect(runtime.listAcquisitions).toBeUndefined(); + expect(runtime.setActiveAsset).toBeUndefined(); + }); + + it("accepts a runtime that also implements listAcquisitions and setActiveAsset", () => { + const runtime: VisionSetBrowserInferenceRuntime = { + listTargets: async () => [], + executorFor: () => ({ suggest: async () => { throw new Error("unused"); } }), + listAcquisitions: () => [{ id: "m", label: "Model", approxBytes: 10, acquire: async () => {} }], + setActiveAsset: () => {}, + }; + expect(runtime.listAcquisitions?.()).toHaveLength(1); + }); +}); diff --git a/frontend/ui-core/src/inference/browserPort.ts b/frontend/ui-core/src/inference/browserPort.ts index 1026891d..b850fee3 100644 --- a/frontend/ui-core/src/inference/browserPort.ts +++ b/frontend/ui-core/src/inference/browserPort.ts @@ -20,6 +20,7 @@ * cannot answer yet, and it is answered elsewhere, later, by whatever seam the work that needs * it earns. */ +import type { RgbPixels } from "@visionset/annotator"; import type { SuggestionExecutor } from "./suggestionExecutor.js"; /** A model this browser can run a suggestion with, right now. */ @@ -38,6 +39,44 @@ export interface BrowserSuggestionTarget { readonly modelRef: string; } +/** + * A lease on the currently displayed asset's pixels, host-built from `AnnotatorCanvas`'s + * `onImageReady`. Never persisted — a browser executor reads it once per `suggest()` call + * and must re-check `assetId` against its own request before trusting anything cached + * against it, because the active asset can change while a `readRgb`/`prepareImage` is + * still in flight. + */ +export interface BrowserSuggestionAssetSource { + readonly assetId: string; + readonly width: number; + readonly height: number; + readRgb(): RgbPixels; +} + +/** + * One model this browser could run, once its bytes are fetched and verified. + * + * Deliberately not reactive — no `getState()`/`subscribe()`. Phase F ships exactly one + * acquirable model; the transient idle/acquiring/failed state around `acquire()` is the + * UI's own, not this port's. + */ +export interface BrowserModelAcquisition { + readonly id: string; + readonly label: string; + readonly approxBytes: number; + acquire(options?: { readonly signal?: AbortSignal }): Promise; +} + +/** + * Which kind of thing answers a suggestion now: a workspace connection, or a browser + * target. Never persisted as an `InferenceConnection` — a browser target answers + * questions a connection cannot ("can this browser run something now"), and the two + * are not interchangeable rows of the same table. + */ +export type ActiveSuggestionTarget = + | { readonly kind: "server"; readonly connectionId: string } + | { readonly kind: "browser"; readonly targetId: string }; + export interface VisionSetBrowserInferenceRuntime { /** * The targets that can answer *now*. @@ -49,4 +88,8 @@ export interface VisionSetBrowserInferenceRuntime { listTargets(): Promise; /** How to ask one of them. The same contract the server path answers through. */ executorFor(targetId: string): SuggestionExecutor; + /** Models not yet ready, each with its own explicit `acquire()`. Absent hosts offer none. */ + listAcquisitions?(): readonly BrowserModelAcquisition[]; + /** The displayed asset, or `null` between assets. Feeds the executor's race-safety checks. */ + setActiveAsset?(source: BrowserSuggestionAssetSource | null): void; } From 5c7d6999097595103d88969ef6800b6a0315867d Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya <1445792+JArmandoAnaya@users.noreply.github.com> Date: Thu, 17 Sep 2026 00:02:26 -0700 Subject: [PATCH 02/42] feat(ui-core): compute suggest blockers per active target, not per connection --- frontend/ui-core/src/index.ts | 1 + .../src/inference/targetBlocker.test.ts | 27 +++++++++++++++++++ .../ui-core/src/inference/targetBlocker.ts | 17 ++++++++++++ 3 files changed, 45 insertions(+) create mode 100644 frontend/ui-core/src/inference/targetBlocker.test.ts create mode 100644 frontend/ui-core/src/inference/targetBlocker.ts diff --git a/frontend/ui-core/src/index.ts b/frontend/ui-core/src/index.ts index abc11b12..fd9d1f56 100644 --- a/frontend/ui-core/src/index.ts +++ b/frontend/ui-core/src/index.ts @@ -129,6 +129,7 @@ export type { BrowserSuggestionTarget, VisionSetBrowserInferenceRuntime, } from "./inference/browserPort.js"; +export { computeSuggestBlocker } from "./inference/targetBlocker.js"; export { useServerSuggestionExecutor, type SuggestionExecutor, diff --git a/frontend/ui-core/src/inference/targetBlocker.test.ts b/frontend/ui-core/src/inference/targetBlocker.test.ts new file mode 100644 index 00000000..7ed29a16 --- /dev/null +++ b/frontend/ui-core/src/inference/targetBlocker.test.ts @@ -0,0 +1,27 @@ +import { describe, expect, it } from "vitest"; +import { computeSuggestBlocker } from "./targetBlocker.js"; +import type { BrowserSuggestionTarget } from "./browserPort.js"; + +const READY_TARGET: BrowserSuggestionTarget = { id: "efficient-sam-ti", label: "EfficientSAM-Ti", modelRef: "x" }; + +describe("computeSuggestBlocker", () => { + it("passes the server blocker through unchanged when the server is the active target", () => { + expect(computeSuggestBlocker({ kind: "server", connectionId: "c1" }, "no-connections", undefined)).toBe( + "no-connections", + ); + expect(computeSuggestBlocker({ kind: "server", connectionId: "c1" }, null, [READY_TARGET])).toBeNull(); + }); + + it("is answerable when a ready browser target is active, regardless of server state", () => { + expect(computeSuggestBlocker({ kind: "browser", targetId: "efficient-sam-ti" }, "no-connections", [READY_TARGET])).toBeNull(); + expect(computeSuggestBlocker({ kind: "browser", targetId: "efficient-sam-ti" }, "not-ready", [READY_TARGET])).toBeNull(); + }); + + it("is 'checking' while the browser target list has not loaded yet", () => { + expect(computeSuggestBlocker({ kind: "browser", targetId: "efficient-sam-ti" }, null, undefined)).toBe("checking"); + }); + + it("is 'not-ready' when the browser target list loaded but does not contain the active target", () => { + expect(computeSuggestBlocker({ kind: "browser", targetId: "efficient-sam-ti" }, null, [])).toBe("not-ready"); + }); +}); diff --git a/frontend/ui-core/src/inference/targetBlocker.ts b/frontend/ui-core/src/inference/targetBlocker.ts new file mode 100644 index 00000000..e74ae3d1 --- /dev/null +++ b/frontend/ui-core/src/inference/targetBlocker.ts @@ -0,0 +1,17 @@ +import type { SuggestBlocker } from "../data/inferenceQueries.js"; +import type { ActiveSuggestionTarget, BrowserSuggestionTarget } from "./browserPort.js"; + +/** + * `SuggestBlocker` is server-connection vocabulary. A ready browser target is answerable + * on its own terms, so it must never inherit "no-connections"/"not-ready"/"not-capable" + * from a server that the active target isn't even asking. + */ +export function computeSuggestBlocker( + target: ActiveSuggestionTarget, + serverBlocker: SuggestBlocker | null | undefined, + browserTargets: readonly BrowserSuggestionTarget[] | undefined, +): SuggestBlocker | null | undefined { + if (target.kind === "server") return serverBlocker; + if (browserTargets === undefined) return "checking"; + return browserTargets.some((row) => row.id === target.targetId) ? null : "not-ready"; +} From d957ca83fed71a7b159e5d9818dda77db478ae36 Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya <1445792+JArmandoAnaya@users.noreply.github.com> Date: Thu, 17 Sep 2026 00:04:11 -0700 Subject: [PATCH 03/42] feat(ui-core): add refusal prose for browser negative-point requests --- frontend/ui-core/src/data/refusals.test.ts | 14 ++++++++++++++ frontend/ui-core/src/data/refusals.ts | 6 ++++++ 2 files changed, 20 insertions(+) create mode 100644 frontend/ui-core/src/data/refusals.test.ts diff --git a/frontend/ui-core/src/data/refusals.test.ts b/frontend/ui-core/src/data/refusals.test.ts new file mode 100644 index 00000000..96a7dd42 --- /dev/null +++ b/frontend/ui-core/src/data/refusals.test.ts @@ -0,0 +1,14 @@ +import { describe, expect, it } from "vitest"; +import { ApiError } from "./errors.js"; +import { refusalProse } from "./refusals.js"; + +describe("refusalProse — browser target refusals", () => { + it("explains that this device is positive-point only, without inventing a server-error sentence", () => { + const error = new ApiError({ + code: "BROWSER_NEGATIVE_POINTS_UNSUPPORTED", + message: "unused — REFUSAL_PROSE wins", + }); + expect(refusalProse(error)).toMatch(/positive-point/i); + expect(refusalProse(error)).not.toMatch(/could not be reached/i); + }); +}); diff --git a/frontend/ui-core/src/data/refusals.ts b/frontend/ui-core/src/data/refusals.ts index 536a2b10..337fb9ec 100644 --- a/frontend/ui-core/src/data/refusals.ts +++ b/frontend/ui-core/src/data/refusals.ts @@ -209,6 +209,12 @@ export const REFUSAL_PROSE: Record = { // this" would be wrong about where the problem is. NETWORK_ERROR: "The server could not be reached — check the connection and try again.", MALFORMED_RESPONSE: "The server answered with something this app does not recognise.", + + // Browser-local suggestion. EfficientSAM-Ti has no true negative point; this is the + // executor's own pre-flight refusal, never the model's internal throw, so the message + // a person sees is contract-tested at this boundary rather than assumed to propagate. + BROWSER_NEGATIVE_POINTS_UNSUPPORTED: + "This device supports positive-point refinement only. Choose Server to add a negative point.", }; /** From ee5d0b92f70cb011dd71888cd4a9aa28672e04f6 Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya <1445792+JArmandoAnaya@users.noreply.github.com> Date: Thu, 17 Sep 2026 00:10:20 -0700 Subject: [PATCH 04/42] feat(ui-core): feed the displayed asset to the browser inference runtime --- .../ui-core/src/annotator/AnnotationPage.tsx | 19 +++++++++++++ .../src/inference/browserRuntime.test.tsx | 27 ++++++++++++++++++- 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/frontend/ui-core/src/annotator/AnnotationPage.tsx b/frontend/ui-core/src/annotator/AnnotationPage.tsx index bc4f9230..616dbbec 100644 --- a/frontend/ui-core/src/annotator/AnnotationPage.tsx +++ b/frontend/ui-core/src/annotator/AnnotationPage.tsx @@ -188,6 +188,8 @@ import { SuggestPanel } from "./SuggestPanel"; import { useConnections, usableConnection } from "../data/inferenceQueries"; import type { SuggestionOut } from "../data/inferenceQueries"; import { useServerSuggestionExecutor } from "../inference/suggestionExecutor"; +import { useBrowserInferenceRuntime } from "../inference/VisionSetBrowserInferenceProvider.js"; +import type { BrowserSuggestionAssetSource } from "../inference/browserPort.js"; import { readPref, writePref } from "../data/prefs"; /** @@ -923,6 +925,12 @@ function Workspace({ */ const [adjusting, setAdjusting] = useState(false); + const browserRuntime = useBrowserInferenceRuntime(); + + useEffect(() => { + return () => browserRuntime?.setActiveAsset?.(null); + }, [browserRuntime]); + /** * The connection list, fetched **only once the tool is armed**. * @@ -2549,6 +2557,17 @@ function Workspace({ // its own and lose it on every navigation. clipboard={clipboard} onHostAction={hostAction} + onImageReady={(image) => { + const width = image.image.naturalWidth; + const height = image.image.naturalHeight; + const source: BrowserSuggestionAssetSource = { + assetId: asset.id, + width, + height, + readRgb: () => image.readRgb(width, height), + }; + browserRuntime?.setActiveAsset?.(source); + }} // A right-click on a shape: select it, then open its class // picker over it. Selecting is what makes the picker's // subject unambiguous — it anchors to the selection, and a menu diff --git a/frontend/ui-core/src/inference/browserRuntime.test.tsx b/frontend/ui-core/src/inference/browserRuntime.test.tsx index 462dfdf0..a41cf4f8 100644 --- a/frontend/ui-core/src/inference/browserRuntime.test.tsx +++ b/frontend/ui-core/src/inference/browserRuntime.test.tsx @@ -8,7 +8,7 @@ import { useBrowserInferenceRuntime, VisionSetBrowserInferenceProvider, } from "./VisionSetBrowserInferenceProvider"; -import type { VisionSetBrowserInferenceRuntime } from "./browserPort"; +import type { BrowserSuggestionAssetSource, VisionSetBrowserInferenceRuntime } from "./browserPort"; import { clearPrefs } from "../data/prefs"; import { AnnotationPage } from "../annotator/AnnotationPage"; import { TooltipProvider } from "@robomous/ui-core"; @@ -294,3 +294,28 @@ describe("an injected browser runtime changes nothing on the wire", () => { expect(withRuntime).toEqual(withoutRuntime); }); }); + +describe("BrowserSuggestionAssetSource", () => { + it("hands the browser runtime a BrowserSuggestionAssetSource once the asset image loads", async () => { + const setActiveAsset = vi.fn(); + const runtime: VisionSetBrowserInferenceRuntime = { + listTargets: async () => [], + executorFor: () => ({ + suggest: async () => { + throw new Error("unused"); + }, + }), + setActiveAsset, + }; + + await open(runtime); + + const image = screen.getByTestId("annotator-image"); + fireEvent.load(image); + + await waitFor(() => expect(setActiveAsset).toHaveBeenCalledTimes(1)); + const [source] = setActiveAsset.mock.calls[0] as [BrowserSuggestionAssetSource]; + expect(source.assetId).toBe(ASSET); + expect(typeof source.readRgb).toBe("function"); + }); +}); From 7fa1d27bea4fc08290c30337947fb52adcedf874 Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya <1445792+JArmandoAnaya@users.noreply.github.com> Date: Thu, 17 Sep 2026 00:19:50 -0700 Subject: [PATCH 05/42] feat(ui-core): route suggestions through the active target, server or browser --- .../ui-core/src/annotator/AnnotationPage.tsx | 73 +++++++++++++++++-- .../src/inference/browserRuntime.test.tsx | 42 ++++++++++- 2 files changed, 109 insertions(+), 6 deletions(-) diff --git a/frontend/ui-core/src/annotator/AnnotationPage.tsx b/frontend/ui-core/src/annotator/AnnotationPage.tsx index 616dbbec..febf0d4f 100644 --- a/frontend/ui-core/src/annotator/AnnotationPage.tsx +++ b/frontend/ui-core/src/annotator/AnnotationPage.tsx @@ -189,7 +189,8 @@ import { useConnections, usableConnection } from "../data/inferenceQueries"; import type { SuggestionOut } from "../data/inferenceQueries"; import { useServerSuggestionExecutor } from "../inference/suggestionExecutor"; import { useBrowserInferenceRuntime } from "../inference/VisionSetBrowserInferenceProvider.js"; -import type { BrowserSuggestionAssetSource } from "../inference/browserPort.js"; +import type { ActiveSuggestionTarget, BrowserSuggestionAssetSource, BrowserSuggestionTarget } from "../inference/browserPort.js"; +import { computeSuggestBlocker } from "../inference/targetBlocker.js"; import { readPref, writePref } from "../data/prefs"; /** @@ -204,6 +205,26 @@ function preferredConnectionKey(projectId: string): string { return `suggest.connection.${projectId}`; } +/** Where a project's browser-vs-server suggest target is remembered — separate from, + * and never overwriting, `preferredConnectionKey`'s server-model preference. */ +function suggestTargetKey(projectId: string): string { + return `suggest.target.${projectId}`; +} + +type StoredSuggestTarget = { readonly kind: "server" } | { readonly kind: "browser"; readonly targetId: string }; + +function readStoredSuggestTarget(projectId: string): StoredSuggestTarget { + const raw = readPref(suggestTargetKey(projectId)); + if (raw !== null && raw.startsWith("browser:")) { + return { kind: "browser", targetId: raw.slice("browser:".length) }; + } + return { kind: "server" }; +} + +function writeStoredSuggestTarget(projectId: string, target: ActiveSuggestionTarget): void { + writePref(suggestTargetKey(projectId), target.kind === "browser" ? `browser:${target.targetId}` : "server"); +} + /** * Where "a trackpad has been seen on this browser" is remembered. * @@ -958,13 +979,50 @@ function Workspace({ const [preferredConnection, setPreferredConnection] = useState(() => readPref(preferredConnectionKey(projectId)), ); - const { connection, candidates, blocker } = usableConnection( + const { connection, candidates, blocker: serverBlocker } = usableConnection( connections.data?.items, preferredConnection, ); - // Server-only today, and `null` is how "nowhere to send this" arrives — the same fact - // `usableConnection`'s blocker states, which is what the panel renders. - const executor = useServerSuggestionExecutor(connection?.id ?? null); + const serverExecutor = useServerSuggestionExecutor(connection?.id ?? null); + + const [browserTargetsRefreshKey, setBrowserTargetsRefreshKey] = useState(0); + const [browserTargets, setBrowserTargets] = useState(undefined); + useEffect(() => { + if (browserRuntime === null) { + setBrowserTargets([]); + return; + } + let cancelled = false; + setBrowserTargets(undefined); + void browserRuntime.listTargets().then((targets) => { + if (!cancelled) setBrowserTargets(targets); + }); + return () => { + cancelled = true; + }; + }, [browserRuntime, browserTargetsRefreshKey]); + + const [storedTarget, setStoredTarget] = useState(() => readStoredSuggestTarget(projectId)); + // A stale/unavailable stored preference (no runtime wired, or the stored target id isn't + // ready) falls back to Server silently — the safely-fallback-able case. An *explicit* + // choice that later fails is a different thing (surfaced via `blocker`/`refusal`, never + // auto-switched), which is why this fallback lives only here, at read time. + const activeTarget: ActiveSuggestionTarget = + browserRuntime !== null && storedTarget.kind === "browser" + ? { kind: "browser", targetId: storedTarget.targetId } + : { kind: "server", connectionId: connection?.id ?? "" }; + + const blocker = computeSuggestBlocker(activeTarget, serverBlocker, browserTargets); + const executor = + activeTarget.kind === "browser" && browserRuntime !== null + ? browserRuntime.executorFor(activeTarget.targetId) + : serverExecutor; + + function chooseTarget(target: ActiveSuggestionTarget): void { + setStoredTarget(target.kind === "browser" ? { kind: "browser", targetId: target.targetId } : { kind: "server" }); + writeStoredSuggestTarget(projectId, target); + if (target.kind === "server") setPreferredConnection(connection?.id ?? preferredConnection); + } /** * One clock over the wait, read by the canvas and by the panel alike. @@ -2773,6 +2831,11 @@ function Workspace({ // matching while parked — the one reading that has to name it. heldClass={activeClass} blocker={blocker} + browserTargets={browserRuntime === null ? undefined : browserTargets} + browserAcquisitions={browserRuntime?.listAcquisitions?.()} + activeTarget={activeTarget} + onChooseTarget={chooseTarget} + onAcquired={() => setBrowserTargetsRefreshKey((key) => key + 1)} refusal={suggesting.refusal} candidates={candidates} connectionId={connection?.id ?? null} diff --git a/frontend/ui-core/src/inference/browserRuntime.test.tsx b/frontend/ui-core/src/inference/browserRuntime.test.tsx index a41cf4f8..161c4552 100644 --- a/frontend/ui-core/src/inference/browserRuntime.test.tsx +++ b/frontend/ui-core/src/inference/browserRuntime.test.tsx @@ -9,7 +9,7 @@ import { VisionSetBrowserInferenceProvider, } from "./VisionSetBrowserInferenceProvider"; import type { BrowserSuggestionAssetSource, VisionSetBrowserInferenceRuntime } from "./browserPort"; -import { clearPrefs } from "../data/prefs"; +import { clearPrefs, writePref } from "../data/prefs"; import { AnnotationPage } from "../annotator/AnnotationPage"; import { TooltipProvider } from "@robomous/ui-core"; import { renderWithData } from "../testing/dataHarness"; @@ -295,6 +295,46 @@ describe("an injected browser runtime changes nothing on the wire", () => { }); }); +describe("target selection routes a ready browser target around the server", () => { + it("a ready browser target answers suggestions even with no server connections", async () => { + connections = []; + writePref(`suggest.target.${PROJECT}`, "browser:efficient-sam-ti"); + + const browserSuggest = vi.fn().mockResolvedValue({ + model_ref: "efficient-sam-ti@rev", + confidence: 0.9, + regions: [{ geometry: { type: "polygon", points: [[0, 0], [1, 0], [1, 1]] }, contour: [] }], + applied: { tolerance: 1 }, + parameters: ["tolerance"], + }); + const runtime: VisionSetBrowserInferenceRuntime = { + listTargets: async () => [ + { id: "efficient-sam-ti", label: "EfficientSAM-Ti", modelRef: "efficient-sam-ti@rev" }, + ], + executorFor: (id) => { + expect(id).toBe("efficient-sam-ti"); + return { suggest: browserSuggest }; + }, + }; + + const unmount = await open(runtime); + await arm(); + + // The blocker the panel renders must clear once the browser target reports + // ready, even though the server side has nothing — "no-connections" must + // never win once the active target isn't asking the server anything. + await screen.findByTestId("suggest-idle"); + expect(screen.queryByTestId("suggest-no-connections")).toBeNull(); + + clickCanvas(); + + await waitFor(() => expect(browserSuggest).toHaveBeenCalledTimes(1)); + expect(asks()).toHaveLength(0); + + unmount(); + }); +}); + describe("BrowserSuggestionAssetSource", () => { it("hands the browser runtime a BrowserSuggestionAssetSource once the asset image loads", async () => { const setActiveAsset = vi.fn(); From 911f94ff571d8226bbb970df650eafa2f5f19e90 Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya <1445792+JArmandoAnaya@users.noreply.github.com> Date: Thu, 17 Sep 2026 00:30:51 -0700 Subject: [PATCH 06/42] fix(ui-core): fall back to Server silently for a stale stored browser target A stored suggest-target preference naming a browser target that listTargets() no longer reports is the ordinary shape of every reload for someone who picked "This device" last time, since acquired model bytes are never persisted across page loads. It must revert to Server without a visible error. An explicit in-session choice that later drops out is different and must keep surfacing through blocker/refusal, never auto-switch back. --- .../ui-core/src/annotator/AnnotationPage.tsx | 49 +++++++++++++++-- .../src/inference/browserRuntime.test.tsx | 52 ++++++++++++++++++- 2 files changed, 96 insertions(+), 5 deletions(-) diff --git a/frontend/ui-core/src/annotator/AnnotationPage.tsx b/frontend/ui-core/src/annotator/AnnotationPage.tsx index febf0d4f..06c444f6 100644 --- a/frontend/ui-core/src/annotator/AnnotationPage.tsx +++ b/frontend/ui-core/src/annotator/AnnotationPage.tsx @@ -225,6 +225,28 @@ function writeStoredSuggestTarget(projectId: string, target: ActiveSuggestionTar writePref(suggestTargetKey(projectId), target.kind === "browser" ? `browser:${target.targetId}` : "server"); } +/** + * Whether a stored browser-target preference is stale enough to fall back to Server + * silently. + * + * Acquired model bytes are never persisted across page loads, so "the stored browser + * target isn't in `listTargets()`'s answer" is the ordinary shape of every reload for + * someone who previously picked a browser target — not a rare failure, and it must not + * surface as a `blocker`. `explicitlyChosen` is what keeps this from also catching a + * target this *session* picked and which later drops out: that one is pinned, and stays + * pinned to `not-ready`/`refusal` rather than silently reverting. + */ +export function staleStoredBrowserTarget( + storedTarget: StoredSuggestTarget, + browserTargets: readonly BrowserSuggestionTarget[] | undefined, + explicitlyChosen: boolean, +): boolean { + if (browserTargets === undefined) return false; + if (storedTarget.kind !== "browser") return false; + if (explicitlyChosen) return false; + return !browserTargets.some((row) => row.id === storedTarget.targetId); +} + /** * Where "a trackpad has been seen on this browser" is remembered. * @@ -1003,10 +1025,28 @@ function Workspace({ }, [browserRuntime, browserTargetsRefreshKey]); const [storedTarget, setStoredTarget] = useState(() => readStoredSuggestTarget(projectId)); - // A stale/unavailable stored preference (no runtime wired, or the stored target id isn't - // ready) falls back to Server silently — the safely-fallback-able case. An *explicit* - // choice that later fails is a different thing (surfaced via `blocker`/`refusal`, never - // auto-switched), which is why this fallback lives only here, at read time. + // Whether *this session* picked a browser target through `chooseTarget`, as opposed to one + // merely read back from a persisted preference. Acquired model bytes are never persisted + // across page loads (a later phase's work), so "the stored browser target isn't in + // `listTargets()`'s answer" is the ordinary shape of every reload for someone who picked + // "This device" last time — not a rare failure. That case must fall back to Server + // silently. A target this session explicitly chose and which later drops out is a + // different thing (surfaced via `blocker`/`refusal`, never auto-switched), and this ref is + // what tells the two apart. + const explicitlyChosenBrowser = useRef(false); + + // A stale/unavailable *stored* preference falls back to Server silently, once the browser + // target list has actually resolved enough to say the stored id isn't in it — not the + // in-memory `storedTarget` state read at mount, so it does not fire on a spurious first + // render before `listTargets()` has answered. It never rewrites the persisted + // `suggest.target.` key, so a later reload re-checks the same id once that + // model has actually been re-acquired. + useEffect(() => { + if (staleStoredBrowserTarget(storedTarget, browserTargets, explicitlyChosenBrowser.current)) { + setStoredTarget({ kind: "server" }); + } + }, [browserTargets, storedTarget]); + const activeTarget: ActiveSuggestionTarget = browserRuntime !== null && storedTarget.kind === "browser" ? { kind: "browser", targetId: storedTarget.targetId } @@ -1019,6 +1059,7 @@ function Workspace({ : serverExecutor; function chooseTarget(target: ActiveSuggestionTarget): void { + if (target.kind === "browser") explicitlyChosenBrowser.current = true; setStoredTarget(target.kind === "browser" ? { kind: "browser", targetId: target.targetId } : { kind: "server" }); writeStoredSuggestTarget(projectId, target); if (target.kind === "server") setPreferredConnection(connection?.id ?? preferredConnection); diff --git a/frontend/ui-core/src/inference/browserRuntime.test.tsx b/frontend/ui-core/src/inference/browserRuntime.test.tsx index 161c4552..497961ee 100644 --- a/frontend/ui-core/src/inference/browserRuntime.test.tsx +++ b/frontend/ui-core/src/inference/browserRuntime.test.tsx @@ -10,7 +10,7 @@ import { } from "./VisionSetBrowserInferenceProvider"; import type { BrowserSuggestionAssetSource, VisionSetBrowserInferenceRuntime } from "./browserPort"; import { clearPrefs, writePref } from "../data/prefs"; -import { AnnotationPage } from "../annotator/AnnotationPage"; +import { AnnotationPage, staleStoredBrowserTarget } from "../annotator/AnnotationPage"; import { TooltipProvider } from "@robomous/ui-core"; import { renderWithData } from "../testing/dataHarness"; import { stubResizeObserver } from "../testing/resizeObserver.js"; @@ -333,6 +333,56 @@ describe("target selection routes a ready browser target around the server", () unmount(); }); + + it("falls back to Server silently when the stored browser target isn't in a resolved list", async () => { + // Acquired model bytes are never persisted across a reload, so this — a stored + // preference naming a browser target `listTargets()` no longer reports — is the + // ordinary shape of every reload for someone who last picked "This device", not a + // rare failure. It must read as "no connections" (the server's own honest state), + // never as a browser "not-ready" the person never asked to see again. + connections = []; + writePref(`suggest.target.${PROJECT}`, "browser:gone-model"); + + const runtime: VisionSetBrowserInferenceRuntime = { + listTargets: async () => [], + executorFor: () => ({ + suggest: async () => { + throw new Error("unused"); + }, + }), + }; + + const unmount = await open(runtime); + await arm(); + + await screen.findByTestId("suggest-no-connections"); + expect(screen.queryByTestId("suggest-not-ready")).toBeNull(); + + unmount(); + }); +}); + +describe("staleStoredBrowserTarget", () => { + const listed = [{ id: "t1", label: "T1", modelRef: "m@rev" }]; + + it("flags a stored (non-explicit) browser preference missing from a resolved list", () => { + expect(staleStoredBrowserTarget({ kind: "browser", targetId: "gone" }, listed, false)).toBe(true); + }); + + it("does not flag a stored preference that is in the resolved list", () => { + expect(staleStoredBrowserTarget({ kind: "browser", targetId: "t1" }, listed, false)).toBe(false); + }); + + it("does not flag a server preference, or a list still resolving", () => { + expect(staleStoredBrowserTarget({ kind: "server" }, [], false)).toBe(false); + expect(staleStoredBrowserTarget({ kind: "browser", targetId: "gone" }, undefined, false)).toBe(false); + }); + + it("never flags a target this session explicitly chose, even once it drops out of the list", () => { + // This is the other half of the same rule: an explicit in-session choice that later + // fails must keep surfacing through `blocker`/`refusal`, not silently revert. + expect(staleStoredBrowserTarget({ kind: "browser", targetId: "gone" }, [], true)).toBe(false); + }); }); describe("BrowserSuggestionAssetSource", () => { From 8839e066dde5848760d95a8c791a30900ecd91e6 Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya <1445792+JArmandoAnaya@users.noreply.github.com> Date: Thu, 17 Sep 2026 00:40:55 -0700 Subject: [PATCH 07/42] feat(ui-core): add a This device section to the suggest chooser --- .../ui-core/src/annotator/SuggestPanel.tsx | 201 +++++++++++++++++- .../src/annotator/suggestPanel.test.tsx | 124 ++++++++++- 2 files changed, 319 insertions(+), 6 deletions(-) diff --git a/frontend/ui-core/src/annotator/SuggestPanel.tsx b/frontend/ui-core/src/annotator/SuggestPanel.tsx index 0606b4fe..dce6fe48 100644 --- a/frontend/ui-core/src/annotator/SuggestPanel.tsx +++ b/frontend/ui-core/src/annotator/SuggestPanel.tsx @@ -64,11 +64,28 @@ import { type SuggestionState, } from "@visionset/annotator"; import { Check, Loader2, Sparkles, TriangleAlert, X } from "lucide-react"; -import type { JSX, ReactNode } from "react"; +import { useState, type JSX, type ReactNode } from "react"; import { EditorNotice } from "./EditorNotice"; -import { Button, Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "@robomous/ui-core"; +import { + Badge, + Button, + Select, + SelectContent, + SelectItem, + SelectTrigger, + SelectValue, + Tabs, + TabsContent, + TabsList, + TabsTrigger, +} from "@robomous/ui-core"; import type { Connection, SuggestBlocker } from "../data/inferenceQueries"; +import type { + ActiveSuggestionTarget, + BrowserModelAcquisition, + BrowserSuggestionTarget, +} from "../inference/browserPort.js"; export interface SuggestPanelProps { /** The session, whose status decides which sentence this card carries. */ @@ -113,6 +130,14 @@ export interface SuggestPanelProps { readonly onChooseConnection?: (connectionId: string) => void; /** Where a person goes to make or finish a connection, if the host has one. */ readonly onConfigure?: () => void; + /** Browser targets ready right now. `undefined` when no browser runtime is wired at all. */ + readonly browserTargets?: readonly BrowserSuggestionTarget[]; + /** Models not yet acquired. `undefined` when no browser runtime is wired at all. */ + readonly browserAcquisitions?: readonly BrowserModelAcquisition[]; + readonly activeTarget?: ActiveSuggestionTarget; + readonly onChooseTarget?: (target: ActiveSuggestionTarget) => void; + /** Called once an `acquire()` this panel started resolves, so the host can re-read `listTargets()`. */ + readonly onAcquired?: () => void; readonly onAccept: () => void; readonly onDiscard: () => void; /** Whether the adjustments are open. Owned by the host, because `Esc` layers on it. */ @@ -195,6 +220,11 @@ export function SuggestPanel({ connectionId = null, onChooseConnection, onConfigure, + browserTargets, + browserAcquisitions, + activeTarget, + onChooseTarget, + onAcquired, onAccept, onDiscard, adjusting, @@ -411,14 +441,31 @@ export function SuggestPanel({ genuinely out. Stating the rule where it is enforced is what makes the branch ordering an implementation detail rather than the guarantee. */} - {!hasPending(session) && ( - + {!hasPending(session) && ( + + )} + {hasPending(session) && } + + ) : ( + )} - {hasPending(session) && } ); } @@ -480,6 +527,150 @@ function Through({ ); } +/** + * Server or this device, once a browser runtime is wired in at all. + * + * `Tabs` rather than a picker: the choice is binary and always available once a + * runtime exists, so a segmented control reads better than a `Select` built for + * an open-ended candidate list. The server tab holds exactly what rendered + * before this component existed — `Through`/`Discard`, untouched — so choosing + * "Server" is never a behavior change from the pre-Task-6 panel. + */ +function TargetChooser({ + candidates, + connectionId, + onChoose, + pending, + onDiscard, + browserTargets, + browserAcquisitions, + activeTarget, + onChooseTarget, + onAcquired, +}: { + readonly candidates: readonly Connection[]; + readonly connectionId: string | null; + readonly onChoose?: (connectionId: string) => void; + readonly pending: boolean; + readonly onDiscard: () => void; + readonly browserTargets: readonly BrowserSuggestionTarget[]; + readonly browserAcquisitions: readonly BrowserModelAcquisition[]; + readonly activeTarget: ActiveSuggestionTarget | undefined; + readonly onChooseTarget?: (target: ActiveSuggestionTarget) => void; + readonly onAcquired?: () => void; +}): JSX.Element { + const value = activeTarget?.kind ?? "server"; + + return ( + { + if (next === "server") { + onChooseTarget?.({ kind: "server", connectionId: connectionId ?? "" }); + return; + } + const targetId = browserTargets[0]?.id ?? browserAcquisitions[0]?.id; + if (targetId !== undefined) { + onChooseTarget?.({ kind: "browser", targetId }); + } + }} + > + + + Server + + + This device + + + + {!pending && ( + + )} + {pending && } + + + + + + ); +} + +/** + * What "this device" has to say: a ready target's name and a success pill, or + * an unacquired model's size and a download button. + * + * Phase F ships exactly one browser-suggestible model, so this never has to + * choose between several targets or several acquisitions — it reads the first + * of whichever list is non-empty. The acquiring/failed state is local and + * transient, on `BrowserModelAcquisition`'s own contract: `acquire()` is not + * reactive, so the UI owns the story of one attempt in flight. + */ +function DeviceTab({ + targets, + acquisitions, + onAcquired, +}: { + readonly targets: readonly BrowserSuggestionTarget[]; + readonly acquisitions: readonly BrowserModelAcquisition[]; + readonly onAcquired?: () => void; +}): JSX.Element | null { + const [acquiring, setAcquiring] = useState(false); + const [failed, setFailed] = useState(false); + + const target = targets[0]; + if (target !== undefined) { + return ( +

+ {target.label} + Ready +

+ ); + } + + const acquisition = acquisitions[0]; + if (acquisition === undefined) return null; + + return ( +
+

+ {acquisition.label} — ~{Math.round(acquisition.approxBytes / 1_000_000)} MB +

+ + {failed &&

Download failed. Try again.

} +
+ ); +} + /** The take-back, where a state has something to take back and nothing to accept. */ function Discard({ onDiscard }: { readonly onDiscard: () => void }): JSX.Element { return ( diff --git a/frontend/ui-core/src/annotator/suggestPanel.test.tsx b/frontend/ui-core/src/annotator/suggestPanel.test.tsx index 070de471..6ebce1a2 100644 --- a/frontend/ui-core/src/annotator/suggestPanel.test.tsx +++ b/frontend/ui-core/src/annotator/suggestPanel.test.tsx @@ -9,7 +9,7 @@ * never a dead button. */ -import { cleanup, fireEvent, render, screen } from "@testing-library/react"; +import { cleanup, fireEvent, render, screen, waitFor } from "@testing-library/react"; import { userEvent } from "@testing-library/user-event"; import { describe, expect, it, vi } from "vitest"; import type { JSX } from "react"; @@ -20,6 +20,7 @@ import type { Suggestion, SuggestionState } from "@visionset/annotator"; import { SuggestPanel } from "./SuggestPanel"; import type { Answer } from "@visionset/annotator"; import { usableConnection, type Connection } from "../data/inferenceQueries"; +import type { BrowserModelAcquisition, BrowserSuggestionTarget } from "../inference/browserPort.js"; const A_BOX = { type: "bbox", x: 10, y: 20, width: 30, height: 40 } as const; @@ -639,3 +640,124 @@ describe("the adjustments, which are a section and never a popup", () => { ); }); }); + +describe("this device, once a browser runtime is wired", () => { + const READY: BrowserSuggestionTarget = { + id: "efficient-sam-ti", + label: "EfficientSAM-Ti", + modelRef: "efficient-sam-ti@browser", + }; + + function acquisition(overrides: Partial = {}): BrowserModelAcquisition { + return { + id: "efficient-sam-ti", + label: "EfficientSAM-Ti", + approxBytes: 41_000_000, + acquire: vi.fn().mockResolvedValue(undefined), + ...overrides, + }; + } + + it("renders no device section, and no tab chooser, when no runtime is wired at all", () => { + render(mount()); + + expect(screen.queryByTestId("suggest-device-section")).toBeNull(); + expect(screen.queryByTestId("suggest-target-server")).toBeNull(); + expect(screen.queryByTestId("suggest-target-browser")).toBeNull(); + // The rest of the idle card renders exactly as it does today. + expect(screen.getByTestId("suggest-idle")).toBeTruthy(); + }); + + it("shows the ready target with a success badge, and offers the two tabs", () => { + render( + mount({ + browserTargets: [READY], + browserAcquisitions: [], + activeTarget: { kind: "browser", targetId: READY.id }, + onChooseTarget: vi.fn(), + }), + ); + + expect(screen.getByTestId("suggest-target-server")).toBeTruthy(); + expect(screen.getByTestId("suggest-target-browser")).toBeTruthy(); + const section = screen.getByTestId("suggest-device-section"); + expect(section.textContent).toContain("EfficientSAM-Ti"); + expect(section.textContent).toContain("Ready"); + }); + + it("switches to the server target when the Server tab is chosen", async () => { + const onChooseTarget = vi.fn(); + const user = userEvent.setup(); + render( + mount({ + browserTargets: [READY], + browserAcquisitions: [], + activeTarget: { kind: "browser", targetId: READY.id }, + onChooseTarget, + connectionId: "c1", + }), + ); + + await user.click(screen.getByTestId("suggest-target-server")); + expect(onChooseTarget).toHaveBeenCalledWith({ kind: "server", connectionId: "c1" }); + }); + + it("switches to the browser target when the This device tab is chosen", async () => { + const onChooseTarget = vi.fn(); + const user = userEvent.setup(); + render( + mount({ + browserTargets: [READY], + browserAcquisitions: [], + activeTarget: { kind: "server", connectionId: "c1" }, + onChooseTarget, + }), + ); + + await user.click(screen.getByTestId("suggest-target-browser")); + expect(onChooseTarget).toHaveBeenCalledWith({ kind: "browser", targetId: READY.id }); + }); + + it("offers a download for a model not yet acquired, and reports success", async () => { + const onAcquired = vi.fn(); + const acquire = vi.fn().mockResolvedValue(undefined); + render( + mount({ + browserTargets: [], + browserAcquisitions: [acquisition({ acquire })], + activeTarget: { kind: "browser", targetId: "efficient-sam-ti" }, + onChooseTarget: vi.fn(), + onAcquired, + }), + ); + + const button = screen.getByTestId("suggest-device-acquire-efficient-sam-ti"); + expect(button.textContent).toContain("Download to this browser"); + expect(screen.getByTestId("suggest-device-section").textContent).toContain("41 MB"); + + fireEvent.click(button); + expect(acquire).toHaveBeenCalledTimes(1); + expect(button).toHaveProperty("disabled", true); + + await waitFor(() => expect(onAcquired).toHaveBeenCalledTimes(1)); + }); + + it("shows an alert and re-enables the button when the download fails", async () => { + const acquire = vi.fn().mockRejectedValue(new Error("network down")); + render( + mount({ + browserTargets: [], + browserAcquisitions: [acquisition({ acquire })], + activeTarget: { kind: "browser", targetId: "efficient-sam-ti" }, + onChooseTarget: vi.fn(), + }), + ); + + const button = screen.getByTestId("suggest-device-acquire-efficient-sam-ti"); + fireEvent.click(button); + + await waitFor(() => expect(screen.getByRole("alert")).toBeTruthy()); + expect(screen.getByRole("alert").textContent).toContain("Download failed"); + expect(button).toHaveProperty("disabled", false); + }); +}); From 0fd7c6ac148bf7192d9520a04ac1a18df5ea8c85 Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya <1445792+JArmandoAnaya@users.noreply.github.com> Date: Thu, 17 Sep 2026 00:49:35 -0700 Subject: [PATCH 08/42] feat(app): fetch and verify EfficientSAM-Ti artifacts against pinned SHA-256 --- .../acquireEfficientSam.test.ts | 95 +++++++++++++++++++ .../browserInference/acquireEfficientSam.ts | 48 ++++++++++ .../app/src/data/browserInference/manifest.ts | 30 ++++++ 3 files changed, 173 insertions(+) create mode 100644 frontend/app/src/data/browserInference/acquireEfficientSam.test.ts create mode 100644 frontend/app/src/data/browserInference/acquireEfficientSam.ts create mode 100644 frontend/app/src/data/browserInference/manifest.ts diff --git a/frontend/app/src/data/browserInference/acquireEfficientSam.test.ts b/frontend/app/src/data/browserInference/acquireEfficientSam.test.ts new file mode 100644 index 00000000..51670c16 --- /dev/null +++ b/frontend/app/src/data/browserInference/acquireEfficientSam.test.ts @@ -0,0 +1,95 @@ +/** + * @vitest-environment node + * + * The same realm problem `ossClient.test.ts` and `frameSink.test.ts` document at + * length: vitest's jsdom environment gives `Uint8Array`, `Response` and `fetch` + * their own jsdom-realm identities, and a jsdom-realm `Uint8Array` compares unequal + * to a Node-realm one built from the same bytes even though `toEqual` reports "no + * visual difference". This file never touches the DOM, so the node environment + * sidesteps the mismatch instead of working around it. + */ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import { fetchVerified } from "./acquireEfficientSam.js"; + +// `Uint8Array`, not the bare `Uint8Array` — see the same note in +// acquireEfficientSam.ts: TypeScript 6's `lib.dom.d.ts` requires the concrete +// `ArrayBuffer` variant everywhere these bytes flow into `Response` or `crypto.subtle`. +function bytesOf(text: string): Uint8Array { + return new TextEncoder().encode(text); +} + +async function sha256Of(bytes: Uint8Array): Promise { + const digest = await crypto.subtle.digest("SHA-256", bytes); + return Array.from(new Uint8Array(digest)).map((b) => b.toString(16).padStart(2, "0")).join(""); +} + +describe("fetchVerified", () => { + beforeEach(() => vi.restoreAllMocks()); + + it("returns the bytes when size and SHA-256 both match", async () => { + const bytes = bytesOf("hello world"); + const sha256 = await sha256Of(bytes); + vi.stubGlobal("fetch", vi.fn().mockResolvedValue(new Response(bytes))); + + const result = await fetchVerified("https://cdn.example/x.onnx", { bytes: bytes.byteLength, sha256 }); + expect(result).toEqual(bytes); + }); + + it("throws, without touching the network response's content, when the byte count is wrong", async () => { + const bytes = bytesOf("hello world"); + vi.stubGlobal("fetch", vi.fn().mockResolvedValue(new Response(bytes))); + await expect( + fetchVerified("https://cdn.example/x.onnx", { bytes: bytes.byteLength + 1, sha256: "deadbeef" }), + ).rejects.toThrow(/size mismatch/i); + }); + + it("throws when the SHA-256 does not match, even though the size does", async () => { + const bytes = bytesOf("hello world"); + vi.stubGlobal("fetch", vi.fn().mockResolvedValue(new Response(bytes))); + await expect( + fetchVerified("https://cdn.example/x.onnx", { bytes: bytes.byteLength, sha256: "0".repeat(64) }), + ).rejects.toThrow(/sha-256 mismatch/i); + }); +}); + +describe("acquireEfficientSam", () => { + it("fetches the manifest, then each artifact — never before the manifest resolves", async () => { + const encoderBytes = bytesOf("encoder-fixture"); + const decoderBytes = bytesOf("decoder-fixture"); + const fetchMock = vi.fn(async (url: string) => { + if (url.endsWith("manifest.json")) { + return new Response(JSON.stringify({ encoder: { path: "/encoder.onnx" }, decoder: { path: "/decoder.onnx" } })); + } + if (url.endsWith("encoder.onnx")) return new Response(encoderBytes); + if (url.endsWith("decoder.onnx")) return new Response(decoderBytes); + throw new Error(`unexpected url ${url}`); + }); + vi.stubGlobal("fetch", fetchMock); + // `fetchVerified` is already imported statically at the top of this file, so its + // module graph (including `./manifest.js`) is cached before this test runs. + // `vi.doMock` only rewrites *future* resolutions of a specifier — it does not + // retroactively patch an already-loaded module — so `vi.resetModules()` clears + // the cache first, forcing the dynamic `import()` below to re-evaluate both + // modules fresh, this time picking up the mock. + vi.resetModules(); + vi.doMock("./manifest.js", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + EFFICIENT_SAM_TI_EXPECTED: { + encoder: { sha256: await sha256Of(encoderBytes), bytes: encoderBytes.byteLength }, + decoder: { sha256: await sha256Of(decoderBytes), bytes: decoderBytes.byteLength }, + }, + }; + }); + const { acquireEfficientSam } = await import("./acquireEfficientSam.js"); + + const result = await acquireEfficientSam(); + + expect(result.encoder).toEqual(encoderBytes); + expect(result.decoder).toEqual(decoderBytes); + expect(fetchMock).toHaveBeenCalledTimes(3); + expect(fetchMock.mock.calls[0]![0]).toMatch(/manifest\.json$/); + }); +}); diff --git a/frontend/app/src/data/browserInference/acquireEfficientSam.ts b/frontend/app/src/data/browserInference/acquireEfficientSam.ts new file mode 100644 index 00000000..ba53a399 --- /dev/null +++ b/frontend/app/src/data/browserInference/acquireEfficientSam.ts @@ -0,0 +1,48 @@ +import { EFFICIENT_SAM_TI_EXPECTED, MODEL_CDN_BASE_URL, fetchEfficientSamManifest } from "./manifest.js"; + +// `Uint8Array`, not the bare `Uint8Array` (which now defaults to the +// wider `Uint8Array`) — TypeScript 6's `lib.dom.d.ts` types +// `crypto.subtle.digest`'s `BufferSource` parameter as requiring the concrete +// `ArrayBuffer` variant. +async function sha256Hex(bytes: Uint8Array): Promise { + const digest = await crypto.subtle.digest("SHA-256", bytes); + return Array.from(new Uint8Array(digest)) + .map((byte) => byte.toString(16).padStart(2, "0")) + .join(""); +} + +export async function fetchVerified( + url: string, + expected: { readonly bytes: number; readonly sha256: string }, + signal?: AbortSignal, +): Promise { + const response = await fetch(url, { signal }); + if (!response.ok) throw new Error(`artifact fetch failed: ${response.status} ${response.statusText}`); + const bytes = new Uint8Array(await response.arrayBuffer()); + if (bytes.byteLength !== expected.bytes) { + throw new Error(`artifact size mismatch for ${url}: got ${bytes.byteLength} bytes, expected ${expected.bytes}`); + } + const digest = await sha256Hex(bytes); + if (digest !== expected.sha256) { + throw new Error(`artifact SHA-256 mismatch for ${url}: got ${digest}, expected ${expected.sha256}`); + } + return bytes; +} + +/** Fetches nothing until called, and hard-fails rather than constructing a runtime on any mismatch. */ +export async function acquireEfficientSam( + signal?: AbortSignal, +): Promise<{ readonly encoder: Uint8Array; readonly decoder: Uint8Array }> { + const manifest = await fetchEfficientSamManifest(signal); + const encoder = await fetchVerified( + `${MODEL_CDN_BASE_URL}${manifest.encoder.path}`, + EFFICIENT_SAM_TI_EXPECTED.encoder, + signal, + ); + const decoder = await fetchVerified( + `${MODEL_CDN_BASE_URL}${manifest.decoder.path}`, + EFFICIENT_SAM_TI_EXPECTED.decoder, + signal, + ); + return { encoder, decoder }; +} diff --git a/frontend/app/src/data/browserInference/manifest.ts b/frontend/app/src/data/browserInference/manifest.ts new file mode 100644 index 00000000..48e1eb3c --- /dev/null +++ b/frontend/app/src/data/browserInference/manifest.ts @@ -0,0 +1,30 @@ +/** + * The only file in this repository allowed to name models.robomous.ai — see + * tests/scripts/ui_core_boundary.test.mjs and tests/scripts/cdn_vendor_boundary.test.mjs. + * Self-hosted deployments override VITE_MODEL_CDN_BASE_URL to point at their own mirror + * of this same manifest layout. + */ +export const MODEL_CDN_BASE_URL: string = + (import.meta.env["VITE_MODEL_CDN_BASE_URL"] as string | undefined) ?? "https://models.robomous.ai"; + +export const EFFICIENT_SAM_TI_REVISION = "b19782d049c0-843761ca46f4"; + +export const EFFICIENT_SAM_TI_MANIFEST_URL = + `${MODEL_CDN_BASE_URL}/models/efficient-sam-ti/${EFFICIENT_SAM_TI_REVISION}/manifest.json`; + +/** Verified 2026-09-16 against the live models.robomous.ai release — see the design doc §5. */ +export const EFFICIENT_SAM_TI_EXPECTED = { + encoder: { sha256: "b19782d049c09a8f1cc36ccc6029264ca23c8ac35e6379fd9ef9f1bc6d81e7f2", bytes: 24_799_777 }, + decoder: { sha256: "843761ca46f4aa00b09fdcf0c94271321f76eece092a744296c742d682a86172", bytes: 16_501_901 }, +} as const; + +export interface EfficientSamManifest { + readonly encoder: { readonly path: string }; + readonly decoder: { readonly path: string }; +} + +export async function fetchEfficientSamManifest(signal?: AbortSignal): Promise { + const response = await fetch(EFFICIENT_SAM_TI_MANIFEST_URL, { signal }); + if (!response.ok) throw new Error(`manifest fetch failed: ${response.status} ${response.statusText}`); + return (await response.json()) as EfficientSamManifest; +} From 0c4f688e59b3b15d23e0cc6b297750f07286343d Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya <1445792+JArmandoAnaya@users.noreply.github.com> Date: Thu, 17 Sep 2026 01:02:27 -0700 Subject: [PATCH 09/42] feat(app): add the race-safe browser suggestion executor --- .../BrowserSuggestionExecutor.test.ts | 255 ++++++++++++++++++ .../BrowserSuggestionExecutor.ts | 106 ++++++++ 2 files changed, 361 insertions(+) create mode 100644 frontend/app/src/data/browserInference/BrowserSuggestionExecutor.test.ts create mode 100644 frontend/app/src/data/browserInference/BrowserSuggestionExecutor.ts diff --git a/frontend/app/src/data/browserInference/BrowserSuggestionExecutor.test.ts b/frontend/app/src/data/browserInference/BrowserSuggestionExecutor.test.ts new file mode 100644 index 00000000..5bb4d322 --- /dev/null +++ b/frontend/app/src/data/browserInference/BrowserSuggestionExecutor.test.ts @@ -0,0 +1,255 @@ +import { describe, expect, it, vi } from "vitest"; +import type { PromptableSegmentationRuntime } from "@visionset/browser-inference"; +import { ApiError } from "@visionset/ui-core"; +import type { BrowserSuggestionAssetSource, SuggestionRequest } from "@visionset/ui-core"; + +import { createBrowserSuggestionExecutor } from "./BrowserSuggestionExecutor.js"; + +function requestFor(assetId: string, overrides?: Partial): SuggestionRequest { + return { + projectId: "p1", + assetId, + positive: [[10, 10]], + negative: [], + allowedGeometries: ["polygon"], + adjustments: { tolerance: 2 }, + ...overrides, + }; +} + +function sourceFor(assetId: string, rgb = new Uint8Array(3 * 4 * 4)): BrowserSuggestionAssetSource { + return { assetId, width: 4, height: 4, readRgb: () => ({ width: 4, height: 4, rgb }) }; +} + +/** + * A runtime whose two interesting methods the caller supplies. + * + * `ready`/`dispose` are stubbed rather than cast away, so a signature change on the + * port breaks these tests instead of being hidden behind an `any`. + */ +function runtimeWith(parts: Partial): PromptableSegmentationRuntime { + return { + ready: async () => [], + prepareImage: async () => ({ width: 4, height: 4 }), + suggest: async () => ({ width: 4, height: 4, mask: new Uint8Array(16), confidence: 0 }), + dispose: () => undefined, + ...parts, + }; +} + +describe("createBrowserSuggestionExecutor", () => { + it("refuses a negative-point request before touching the model", async () => { + const prepareImage = vi.fn(); + const suggest = vi.fn(); + const executor = createBrowserSuggestionExecutor({ + modelRef: "efficient-sam-ti@rev", + runtime: runtimeWith({ prepareImage, suggest }), + getActiveSource: () => sourceFor("a1"), + }); + const request = requestFor("a1", { negative: [[1, 1]] }); + await expect(executor.suggest(request)).rejects.toBeInstanceOf(ApiError); + await expect(executor.suggest(request)).rejects.toMatchObject({ + code: "BROWSER_NEGATIVE_POINTS_UNSUPPORTED", + }); + expect(prepareImage).not.toHaveBeenCalled(); + expect(suggest).not.toHaveBeenCalled(); + }); + + it("refuses when the active source's assetId does not match the request", async () => { + const prepareImage = vi.fn(); + const suggest = vi.fn(); + const executor = createBrowserSuggestionExecutor({ + modelRef: "efficient-sam-ti@rev", + runtime: runtimeWith({ prepareImage, suggest }), + getActiveSource: () => sourceFor("a1"), + }); + await expect(executor.suggest(requestFor("a2"))).rejects.toThrow(); + expect(prepareImage).not.toHaveBeenCalled(); + }); + + it("refuses when there is no active source at all", async () => { + const prepareImage = vi.fn(); + const executor = createBrowserSuggestionExecutor({ + modelRef: "efficient-sam-ti@rev", + runtime: runtimeWith({ prepareImage }), + getActiveSource: () => null, + }); + await expect(executor.suggest(requestFor("a1"))).rejects.toThrow(); + expect(prepareImage).not.toHaveBeenCalled(); + }); + + it("prepares the image once for N refinements on the same source", async () => { + const source = sourceFor("a1"); + const prepareImage = vi.fn().mockResolvedValue({ width: 4, height: 4 }); + const suggest = vi + .fn() + .mockResolvedValue({ width: 4, height: 4, mask: new Uint8Array(16), confidence: 0.8 }); + const executor = createBrowserSuggestionExecutor({ + modelRef: "efficient-sam-ti@rev", + runtime: runtimeWith({ prepareImage, suggest }), + getActiveSource: () => source, + }); + await executor.suggest(requestFor("a1")); + await executor.suggest( + requestFor("a1", { + positive: [ + [10, 10], + [12, 12], + ], + }), + ); + expect(prepareImage).toHaveBeenCalledTimes(1); + expect(suggest).toHaveBeenCalledTimes(2); + }); + + it("prepares each source separately, even for the same assetId", async () => { + const prepareImage = vi.fn().mockResolvedValue({ width: 4, height: 4 }); + const suggest = vi + .fn() + .mockResolvedValue({ width: 4, height: 4, mask: new Uint8Array(16), confidence: 0.8 }); + let source = sourceFor("a1"); + const executor = createBrowserSuggestionExecutor({ + modelRef: "efficient-sam-ti@rev", + runtime: runtimeWith({ prepareImage, suggest }), + getActiveSource: () => source, + }); + await executor.suggest(requestFor("a1")); + source = sourceFor("a1"); + await executor.suggest(requestFor("a1")); + expect(prepareImage).toHaveBeenCalledTimes(2); + }); + + it("never lets a stale in-flight prepareImage answer for a source that changed underneath it", async () => { + let resolvePrepare!: (value: { width: number; height: number }) => void; + const prepareImage = vi + .fn() + .mockReturnValue(new Promise<{ width: number; height: number }>((resolve) => (resolvePrepare = resolve))); + const suggest = vi.fn(); + let active: BrowserSuggestionAssetSource | null = sourceFor("a1"); + const executor = createBrowserSuggestionExecutor({ + modelRef: "efficient-sam-ti@rev", + runtime: runtimeWith({ prepareImage, suggest }), + getActiveSource: () => active, + }); + + const pending = executor.suggest(requestFor("a1")); + expect(prepareImage).toHaveBeenCalledTimes(1); + active = sourceFor("b1"); // asset switch while prepareImage(a1) is still in flight + resolvePrepare({ width: 4, height: 4 }); + + await expect(pending).rejects.toThrow(); + expect(suggest).not.toHaveBeenCalled(); + }); + + it("never lets a stale in-flight suggest paint onto a source that changed underneath it", async () => { + let resolveSuggest!: (value: { + width: number; + height: number; + mask: Uint8Array; + confidence: number; + }) => void; + const prepareImage = vi.fn().mockResolvedValue({ width: 4, height: 4 }); + const suggest = vi.fn().mockReturnValue( + new Promise<{ width: number; height: number; mask: Uint8Array; confidence: number }>( + (resolve) => (resolveSuggest = resolve), + ), + ); + let active: BrowserSuggestionAssetSource | null = sourceFor("a1"); + const executor = createBrowserSuggestionExecutor({ + modelRef: "efficient-sam-ti@rev", + runtime: runtimeWith({ prepareImage, suggest }), + getActiveSource: () => active, + }); + + const pending = executor.suggest(requestFor("a1")); + await vi.waitFor(() => expect(suggest).toHaveBeenCalledTimes(1)); + active = sourceFor("b1"); // asset switch while suggest(a1) is still in flight + resolveSuggest({ width: 4, height: 4, mask: new Uint8Array(16).fill(1), confidence: 0.9 }); + + await expect(pending).rejects.toThrow(); + }); + + it("builds a SuggestionOut matching the authoritative shape: model_ref, confidence, regions, applied.tolerance, parameters", async () => { + const source = sourceFor("a1"); + const prepareImage = vi.fn().mockResolvedValue({ width: 4, height: 4 }); + const mask = new Uint8Array(16).fill(1); + const suggest = vi.fn().mockResolvedValue({ width: 4, height: 4, mask, confidence: 0.75 }); + const executor = createBrowserSuggestionExecutor({ + modelRef: "efficient-sam-ti@rev", + runtime: runtimeWith({ prepareImage, suggest }), + getActiveSource: () => source, + }); + + const out = await executor.suggest( + requestFor("a1", { allowedGeometries: ["polygon"], adjustments: { tolerance: 3 } }), + ); + + expect(out.model_ref).toBe("efficient-sam-ti@rev"); + expect(out.confidence).toBe(0.75); + expect(out.applied).toEqual({ tolerance: 3 }); + expect(out.parameters).toEqual(["tolerance"]); + expect(Array.isArray(out.regions)).toBe(true); + expect(out.regions.length).toBeGreaterThan(0); + for (const region of out.regions) { + expect(region.geometry).toMatchObject({ type: "polygon" }); + expect(Array.isArray(region.contour)).toBe(true); + } + expect(out).not.toHaveProperty("regions.0.confidence"); + }); + + it("passes the request's positive points and tolerance through to the geometry step", async () => { + const source = sourceFor("a1"); + const mask = new Uint8Array(16).fill(1); + const suggest = vi.fn().mockResolvedValue({ width: 4, height: 4, mask, confidence: 0.5 }); + const executor = createBrowserSuggestionExecutor({ + modelRef: "efficient-sam-ti@rev", + runtime: runtimeWith({ + prepareImage: vi.fn().mockResolvedValue({ width: 4, height: 4 }), + suggest, + }), + getActiveSource: () => source, + }); + + await executor.suggest(requestFor("a1", { positive: [[1, 1]] })); + + expect(suggest).toHaveBeenCalledWith( + { width: 4, height: 4 }, + { positive: [[1, 1]], negative: [] }, + ); + }); + + it("parameters is empty when polygon is not among the allowed geometries", async () => { + const source = sourceFor("a1"); + const prepareImage = vi.fn().mockResolvedValue({ width: 4, height: 4 }); + const suggest = vi + .fn() + .mockResolvedValue({ width: 4, height: 4, mask: new Uint8Array(16).fill(1), confidence: 0.5 }); + const executor = createBrowserSuggestionExecutor({ + modelRef: "efficient-sam-ti@rev", + runtime: runtimeWith({ prepareImage, suggest }), + getActiveSource: () => source, + }); + const out = await executor.suggest(requestFor("a1", { allowedGeometries: ["bbox"] })); + expect(out.parameters).toEqual([]); + }); + + it("reads the source's pixels for the encode", async () => { + const rgb = new Uint8Array(3 * 4 * 4).fill(7); + const source = sourceFor("a1", rgb); + const prepareImage = vi.fn().mockResolvedValue({ width: 4, height: 4 }); + const executor = createBrowserSuggestionExecutor({ + modelRef: "efficient-sam-ti@rev", + runtime: runtimeWith({ + prepareImage, + suggest: vi + .fn() + .mockResolvedValue({ width: 4, height: 4, mask: new Uint8Array(16), confidence: 0.1 }), + }), + getActiveSource: () => source, + }); + + await executor.suggest(requestFor("a1")); + + expect(prepareImage).toHaveBeenCalledWith({ width: 4, height: 4, rgb }); + }); +}); diff --git a/frontend/app/src/data/browserInference/BrowserSuggestionExecutor.ts b/frontend/app/src/data/browserInference/BrowserSuggestionExecutor.ts new file mode 100644 index 00000000..62fc0f9e --- /dev/null +++ b/frontend/app/src/data/browserInference/BrowserSuggestionExecutor.ts @@ -0,0 +1,106 @@ +/** + * The browser half of the suggestion contract: a `SuggestionExecutor` that answers from a + * model running on this device. + * + * It answers the same question the server executor answers, and the caller cannot tell them + * apart — which is the point of the seam. What is different is that everything here happens + * inside one browser tab while the user keeps clicking, so two facts the server path gets for + * free have to be established by hand: + * + * 1. **One encode per asset.** The expensive half of a promptable segmentation is the image + * embedding; a refinement click only re-runs the decoder. The `prepareImage` promise is + * therefore cached against the *source object's identity* — not its `assetId` — because the + * embedding belongs to the pixels that source leased, and a second lease over the same + * logical asset is a second set of pixels as far as this module is allowed to assume. + * + * 2. **A stale answer never paints.** The active asset can change at any await point. The + * source is captured once, before the first await, and re-checked after *both* the encode + * and the decode: an embedding for the asset the user has left must not become "the prepared + * image" for the one they are looking at, and its late answer must not reach the canvas. + * A refusal is cheap here — the session's serial would drop the answer anyway, and refusing + * keeps a superseded run from spending the decoder. + * + * Geometry is not reimplemented: `shapesFromMask` is the one mask-to-geometry pipeline, shared + * with the server's Python and pinned to it by a fixture. + */ +import { shapesFromMask } from "@visionset/annotator"; +import type { PreparedImage, PromptableSegmentationRuntime } from "@visionset/browser-inference"; +import { ApiError } from "@visionset/ui-core"; +import type { + BrowserSuggestionAssetSource, + SuggestionExecutor, + SuggestionOut, + SuggestionRequest, +} from "@visionset/ui-core"; + +interface Deps { + /** What an accepted suggestion is attributed to. The artifact's identity, not the runtime's. */ + readonly modelRef: string; + readonly runtime: PromptableSegmentationRuntime; + /** Read afresh at every await point — this is what makes the staleness checks mean anything. */ + readonly getActiveSource: () => BrowserSuggestionAssetSource | null; +} + +export function createBrowserSuggestionExecutor(deps: Deps): SuggestionExecutor { + const prepared = new WeakMap>(); + + return { + async suggest(request: SuggestionRequest): Promise { + // Refused before the model is touched, because EfficientSAM-Ti's prompt encoder has no + // embedding for a background label: a negative point would reach the graph with no + // polarity and come back as something that is not an exclusion. + if (request.negative.length > 0) { + throw new ApiError({ + code: "BROWSER_NEGATIVE_POINTS_UNSUPPORTED", + message: "This device supports positive-point refinement only.", + }); + } + + const source = deps.getActiveSource(); + if (source === null || source.assetId !== request.assetId) { + throw new Error(`no active browser asset source for asset ${request.assetId}`); + } + + let preparing = prepared.get(source); + if (preparing === undefined) { + preparing = deps.runtime.prepareImage({ + width: source.width, + height: source.height, + rgb: source.readRgb().rgb, + }); + prepared.set(source, preparing); + } + const preparedImage = await preparing; + if (deps.getActiveSource() !== source) { + throw new Error("the active asset changed while this device was preparing the image"); + } + + const raw = await deps.runtime.suggest(preparedImage, { + positive: request.positive, + negative: [], + }); + if (deps.getActiveSource() !== source) { + throw new Error("the active asset changed while this device was answering"); + } + + const shapes = shapesFromMask( + { width: raw.width, height: raw.height, mask: raw.mask }, + { + allowed: request.allowedGeometries, + tolerance: request.adjustments.tolerance, + at: request.positive, + }, + ); + + return { + model_ref: deps.modelRef, + confidence: raw.confidence, + regions: shapes.map((shape) => ({ geometry: shape.geometry, contour: shape.contour })), + applied: { tolerance: request.adjustments.tolerance }, + // The kernel's rule, not a second copy of it: a box does not depend on the tolerance, + // so a class that admits no polygon has no setting worth showing. + parameters: request.allowedGeometries.includes("polygon") ? ["tolerance"] : [], + }; + }, + }; +} From 64ec4ba3699d6daa51e1fec1c2ac9638e7de2d27 Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya <1445792+JArmandoAnaya@users.noreply.github.com> Date: Thu, 17 Sep 2026 01:05:33 -0700 Subject: [PATCH 10/42] fix(app): retry the encode after a failed prepareImage instead of caching the rejection --- .../BrowserSuggestionExecutor.test.ts | 32 +++++++++++++++++++ .../BrowserSuggestionExecutor.ts | 19 ++++++++--- 2 files changed, 46 insertions(+), 5 deletions(-) diff --git a/frontend/app/src/data/browserInference/BrowserSuggestionExecutor.test.ts b/frontend/app/src/data/browserInference/BrowserSuggestionExecutor.test.ts index 5bb4d322..a69c86cd 100644 --- a/frontend/app/src/data/browserInference/BrowserSuggestionExecutor.test.ts +++ b/frontend/app/src/data/browserInference/BrowserSuggestionExecutor.test.ts @@ -119,6 +119,38 @@ describe("createBrowserSuggestionExecutor", () => { expect(prepareImage).toHaveBeenCalledTimes(2); }); + it("retries the encode after a failed prepareImage instead of replaying the rejection", async () => { + const source = sourceFor("a1"); + const prepareImage = vi + .fn() + .mockRejectedValueOnce(new Error("the encoder gave out")) + .mockResolvedValue({ width: 4, height: 4 }); + const suggest = vi + .fn() + .mockResolvedValue({ width: 4, height: 4, mask: new Uint8Array(16).fill(1), confidence: 0.6 }); + const executor = createBrowserSuggestionExecutor({ + modelRef: "efficient-sam-ti@rev", + runtime: runtimeWith({ prepareImage, suggest }), + getActiveSource: () => source, + }); + + await expect(executor.suggest(requestFor("a1"))).rejects.toThrow("the encoder gave out"); + expect(prepareImage).toHaveBeenCalledTimes(1); + expect(suggest).not.toHaveBeenCalled(); + + // The same still-active source, so a cached rejection would answer this without a second + // encode. It gets a fresh one, and the click succeeds. + const out = await executor.suggest(requestFor("a1")); + expect(prepareImage).toHaveBeenCalledTimes(2); + expect(suggest).toHaveBeenCalledTimes(1); + expect(out.confidence).toBe(0.6); + + // ...and the retry's embedding is cached in its turn: a third click re-decodes only. + await executor.suggest(requestFor("a1")); + expect(prepareImage).toHaveBeenCalledTimes(2); + expect(suggest).toHaveBeenCalledTimes(2); + }); + it("never lets a stale in-flight prepareImage answer for a source that changed underneath it", async () => { let resolvePrepare!: (value: { width: number; height: number }) => void; const prepareImage = vi diff --git a/frontend/app/src/data/browserInference/BrowserSuggestionExecutor.ts b/frontend/app/src/data/browserInference/BrowserSuggestionExecutor.ts index 62fc0f9e..0217c366 100644 --- a/frontend/app/src/data/browserInference/BrowserSuggestionExecutor.ts +++ b/frontend/app/src/data/browserInference/BrowserSuggestionExecutor.ts @@ -63,11 +63,20 @@ export function createBrowserSuggestionExecutor(deps: Deps): SuggestionExecutor let preparing = prepared.get(source); if (preparing === undefined) { - preparing = deps.runtime.prepareImage({ - width: source.width, - height: source.height, - rgb: source.readRgb().rgb, - }); + // Only a *settled* embedding is worth keeping. A rejected promise left in the cache + // would answer every later click on this asset with the same dead error, so one + // transient encoder failure would break suggestion here until the host re-leased the + // pixels — evicting on rejection is what makes the next click a retry. + preparing = deps.runtime + .prepareImage({ + width: source.width, + height: source.height, + rgb: source.readRgb().rgb, + }) + .catch((error: unknown) => { + prepared.delete(source); + throw error; + }); prepared.set(source, preparing); } const preparedImage = await preparing; From 6f95ba01754c1f7cc99a8c5d177edc909f44b45f Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya <1445792+JArmandoAnaya@users.noreply.github.com> Date: Thu, 17 Sep 2026 01:17:49 -0700 Subject: [PATCH 11/42] fix(app): hold one embedding slot, and pin the prompt and tolerance with discriminating fixtures --- .../BrowserSuggestionExecutor.test.ts | 227 ++++++++++++++++-- .../BrowserSuggestionExecutor.ts | 54 +++-- 2 files changed, 250 insertions(+), 31 deletions(-) diff --git a/frontend/app/src/data/browserInference/BrowserSuggestionExecutor.test.ts b/frontend/app/src/data/browserInference/BrowserSuggestionExecutor.test.ts index a69c86cd..3b91d269 100644 --- a/frontend/app/src/data/browserInference/BrowserSuggestionExecutor.test.ts +++ b/frontend/app/src/data/browserInference/BrowserSuggestionExecutor.test.ts @@ -5,11 +5,19 @@ import type { BrowserSuggestionAssetSource, SuggestionRequest } from "@visionset import { createBrowserSuggestionExecutor } from "./BrowserSuggestionExecutor.js"; +interface Extent { + readonly width: number; + readonly height: number; +} + +const SMALL: Extent = { width: 4, height: 4 }; + +/** `[1, 1]`, so the prompt lands genuinely *inside* even the smallest fixture mask. */ function requestFor(assetId: string, overrides?: Partial): SuggestionRequest { return { projectId: "p1", assetId, - positive: [[10, 10]], + positive: [[1, 1]], negative: [], allowedGeometries: ["polygon"], adjustments: { tolerance: 2 }, @@ -17,8 +25,43 @@ function requestFor(assetId: string, overrides?: Partial): Su }; } -function sourceFor(assetId: string, rgb = new Uint8Array(3 * 4 * 4)): BrowserSuggestionAssetSource { - return { assetId, width: 4, height: 4, readRgb: () => ({ width: 4, height: 4, rgb }) }; +function sourceFor( + assetId: string, + extent: Extent = SMALL, + rgb = new Uint8Array(extent.width * extent.height * 3), +): BrowserSuggestionAssetSource { + return { assetId, ...extent, readRgb: () => ({ ...extent, rgb }) }; +} + +/** What the runtime answers with: a mask over `extent`, plus a confidence. */ +function segmentationOf(extent: Extent, mask: Uint8Array, confidence: number) { + return { ...extent, mask, confidence }; +} + +function solid(extent: Extent): Uint8Array { + return new Uint8Array(extent.width * extent.height).fill(1); +} + +function litRect(mask: Uint8Array, extent: Extent, x0: number, y0: number, w: number, h: number): void { + for (let y = y0; y < y0 + h; y += 1) { + for (let x = x0; x < x0 + w; x += 1) mask[y * extent.width + x] = 1; + } +} + +/** The polygon's own points, after asserting the geometry really is one. */ +function polygonPoints(geometry: unknown): readonly (readonly number[])[] { + const shape = geometry as { + readonly type?: unknown; + readonly points?: readonly (readonly number[])[]; + }; + expect(shape.type).toBe("polygon"); + expect(shape.points).toBeDefined(); + return shape.points ?? []; +} + +function xRangeOf(points: readonly (readonly number[])[]): { readonly min: number; readonly max: number } { + const xs = points.map((point) => point[0] ?? NaN); + return { min: Math.min(...xs), max: Math.max(...xs) }; } /** @@ -31,7 +74,7 @@ function runtimeWith(parts: Partial): PromptableS return { ready: async () => [], prepareImage: async () => ({ width: 4, height: 4 }), - suggest: async () => ({ width: 4, height: 4, mask: new Uint8Array(16), confidence: 0 }), + suggest: async () => segmentationOf(SMALL, solid(SMALL), 0), dispose: () => undefined, ...parts, }; @@ -83,7 +126,7 @@ describe("createBrowserSuggestionExecutor", () => { const prepareImage = vi.fn().mockResolvedValue({ width: 4, height: 4 }); const suggest = vi .fn() - .mockResolvedValue({ width: 4, height: 4, mask: new Uint8Array(16), confidence: 0.8 }); + .mockResolvedValue(segmentationOf(SMALL, solid(SMALL), 0.8)); const executor = createBrowserSuggestionExecutor({ modelRef: "efficient-sam-ti@rev", runtime: runtimeWith({ prepareImage, suggest }), @@ -93,8 +136,8 @@ describe("createBrowserSuggestionExecutor", () => { await executor.suggest( requestFor("a1", { positive: [ - [10, 10], - [12, 12], + [1, 1], + [2, 2], ], }), ); @@ -102,11 +145,44 @@ describe("createBrowserSuggestionExecutor", () => { expect(suggest).toHaveBeenCalledTimes(2); }); + it("re-prepares a source whose embedding a later source superseded", async () => { + const first = sourceFor("a1"); + const second = sourceFor("b1"); + const prepareImage = vi.fn().mockResolvedValue({ width: 4, height: 4 }); + const suggest = vi.fn().mockResolvedValue(segmentationOf(SMALL, solid(SMALL), 0.4)); + let active = first; + const executor = createBrowserSuggestionExecutor({ + modelRef: "efficient-sam-ti@rev", + runtime: runtimeWith({ prepareImage, suggest }), + getActiveSource: () => active, + }); + + await executor.suggest(requestFor("a1")); + expect(prepareImage).toHaveBeenCalledTimes(1); + + // Preparing another image invalidates the first handle inside the runtime. + active = second; + await executor.suggest(requestFor("b1")); + expect(prepareImage).toHaveBeenCalledTimes(2); + + // Back to the *same object* as the first time. A per-source cache would hit here and hand + // the runtime a handle it has already invalidated, forever. It gets a fresh encode. + active = first; + const out = await executor.suggest(requestFor("a1")); + expect(prepareImage).toHaveBeenCalledTimes(3); + expect(out.confidence).toBe(0.4); + + // Still one encode per source, though: a refinement click on the live source re-decodes only. + await executor.suggest(requestFor("a1", { positive: [[2, 2]] })); + expect(prepareImage).toHaveBeenCalledTimes(3); + expect(suggest).toHaveBeenCalledTimes(4); + }); + it("prepares each source separately, even for the same assetId", async () => { const prepareImage = vi.fn().mockResolvedValue({ width: 4, height: 4 }); const suggest = vi .fn() - .mockResolvedValue({ width: 4, height: 4, mask: new Uint8Array(16), confidence: 0.8 }); + .mockResolvedValue(segmentationOf(SMALL, solid(SMALL), 0.8)); let source = sourceFor("a1"); const executor = createBrowserSuggestionExecutor({ modelRef: "efficient-sam-ti@rev", @@ -127,7 +203,7 @@ describe("createBrowserSuggestionExecutor", () => { .mockResolvedValue({ width: 4, height: 4 }); const suggest = vi .fn() - .mockResolvedValue({ width: 4, height: 4, mask: new Uint8Array(16).fill(1), confidence: 0.6 }); + .mockResolvedValue(segmentationOf(SMALL, solid(SMALL), 0.6)); const executor = createBrowserSuggestionExecutor({ modelRef: "efficient-sam-ti@rev", runtime: runtimeWith({ prepareImage, suggest }), @@ -151,6 +227,37 @@ describe("createBrowserSuggestionExecutor", () => { expect(suggest).toHaveBeenCalledTimes(2); }); + it("does not let a failed encode evict a newer source's good embedding", async () => { + let rejectFirst!: (error: Error) => void; + const prepareImage = vi + .fn() + .mockReturnValueOnce(new Promise<{ width: number; height: number }>((_, reject) => (rejectFirst = reject))) + .mockResolvedValue({ width: 4, height: 4 }); + const suggest = vi.fn().mockResolvedValue(segmentationOf(SMALL, solid(SMALL), 0.3)); + const first = sourceFor("a1"); + const second = sourceFor("b1"); + let active = first; + const executor = createBrowserSuggestionExecutor({ + modelRef: "efficient-sam-ti@rev", + runtime: runtimeWith({ prepareImage, suggest }), + getActiveSource: () => active, + }); + + const pending = executor.suggest(requestFor("a1")); // encode for A, left in flight + active = second; + await executor.suggest(requestFor("b1")); // encode for B, which succeeds and takes the slot + expect(prepareImage).toHaveBeenCalledTimes(2); + + rejectFirst(new Error("the first encode gave out")); + await expect(pending).rejects.toThrow("the first encode gave out"); + + // A's failure must clear only *its own* slot, and B has since taken it. Re-encoding B here + // would be a needless second encode of an image the runtime is already holding. + await executor.suggest(requestFor("b1")); + expect(prepareImage).toHaveBeenCalledTimes(2); + expect(suggest).toHaveBeenCalledTimes(2); + }); + it("never lets a stale in-flight prepareImage answer for a source that changed underneath it", async () => { let resolvePrepare!: (value: { width: number; height: number }) => void; const prepareImage = vi @@ -196,7 +303,7 @@ describe("createBrowserSuggestionExecutor", () => { const pending = executor.suggest(requestFor("a1")); await vi.waitFor(() => expect(suggest).toHaveBeenCalledTimes(1)); active = sourceFor("b1"); // asset switch while suggest(a1) is still in flight - resolveSuggest({ width: 4, height: 4, mask: new Uint8Array(16).fill(1), confidence: 0.9 }); + resolveSuggest(segmentationOf(SMALL, solid(SMALL), 0.9)); await expect(pending).rejects.toThrow(); }); @@ -229,10 +336,9 @@ describe("createBrowserSuggestionExecutor", () => { expect(out).not.toHaveProperty("regions.0.confidence"); }); - it("passes the request's positive points and tolerance through to the geometry step", async () => { + it("hands the model the request's prompt verbatim, with no coordinate conversion", async () => { const source = sourceFor("a1"); - const mask = new Uint8Array(16).fill(1); - const suggest = vi.fn().mockResolvedValue({ width: 4, height: 4, mask, confidence: 0.5 }); + const suggest = vi.fn().mockResolvedValue(segmentationOf(SMALL, solid(SMALL), 0.5)); const executor = createBrowserSuggestionExecutor({ modelRef: "efficient-sam-ti@rev", runtime: runtimeWith({ @@ -244,18 +350,105 @@ describe("createBrowserSuggestionExecutor", () => { await executor.suggest(requestFor("a1", { positive: [[1, 1]] })); + // Tuples straight through — not `{x, y}` objects, and not re-scaled to the model's frame. expect(suggest).toHaveBeenCalledWith( { width: 4, height: 4 }, { positive: [[1, 1]], negative: [] }, ); }); + /** + * The prompt reaching the *model* is not the same claim as the prompt reaching the + * *geometry* step, and a solid mask cannot tell the two apart — every `at` selects the one + * component and every tolerance simplifies a rectangle identically. These two use fixtures + * that can actually discriminate. + */ + it("derives the geometry from the mask component the prompt points at", async () => { + const extent: Extent = { width: 8, height: 8 }; + const mask = new Uint8Array(extent.width * extent.height); + litRect(mask, extent, 1, 1, 2, 2); // one square, top-left + litRect(mask, extent, 5, 5, 2, 2); // a second, disjoint, exactly the same area + const source = sourceFor("a1", extent); + const executor = createBrowserSuggestionExecutor({ + modelRef: "efficient-sam-ti@rev", + runtime: runtimeWith({ + prepareImage: vi.fn().mockResolvedValue(extent), + suggest: vi.fn().mockResolvedValue(segmentationOf(extent, mask, 0.5)), + }), + getActiveSource: () => source, + }); + + const geometryAt = async (point: readonly [number, number]) => { + const out = await executor.suggest( + requestFor("a1", { positive: [point], adjustments: { tolerance: 1 } }), + ); + expect(out.regions).toHaveLength(1); + return polygonPoints(out.regions[0]?.geometry); + }; + + const nearOrigin = xRangeOf(await geometryAt([1, 1])); + const farCorner = xRangeOf(await geometryAt([6, 6])); + + // Equal-area components, so "the largest piece" cannot discriminate them: the answers can + // only differ if `at` is what selected the piece. They are disjoint along x. + expect(nearOrigin.max).toBeLessThan(farCorner.min); + expect(nearOrigin.min).toBeGreaterThanOrEqual(1); + expect(nearOrigin.max).toBeLessThanOrEqual(3); + expect(farCorner.min).toBeGreaterThanOrEqual(5); + expect(farCorner.max).toBeLessThanOrEqual(7); + }); + + it("simplifies the outline more aggressively at a coarser tolerance", async () => { + const extent: Extent = { width: 24, height: 24 }; + const mask = new Uint8Array(extent.width * extent.height); + const centre = 11.5; + for (let y = 0; y < extent.height; y += 1) { + for (let x = 0; x < extent.width; x += 1) { + // A rough disc: a stair-stepped boundary with far more vertices than a rectangle's, + // which is what gives Douglas-Peucker something to actually remove. + if (Math.hypot(x - centre, y - centre) <= 9.3) mask[y * extent.width + x] = 1; + } + } + const source = sourceFor("a1", extent); + const executor = createBrowserSuggestionExecutor({ + modelRef: "efficient-sam-ti@rev", + runtime: runtimeWith({ + prepareImage: vi.fn().mockResolvedValue(extent), + suggest: vi.fn().mockResolvedValue(segmentationOf(extent, mask, 0.5)), + }), + getActiveSource: () => source, + }); + + const answerAt = async (tolerance: number) => { + const out = await executor.suggest( + requestFor("a1", { positive: [[11, 11]], adjustments: { tolerance } }), + ); + expect(out.applied).toEqual({ tolerance }); + const region = out.regions[0]; + expect(region).toBeDefined(); + return { points: polygonPoints(region?.geometry), contour: region?.contour ?? [] }; + }; + + const fine = await answerAt(1); // the default + const coarse = await answerAt(16); // MAXIMUM_TOLERANCE + + // 11 points against 3, on this fixture. Asserted as the relation rather than the two + // numbers: the counts belong to the annotator's simplifier, which is pinned to the + // server's Python by its own fixture, not by this test. + expect(fine.points.length).toBeGreaterThan(coarse.points.length); + expect(coarse.points.length).toBeGreaterThanOrEqual(3); + // The *unsimplified* outline is identical either way, so what differs is the tolerance + // doing work downstream of the mask, not a different mask or a different component. + expect(fine.contour).toEqual(coarse.contour); + expect(fine.contour.length).toBeGreaterThan(fine.points.length); + }); + it("parameters is empty when polygon is not among the allowed geometries", async () => { const source = sourceFor("a1"); const prepareImage = vi.fn().mockResolvedValue({ width: 4, height: 4 }); const suggest = vi .fn() - .mockResolvedValue({ width: 4, height: 4, mask: new Uint8Array(16).fill(1), confidence: 0.5 }); + .mockResolvedValue(segmentationOf(SMALL, solid(SMALL), 0.5)); const executor = createBrowserSuggestionExecutor({ modelRef: "efficient-sam-ti@rev", runtime: runtimeWith({ prepareImage, suggest }), @@ -266,8 +459,8 @@ describe("createBrowserSuggestionExecutor", () => { }); it("reads the source's pixels for the encode", async () => { - const rgb = new Uint8Array(3 * 4 * 4).fill(7); - const source = sourceFor("a1", rgb); + const rgb = new Uint8Array(SMALL.width * SMALL.height * 3).fill(7); + const source = sourceFor("a1", SMALL, rgb); const prepareImage = vi.fn().mockResolvedValue({ width: 4, height: 4 }); const executor = createBrowserSuggestionExecutor({ modelRef: "efficient-sam-ti@rev", @@ -275,7 +468,7 @@ describe("createBrowserSuggestionExecutor", () => { prepareImage, suggest: vi .fn() - .mockResolvedValue({ width: 4, height: 4, mask: new Uint8Array(16), confidence: 0.1 }), + .mockResolvedValue(segmentationOf(SMALL, solid(SMALL), 0.1)), }), getActiveSource: () => source, }); diff --git a/frontend/app/src/data/browserInference/BrowserSuggestionExecutor.ts b/frontend/app/src/data/browserInference/BrowserSuggestionExecutor.ts index 0217c366..7057e11b 100644 --- a/frontend/app/src/data/browserInference/BrowserSuggestionExecutor.ts +++ b/frontend/app/src/data/browserInference/BrowserSuggestionExecutor.ts @@ -8,10 +8,16 @@ * free have to be established by hand: * * 1. **One encode per asset.** The expensive half of a promptable segmentation is the image - * embedding; a refinement click only re-runs the decoder. The `prepareImage` promise is - * therefore cached against the *source object's identity* — not its `assetId` — because the - * embedding belongs to the pixels that source leased, and a second lease over the same - * logical asset is a second set of pixels as far as this module is allowed to assume. + * embedding; a refinement click only re-runs the decoder. So the `prepareImage` promise is + * held and reused — but in a *single slot*, not a per-source map, because that is the shape + * of the thing being cached: `PromptableSegmentationRuntime` "holds exactly one embedding at + * a time; preparing another image invalidates this one". A map could hold two entries the + * runtime cannot both honour, and the second one would be a handle the runtime has already + * invalidated — refused on every later click, with nothing to trigger a retry. + * + * The slot is keyed on the *source object's identity*, not its `assetId`: the embedding + * belongs to the pixels that source leased, and a second lease over the same logical asset + * is a second set of pixels as far as this module is allowed to assume. * * 2. **A stale answer never paints.** The active asset can change at any await point. The * source is captured once, before the first await, and re-checked after *both* the encode @@ -42,9 +48,19 @@ interface Deps { } export function createBrowserSuggestionExecutor(deps: Deps): SuggestionExecutor { - const prepared = new WeakMap>(); + // The runtime's one embedding, and which source leased the pixels behind it. One slot, because + // the runtime has one slot; see the note at the top of the file. + let currentSource: BrowserSuggestionAssetSource | null = null; + let currentPrepared: Promise | null = null; return { + /** + * `signal` is deliberately not forwarded to the model calls, matching + * `useServerSuggestionExecutor`, which does not honour it either: the session's serial in + * `AnnotationPage` is what keeps a late answer off the screen. Forwarding it to + * `prepareImage` would additionally be wrong, since that promise is shared — one caller's + * abort would take the embedding out from under every other caller of this source. + */ async suggest(request: SuggestionRequest): Promise { // Refused before the model is touched, because EfficientSAM-Ti's prompt encoder has no // embedding for a background label: a negative point would reach the graph with no @@ -61,23 +77,33 @@ export function createBrowserSuggestionExecutor(deps: Deps): SuggestionExecutor throw new Error(`no active browser asset source for asset ${request.assetId}`); } - let preparing = prepared.get(source); - if (preparing === undefined) { - // Only a *settled* embedding is worth keeping. A rejected promise left in the cache - // would answer every later click on this asset with the same dead error, so one - // transient encoder failure would break suggestion here until the host re-leased the - // pixels — evicting on rejection is what makes the next click a retry. - preparing = deps.runtime + let preparing: Promise; + if (source === currentSource && currentPrepared !== null) { + preparing = currentPrepared; + } else { + // Only a *settled* embedding is worth keeping. A rejected promise left in the slot + // would answer every later click with the same dead error, so one transient encoder + // failure would break suggestion here until the host re-leased the pixels — clearing + // the slot is what makes the next click a retry. Cleared only if it is still *this* + // encode's slot: a newer source has already taken it, and its embedding is good. + const started: Promise = deps.runtime .prepareImage({ width: source.width, height: source.height, rgb: source.readRgb().rgb, }) .catch((error: unknown) => { - prepared.delete(source); + if (currentPrepared === started) { + currentSource = null; + currentPrepared = null; + } throw error; }); - prepared.set(source, preparing); + // Published before the first await, so a second click on this source reuses this encode + // rather than starting a rival one. + currentSource = source; + currentPrepared = started; + preparing = started; } const preparedImage = await preparing; if (deps.getActiveSource() !== source) { From 74be52ddab3c668b3b79469fd7a8c4bda1b35fa5 Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya <1445792+JArmandoAnaya@users.noreply.github.com> Date: Thu, 17 Sep 2026 01:26:03 -0700 Subject: [PATCH 12/42] feat(app): add the concrete browser inference runtime and acquisition state Also adds the missing @visionset/browser-inference workspace dependency to frontend/app/package.json (and the resulting pnpm-lock.yaml update), without which BrowserInferenceRuntime.ts's import could never resolve. --- frontend/app/package.json | 1 + .../BrowserInferenceRuntime.test.ts | 56 ++++++++++++++ .../BrowserInferenceRuntime.ts | 73 +++++++++++++++++++ pnpm-lock.yaml | 3 + 4 files changed, 133 insertions(+) create mode 100644 frontend/app/src/data/browserInference/BrowserInferenceRuntime.test.ts create mode 100644 frontend/app/src/data/browserInference/BrowserInferenceRuntime.ts diff --git a/frontend/app/package.json b/frontend/app/package.json index d83b5c68..025369d8 100644 --- a/frontend/app/package.json +++ b/frontend/app/package.json @@ -19,6 +19,7 @@ "dependencies": { "@robomous/ui-core": "^0.2.1", "@visionset/annotator": "workspace:*", + "@visionset/browser-inference": "workspace:*", "@visionset/media": "workspace:*", "@visionset/ui-core": "workspace:*", "lucide-react": "^1.44.0", diff --git a/frontend/app/src/data/browserInference/BrowserInferenceRuntime.test.ts b/frontend/app/src/data/browserInference/BrowserInferenceRuntime.test.ts new file mode 100644 index 00000000..c9427dcb --- /dev/null +++ b/frontend/app/src/data/browserInference/BrowserInferenceRuntime.test.ts @@ -0,0 +1,56 @@ +import { describe, expect, it, vi } from "vitest"; +import { createOssBrowserInferenceRuntime } from "./BrowserInferenceRuntime.js"; + +function fakeDeps(overrides?: { acquire?: () => Promise<{ encoder: Uint8Array; decoder: Uint8Array }> }) { + const createRuntime = vi.fn(() => ({ + ready: async () => [], + prepareImage: async () => ({ width: 1, height: 1 }), + suggest: async () => ({ width: 1, height: 1, mask: new Uint8Array(1), confidence: 1 }), + dispose: () => {}, + })); + const acquire = vi.fn(overrides?.acquire ?? (async () => ({ encoder: new Uint8Array(1), decoder: new Uint8Array(1) }))); + return { acquire, createRuntime }; +} + +describe("createOssBrowserInferenceRuntime", () => { + it("lists no targets and one acquisition before acquiring", async () => { + const deps = fakeDeps(); + const runtime = createOssBrowserInferenceRuntime(deps); + expect(await runtime.listTargets()).toEqual([]); + expect(runtime.listAcquisitions?.()).toHaveLength(1); + }); + + it("lists the target and no acquisitions after acquiring", async () => { + const deps = fakeDeps(); + const runtime = createOssBrowserInferenceRuntime(deps); + await runtime.listAcquisitions?.()[0]!.acquire(); + expect(await runtime.listTargets()).toHaveLength(1); + expect(runtime.listAcquisitions?.()).toEqual([]); + expect(deps.acquire).toHaveBeenCalledTimes(1); + expect(deps.createRuntime).toHaveBeenCalledTimes(1); + }); + + it("leaves the model unacquired (retryable) when acquire() rejects", async () => { + const deps = fakeDeps({ acquire: () => Promise.reject(new Error("network down")) }); + const runtime = createOssBrowserInferenceRuntime(deps); + await expect(runtime.listAcquisitions?.()[0]!.acquire()).rejects.toThrow("network down"); + expect(await runtime.listTargets()).toEqual([]); + expect(runtime.listAcquisitions?.()).toHaveLength(1); + }); + + it("dedupes a second acquire() call while the first is still in flight", async () => { + let resolveAcquire!: (value: { encoder: Uint8Array; decoder: Uint8Array }) => void; + const deps = fakeDeps({ acquire: () => new Promise((resolve) => (resolveAcquire = resolve)) }); + const runtime = createOssBrowserInferenceRuntime(deps); + const first = runtime.listAcquisitions?.()[0]!.acquire(); + const second = runtime.listAcquisitions?.()[0]!.acquire(); + resolveAcquire({ encoder: new Uint8Array(1), decoder: new Uint8Array(1) }); + await Promise.all([first, second]); + expect(deps.acquire).toHaveBeenCalledTimes(1); + }); + + it("throws from executorFor before any acquisition has succeeded", () => { + const runtime = createOssBrowserInferenceRuntime(fakeDeps()); + expect(() => runtime.executorFor("efficient-sam-ti")).toThrow(); + }); +}); diff --git a/frontend/app/src/data/browserInference/BrowserInferenceRuntime.ts b/frontend/app/src/data/browserInference/BrowserInferenceRuntime.ts new file mode 100644 index 00000000..65197b0e --- /dev/null +++ b/frontend/app/src/data/browserInference/BrowserInferenceRuntime.ts @@ -0,0 +1,73 @@ +import type { PromptableSegmentationRuntime } from "@visionset/browser-inference"; +import { createEfficientSamRuntime } from "@visionset/browser-inference/browser"; +import type { + BrowserSuggestionAssetSource, + BrowserSuggestionTarget, + SuggestionExecutor, + VisionSetBrowserInferenceRuntime, +} from "@visionset/ui-core"; + +import { acquireEfficientSam } from "./acquireEfficientSam.js"; +import { EFFICIENT_SAM_TI_EXPECTED, EFFICIENT_SAM_TI_REVISION } from "./manifest.js"; +import { createBrowserSuggestionExecutor } from "./BrowserSuggestionExecutor.js"; + +const MODEL_ID = "efficient-sam-ti"; +const MODEL_REF = `efficient-sam-ti@${EFFICIENT_SAM_TI_REVISION}`; + +interface Deps { + readonly acquire: (signal?: AbortSignal) => Promise<{ encoder: Uint8Array; decoder: Uint8Array }>; + readonly createRuntime: (artifacts: { encoder: Uint8Array; decoder: Uint8Array }) => PromptableSegmentationRuntime; +} + +const REAL_DEPS: Deps = { acquire: acquireEfficientSam, createRuntime: createEfficientSamRuntime }; + +type State = { readonly kind: "unacquired" } | { readonly kind: "ready"; readonly runtime: PromptableSegmentationRuntime }; + +export function createOssBrowserInferenceRuntime(deps: Deps = REAL_DEPS): VisionSetBrowserInferenceRuntime { + let state: State = { kind: "unacquired" }; + let inFlight: Promise | null = null; + let activeAssetSource: BrowserSuggestionAssetSource | null = null; + + return { + async listTargets(): Promise { + return state.kind === "ready" ? [{ id: MODEL_ID, label: "EfficientSAM-Ti", modelRef: MODEL_REF }] : []; + }, + listAcquisitions() { + if (state.kind === "ready") return []; + return [ + { + id: MODEL_ID, + label: "EfficientSAM-Ti", + approxBytes: EFFICIENT_SAM_TI_EXPECTED.encoder.bytes + EFFICIENT_SAM_TI_EXPECTED.decoder.bytes, + acquire(options?: { readonly signal?: AbortSignal }): Promise { + if (state.kind === "ready") return Promise.resolve(); + if (inFlight !== null) return inFlight; + inFlight = (async () => { + try { + const artifacts = await deps.acquire(options?.signal); + const runtime = deps.createRuntime(artifacts); + state = { kind: "ready", runtime }; + } finally { + inFlight = null; + } + })(); + return inFlight; + }, + }, + ]; + }, + executorFor(targetId: string): SuggestionExecutor { + if (state.kind !== "ready" || targetId !== MODEL_ID) { + throw new Error(`no ready browser target "${targetId}"`); + } + return createBrowserSuggestionExecutor({ + modelRef: MODEL_REF, + runtime: state.runtime, + getActiveSource: () => activeAssetSource, + }); + }, + setActiveAsset(source: BrowserSuggestionAssetSource | null): void { + activeAssetSource = source; + }, + }; +} diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 1ea89337..b47b1234 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -220,6 +220,9 @@ importers: '@visionset/annotator': specifier: workspace:* version: link:../annotator + '@visionset/browser-inference': + specifier: workspace:* + version: link:../browser-inference '@visionset/media': specifier: workspace:* version: link:../media From e7b837d02a43f4820fd8b95d823bf40453d42ce2 Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya <1445792+JArmandoAnaya@users.noreply.github.com> Date: Thu, 17 Sep 2026 01:30:45 -0700 Subject: [PATCH 13/42] feat(app): inject the browser inference runtime into the OSS session --- frontend/app/src/data/OssSession.tsx | 14 +++++++++++-- frontend/app/src/data/ossSession.test.tsx | 25 ++++++++++++++++++++++- 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/frontend/app/src/data/OssSession.tsx b/frontend/app/src/data/OssSession.tsx index 9de57b9e..60c834d2 100644 --- a/frontend/app/src/data/OssSession.tsx +++ b/frontend/app/src/data/OssSession.tsx @@ -34,9 +34,10 @@ import { type ReactNode, } from "react"; import type { QueryClient } from "@tanstack/react-query"; -import { VisionSetDataProvider, VisionSetMediaProvider } from "@visionset/ui-core"; +import { VisionSetBrowserInferenceProvider, VisionSetDataProvider, VisionSetMediaProvider } from "@visionset/ui-core"; import { MediabunnyVideoMaterializer } from "@visionset/media/mediabunny"; +import { createOssBrowserInferenceRuntime } from "./browserInference/BrowserInferenceRuntime"; import { createOssDataClient, requestSession } from "./ossClient"; import { createLocalApiFrameSink } from "./frameSink"; import { clearToken, readToken, writeToken } from "./token"; @@ -158,6 +159,13 @@ export function OssSessionProvider({ [client], ); + /** + * The browser inference runtime: this host's only door into ONNX Runtime, CDN manifests + * and SHA-256 verification. Built once per session — its acquisition state (unacquired + * vs. ready) must survive across asset navigation, not reset on every render. + */ + const browserInferenceRuntime = useMemo(() => createOssBrowserInferenceRuntime(), []); + const signIn = useCallback((next: string) => { writeToken(next); setToken(next); @@ -215,7 +223,9 @@ export function OssSessionProvider({ onUnauthorized={signOut} makeQueryClient={makeQueryClient} > - {children} + + {children} + ); diff --git a/frontend/app/src/data/ossSession.test.tsx b/frontend/app/src/data/ossSession.test.tsx index 108258f2..bda8aeed 100644 --- a/frontend/app/src/data/ossSession.test.tsx +++ b/frontend/app/src/data/ossSession.test.tsx @@ -28,7 +28,7 @@ import { userEvent } from "@testing-library/user-event"; import { afterEach, describe, expect, it, vi } from "vitest"; import type { JSX } from "react"; -import { useApiClient, Async, unwrap, checks } from "@visionset/ui-core"; +import { useApiClient, useBrowserInferenceRuntime, Async, unwrap, checks } from "@visionset/ui-core"; import { readToken, writeToken } from "./token"; import { OssSessionProvider, useOssSession } from "./OssSession"; import { TokenGate } from "../shell/TokenGate"; @@ -120,6 +120,29 @@ describe("the client carries the credential", () => { }); }); +describe("the browser inference runtime", () => { + it("provides a browser inference runtime to descendants", async () => { + const stub = stubFetch([[200, { items: [], total: 0 }]]); + vi.stubGlobal("fetch", stub.fetch); + + let seen: unknown; + function Probe(): null { + seen = useBrowserInferenceRuntime(); + return null; + } + + render( + + + , + ); + + await waitFor(() => expect(seen).not.toBeNull()); + + vi.unstubAllGlobals(); + }); +}); + describe("the browser session", () => { it("signs in with no token at all when the server issues one", async () => { const stub = stubFetch([[200, { items: [{ id: "p1", name: "highway", description: null, thumbnail_asset_id: null, thumbnail_hash: null, created_at: null }], total: 1 }]], { From e07405c52eb91049cfb2376d54c61d6c68591b8e Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya <1445792+JArmandoAnaya@users.noreply.github.com> Date: Thu, 17 Sep 2026 01:32:57 -0700 Subject: [PATCH 14/42] test(scripts): ban @visionset/browser-inference and CDN literals from ui-core --- tests/scripts/ui_core_boundary.test.mjs | 69 +++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/tests/scripts/ui_core_boundary.test.mjs b/tests/scripts/ui_core_boundary.test.mjs index addfc40d..1765bf9c 100644 --- a/tests/scripts/ui_core_boundary.test.mjs +++ b/tests/scripts/ui_core_boundary.test.mjs @@ -114,6 +114,33 @@ const MEDIA_HOST_BOUNDARY = [ ], ]; +/** + * `ui-core` may compose a browser inference runtime's *port* (already exported from + * `@visionset/ui-core`'s own root), but never the concrete package that runs ONNX Runtime, + * nor any CDN/vendor detail behind it — that composition belongs to `frontend/app` alone. + */ +const BROWSER_INFERENCE_HOST_BOUNDARY = [ + [ + /\bfrom\s+["']@visionset\/browser-inference(\/browser)?["']/, + "imports @visionset/browser-inference — the concrete runtime is the host's choice, not ui-core's", + ], + [ + /\brequire\(\s*["']@visionset\/browser-inference(\/browser)?["']\s*\)/, + "require()s @visionset/browser-inference", + ], + [/\bimport\(\s*["']@visionset\/browser-inference(\/browser)?["']\s*\)/, "dynamically imports @visionset/browser-inference"], + [/\bonnxruntime-web\b/, "names onnxruntime-web directly"], +]; + +/** Concrete CDN/vendor identity. Named literals only — never a generic substring like "s3". */ +const CDN_VENDOR_LITERALS = [ + [/\bmodels\.robomous\.ai\b/, "names models.robomous.ai — only frontend/app may know this hostname"], + [/\bcloudflare\b/i, "names cloudflare"], + [/\bcloudfront\b/i, "names cloudfront"], + [/\bamazonaws\.com\b/, "names amazonaws.com"], + [/\br2\.cloudflarestorage\.com\b/, "names r2.cloudflarestorage.com"], +]; + /** * Reading a `DataResult`'s status as meaning. * @@ -215,6 +242,14 @@ test("the reusable UI never reaches for the browser video materializer directly" assert.deepEqual(violations(shippedSource(), MEDIA_HOST_BOUNDARY), []); }); +test("the reusable UI never reaches for @visionset/browser-inference or onnxruntime-web directly", () => { + assert.deepEqual(violations(shippedSource(), BROWSER_INFERENCE_HOST_BOUNDARY), []); +}); + +test("the reusable UI names no CDN/vendor identity", () => { + assert.deepEqual(violations(shippedSource(), CDN_VENDOR_LITERALS), []); +}); + test("no semantic branching on DataResult.status in reusable ui-core", () => { assert.deepEqual(violations(shippedSource(), STATUS_AS_MEANING), []); }); @@ -284,6 +319,20 @@ test("the gate fires on a violation", () => { mediaBoundaryViolations.length, ); + const browserInferenceViolations = [ + { path: "frontend/ui-core/src/inference/BadImport.ts", text: 'import { createEfficientSamRuntime } from "@visionset/browser-inference/browser";\n' }, + { path: "frontend/ui-core/src/inference/BadRequire.ts", text: 'const m = require("@visionset/browser-inference");\n' }, + { path: "frontend/ui-core/src/inference/BadDynamic.ts", text: 'const m = await import("@visionset/browser-inference");\n' }, + { path: "frontend/ui-core/src/inference/BadOrt.ts", text: 'import * as ort from "onnxruntime-web/webgpu";\n' }, + ]; + assert.equal(violations(browserInferenceViolations, BROWSER_INFERENCE_HOST_BOUNDARY).length, browserInferenceViolations.length); + + const cdnViolations = [ + { path: "frontend/ui-core/src/inference/BadCdn.ts", text: 'const url = "https://models.robomous.ai/registry/v1.json";\n' }, + { path: "frontend/ui-core/src/inference/BadVendor.ts", text: "// served from Cloudflare\n" }, + ]; + assert.equal(violations(cdnViolations, CDN_VENDOR_LITERALS).length, cdnViolations.length); + assert.deepEqual(offeredRemovedExports('export type { ApiProviderProps } from "./x.js";\n'), ["ApiProviderProps"]); assert.deepEqual(offeredRemovedExports('export type { TokenGateProps } from "./x.js";\n'), ["TokenGateProps"]); assert.deepEqual(offeredRemovedExports('export type { Access } from "./x.js";\n'), ["Access"]); @@ -340,6 +389,26 @@ test("the gate does NOT fire on the legitimate neighbouring form", () => { ]; assert.deepEqual(violations(legitimateMediaBoundary, MEDIA_HOST_BOUNDARY), []); + const legitimateBrowserInference = [ + { + path: "frontend/ui-core/src/inference/browserPort.ts", + // The port's own types, imported from @visionset/annotator — never from + // @visionset/browser-inference, and this line must not trip the rule. + text: 'import type { RgbPixels } from "@visionset/annotator";\n', + }, + ]; + assert.deepEqual(violations(legitimateBrowserInference, BROWSER_INFERENCE_HOST_BOUNDARY), []); + + const legitimateCdn = [ + { + path: "frontend/ui-core/src/inference/browserPort.ts", + // "cloud" alone, not "cloudflare" — the rule is a named-vendor ban, not a generic + // substring ban, and must leave ordinary words alone. + text: "// this runtime may run in the cloud someday\n", + }, + ]; + assert.deepEqual(violations(legitimateCdn, CDN_VENDOR_LITERALS), []); + const legitimateStatus = [ { path: "frontend/ui-core/src/data/errors.ts", From 18f0494dc079889097e81336a3a481257feef4f1 Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya <1445792+JArmandoAnaya@users.noreply.github.com> Date: Thu, 17 Sep 2026 01:35:56 -0700 Subject: [PATCH 15/42] test(scripts): add missing CDN violation examples for cloudfront, amazonaws, and r2 --- tests/scripts/ui_core_boundary.test.mjs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/scripts/ui_core_boundary.test.mjs b/tests/scripts/ui_core_boundary.test.mjs index 1765bf9c..451d48fa 100644 --- a/tests/scripts/ui_core_boundary.test.mjs +++ b/tests/scripts/ui_core_boundary.test.mjs @@ -330,6 +330,9 @@ test("the gate fires on a violation", () => { const cdnViolations = [ { path: "frontend/ui-core/src/inference/BadCdn.ts", text: 'const url = "https://models.robomous.ai/registry/v1.json";\n' }, { path: "frontend/ui-core/src/inference/BadVendor.ts", text: "// served from Cloudflare\n" }, + { path: "frontend/ui-core/src/inference/BadCloudfront.ts", text: 'const url = "https://d123.cloudfront.net/x";\n' }, + { path: "frontend/ui-core/src/inference/BadAmazon.ts", text: 'const url = "https://bucket.s3.amazonaws.com/x";\n' }, + { path: "frontend/ui-core/src/inference/BadR2.ts", text: 'const url = "https://abc.r2.cloudflarestorage.com/x";\n' }, ]; assert.equal(violations(cdnViolations, CDN_VENDOR_LITERALS).length, cdnViolations.length); From 48e49d17e996bb82a037f8615f8ff9c9f3a53d59 Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya <1445792+JArmandoAnaya@users.noreply.github.com> Date: Thu, 17 Sep 2026 01:38:14 -0700 Subject: [PATCH 16/42] test(scripts): ban CDN/vendor literals from annotator and browser-inference --- tests/scripts/cdn_vendor_boundary.test.mjs | 72 ++++++++++++++++++++++ 1 file changed, 72 insertions(+) create mode 100644 tests/scripts/cdn_vendor_boundary.test.mjs diff --git a/tests/scripts/cdn_vendor_boundary.test.mjs b/tests/scripts/cdn_vendor_boundary.test.mjs new file mode 100644 index 00000000..8bd93e1f --- /dev/null +++ b/tests/scripts/cdn_vendor_boundary.test.mjs @@ -0,0 +1,72 @@ +// tests/scripts/cdn_vendor_boundary.test.mjs +// Run with: pnpm test:scripts +// +// The CDN this repository's OSS build points at by default is frontend/app's own choice +// (see frontend/app/src/data/browserInference/manifest.ts), never a fact @visionset/ +// annotator or @visionset/browser-inference are allowed to know — those two packages ship +// to npm and must work against any self-hosted mirror. Mirrors ui_core_boundary.test.mjs's +// CDN_VENDOR_LITERALS rule, scoped to the two packages that rule does not cover. +import assert from "node:assert/strict"; +import { execFileSync } from "node:child_process"; +import { readFileSync } from "node:fs"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; +import { test } from "node:test"; + +const REPO = fileURLToPath(new URL("../../", import.meta.url)); + +function shippedSourceUnder(dir) { + return execFileSync("git", ["ls-files", dir], { cwd: REPO, encoding: "utf8" }) + .split("\n") + .filter((file) => /\.[cm]?[jt]sx?$/.test(file)) + .filter((file) => !/\.test\.[jt]sx?$/.test(file)) + .map((file) => ({ path: file, text: readFileSync(path.join(REPO, file), "utf8") })); +} + +const CDN_VENDOR_LITERALS = [ + [/\bmodels\.robomous\.ai\b/, "names models.robomous.ai — only frontend/app may know this hostname"], + [/\bcloudflare\b/i, "names cloudflare"], + [/\bcloudfront\b/i, "names cloudfront"], + [/\bamazonaws\.com\b/, "names amazonaws.com"], + [/\br2\.cloudflarestorage\.com\b/, "names r2.cloudflarestorage.com"], +]; + +function violations(files, rules) { + const found = []; + for (const { path: file, text } of files) { + for (const [pattern, reason] of rules) { + if (pattern.test(text)) found.push(`${file}: ${reason}`); + } + } + return found; +} + +test("@visionset/annotator names no CDN/vendor identity", () => { + assert.deepEqual(violations(shippedSourceUnder("frontend/annotator/src"), CDN_VENDOR_LITERALS), []); +}); + +test("@visionset/browser-inference names no CDN/vendor identity", () => { + assert.deepEqual(violations(shippedSourceUnder("frontend/browser-inference/src"), CDN_VENDOR_LITERALS), []); +}); + +test("the gate fires on a violation", () => { + // One planted line per rule, each written so it trips exactly one pattern — + // verified so a neutered pattern changes the count, not just its sign, and a + // mutation to one pattern cannot hide behind another's hit. + const planted = [ + { path: "frontend/annotator/src/Bad.ts", text: 'const url = "https://models.robomous.ai/x";\n' }, + { path: "frontend/annotator/src/BadCloudflare.ts", text: "// hosted on Cloudflare R2\n" }, + { path: "frontend/browser-inference/src/BadCloudfront.ts", text: 'const url = "https://d123.cloudfront.net/x";\n' }, + { path: "frontend/browser-inference/src/BadAmazon.ts", text: 'const url = "https://bucket.s3.amazonaws.com/x";\n' }, + { path: "frontend/annotator/src/BadR2.ts", text: 'const url = "https://abc.r2.cloudflarestorage.com/x";\n' }, + ]; + assert.equal(violations(planted, CDN_VENDOR_LITERALS).length, planted.length); +}); + +test("the gate does NOT fire on the legitimate neighbouring form", () => { + const legitimate = [ + { path: "frontend/annotator/src/Fine.ts", text: "// this runtime may run in the cloud someday\n" }, + { path: "frontend/browser-inference/src/Fine.ts", text: 'const label = "amazon-style layout";\n' }, + ]; + assert.deepEqual(violations(legitimate, CDN_VENDOR_LITERALS), []); +}); From a9ccb4a3d62bb544946e4a9064c40b0256f7f8ca Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya <1445792+JArmandoAnaya@users.noreply.github.com> Date: Thu, 17 Sep 2026 01:51:21 -0700 Subject: [PATCH 17/42] test(app): extract serveApi/openJob into a shared _wireApiStub annotate.spec.ts's stubbed API route table was module-private, so a second Playwright spec had no way to reuse it without duplicating ~440 lines. Moved verbatim into frontend/app/e2e/_wireApiStub.ts, exporting the pieces other specs and annotate.spec.ts's own remaining tests need (serveApi, openJob, PROJECT/BATCH/JOB, progressStore, openedWorld); annotate.spec.ts now imports them instead of defining its own copy. --- frontend/app/e2e/_wireApiStub.ts | 444 ++++++++++++++++++++++++++++++ frontend/app/e2e/annotate.spec.ts | 436 +---------------------------- 2 files changed, 446 insertions(+), 434 deletions(-) create mode 100644 frontend/app/e2e/_wireApiStub.ts diff --git a/frontend/app/e2e/_wireApiStub.ts b/frontend/app/e2e/_wireApiStub.ts new file mode 100644 index 00000000..ddd29feb --- /dev/null +++ b/frontend/app/e2e/_wireApiStub.ts @@ -0,0 +1,444 @@ +/** + * `annotate.spec.ts`'s stubbed API, lifted out so a second spec can drive the same + * job without duplicating the route table. + * + * Everything is routed under `/api/`, which is where the app sends requests in + * development. Routing the bare paths would also intercept the *document* + * navigation, and the failure reads as "the shell disappeared". + */ + +import { expect, type Page, type Request } from "@playwright/test"; +import { assetActions, batchActions, jobActions, type Wire } from "./_wire"; + +export const PROJECT = "11111111-1111-4111-8111-111111111111"; +export const BATCH = "22222222-2222-4222-8222-222222222222"; +export const JOB = "33333333-3333-4333-8333-333333333333"; + +const SCHEMA = { + project_id: PROJECT, + version: 3, + classes: [ + { name: "vehicle", geometries: ["bbox"], color: "#38bdf8", attributes: [] }, + { name: "lane", geometries: ["polygon"], color: "#f97316", attributes: [] }, + // A second **bbox** class, so a reassignment has somewhere to land. + // It adds no tool — the palette is per geometry — and one hotkey row, which + // the shortcut-sheet scenario below counts. + { name: "pedestrian", geometries: ["bbox"], color: "#22c55e", attributes: [] }, + ], +} satisfies Wire["SchemaVersionOut"]; + +function asset( + index: number, + progress: Wire["AssetProgress"], + batchState: Wire["BatchState"] = "in_annotation", + jobState: Wire["AnnotationJobState"] = "in_progress", +): Wire["BatchAssetOut"] { + return { + id: `asset-${index}`, + project_id: PROJECT, + modality: "image", + content_hash: `${index}`.repeat(8) + "abcdef", + width: 640, + height: 480, + format: "png", + source_id: null, + frame_index: index, + frame_timestamp: null, + thumbnail_hash: "ab".repeat(32), + ingested_at: null, + job_id: JOB, + progress, + // Threaded from the batch **and from the job**, because that is what the + // server does: `asset_actions` returns `[]` for every frame of a batch that + // is not `in_annotation` and for every frame of a job that has been + // completed, whatever the frame's own progress is. Without the first a mock + // would declare `annotate` on a completed batch; without the second it would + // declare it on a finished job — and since the job's state is what the + // Finish press moves, that is the whole of the live transition below. + allowed_actions: assetActions(progress, { batchState, jobState }), + annotation_count: 0, + min_confidence: null, + }; +} + +/** A 1x1 PNG, so the canvas has real pixels to lay out. */ +const PIXEL = Buffer.from( + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8z8BQDwAEhQGAhKmMIQAAAABJRU5ErkJggg==", + "base64", +); + +/** + * The stub's progress, which a `PUT` actually moves. + * + * Every other piece of this stub is static, and that is right for a suite about + * what the page *sends*. Progress is the exception because the skip claims are + * about what the page *shows afterwards*: a `PUT` the server accepts and a listing that + * keeps answering the old value is exactly the state the defect looked like from + * the user's side, and a static stub would reproduce the bug rather than the fix. + */ +export function progressStore( + seed: Readonly>, +): Map { + return new Map(Object.entries(seed)); +} + +/** + * The lifecycle half of the stub: batch and job state that the two `start` + * POSTs actually move, on `progressStore`'s reasoning. The default is everything + * already open; the + * approved-batch scenarios are claims about the moves the page itself makes on + * open, and a stub whose state never moved would reproduce the bug rather than + * the fix. + */ +export interface Lifecycle { + batch: Wire["BatchState"]; + job: Wire["AnnotationJobState"]; + /** When set, `POST /batches/{id}/start` refuses 409 with this code instead. */ + refuseBatchStart?: string; + /** + * When set, `POST /jobs/{id}/start` refuses 409 with this code instead. + * + * The stale-read case, made deterministic: the client's cached `JobOut` + * says `pending` and declares `start`, while the server's job has already been + * started. In a real browser that window is opened by an invalidation whose + * refetch has not landed yet, which is why it only ever appeared on a loaded + * CI runner. Here it is simply what the stub answers, every time. + */ + refuseJobStart?: string; + /** When set, every `PUT .../progress` refuses 409 with this code instead. */ + refuseProgress?: string; + /** + * When set, every write to `/annotations` refuses 409 with this code and this + * message. + * + * The message is the interesting half: a code with no entry in `REFUSAL_PROSE` + * falls through to the server's own wording, which is how an install command — + * or a model reference — reaches a person verbatim. It is also the only way to + * put an arbitrarily long unbroken token on screen. + */ + refuseSave?: { code: string; message: string }; + /** When set, `POST /jobs/{id}/complete` refuses 409 with this code instead. */ + refuseJobComplete?: string; + /** + * Whether every asset is settled, which is what makes the job declare + * `complete`. Defaults true; the withheld Finish-job scenarios set it false. + */ + jobSettled?: boolean; +} + +export function openedWorld(): Lifecycle { + return { batch: "in_annotation", job: "in_progress" }; +} + +/** + * How many classes the served schema declares. + * + * Only the classes-region scenarios pass one — everything else wants the three + * `SCHEMA` names its assertions are written against. Padding rather than + * replacing, so a scenario asking for twelve still gets `vehicle` and `lane` + * where it expects them. + */ +export interface SchemaSize { + readonly classes?: number; +} + +function schemaOfSize(size: SchemaSize | undefined): Wire["SchemaVersionOut"] { + const want = size?.classes ?? SCHEMA.classes.length; + if (want <= SCHEMA.classes.length) return SCHEMA; + return { + ...SCHEMA, + classes: [ + ...SCHEMA.classes, + ...Array.from( + { length: want - SCHEMA.classes.length }, + (_unused, index): Wire["SchemaVersionOut"]["classes"][number] => ({ + name: `filler-${index + 1}`, + geometries: ["bbox"], + color: "#94a3b8", + attributes: [], + }), + ), + ], + }; +} + +/** + * A workspace with a segmenter in it, for the one scenario that needs the + * suggest tool to actually work. + * + * Off by default, because the interesting answer for every other test here is + * the empty list — that is the state the tool's explanation panel exists for, + * and it is what a workspace that has never been to the Models page is in. + */ +const READY_SAM = { + id: "66666666-6666-4666-8666-666666666666", + name: "local sam", + connection_type: "local", + model_id: "facebook/sam2-hiera-base-plus", + model_revision: "main", + device: "cuda", + precision: "fp16", + endpoint_url: null, + provider_id: "sam", + credential_env: null, + origin: "huggingface", + setup_state: "ready", + allowed_actions: [], + capabilities: ["point_suggest"], + produces: ["bbox", "polygon"], + download: null, + integrity_check: null, + created_at: "2026-08-08T00:00:00Z", + updated_at: "2026-08-08T00:00:00Z", +} satisfies Wire["ConnectionOut"]; + +export async function serveApi( + page: Page, + sent: Request[], + progress: Map = progressStore({ + "asset-1": "unannotated", + "asset-2": "annotated", + }), + lifecycle: Lifecycle = openedWorld(), + size?: SchemaSize, + seeded: readonly Wire["AnnotationOut"][] = [], + suggestible = false, +): Promise { + const stored: Wire["AnnotationOut"][] = [...seeded]; + const batchBody = (): Wire["BatchOut"] => ({ + id: BATCH, + project_id: PROJECT, + name: "drive-01", + state: lifecycle.batch, + schema_version: 3, + asset_count: 2, + allowed_actions: batchActions(lifecycle.batch), + promoted_asset_count: 0, + parent_batch_id: null, + pre_label_run: null, + progress: { + unannotated: 2, + pre_labeled: 0, + annotated: 0, + skipped: 0, + review_pending: 0, + accepted: 0, + total: 2, + }, + }); + const jobBody = (): Wire["JobOut"] => ({ + id: JOB, + batch_id: BATCH, + state: lifecycle.job, + asset_count: 2, + allowed_actions: jobActions(lifecycle.job, { + batchState: lifecycle.batch, + settled: lifecycle.jobSettled ?? true, + }), + assignee: null, + pre_label_run: null, + }); + await page.route("**/api/**", async (route) => { + const request = route.request(); + const path = new URL(request.url()).pathname.replace(/^\/api/, ""); + + // Answered before anything is recorded: every page load asks whether this + // server will sign the browser in by itself, and here it will not — + // this suite is about the annotation page, and it reaches it with a token. + if (path === "/session") return route.fulfill({ json: { issued: false } }); + + sent.push(request); + + if (path === `/jobs/${JOB}/start` && request.method() === "POST") { + if (lifecycle.refuseJobStart !== undefined) { + return route.fulfill({ + status: 409, + json: { code: lifecycle.refuseJobStart, message: "the kernel's own wording" }, + }); + } + lifecycle.job = "in_progress"; + return route.fulfill({ json: jobBody() }); + } + if (path === `/jobs/${JOB}/complete` && request.method() === "POST") { + if (lifecycle.refuseJobComplete !== undefined) { + return route.fulfill({ + status: 409, + json: { code: lifecycle.refuseJobComplete, message: "the kernel's own wording" }, + }); + } + lifecycle.job = "completed"; + return route.fulfill({ json: jobBody() }); + } + if (path === `/jobs/${JOB}`) { + return route.fulfill({ json: jobBody() }); + } + if (path === `/batches/${BATCH}/start` && request.method() === "POST") { + if (lifecycle.refuseBatchStart !== undefined) { + return route.fulfill({ + status: 409, + json: { code: lifecycle.refuseBatchStart, message: "the stub refuses" }, + }); + } + lifecycle.batch = "in_annotation"; + return route.fulfill({ json: batchBody() }); + } + if (path === `/batches/${BATCH}`) { + return route.fulfill({ json: batchBody() }); + } + if (path.endsWith("/schema/versions/3")) return route.fulfill({ json: schemaOfSize(size) }); + if (path.endsWith("/assets") && path.startsWith("/batches")) { + return route.fulfill({ + json: { + items: [ + asset(1, progress.get("asset-1") ?? "unannotated", lifecycle.batch, lifecycle.job), + asset(2, progress.get("asset-2") ?? "annotated", lifecycle.batch, lifecycle.job), + ], + total: 2, + } satisfies Wire["BatchAssetPage"], + }); + } + if (path.endsWith("/annotations") && request.method() === "GET") { + // **Per asset**, because the route is `/jobs/{id}/assets/{asset_id}/annotations` + // and that is what it answers. It used to hand back everything stored, which + // was harmless only while nothing was saved before navigating — the moment + // something was, the next frame's document was built from an annotation + // belonging to the previous one and `createDocument` refuses it outright. + // Cross-frame paste is what walks that path. + const assetId = path.split("/").at(-2) ?? ""; + const mine = stored.filter((one) => one.asset_id === assetId); + return route.fulfill({ + json: { items: mine, total: mine.length } satisfies Wire["AnnotationPage"], + }); + } + if (path.endsWith("/annotations") && request.method() !== "GET" && lifecycle.refuseSave !== undefined) { + return route.fulfill({ status: 409, json: lifecycle.refuseSave }); + } + if (path.endsWith("/annotations") && request.method() === "POST") { + // Kept, and stamped with a server id — the kernel mints its own and the page + // refetches to learn them (`jobQueries.ts`). A stub that answered an empty + // list would leave the page permanently dirty and "Saved" unreachable, which + // says nothing about the product. + const body = JSON.parse(request.postData() ?? "[]") as Wire["AnnotationCreate"][]; + body.forEach((one, at) => + stored.push({ + ...one, + id: `server-${stored.length + at}`, + // `asset_id` is the client's own, not a literal: `AnnotationCreate` + // carries it and the kernel writes the label against it. + schema_version: 3, + attributes: {}, + provenance: "human", + model_ref: null, + confidence: null, + job_id: null, + }), + ); + const written = stored.filter((one) => one.asset_id === body[0]?.asset_id); + return route.fulfill({ + status: 201, + json: { items: written, total: written.length } satisfies Wire["AnnotationPage"], + }); + } + if (path.endsWith("/progress") && request.method() === "GET") { + // **Derived from the same map the PUTs move**, not a frozen literal. It + // was a literal — `unannotated: 2, annotated: 0` — which meant the counts + // described a job nobody had touched however far the test had walked it, + // and any claim about the readout was a claim about the stub. That is the + // habit worth making impossible: a mock that answers something the + // endpoint would never have sent is worse than no mock. + const states = [...progress.values()]; + const count = (of: Wire["AssetProgress"]): number => + states.filter((one) => one === of).length; + return route.fulfill({ + json: { + unannotated: count("unannotated"), + pre_labeled: count("pre_labeled"), + annotated: count("annotated"), + skipped: count("skipped"), + review_pending: count("review_pending"), + accepted: count("accepted"), + total: states.length, + } satisfies Wire["ProgressCounts"], + }); + } + if (path.endsWith("/progress") && request.method() === "PUT") { + if (lifecycle.refuseProgress !== undefined) { + return route.fulfill({ + status: 409, + json: { code: lifecycle.refuseProgress, message: "the kernel's own wording" }, + }); + } + const assetId = path.split("/").at(-2) ?? ""; + const body = JSON.parse(request.postData() ?? "{}") as { + progress?: Wire["AssetProgress"]; + }; + if (body.progress !== undefined) progress.set(assetId, body.progress); + // `AssetProgressOut`, not `{}`. The route answers where the asset now is, and a + // stub that answered an empty object was describing a response the endpoint has + // never sent. + return route.fulfill({ + status: 200, + json: { + asset_id: assetId, + progress: progress.get(assetId) ?? "unannotated", + } satisfies Wire["AssetProgressOut"], + }); + } + if (path.endsWith("/content") || path.endsWith("/thumbnail")) { + return route.fulfill({ contentType: "image/png", body: PIXEL }); + } + if (path === "/projects") { + return route.fulfill({ json: { items: [], total: 0 } satisfies Wire["ProjectPage"] }); + } + // The suggest tool's own read. Empty is the interesting answer + // here: it is the state the panel's explanation exists for, and it is what a workspace + // that has never been to the Models page is in. + if (path === "/inference/connections") { + const items = suggestible ? [READY_SAM] : []; + return route.fulfill({ + json: { items, total: items.length } satisfies Wire["ConnectionPage"], + }); + } + if (path === "/inference/suggest" && request.method() === "POST") { + // `SuggestionOut`, in full: the score rides on the answer, the shapes are + // a list, each carries the contour it was reduced from, and `parameters` + // declares which settings apply to this kind. Every field is required, and + // a shape missing one is refused by the generated runtime check — which + // reads as "the server answered something this app does not recognise" and + // looks nothing like a stub bug. + return route.fulfill({ + json: { + model_ref: "facebook/sam2-hiera-base-plus@main", + confidence: 0.91, + regions: [ + { + geometry: { type: "bbox", x: 100, y: 100, width: 80, height: 60 }, + contour: [], + }, + ], + applied: { tolerance: 1 }, + // A box class, so the wire names no settings at all — which is how the + // editor is told to render no adjustments section (#557). + parameters: [], + } satisfies Wire["SuggestionOut"], + }); + } + return route.fulfill({ status: 500, json: { code: "NO_STUB", message: path } }); + }); +} + +export async function openJob( + page: Page, + sent: Request[], + progress?: Map, + lifecycle?: Lifecycle, + size?: SchemaSize, + seeded?: readonly Wire["AnnotationOut"][], + suggestible?: boolean, +): Promise { + await serveApi(page, sent, progress, lifecycle, size, seeded, suggestible); + await page.goto(`/jobs/${JOB}`); + await page.getByTestId("token-input").fill("a-token"); + await page.getByTestId("token-submit").click(); + await expect(page.getByTestId("annotation-page")).toBeVisible(); +} diff --git a/frontend/app/e2e/annotate.spec.ts b/frontend/app/e2e/annotate.spec.ts index 3e855620..dcf47c28 100644 --- a/frontend/app/e2e/annotate.spec.ts +++ b/frontend/app/e2e/annotate.spec.ts @@ -12,7 +12,7 @@ */ import { expect, test, type Page, type Request } from "@playwright/test"; -import { assetActions, batchActions, jobActions, type Wire } from "./_wire"; +import type { Wire } from "./_wire"; import { closeOverflow, expectNothingToSave, @@ -21,439 +21,7 @@ import { saveNow, zoomWheel, } from "./_frame"; - -const PROJECT = "11111111-1111-4111-8111-111111111111"; -const BATCH = "22222222-2222-4222-8222-222222222222"; -const JOB = "33333333-3333-4333-8333-333333333333"; - -const SCHEMA = { - project_id: PROJECT, - version: 3, - classes: [ - { name: "vehicle", geometries: ["bbox"], color: "#38bdf8", attributes: [] }, - { name: "lane", geometries: ["polygon"], color: "#f97316", attributes: [] }, - // A second **bbox** class, so a reassignment has somewhere to land. - // It adds no tool — the palette is per geometry — and one hotkey row, which - // the shortcut-sheet scenario below counts. - { name: "pedestrian", geometries: ["bbox"], color: "#22c55e", attributes: [] }, - ], -} satisfies Wire["SchemaVersionOut"]; - -function asset( - index: number, - progress: Wire["AssetProgress"], - batchState: Wire["BatchState"] = "in_annotation", - jobState: Wire["AnnotationJobState"] = "in_progress", -): Wire["BatchAssetOut"] { - return { - id: `asset-${index}`, - project_id: PROJECT, - modality: "image", - content_hash: `${index}`.repeat(8) + "abcdef", - width: 640, - height: 480, - format: "png", - source_id: null, - frame_index: index, - frame_timestamp: null, - thumbnail_hash: "ab".repeat(32), - ingested_at: null, - job_id: JOB, - progress, - // Threaded from the batch **and from the job**, because that is what the - // server does: `asset_actions` returns `[]` for every frame of a batch that - // is not `in_annotation` and for every frame of a job that has been - // completed, whatever the frame's own progress is. Without the first a mock - // would declare `annotate` on a completed batch; without the second it would - // declare it on a finished job — and since the job's state is what the - // Finish press moves, that is the whole of the live transition below. - allowed_actions: assetActions(progress, { batchState, jobState }), - annotation_count: 0, - min_confidence: null, - }; -} - -/** A 1x1 PNG, so the canvas has real pixels to lay out. */ -const PIXEL = Buffer.from( - "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8z8BQDwAEhQGAhKmMIQAAAABJRU5ErkJggg==", - "base64", -); - -/** - * The stub's progress, which a `PUT` actually moves. - * - * Every other piece of this stub is static, and that is right for a suite about - * what the page *sends*. Progress is the exception because the skip claims are - * about what the page *shows afterwards*: a `PUT` the server accepts and a listing that - * keeps answering the old value is exactly the state the defect looked like from - * the user's side, and a static stub would reproduce the bug rather than the fix. - */ -function progressStore( - seed: Readonly>, -): Map { - return new Map(Object.entries(seed)); -} - -/** - * The lifecycle half of the stub: batch and job state that the two `start` - * POSTs actually move, on `progressStore`'s reasoning. The default is everything - * already open; the - * approved-batch scenarios are claims about the moves the page itself makes on - * open, and a stub whose state never moved would reproduce the bug rather than - * the fix. - */ -interface Lifecycle { - batch: Wire["BatchState"]; - job: Wire["AnnotationJobState"]; - /** When set, `POST /batches/{id}/start` refuses 409 with this code instead. */ - refuseBatchStart?: string; - /** - * When set, `POST /jobs/{id}/start` refuses 409 with this code instead. - * - * The stale-read case, made deterministic: the client's cached `JobOut` - * says `pending` and declares `start`, while the server's job has already been - * started. In a real browser that window is opened by an invalidation whose - * refetch has not landed yet, which is why it only ever appeared on a loaded - * CI runner. Here it is simply what the stub answers, every time. - */ - refuseJobStart?: string; - /** When set, every `PUT .../progress` refuses 409 with this code instead. */ - refuseProgress?: string; - /** - * When set, every write to `/annotations` refuses 409 with this code and this - * message. - * - * The message is the interesting half: a code with no entry in `REFUSAL_PROSE` - * falls through to the server's own wording, which is how an install command — - * or a model reference — reaches a person verbatim. It is also the only way to - * put an arbitrarily long unbroken token on screen. - */ - refuseSave?: { code: string; message: string }; - /** When set, `POST /jobs/{id}/complete` refuses 409 with this code instead. */ - refuseJobComplete?: string; - /** - * Whether every asset is settled, which is what makes the job declare - * `complete`. Defaults true; the withheld Finish-job scenarios set it false. - */ - jobSettled?: boolean; -} - -function openedWorld(): Lifecycle { - return { batch: "in_annotation", job: "in_progress" }; -} - -/** - * How many classes the served schema declares. - * - * Only the classes-region scenarios pass one — everything else wants the three - * `SCHEMA` names its assertions are written against. Padding rather than - * replacing, so a scenario asking for twelve still gets `vehicle` and `lane` - * where it expects them. - */ -interface SchemaSize { - readonly classes?: number; -} - -function schemaOfSize(size: SchemaSize | undefined): Wire["SchemaVersionOut"] { - const want = size?.classes ?? SCHEMA.classes.length; - if (want <= SCHEMA.classes.length) return SCHEMA; - return { - ...SCHEMA, - classes: [ - ...SCHEMA.classes, - ...Array.from( - { length: want - SCHEMA.classes.length }, - (_unused, index): Wire["SchemaVersionOut"]["classes"][number] => ({ - name: `filler-${index + 1}`, - geometries: ["bbox"], - color: "#94a3b8", - attributes: [], - }), - ), - ], - }; -} - -/** - * A workspace with a segmenter in it, for the one scenario that needs the - * suggest tool to actually work. - * - * Off by default, because the interesting answer for every other test here is - * the empty list — that is the state the tool's explanation panel exists for, - * and it is what a workspace that has never been to the Models page is in. - */ -const READY_SAM = { - id: "66666666-6666-4666-8666-666666666666", - name: "local sam", - connection_type: "local", - model_id: "facebook/sam2-hiera-base-plus", - model_revision: "main", - device: "cuda", - precision: "fp16", - endpoint_url: null, - provider_id: "sam", - credential_env: null, - origin: "huggingface", - setup_state: "ready", - allowed_actions: [], - capabilities: ["point_suggest"], - produces: ["bbox", "polygon"], - download: null, - integrity_check: null, - created_at: "2026-08-08T00:00:00Z", - updated_at: "2026-08-08T00:00:00Z", -} satisfies Wire["ConnectionOut"]; - -async function serveApi( - page: Page, - sent: Request[], - progress: Map = progressStore({ - "asset-1": "unannotated", - "asset-2": "annotated", - }), - lifecycle: Lifecycle = openedWorld(), - size?: SchemaSize, - seeded: readonly Wire["AnnotationOut"][] = [], - suggestible = false, -): Promise { - const stored: Wire["AnnotationOut"][] = [...seeded]; - const batchBody = (): Wire["BatchOut"] => ({ - id: BATCH, - project_id: PROJECT, - name: "drive-01", - state: lifecycle.batch, - schema_version: 3, - asset_count: 2, - allowed_actions: batchActions(lifecycle.batch), - promoted_asset_count: 0, - parent_batch_id: null, - pre_label_run: null, - progress: { - unannotated: 2, - pre_labeled: 0, - annotated: 0, - skipped: 0, - review_pending: 0, - accepted: 0, - total: 2, - }, - }); - const jobBody = (): Wire["JobOut"] => ({ - id: JOB, - batch_id: BATCH, - state: lifecycle.job, - asset_count: 2, - allowed_actions: jobActions(lifecycle.job, { - batchState: lifecycle.batch, - settled: lifecycle.jobSettled ?? true, - }), - assignee: null, - pre_label_run: null, - }); - await page.route("**/api/**", async (route) => { - const request = route.request(); - const path = new URL(request.url()).pathname.replace(/^\/api/, ""); - - // Answered before anything is recorded: every page load asks whether this - // server will sign the browser in by itself, and here it will not — - // this suite is about the annotation page, and it reaches it with a token. - if (path === "/session") return route.fulfill({ json: { issued: false } }); - - sent.push(request); - - if (path === `/jobs/${JOB}/start` && request.method() === "POST") { - if (lifecycle.refuseJobStart !== undefined) { - return route.fulfill({ - status: 409, - json: { code: lifecycle.refuseJobStart, message: "the kernel's own wording" }, - }); - } - lifecycle.job = "in_progress"; - return route.fulfill({ json: jobBody() }); - } - if (path === `/jobs/${JOB}/complete` && request.method() === "POST") { - if (lifecycle.refuseJobComplete !== undefined) { - return route.fulfill({ - status: 409, - json: { code: lifecycle.refuseJobComplete, message: "the kernel's own wording" }, - }); - } - lifecycle.job = "completed"; - return route.fulfill({ json: jobBody() }); - } - if (path === `/jobs/${JOB}`) { - return route.fulfill({ json: jobBody() }); - } - if (path === `/batches/${BATCH}/start` && request.method() === "POST") { - if (lifecycle.refuseBatchStart !== undefined) { - return route.fulfill({ - status: 409, - json: { code: lifecycle.refuseBatchStart, message: "the stub refuses" }, - }); - } - lifecycle.batch = "in_annotation"; - return route.fulfill({ json: batchBody() }); - } - if (path === `/batches/${BATCH}`) { - return route.fulfill({ json: batchBody() }); - } - if (path.endsWith("/schema/versions/3")) return route.fulfill({ json: schemaOfSize(size) }); - if (path.endsWith("/assets") && path.startsWith("/batches")) { - return route.fulfill({ - json: { - items: [ - asset(1, progress.get("asset-1") ?? "unannotated", lifecycle.batch, lifecycle.job), - asset(2, progress.get("asset-2") ?? "annotated", lifecycle.batch, lifecycle.job), - ], - total: 2, - } satisfies Wire["BatchAssetPage"], - }); - } - if (path.endsWith("/annotations") && request.method() === "GET") { - // **Per asset**, because the route is `/jobs/{id}/assets/{asset_id}/annotations` - // and that is what it answers. It used to hand back everything stored, which - // was harmless only while nothing was saved before navigating — the moment - // something was, the next frame's document was built from an annotation - // belonging to the previous one and `createDocument` refuses it outright. - // Cross-frame paste is what walks that path. - const assetId = path.split("/").at(-2) ?? ""; - const mine = stored.filter((one) => one.asset_id === assetId); - return route.fulfill({ - json: { items: mine, total: mine.length } satisfies Wire["AnnotationPage"], - }); - } - if (path.endsWith("/annotations") && request.method() !== "GET" && lifecycle.refuseSave !== undefined) { - return route.fulfill({ status: 409, json: lifecycle.refuseSave }); - } - if (path.endsWith("/annotations") && request.method() === "POST") { - // Kept, and stamped with a server id — the kernel mints its own and the page - // refetches to learn them (`jobQueries.ts`). A stub that answered an empty - // list would leave the page permanently dirty and "Saved" unreachable, which - // says nothing about the product. - const body = JSON.parse(request.postData() ?? "[]") as Wire["AnnotationCreate"][]; - body.forEach((one, at) => - stored.push({ - ...one, - id: `server-${stored.length + at}`, - // `asset_id` is the client's own, not a literal: `AnnotationCreate` - // carries it and the kernel writes the label against it. - schema_version: 3, - attributes: {}, - provenance: "human", - model_ref: null, - confidence: null, - job_id: null, - }), - ); - const written = stored.filter((one) => one.asset_id === body[0]?.asset_id); - return route.fulfill({ - status: 201, - json: { items: written, total: written.length } satisfies Wire["AnnotationPage"], - }); - } - if (path.endsWith("/progress") && request.method() === "GET") { - // **Derived from the same map the PUTs move**, not a frozen literal. It - // was a literal — `unannotated: 2, annotated: 0` — which meant the counts - // described a job nobody had touched however far the test had walked it, - // and any claim about the readout was a claim about the stub. That is the - // habit worth making impossible: a mock that answers something the - // endpoint would never have sent is worse than no mock. - const states = [...progress.values()]; - const count = (of: Wire["AssetProgress"]): number => - states.filter((one) => one === of).length; - return route.fulfill({ - json: { - unannotated: count("unannotated"), - pre_labeled: count("pre_labeled"), - annotated: count("annotated"), - skipped: count("skipped"), - review_pending: count("review_pending"), - accepted: count("accepted"), - total: states.length, - } satisfies Wire["ProgressCounts"], - }); - } - if (path.endsWith("/progress") && request.method() === "PUT") { - if (lifecycle.refuseProgress !== undefined) { - return route.fulfill({ - status: 409, - json: { code: lifecycle.refuseProgress, message: "the kernel's own wording" }, - }); - } - const assetId = path.split("/").at(-2) ?? ""; - const body = JSON.parse(request.postData() ?? "{}") as { - progress?: Wire["AssetProgress"]; - }; - if (body.progress !== undefined) progress.set(assetId, body.progress); - // `AssetProgressOut`, not `{}`. The route answers where the asset now is, and a - // stub that answered an empty object was describing a response the endpoint has - // never sent. - return route.fulfill({ - status: 200, - json: { - asset_id: assetId, - progress: progress.get(assetId) ?? "unannotated", - } satisfies Wire["AssetProgressOut"], - }); - } - if (path.endsWith("/content") || path.endsWith("/thumbnail")) { - return route.fulfill({ contentType: "image/png", body: PIXEL }); - } - if (path === "/projects") { - return route.fulfill({ json: { items: [], total: 0 } satisfies Wire["ProjectPage"] }); - } - // The suggest tool's own read. Empty is the interesting answer - // here: it is the state the panel's explanation exists for, and it is what a workspace - // that has never been to the Models page is in. - if (path === "/inference/connections") { - const items = suggestible ? [READY_SAM] : []; - return route.fulfill({ - json: { items, total: items.length } satisfies Wire["ConnectionPage"], - }); - } - if (path === "/inference/suggest" && request.method() === "POST") { - // `SuggestionOut`, in full: the score rides on the answer, the shapes are - // a list, each carries the contour it was reduced from, and `parameters` - // declares which settings apply to this kind. Every field is required, and - // a shape missing one is refused by the generated runtime check — which - // reads as "the server answered something this app does not recognise" and - // looks nothing like a stub bug. - return route.fulfill({ - json: { - model_ref: "facebook/sam2-hiera-base-plus@main", - confidence: 0.91, - regions: [ - { - geometry: { type: "bbox", x: 100, y: 100, width: 80, height: 60 }, - contour: [], - }, - ], - applied: { tolerance: 1 }, - // A box class, so the wire names no settings at all — which is how the - // editor is told to render no adjustments section (#557). - parameters: [], - } satisfies Wire["SuggestionOut"], - }); - } - return route.fulfill({ status: 500, json: { code: "NO_STUB", message: path } }); - }); -} - -async function openJob( - page: Page, - sent: Request[], - progress?: Map, - lifecycle?: Lifecycle, - size?: SchemaSize, - seeded?: readonly Wire["AnnotationOut"][], - suggestible?: boolean, -): Promise { - await serveApi(page, sent, progress, lifecycle, size, seeded, suggestible); - await page.goto(`/jobs/${JOB}`); - await page.getByTestId("token-input").fill("a-token"); - await page.getByTestId("token-submit").click(); - await expect(page.getByTestId("annotation-page")).toBeVisible(); -} +import { BATCH, JOB, openedWorld, openJob, PROJECT, progressStore, serveApi } from "./_wireApiStub"; test("the page loads the job's assets, its pinned schema and its progress", async ({ page }) => { const sent: Request[] = []; From a05734642df98ac5fbca6ccecd5f2016193c3a12 Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya <1445792+JArmandoAnaya@users.noreply.github.com> Date: Thu, 17 Sep 2026 01:51:23 -0700 Subject: [PATCH 18/42] test(app): add hermetic Playwright coverage for browser suggestion --- frontend/app/e2e/browserSuggestion.spec.ts | 191 +++++++++++++++++++++ 1 file changed, 191 insertions(+) create mode 100644 frontend/app/e2e/browserSuggestion.spec.ts diff --git a/frontend/app/e2e/browserSuggestion.spec.ts b/frontend/app/e2e/browserSuggestion.spec.ts new file mode 100644 index 00000000..a645e514 --- /dev/null +++ b/frontend/app/e2e/browserSuggestion.spec.ts @@ -0,0 +1,191 @@ +/** + * Browser-local point suggestion, end to end: EfficientSAM-Ti runs in a real Chromium + * against the real editor, using the Phase C hermetic fixture (never models.robomous.ai — + * the real CDN is proved separately, once, by hand — see Task 16). Skipped whole-file when + * the fixture is absent; VISIONSET_REQUIRE_BROWSER_MODELS=1 turns that into a hard failure, + * the same bargain frontend/browser-inference/browser/efficientSam.spec.ts strikes. + * + * The Server/"This device" choice is a real `Tabs` component (`SuggestPanel.tsx`) + * defaulting to the "server" tab, so every browser-target scenario here clicks + * `suggest-target-browser` before touching the acquire button. There is no dedicated + * "ready" testid: `DeviceTab` renders a plain `Ready` once + * `listTargets()` answers non-empty, so "now ready" is read as that text appearing + * inside `suggest-device-section` — and a failed acquisition renders `role="alert"` + * beside the still-present acquire button (`SuggestPanel.tsx`'s `DeviceTab`). + */ +import { existsSync, readFileSync } from "node:fs"; +import path from "node:path"; +import { expect, test, type Page, type Request } from "@playwright/test"; +import { openJob } from "./_wireApiStub"; + +const ARTIFACTS_DIR = path.resolve( + import.meta.dirname, "..", "..", "browser-inference", "model-artifacts", "efficientsam-ti", +); +const ENCODER_PATH = path.join(ARTIFACTS_DIR, "encoder.onnx"); +const DECODER_PATH = path.join(ARTIFACTS_DIR, "decoder.onnx"); +const HAS_ARTIFACTS = existsSync(ENCODER_PATH) && existsSync(DECODER_PATH); +const REQUIRE_ENV = "VISIONSET_REQUIRE_BROWSER_MODELS"; +const MISSING_MESSAGE = + "model-artifacts/efficientsam-ti/{encoder.onnx,decoder.onnx} are not on disk — see " + + "frontend/browser-inference/browser/efficientSam.spec.ts for how to build them."; + +if (process.env[REQUIRE_ENV] === "1" && !HAS_ARTIFACTS) { + throw new Error(`${MISSING_MESSAGE}\n\n${REQUIRE_ENV}=1 is set, so this is an error rather than a skip.`); +} + +const ENCODER_BYTES = HAS_ARTIFACTS ? readFileSync(ENCODER_PATH) : Buffer.alloc(0); +const DECODER_BYTES = HAS_ARTIFACTS ? readFileSync(DECODER_PATH) : Buffer.alloc(0); + +/** Routes the CDN manifest + artifacts to the local fixture — no network call ever leaves the page. */ +async function mockCdn(page: Page, encoderBytes = ENCODER_BYTES, decoderBytes = DECODER_BYTES): Promise { + await page.route("**/models.robomous.ai/models/efficient-sam-ti/**/manifest.json", (route) => + route.fulfill({ json: { encoder: { path: "/encoder.onnx" }, decoder: { path: "/decoder.onnx" } } }), + ); + await page.route("**/models.robomous.ai/encoder.onnx", (route) => + route.fulfill({ body: encoderBytes, contentType: "application/octet-stream" }), + ); + await page.route("**/models.robomous.ai/decoder.onnx", (route) => + route.fulfill({ body: decoderBytes, contentType: "application/octet-stream" }), + ); +} + +async function openJobWithBrowserRuntime( + page: Page, + sent: Request[], + suggestible: boolean, +): Promise { + await mockCdn(page); + await openJob(page, sent, undefined, undefined, undefined, undefined, suggestible); +} + +function suggestCallsOf(sent: Request[]): Request[] { + return sent.filter((r) => r.method() === "POST" && r.url().endsWith("/inference/suggest")); +} + +/** + * Counts requests to the model CDN from here on — via `page.on("request", ...)` + * rather than a second `page.route`, so it never interferes with `mockCdn`'s own + * fulfil handlers (a later-added `page.route` for the same pattern would run first + * and could shadow them). + */ +function countModelRequestsFromNow(page: Page): () => number { + let count = 0; + page.on("request", (request) => { + if (request.url().includes("models.robomous.ai")) count += 1; + }); + return () => count; +} + +/** + * Clicks into the "This device" tab and downloads the model, then waits for the + * real "ready" signal: `DeviceTab` swaps the acquire button for a `Badge` reading + * "Ready" once `listTargets()` answers non-empty (`SuggestPanel.tsx`). There is no + * separate target-selection control to click — Phase F ships exactly one browser + * target, and choosing the tab already set it as the active suggestion target. + */ +async function acquireAndSelectBrowserTarget(page: Page): Promise { + await page.getByTestId("tool-suggest").click(); + await page.getByTestId("suggest-target-browser").click(); + await page.getByTestId("suggest-device-acquire-efficient-sam-ti").click(); + await expect(page.getByTestId("suggest-device-section").getByText(/ready/i)).toBeVisible({ timeout: 60_000 }); +} + +test.describe("browser suggestion", () => { + test.skip(!HAS_ARTIFACTS, MISSING_MESSAGE); + + test("Server target: a click issues exactly one /inference/suggest HTTP request", async ({ page }) => { + const sent: Request[] = []; + await openJobWithBrowserRuntime(page, sent, true); + await page.getByTestId("tool-suggest").click(); + await expect(page.getByTestId("suggest-idle")).toBeVisible(); + const picture = (await page.getByTestId("annotator-canvas").boundingBox())!; + await page.mouse.click(picture.x + picture.width / 2, picture.y + picture.height / 2); + await expect(page.getByTestId("suggestion-shape")).toBeVisible(); + expect(suggestCallsOf(sent)).toHaveLength(1); + }); + + test("This device target: a click never issues an /inference/suggest HTTP request", async ({ page }) => { + const sent: Request[] = []; + await openJobWithBrowserRuntime(page, sent, true); + await acquireAndSelectBrowserTarget(page); + const picture = (await page.getByTestId("annotator-canvas").boundingBox())!; + await page.mouse.click(picture.x + picture.width / 2, picture.y + picture.height / 2); + await expect(page.getByTestId("suggestion-shape")).toBeVisible({ timeout: 30_000 }); + expect(suggestCallsOf(sent)).toHaveLength(0); + }); + + test("a ready browser target is never blocked by a server-connection blocker", async ({ page }) => { + const sent: Request[] = []; + await openJobWithBrowserRuntime(page, sent, false); // no server connections + await page.getByTestId("tool-suggest").click(); + await expect(page.getByTestId("suggest-no-connections")).toBeVisible(); + await acquireAndSelectBrowserTarget(page); + await expect(page.getByTestId("suggest-no-connections")).not.toBeVisible(); + const picture = (await page.getByTestId("annotator-canvas").boundingBox())!; + await page.mouse.click(picture.x + picture.width / 2, picture.y + picture.height / 2); + await expect(page.getByTestId("suggestion-shape")).toBeVisible({ timeout: 30_000 }); + }); + + test("the model is never fetched before the user presses Download", async ({ page }) => { + const sent: Request[] = []; + await openJobWithBrowserRuntime(page, sent, true); + const modelRequests = countModelRequestsFromNow(page); + await page.getByTestId("tool-suggest").click(); + const picture = (await page.getByTestId("annotator-canvas").boundingBox())!; + await page.mouse.click(picture.x + picture.width / 2, picture.y + picture.height / 2); + await expect(page.getByTestId("suggestion-shape")).toBeVisible(); + expect(modelRequests()).toBe(0); + }); + + test("a SHA-256 mismatch on the encoder hard-fails acquisition with no target exposed", async ({ page }) => { + const sent: Request[] = []; + const wrongBytes = Buffer.from("not the real encoder, deliberately wrong length and hash"); + await openJobWithBrowserRuntime(page, sent, true); + // mockCdn already ran with the real bytes inside openJobWithBrowserRuntime; re-route + // the encoder specifically — Playwright tries the most-recently-added matching + // handler first, so this one now answers every encoder.onnx request. + await page.route("**/models.robomous.ai/encoder.onnx", (route) => + route.fulfill({ body: wrongBytes, contentType: "application/octet-stream" }), + ); + await page.getByTestId("tool-suggest").click(); + await page.getByTestId("suggest-target-browser").click(); + await page.getByTestId("suggest-device-acquire-efficient-sam-ti").click(); + await expect(page.getByRole("alert")).toBeVisible({ timeout: 30_000 }); + await expect(page.getByTestId("suggest-device-section").getByText(/ready/i)).toHaveCount(0); + await expect(page.getByTestId("suggest-device-acquire-efficient-sam-ti")).toBeVisible(); + }); + + test("two refinements on one asset never re-fetch the model", async ({ page }) => { + const sent: Request[] = []; + await openJobWithBrowserRuntime(page, sent, true); + await acquireAndSelectBrowserTarget(page); + const modelRequests = countModelRequestsFromNow(page); + const picture = (await page.getByTestId("annotator-canvas").boundingBox())!; + await page.mouse.click(picture.x + picture.width / 2, picture.y + picture.height / 2); + await expect(page.getByTestId("suggestion-shape")).toBeVisible({ timeout: 30_000 }); + await page.mouse.click(picture.x + picture.width / 2 + 10, picture.y + picture.height / 2 + 10); + await expect(page.getByTestId("suggestion-shape")).toBeVisible({ timeout: 30_000 }); + expect(modelRequests()).toBe(0); + }); + + test("a negative-point ask on the browser target refuses deterministically, with no HTTP and no Server fallback", async ({ page }) => { + const sent: Request[] = []; + await openJobWithBrowserRuntime(page, sent, true); + await acquireAndSelectBrowserTarget(page); + const picture = (await page.getByTestId("annotator-canvas").boundingBox())!; + await page.mouse.click(picture.x + picture.width / 2, picture.y + picture.height / 2); + await expect(page.getByTestId("suggestion-shape")).toBeVisible({ timeout: 30_000 }); + + await page.keyboard.down("Alt"); + await page.mouse.click(picture.x + picture.width / 2 + 20, picture.y + picture.height / 2 + 20); + await page.keyboard.up("Alt"); + + // "Refused", not "answered nothing": the executor throws before ever calling the + // model (`BrowserSuggestionExecutor.ts`), and the panel's refusal card carries its + // message verbatim, same as a server refusal would. + await expect(page.getByTestId("suggest-refusal")).toHaveText(/positive-point refinement only/i); + // Still on the browser tab — a refusal never silently falls back to Server. + await expect(page.getByTestId("suggest-target-browser")).toHaveAttribute("aria-selected", "true"); + expect(suggestCallsOf(sent)).toHaveLength(0); + }); +}); From 53f19ebac7fab556f6d8d73a7878a396851693f9 Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya <1445792+JArmandoAnaya@users.noreply.github.com> Date: Thu, 17 Sep 2026 02:04:08 -0700 Subject: [PATCH 19/42] fix(ui-core): let a browser target's chooser survive its own not-ready blocker --- .../ui-core/src/annotator/SuggestPanel.tsx | 105 +++++++++++++----- .../src/annotator/suggestPanel.test.tsx | 50 +++++++++ 2 files changed, 129 insertions(+), 26 deletions(-) diff --git a/frontend/ui-core/src/annotator/SuggestPanel.tsx b/frontend/ui-core/src/annotator/SuggestPanel.tsx index dce6fe48..e7aa7d60 100644 --- a/frontend/ui-core/src/annotator/SuggestPanel.tsx +++ b/frontend/ui-core/src/annotator/SuggestPanel.tsx @@ -274,7 +274,19 @@ export function SuggestPanel({ // The blocker outranks everything else: a session over a workspace with no // usable connection has nothing to report about a request it never made. - if (blocker !== null && blocker !== undefined) { + // + // Only when no browser runtime is wired at all. Once one is, a server-side + // blocker (typically "not-ready" for a browser target still sitting in + // `browserAcquisitions`) is a fact about the *server* tab, not the whole + // panel — the panel still has a working "This device" tab to offer, and + // this early return must not hide it. See `TargetChooser`, which renders + // this same `BLOCKER_COPY` message scoped to its server tab instead. + if ( + browserTargets === undefined && + browserAcquisitions === undefined && + blocker !== null && + blocker !== undefined + ) { const copy = BLOCKER_COPY[blocker]; return ( -

- {copy.title} -

-

{copy.body}

- {/* The action's *destination* is the host's, so its absence removes the - control and leaves the explanation — never a dead button. */} - {copy.action !== null && onConfigure !== undefined && ( - - )} + ); } @@ -464,6 +460,8 @@ export function SuggestPanel({ activeTarget={activeTarget} {...(onChooseTarget === undefined ? {} : { onChooseTarget })} {...(onAcquired === undefined ? {} : { onAcquired })} + blocker={blocker ?? null} + {...(onConfigure === undefined ? {} : { onConfigure })} /> )} @@ -534,7 +532,14 @@ function Through({ * runtime exists, so a segmented control reads better than a `Select` built for * an open-ended candidate list. The server tab holds exactly what rendered * before this component existed — `Through`/`Discard`, untouched — so choosing - * "Server" is never a behavior change from the pre-Task-6 panel. + * "Server" is never a behavior change from the pre-Task-6 panel, *except* that + * a server-side `blocker` now renders inside this tab rather than replacing + * the whole card: `computeSuggestBlocker` answers "not-ready" for a browser + * target that has not been acquired yet, and that answer must never hide the + * "This device" tab's own download control (the bug this component's second + * revision exists to fix — a person who picked "This device" could never + * reach `DeviceTab`'s button, because picking it made `blocker` fire and blank + * the whole panel out from under the tabs). */ function TargetChooser({ candidates, @@ -547,6 +552,8 @@ function TargetChooser({ activeTarget, onChooseTarget, onAcquired, + blocker, + onConfigure, }: { readonly candidates: readonly Connection[]; readonly connectionId: string | null; @@ -558,6 +565,8 @@ function TargetChooser({ readonly activeTarget: ActiveSuggestionTarget | undefined; readonly onChooseTarget?: (target: ActiveSuggestionTarget) => void; readonly onAcquired?: () => void; + readonly blocker: SuggestBlocker | null; + readonly onConfigure?: () => void; }): JSX.Element { const value = activeTarget?.kind ?? "server"; @@ -584,15 +593,27 @@ function TargetChooser({ - {!pending && ( - + {blocker !== null ? ( + + ) : ( + <> + {!pending && ( + + )} + {pending && } + )} - {pending && } + {/* + Never gated on `blocker` — a server-side "not-ready"/"no-connections" + is a fact about the server tab and says nothing about whether this + device can run something. `DeviceTab` reads only `browserTargets`/ + `browserAcquisitions`, which is its own, independent readiness. + */} void; +}): JSX.Element { + const copy = BLOCKER_COPY[blocker]; + return ( + <> +

+ {copy.title} +

+

{copy.body}

+ {/* The action's *destination* is the host's, so its absence removes the + control and leaves the explanation — never a dead button. */} + {copy.action !== null && onConfigure !== undefined && ( + + )} + + ); +} + /** * What "this device" has to say: a ready target's name and a success pill, or * an unacquired model's size and a download button. diff --git a/frontend/ui-core/src/annotator/suggestPanel.test.tsx b/frontend/ui-core/src/annotator/suggestPanel.test.tsx index 6ebce1a2..d1cca074 100644 --- a/frontend/ui-core/src/annotator/suggestPanel.test.tsx +++ b/frontend/ui-core/src/annotator/suggestPanel.test.tsx @@ -760,4 +760,54 @@ describe("this device, once a browser runtime is wired", () => { expect(screen.getByRole("alert").textContent).toContain("Download failed"); expect(button).toHaveProperty("disabled", false); }); + + it("reaches the This device tab's own download button even though the server side is not-ready (#Task-13 regression)", () => { + // The real combination `AnnotationPage` produces once "This device" is + // selected and the model has not been acquired: `computeSuggestBlocker` + // answers "not-ready" for the *server* side of things (Task 2's rule), and + // that answer must never blank the whole panel out from under a tab the + // person just chose — it was doing exactly that before this fix, because + // the old top-level `blocker !== null` early return fired regardless of + // which target was active and replaced the entire `Tabs` tree. + const acquire = vi.fn().mockResolvedValue(undefined); + render( + mount({ + browserTargets: [], + browserAcquisitions: [acquisition({ acquire })], + activeTarget: { kind: "browser", targetId: "efficient-sam-ti" }, + onChooseTarget: vi.fn(), + blocker: "not-ready", + }), + ); + + expect(screen.getByTestId("suggest-target-server")).toBeTruthy(); + expect(screen.getByTestId("suggest-target-browser")).toBeTruthy(); + const button = screen.getByTestId("suggest-device-acquire-efficient-sam-ti"); + expect(button).toHaveProperty("disabled", false); + + fireEvent.click(button); + expect(acquire).toHaveBeenCalledTimes(1); + }); + + it("shows the server tab's own blocker without hiding the This device tab", async () => { + const onChooseTarget = vi.fn(); + const user = userEvent.setup(); + render( + mount({ + browserTargets: [READY], + browserAcquisitions: [], + activeTarget: { kind: "server", connectionId: "" }, + onChooseTarget, + blocker: "no-connections", + }), + ); + + expect(screen.getByTestId("suggest-no-connections")).toBeTruthy(); + expect(screen.queryByTestId("suggest-connection")).toBeNull(); + + const browserTab = screen.getByTestId("suggest-target-browser"); + expect(browserTab).toBeTruthy(); + await user.click(browserTab); + expect(onChooseTarget).toHaveBeenCalledWith({ kind: "browser", targetId: READY.id }); + }); }); From 7da925681d2d62397a97c446ad107f7735fc00da Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya <1445792+JArmandoAnaya@users.noreply.github.com> Date: Thu, 17 Sep 2026 02:14:55 -0700 Subject: [PATCH 20/42] fix(ui-core): dedupe the runtime-wired check and give the blocked server tab its own tone/icon --- .../ui-core/src/annotator/SuggestPanel.tsx | 75 ++++++++++++++----- .../src/annotator/suggestPanel.test.tsx | 69 +++++++++++++++++ 2 files changed, 127 insertions(+), 17 deletions(-) diff --git a/frontend/ui-core/src/annotator/SuggestPanel.tsx b/frontend/ui-core/src/annotator/SuggestPanel.tsx index e7aa7d60..48ef5336 100644 --- a/frontend/ui-core/src/annotator/SuggestPanel.tsx +++ b/frontend/ui-core/src/annotator/SuggestPanel.tsx @@ -232,6 +232,13 @@ export function SuggestPanel({ onTolerance, pendingEscalated = false, }: SuggestPanelProps): JSX.Element { + // The one place this fact is decided. Repeating this condition at both the + // early-return guard below and the idle-card render risked them drifting + // apart — which is exactly the shape of the bug fixed alongside this line: + // the guard used to fire on a wired runtime's own "not-ready" and blank out + // the chooser it should have deferred to instead. + const runtimeWired = browserTargets !== undefined || browserAcquisitions !== undefined; + /* Parked outranks even the blocker. A connection this tool will not use is not the thing standing in the way, and "getting the model ready" over a @@ -281,12 +288,7 @@ export function SuggestPanel({ // panel — the panel still has a working "This device" tab to offer, and // this early return must not hide it. See `TargetChooser`, which renders // this same `BLOCKER_COPY` message scoped to its server tab instead. - if ( - browserTargets === undefined && - browserAcquisitions === undefined && - blocker !== null && - blocker !== undefined - ) { + if (!runtimeWired && blocker !== null && blocker !== undefined) { const copy = BLOCKER_COPY[blocker]; return ( }> -

- Click the thing you want -

-

- One click proposes a shape for “{session.labelClass}”. Alt-click marks something - that is not part of it. -

+ {!serverTabBlocked && ( + <> +

+ Click the thing you want +

+

+ One click proposes a shape for “{session.labelClass}”. Alt-click marks something + that is not part of it. +

+ + )} {/* Here and in no other reading. This is the state where nothing is in flight and nothing is waiting to be accepted, so it is the only one where @@ -437,7 +453,7 @@ export function SuggestPanel({ genuinely out. Stating the rule where it is enforced is what makes the branch ordering an implementation detail rather than the guarantee. */} - {browserTargets === undefined && browserAcquisitions === undefined ? ( + {!runtimeWired ? ( <> {!hasPending(session) && ( {blocker !== null ? ( - + ) : ( <> {!pending && ( @@ -625,18 +645,39 @@ function TargetChooser({ ); } -/** The blocker copy, wherever it is read: the whole card once, or scoped to one tab. */ +/** + * The blocker copy, wherever it is read: the whole card once, or scoped to one tab. + * + * `icon` defaults to off, which is what keeps the unwired early return byte-for-byte + * unchanged: its own `EditorNotice` already carries the tone icon in its fixed slot, + * driven by this same `copy.tone`. `TargetChooser`'s idle card has no such slot — its + * `EditorNotice` is fixed to `Sparkles`/calm regardless of which tab is showing what — + * so it opts into drawing the icon here instead, inline with the title. + */ function BlockedMessage({ blocker, onConfigure, + icon = false, }: { readonly blocker: SuggestBlocker; readonly onConfigure?: () => void; + readonly icon?: boolean; }): JSX.Element { const copy = BLOCKER_COPY[blocker]; return ( <> -

+

+ {icon && + (copy.tone === "warn" ? ( +

{copy.body}

diff --git a/frontend/ui-core/src/annotator/suggestPanel.test.tsx b/frontend/ui-core/src/annotator/suggestPanel.test.tsx index d1cca074..1a62807a 100644 --- a/frontend/ui-core/src/annotator/suggestPanel.test.tsx +++ b/frontend/ui-core/src/annotator/suggestPanel.test.tsx @@ -810,4 +810,73 @@ describe("this device, once a browser runtime is wired", () => { await user.click(browserTab); expect(onChooseTarget).toHaveBeenCalledWith({ kind: "browser", targetId: READY.id }); }); + + it("drops the idle invitation to click when the server tab it names is blocked", () => { + // "Click the thing you want" is a promise about the *active* tab. With the + // server tab active and blocked, that promise is false, and it would read + // directly above the sentence explaining why a click will not work. + render( + mount({ + browserTargets: [READY], + browserAcquisitions: [], + activeTarget: { kind: "server", connectionId: "" }, + onChooseTarget: vi.fn(), + blocker: "no-connections", + }), + ); + + expect(screen.queryByTestId("suggest-idle")).toBeNull(); + expect(screen.getByTestId("suggest-no-connections")).toBeTruthy(); + }); + + it("keeps the idle invitation when the browser tab is active, whatever the server blocker says", () => { + // The server's blocker is a fact about the server tab, not about whether + // this device can answer a click — the browser tab may be perfectly ready. + render( + mount({ + browserTargets: [READY], + browserAcquisitions: [], + activeTarget: { kind: "browser", targetId: READY.id }, + onChooseTarget: vi.fn(), + blocker: "no-connections", + }), + ); + + expect(screen.getByTestId("suggest-idle")).toBeTruthy(); + }); + + it("draws the warn icon inline with a warn-tone blocker on the server tab", () => { + render( + mount({ + browserTargets: [READY], + browserAcquisitions: [], + activeTarget: { kind: "server", connectionId: "" }, + onChooseTarget: vi.fn(), + blocker: "not-capable", + }), + ); + + const title = screen.getByTestId("suggest-not-capable"); + const svg = title.querySelector("svg"); + expect(svg).toBeTruthy(); + // The spinning icon is the calm-tone one; a warn-tone blocker must not draw it. + expect(svg?.classList.contains("animate-spin")).toBe(false); + }); + + it("draws the calm spinner inline with a calm-tone blocker on the server tab", () => { + render( + mount({ + browserTargets: [READY], + browserAcquisitions: [], + activeTarget: { kind: "server", connectionId: "" }, + onChooseTarget: vi.fn(), + blocker: "checking", + }), + ); + + const title = screen.getByTestId("suggest-checking"); + const svg = title.querySelector("svg"); + expect(svg).toBeTruthy(); + expect(svg?.classList.contains("animate-spin")).toBe(true); + }); }); From 663e92fb156bdaa291f072eb83054e8d7a0a2312 Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya <1445792+JArmandoAnaya@users.noreply.github.com> Date: Thu, 17 Sep 2026 02:23:32 -0700 Subject: [PATCH 21/42] test(app): fix browser suggestion e2e review findings - Test 7 asserted aria-selected on suggest-target-browser while the refused status has TargetChooser unmounted; press Escape first to drop back to idle, which remounts the chooser without touching activeTarget. - Split acquireAndSelectBrowserTarget into an arming step and a non-arming acquire/select step, since toggleSuggest() clears the whole session on a second press with the tool already armed (test 3 was double-arming it). - Registered the model-request listener before navigating in the "never fetched before Download" test, and added a mid-test assertion that merely switching to the device tab is also zero requests. - SHA-256 mismatch test now uses a correctly-sized but wrong-content buffer, so the size check passes and the SHA-256 check is what actually fails. - Added a catch-all abort route for the model CDN host ahead of the specific fixture routes, so anything unmatched hard-fails instead of reaching the real network. - Removed the routing-paragraph duplicated between annotate.spec.ts and _wireApiStub.ts, keeping it only where the route table now lives. --- frontend/app/e2e/annotate.spec.ts | 5 +- frontend/app/e2e/browserSuggestion.spec.ts | 83 +++++++++++++++++++--- 2 files changed, 74 insertions(+), 14 deletions(-) diff --git a/frontend/app/e2e/annotate.spec.ts b/frontend/app/e2e/annotate.spec.ts index dcf47c28..217bf598 100644 --- a/frontend/app/e2e/annotate.spec.ts +++ b/frontend/app/e2e/annotate.spec.ts @@ -6,9 +6,8 @@ * and what the top bar does — with the API held still, so a failure names the page * rather than the stack under it. * - * Everything is routed under `/api/`, which is where the app sends requests in - * development. Routing the bare paths would also intercept the *document* - * navigation, and the failure reads as "the shell disappeared". + * The stub itself — its route table, and why everything answers under `/api/` — + * lives in `./_wireApiStub`. */ import { expect, test, type Page, type Request } from "@playwright/test"; diff --git a/frontend/app/e2e/browserSuggestion.spec.ts b/frontend/app/e2e/browserSuggestion.spec.ts index a645e514..4998390c 100644 --- a/frontend/app/e2e/browserSuggestion.spec.ts +++ b/frontend/app/e2e/browserSuggestion.spec.ts @@ -12,6 +12,12 @@ * `listTargets()` answers non-empty, so "now ready" is read as that text appearing * inside `suggest-device-section` — and a failed acquisition renders `role="alert"` * beside the still-present acquire button (`SuggestPanel.tsx`'s `DeviceTab`). + * + * `TargetChooser` — and with it `suggest-target-browser` — is only mounted while + * the session is in its ordinary idle/blocked states (`SuggestPanel.tsx`'s final + * fallback render). The "shown" and "refused" cards replace it entirely, so a + * claim about the active tab or the tab list has to be made from idle, never from + * mid-suggestion or mid-refusal. */ import { existsSync, readFileSync } from "node:fs"; import path from "node:path"; @@ -36,8 +42,21 @@ if (process.env[REQUIRE_ENV] === "1" && !HAS_ARTIFACTS) { const ENCODER_BYTES = HAS_ARTIFACTS ? readFileSync(ENCODER_PATH) : Buffer.alloc(0); const DECODER_BYTES = HAS_ARTIFACTS ? readFileSync(DECODER_PATH) : Buffer.alloc(0); +/** + * The real encoder's byte length, from `manifest.ts`'s `EFFICIENT_SAM_TI_EXPECTED. + * encoder.bytes` — kept as a literal rather than imported, because that module + * reads `import.meta.env` at module scope, which only exists under Vite's own + * transform and would throw under this suite's plain Node/tsx loader. + */ +const REAL_ENCODER_BYTE_LENGTH = 24_799_777; + /** Routes the CDN manifest + artifacts to the local fixture — no network call ever leaves the page. */ async function mockCdn(page: Page, encoderBytes = ENCODER_BYTES, decoderBytes = DECODER_BYTES): Promise { + // Registered first, so it is matched *last* (Playwright tries the most-recently-added + // handler first): anything at this host the three specific routes below don't + // recognise is hard-aborted rather than silently reaching the real CDN — including if + // `VITE_MODEL_CDN_BASE_URL` or the manifest layout ever drifts out from under this stub. + await page.route("**/models.robomous.ai/**", (route) => route.abort()); await page.route("**/models.robomous.ai/models/efficient-sam-ti/**/manifest.json", (route) => route.fulfill({ json: { encoder: { path: "/encoder.onnx" }, decoder: { path: "/decoder.onnx" } } }), ); @@ -66,7 +85,8 @@ function suggestCallsOf(sent: Request[]): Request[] { * Counts requests to the model CDN from here on — via `page.on("request", ...)` * rather than a second `page.route`, so it never interferes with `mockCdn`'s own * fulfil handlers (a later-added `page.route` for the same pattern would run first - * and could shadow them). + * and could shadow them). Attach before whatever moment must not fetch, since a + * listener added after the fact cannot see what already happened. */ function countModelRequestsFromNow(page: Page): () => number { let count = 0; @@ -76,15 +96,25 @@ function countModelRequestsFromNow(page: Page): () => number { return () => count; } +/** Arms the suggest tool. The one and only place a scenario should click `tool-suggest`. */ +async function armSuggestTool(page: Page): Promise { + await page.getByTestId("tool-suggest").click(); +} + /** - * Clicks into the "This device" tab and downloads the model, then waits for the - * real "ready" signal: `DeviceTab` swaps the acquire button for a `Badge` reading + * Switches to "This device" and downloads the model, then waits for the real + * "ready" signal: `DeviceTab` swaps the acquire button for a `Badge` reading * "Ready" once `listTargets()` answers non-empty (`SuggestPanel.tsx`). There is no * separate target-selection control to click — Phase F ships exactly one browser * target, and choosing the tab already set it as the active suggestion target. + * + * **Assumes the suggest tool is already armed** (see {@link armSuggestTool}). + * Clicking `tool-suggest` again here would call `toggleSuggest()` a second time — + * and `AnnotationPage.tsx`'s `toggleSuggest` clears the whole session when one + * already exists (`session !== null` → `setSession(null)`), unmounting the very + * panel this helper is trying to drive rather than doing nothing. */ async function acquireAndSelectBrowserTarget(page: Page): Promise { - await page.getByTestId("tool-suggest").click(); await page.getByTestId("suggest-target-browser").click(); await page.getByTestId("suggest-device-acquire-efficient-sam-ti").click(); await expect(page.getByTestId("suggest-device-section").getByText(/ready/i)).toBeVisible({ timeout: 60_000 }); @@ -96,7 +126,7 @@ test.describe("browser suggestion", () => { test("Server target: a click issues exactly one /inference/suggest HTTP request", async ({ page }) => { const sent: Request[] = []; await openJobWithBrowserRuntime(page, sent, true); - await page.getByTestId("tool-suggest").click(); + await armSuggestTool(page); await expect(page.getByTestId("suggest-idle")).toBeVisible(); const picture = (await page.getByTestId("annotator-canvas").boundingBox())!; await page.mouse.click(picture.x + picture.width / 2, picture.y + picture.height / 2); @@ -107,6 +137,7 @@ test.describe("browser suggestion", () => { test("This device target: a click never issues an /inference/suggest HTTP request", async ({ page }) => { const sent: Request[] = []; await openJobWithBrowserRuntime(page, sent, true); + await armSuggestTool(page); await acquireAndSelectBrowserTarget(page); const picture = (await page.getByTestId("annotator-canvas").boundingBox())!; await page.mouse.click(picture.x + picture.width / 2, picture.y + picture.height / 2); @@ -117,7 +148,7 @@ test.describe("browser suggestion", () => { test("a ready browser target is never blocked by a server-connection blocker", async ({ page }) => { const sent: Request[] = []; await openJobWithBrowserRuntime(page, sent, false); // no server connections - await page.getByTestId("tool-suggest").click(); + await armSuggestTool(page); await expect(page.getByTestId("suggest-no-connections")).toBeVisible(); await acquireAndSelectBrowserTarget(page); await expect(page.getByTestId("suggest-no-connections")).not.toBeVisible(); @@ -128,9 +159,25 @@ test.describe("browser suggestion", () => { test("the model is never fetched before the user presses Download", async ({ page }) => { const sent: Request[] = []; - await openJobWithBrowserRuntime(page, sent, true); + // Attached before the page even navigates, so an eager fetch during runtime + // construction or page load — the most likely regression this test guards + // against — cannot happen in an unobserved window before this listener exists. const modelRequests = countModelRequestsFromNow(page); - await page.getByTestId("tool-suggest").click(); + await openJobWithBrowserRuntime(page, sent, true); + await armSuggestTool(page); + await expect(page.getByTestId("suggest-idle")).toBeVisible(); + expect(modelRequests()).toBe(0); + + // Merely switching to "This device" — without ever pressing Download — must + // not fetch anything either. Still idle here, so `suggest-target-browser` and + // the acquire button are both mounted. + await page.getByTestId("suggest-target-browser").click(); + await expect(page.getByTestId("suggest-device-acquire-efficient-sam-ti")).toBeVisible(); + expect(modelRequests()).toBe(0); + + // Back to Server, and the original claim: a Server suggestion never touches + // the model CDN at all. + await page.getByTestId("suggest-target-server").click(); const picture = (await page.getByTestId("annotator-canvas").boundingBox())!; await page.mouse.click(picture.x + picture.width / 2, picture.y + picture.height / 2); await expect(page.getByTestId("suggestion-shape")).toBeVisible(); @@ -139,7 +186,11 @@ test.describe("browser suggestion", () => { test("a SHA-256 mismatch on the encoder hard-fails acquisition with no target exposed", async ({ page }) => { const sent: Request[] = []; - const wrongBytes = Buffer.from("not the real encoder, deliberately wrong length and hash"); + // The *correct* byte length with different content: `fetchVerified` + // (`acquireEfficientSam.ts`) checks size before hashing, so a buffer of the + // wrong length would fail on the size check and never reach the SHA-256 + // comparison this test is named for. + const wrongBytes = Buffer.alloc(REAL_ENCODER_BYTE_LENGTH); await openJobWithBrowserRuntime(page, sent, true); // mockCdn already ran with the real bytes inside openJobWithBrowserRuntime; re-route // the encoder specifically — Playwright tries the most-recently-added matching @@ -147,7 +198,7 @@ test.describe("browser suggestion", () => { await page.route("**/models.robomous.ai/encoder.onnx", (route) => route.fulfill({ body: wrongBytes, contentType: "application/octet-stream" }), ); - await page.getByTestId("tool-suggest").click(); + await armSuggestTool(page); await page.getByTestId("suggest-target-browser").click(); await page.getByTestId("suggest-device-acquire-efficient-sam-ti").click(); await expect(page.getByRole("alert")).toBeVisible({ timeout: 30_000 }); @@ -158,6 +209,7 @@ test.describe("browser suggestion", () => { test("two refinements on one asset never re-fetch the model", async ({ page }) => { const sent: Request[] = []; await openJobWithBrowserRuntime(page, sent, true); + await armSuggestTool(page); await acquireAndSelectBrowserTarget(page); const modelRequests = countModelRequestsFromNow(page); const picture = (await page.getByTestId("annotator-canvas").boundingBox())!; @@ -171,6 +223,7 @@ test.describe("browser suggestion", () => { test("a negative-point ask on the browser target refuses deterministically, with no HTTP and no Server fallback", async ({ page }) => { const sent: Request[] = []; await openJobWithBrowserRuntime(page, sent, true); + await armSuggestTool(page); await acquireAndSelectBrowserTarget(page); const picture = (await page.getByTestId("annotator-canvas").boundingBox())!; await page.mouse.click(picture.x + picture.width / 2, picture.y + picture.height / 2); @@ -184,7 +237,15 @@ test.describe("browser suggestion", () => { // model (`BrowserSuggestionExecutor.ts`), and the panel's refusal card carries its // message verbatim, same as a server refusal would. await expect(page.getByTestId("suggest-refusal")).toHaveText(/positive-point refinement only/i); - // Still on the browser tab — a refusal never silently falls back to Server. + + // The refusal card replaces `TargetChooser` entirely (`SuggestPanel.tsx`'s + // `status === "refused"` branch), so `suggest-target-browser` isn't mounted at + // this instant — asserting on it here would fail on a missing element, not a + // real regression. Escape (`discardSuggestion` → `cleared`, since a refused + // session always `hasPending`) drops the session back to idle, which remounts + // the chooser without touching `activeTarget` — the fact this is actually + // proving no silent fallback to Server. + await page.keyboard.press("Escape"); await expect(page.getByTestId("suggest-target-browser")).toHaveAttribute("aria-selected", "true"); expect(suggestCallsOf(sent)).toHaveLength(0); }); From 128c1899fda5277b42e2d522d4401dbd39725217 Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya <1445792+JArmandoAnaya@users.noreply.github.com> Date: Thu, 17 Sep 2026 02:34:24 -0700 Subject: [PATCH 22/42] fix(ui-core): don't call executorFor on a browser target that isn't ready yet --- .../ui-core/src/annotator/AnnotationPage.tsx | 14 ++++++-- .../src/inference/browserRuntime.test.tsx | 36 +++++++++++++++++++ 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/frontend/ui-core/src/annotator/AnnotationPage.tsx b/frontend/ui-core/src/annotator/AnnotationPage.tsx index 06c444f6..dfc14723 100644 --- a/frontend/ui-core/src/annotator/AnnotationPage.tsx +++ b/frontend/ui-core/src/annotator/AnnotationPage.tsx @@ -188,6 +188,7 @@ import { SuggestPanel } from "./SuggestPanel"; import { useConnections, usableConnection } from "../data/inferenceQueries"; import type { SuggestionOut } from "../data/inferenceQueries"; import { useServerSuggestionExecutor } from "../inference/suggestionExecutor"; +import type { SuggestionExecutor } from "../inference/suggestionExecutor"; import { useBrowserInferenceRuntime } from "../inference/VisionSetBrowserInferenceProvider.js"; import type { ActiveSuggestionTarget, BrowserSuggestionAssetSource, BrowserSuggestionTarget } from "../inference/browserPort.js"; import { computeSuggestBlocker } from "../inference/targetBlocker.js"; @@ -1053,9 +1054,16 @@ function Workspace({ : { kind: "server", connectionId: connection?.id ?? "" }; const blocker = computeSuggestBlocker(activeTarget, serverBlocker, browserTargets); - const executor = - activeTarget.kind === "browser" && browserRuntime !== null - ? browserRuntime.executorFor(activeTarget.targetId) + // `executorFor` throws for a target that isn't actually ready yet — which is exactly the + // state selecting "This device" starts in, before a download ever completes — so this must + // check readiness itself rather than trust `browserRuntime !== null` alone. `null` here is + // this file's existing "nothing to send through" convention, already handled by + // `suggestAt`'s guard. + const executor: SuggestionExecutor | null = + activeTarget.kind === "browser" + ? browserRuntime !== null && browserTargets?.some((row) => row.id === activeTarget.targetId) === true + ? browserRuntime.executorFor(activeTarget.targetId) + : null : serverExecutor; function chooseTarget(target: ActiveSuggestionTarget): void { diff --git a/frontend/ui-core/src/inference/browserRuntime.test.tsx b/frontend/ui-core/src/inference/browserRuntime.test.tsx index 497961ee..16807d60 100644 --- a/frontend/ui-core/src/inference/browserRuntime.test.tsx +++ b/frontend/ui-core/src/inference/browserRuntime.test.tsx @@ -362,6 +362,42 @@ describe("target selection routes a ready browser target around the server", () }); }); +describe("executor selection never calls executorFor on an unready browser target", () => { + it("selecting 'This device' before any download completes does not crash the render", async () => { + // Mirrors `BrowserInferenceRuntime.executorFor`'s real behavior (Task 8): it throws + // synchronously for a target whose acquisition state isn't "ready" yet. Selecting the + // "This device" tab sets `activeTarget` to a browser target before any download has + // completed — that is how the acquisition flow starts — so `executor`'s computation + // must never call this unconditionally for a browser-kind `activeTarget`. + const runtime: VisionSetBrowserInferenceRuntime = { + listTargets: async () => [], + executorFor: (id) => { + throw new Error(`no ready browser target "${id}"`); + }, + listAcquisitions: () => [ + { id: "efficient-sam-ti", label: "EfficientSAM-Ti", approxBytes: 123_456, acquire: async () => {} }, + ], + }; + + const unmount = await open(runtime); + await arm(); + + // This click sets `activeTarget` to `{kind: "browser", targetId: "efficient-sam-ti"}` + // while `browserTargets` is still `[]` — the exact unready state that used to throw + // during render. A throw here would fail this test on its own, uncaught. + await userEvent.click(screen.getByTestId("suggest-target-browser")); + await screen.findByTestId("suggest-device-section"); + expect(screen.queryByTestId("suggest-panel")).not.toBeNull(); + + // With no ready executor, a click on the canvas must be a silent no-op rather than + // falling through to the server executor. + clickCanvas(); + expect(asks()).toHaveLength(0); + + unmount(); + }); +}); + describe("staleStoredBrowserTarget", () => { const listed = [{ id: "t1", label: "T1", modelRef: "m@rev" }]; From 4fa0181710203cbfeca494e2e9ff8a744a5e7993 Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya <1445792+JArmandoAnaya@users.noreply.github.com> Date: Thu, 17 Sep 2026 02:45:25 -0700 Subject: [PATCH 23/42] fix(ui-core): derive executor readiness from blocker, and make the regression test discriminate The executor guard re-derived "is this target in browserTargets" a second time, independently of computeSuggestBlocker's own answer to the same question - the same shape of bug this file just fixed once already. Derive it from blocker instead, so the two can no longer silently drift apart. The regression test's "no fallthrough to the server" assertion also could not actually fail: it checked synchronously right after the click, before an async mutateAsync dispatch would have reached the sent log, and nothing in the test setup guaranteed a genuinely usable server connection existed to fall through to in the first place. Wait for the server tab to resolve ready, and flush a tick before asserting. --- .../ui-core/src/annotator/AnnotationPage.tsx | 10 ++++++---- .../src/inference/browserRuntime.test.tsx | 19 ++++++++++++++++++- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/frontend/ui-core/src/annotator/AnnotationPage.tsx b/frontend/ui-core/src/annotator/AnnotationPage.tsx index dfc14723..075c7b8e 100644 --- a/frontend/ui-core/src/annotator/AnnotationPage.tsx +++ b/frontend/ui-core/src/annotator/AnnotationPage.tsx @@ -1056,12 +1056,14 @@ function Workspace({ const blocker = computeSuggestBlocker(activeTarget, serverBlocker, browserTargets); // `executorFor` throws for a target that isn't actually ready yet — which is exactly the // state selecting "This device" starts in, before a download ever completes — so this must - // check readiness itself rather than trust `browserRuntime !== null` alone. `null` here is - // this file's existing "nothing to send through" convention, already handled by - // `suggestAt`'s guard. + // check readiness itself rather than trust `browserRuntime !== null` alone. Derived from + // `blocker` rather than re-deriving "is this target in `browserTargets`" a second time: + // `computeSuggestBlocker`'s browser case is exactly that check, and re-spelling it here + // would let this guard and that one silently drift apart. `null` here is this file's + // existing "nothing to send through" convention, already handled by `suggestAt`'s guard. const executor: SuggestionExecutor | null = activeTarget.kind === "browser" - ? browserRuntime !== null && browserTargets?.some((row) => row.id === activeTarget.targetId) === true + ? browserRuntime !== null && blocker === null ? browserRuntime.executorFor(activeTarget.targetId) : null : serverExecutor; diff --git a/frontend/ui-core/src/inference/browserRuntime.test.tsx b/frontend/ui-core/src/inference/browserRuntime.test.tsx index 16807d60..9008eb94 100644 --- a/frontend/ui-core/src/inference/browserRuntime.test.tsx +++ b/frontend/ui-core/src/inference/browserRuntime.test.tsx @@ -382,16 +382,33 @@ describe("executor selection never calls executorFor on an unready browser targe const unmount = await open(runtime); await arm(); + // Wait for the server tab's own connection to resolve to the ready one `beforeEach` + // seeds (`connectionRow()`), so the assertion below is a real proof that a click + // doesn't fall through to a genuinely usable server executor — not a false negative + // from `serverExecutor` merely being null too, for an unrelated reason (still loading, + // or no connection at all). `suggest-idle` only renders once the server tab's blocker + // clears, which is exactly that resolution. + await screen.findByTestId("suggest-idle"); + // This click sets `activeTarget` to `{kind: "browser", targetId: "efficient-sam-ti"}` // while `browserTargets` is still `[]` — the exact unready state that used to throw // during render. A throw here would fail this test on its own, uncaught. await userEvent.click(screen.getByTestId("suggest-target-browser")); await screen.findByTestId("suggest-device-section"); expect(screen.queryByTestId("suggest-panel")).not.toBeNull(); + // The user-facing outcome this whole guard exists for: the tab lands on the actual + // Download control rather than a blank or crashed panel. + expect(screen.queryByTestId("suggest-device-acquire-efficient-sam-ti")).not.toBeNull(); // With no ready executor, a click on the canvas must be a silent no-op rather than - // falling through to the server executor. + // falling through to the server executor — which is genuinely non-null here (the + // resolved connection above), so this assertion is a real proof, not a false + // negative from nothing being available to fall through to. The flush lets a + // wrongly-dispatched `mutateAsync` actually reach `sent` before we check — a bare + // synchronous check right after `clickCanvas()` would pass even with a fallthrough + // bug present, since the fetch is dispatched a tick later. clickCanvas(); + await new Promise((resolve) => setTimeout(resolve, 0)); expect(asks()).toHaveLength(0); unmount(); From c3b932884c4cd2e92fb66b386105f3a5c3221cd0 Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya <1445792+JArmandoAnaya@users.noreply.github.com> Date: Thu, 17 Sep 2026 02:50:38 -0700 Subject: [PATCH 24/42] ci: run the browser suggestion e2e suite in the browser-models job --- .github/path-filters.yml | 1 + .github/workflows/ci.yml | 8 ++++++++ 2 files changed, 9 insertions(+) diff --git a/.github/path-filters.yml b/.github/path-filters.yml index 4f47e9ab..67a1b368 100644 --- a/.github/path-filters.yml +++ b/.github/path-filters.yml @@ -99,6 +99,7 @@ docs: # `python` -- and this file says outright that a path may appear in several. browser-models: - "frontend/browser-inference/**" + - "frontend/app/e2e/browserSuggestion.spec.ts" - "scripts/browser_models/**" - "tests/browser_models/**" - "pyproject.toml" diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8ee1c1b6..761d28aa 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1092,6 +1092,14 @@ jobs: if: needs.changes.result != 'success' || needs.changes.outputs['browser-models'] == 'true' run: pnpm --filter @visionset/browser-inference test:browser + - name: Install the app's Playwright browsers + if: needs.changes.result != 'success' || needs.changes.outputs['browser-models'] == 'true' + run: pnpm --filter @visionset/app exec playwright install chromium + + - name: Run the browser suggestion e2e suite, with the model present + if: needs.changes.result != 'success' || needs.changes.outputs['browser-models'] == 'true' + run: pnpm --filter @visionset/app e2e browserSuggestion + # The step that makes this job's green check mean something. Everything above answers # "did what ran succeed?"; a suite that skips itself, or a spec file that stopped being # collected at all, answers that with a cheerful yes. This reads the JSON report the From 79bd15570a1ae015be32c48bd8d51319b31c6c3d Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya <1445792+JArmandoAnaya@users.noreply.github.com> Date: Thu, 17 Sep 2026 03:01:36 -0700 Subject: [PATCH 25/42] test(app): assert the acquired target's modelRef carries the pinned revision Mutation testing found this gap: hardcoding MODEL_REF to drop EFFICIENT_SAM_TI_REVISION left all existing tests green, silently corrupting the provenance carried into accepted annotations. The new test imports EFFICIENT_SAM_TI_REVISION independently from manifest.js and checks the exact expected string, so it fails if either the constant or the implementation drifts. --- .../browserInference/BrowserInferenceRuntime.test.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/frontend/app/src/data/browserInference/BrowserInferenceRuntime.test.ts b/frontend/app/src/data/browserInference/BrowserInferenceRuntime.test.ts index c9427dcb..65e5129a 100644 --- a/frontend/app/src/data/browserInference/BrowserInferenceRuntime.test.ts +++ b/frontend/app/src/data/browserInference/BrowserInferenceRuntime.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it, vi } from "vitest"; import { createOssBrowserInferenceRuntime } from "./BrowserInferenceRuntime.js"; +import { EFFICIENT_SAM_TI_REVISION } from "./manifest.js"; function fakeDeps(overrides?: { acquire?: () => Promise<{ encoder: Uint8Array; decoder: Uint8Array }> }) { const createRuntime = vi.fn(() => ({ @@ -49,6 +50,14 @@ describe("createOssBrowserInferenceRuntime", () => { expect(deps.acquire).toHaveBeenCalledTimes(1); }); + it("carries the pinned revision in the acquired target's modelRef", async () => { + const deps = fakeDeps(); + const runtime = createOssBrowserInferenceRuntime(deps); + await runtime.listAcquisitions?.()[0]!.acquire(); + const targets = await runtime.listTargets(); + expect(targets[0]!.modelRef).toBe(`efficient-sam-ti@${EFFICIENT_SAM_TI_REVISION}`); + }); + it("throws from executorFor before any acquisition has succeeded", () => { const runtime = createOssBrowserInferenceRuntime(fakeDeps()); expect(() => runtime.executorFor("efficient-sam-ti")).toThrow(); From d46ff0a25a822cef19c59eaf9cdfd562b04834d1 Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya <1445792+JArmandoAnaya@users.noreply.github.com> Date: Thu, 17 Sep 2026 03:08:05 -0700 Subject: [PATCH 26/42] fix(app): match the real manifest's nested artifacts schema --- .../acquireEfficientSam.test.ts | 16 +++++++++++++++- .../browserInference/acquireEfficientSam.ts | 19 ++++++++++++++++--- .../app/src/data/browserInference/manifest.ts | 12 ++++++++++-- 3 files changed, 41 insertions(+), 6 deletions(-) diff --git a/frontend/app/src/data/browserInference/acquireEfficientSam.test.ts b/frontend/app/src/data/browserInference/acquireEfficientSam.test.ts index 51670c16..4f3413bb 100644 --- a/frontend/app/src/data/browserInference/acquireEfficientSam.test.ts +++ b/frontend/app/src/data/browserInference/acquireEfficientSam.test.ts @@ -59,7 +59,12 @@ describe("acquireEfficientSam", () => { const decoderBytes = bytesOf("decoder-fixture"); const fetchMock = vi.fn(async (url: string) => { if (url.endsWith("manifest.json")) { - return new Response(JSON.stringify({ encoder: { path: "/encoder.onnx" }, decoder: { path: "/decoder.onnx" } })); + // The real, already-deployed manifest shape: artifacts nested under + // `artifacts`, each `path` a bare filename relative to the manifest's own + // directory — not `{ encoder: { path: "/encoder.onnx" } }` at the top level. + return new Response( + JSON.stringify({ artifacts: { encoder: { path: "encoder.onnx" }, decoder: { path: "decoder.onnx" } } }), + ); } if (url.endsWith("encoder.onnx")) return new Response(encoderBytes); if (url.endsWith("decoder.onnx")) return new Response(decoderBytes); @@ -91,5 +96,14 @@ describe("acquireEfficientSam", () => { expect(result.decoder).toEqual(decoderBytes); expect(fetchMock).toHaveBeenCalledTimes(3); expect(fetchMock.mock.calls[0]![0]).toMatch(/manifest\.json$/); + // The artifact URL is resolved relative to the manifest's own directory, not by + // naively appending the manifest's bare `path` onto the CDN base — this is the + // exact bug the real, live manifest exposed. + expect(fetchMock.mock.calls[1]![0]).toBe( + "https://models.robomous.ai/models/efficient-sam-ti/b19782d049c0-843761ca46f4/encoder.onnx", + ); + expect(fetchMock.mock.calls[2]![0]).toBe( + "https://models.robomous.ai/models/efficient-sam-ti/b19782d049c0-843761ca46f4/decoder.onnx", + ); }); }); diff --git a/frontend/app/src/data/browserInference/acquireEfficientSam.ts b/frontend/app/src/data/browserInference/acquireEfficientSam.ts index ba53a399..481478bb 100644 --- a/frontend/app/src/data/browserInference/acquireEfficientSam.ts +++ b/frontend/app/src/data/browserInference/acquireEfficientSam.ts @@ -1,4 +1,17 @@ -import { EFFICIENT_SAM_TI_EXPECTED, MODEL_CDN_BASE_URL, fetchEfficientSamManifest } from "./manifest.js"; +import { + EFFICIENT_SAM_TI_EXPECTED, + EFFICIENT_SAM_TI_REVISION, + MODEL_CDN_BASE_URL, + fetchEfficientSamManifest, +} from "./manifest.js"; + +// The manifest's `artifacts.*.path` is a bare filename, resolved relative to the +// manifest's own directory — not an absolute path safe to append directly to +// `MODEL_CDN_BASE_URL`. This restates that same directory prefix, which is also how +// `EFFICIENT_SAM_TI_MANIFEST_URL` itself is built. +function artifactUrl(path: string): string { + return `${MODEL_CDN_BASE_URL}/models/efficient-sam-ti/${EFFICIENT_SAM_TI_REVISION}/${path}`; +} // `Uint8Array`, not the bare `Uint8Array` (which now defaults to the // wider `Uint8Array`) — TypeScript 6's `lib.dom.d.ts` types @@ -35,12 +48,12 @@ export async function acquireEfficientSam( ): Promise<{ readonly encoder: Uint8Array; readonly decoder: Uint8Array }> { const manifest = await fetchEfficientSamManifest(signal); const encoder = await fetchVerified( - `${MODEL_CDN_BASE_URL}${manifest.encoder.path}`, + artifactUrl(manifest.artifacts.encoder.path), EFFICIENT_SAM_TI_EXPECTED.encoder, signal, ); const decoder = await fetchVerified( - `${MODEL_CDN_BASE_URL}${manifest.decoder.path}`, + artifactUrl(manifest.artifacts.decoder.path), EFFICIENT_SAM_TI_EXPECTED.decoder, signal, ); diff --git a/frontend/app/src/data/browserInference/manifest.ts b/frontend/app/src/data/browserInference/manifest.ts index 48e1eb3c..0ac33b52 100644 --- a/frontend/app/src/data/browserInference/manifest.ts +++ b/frontend/app/src/data/browserInference/manifest.ts @@ -18,9 +18,17 @@ export const EFFICIENT_SAM_TI_EXPECTED = { decoder: { sha256: "843761ca46f4aa00b09fdcf0c94271321f76eece092a744296c742d682a86172", bytes: 16_501_901 }, } as const; +/** + * Mirrors the real, already-deployed manifest shape (nested under `artifacts`, + * with each `path` a bare filename relative to the manifest's own directory) — not + * an assumed flat shape. Only `path` is read from this; `bytes`/`sha256` are never + * trusted from the manifest itself (see `EFFICIENT_SAM_TI_EXPECTED`). + */ export interface EfficientSamManifest { - readonly encoder: { readonly path: string }; - readonly decoder: { readonly path: string }; + readonly artifacts: { + readonly encoder: { readonly path: string }; + readonly decoder: { readonly path: string }; + }; } export async function fetchEfficientSamManifest(signal?: AbortSignal): Promise { From 6f2e4be7864ca54f679fbe51c4994ccebf756b75 Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya <1445792+JArmandoAnaya@users.noreply.github.com> Date: Thu, 17 Sep 2026 03:14:31 -0700 Subject: [PATCH 27/42] test(app): match browserSuggestion's CDN fixture to the real manifest shape A real-CDN smoke test found the live manifest nests artifacts under "artifacts" with bare relative filenames, not the flat shape this suite's mockCdn assumed (manifest.ts/acquireEfficientSam.ts already fixed upstream). Updates the manifest stub and the encoder/decoder route patterns, including the SHA-mismatch test's own re-route, to match. --- frontend/app/e2e/browserSuggestion.spec.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/frontend/app/e2e/browserSuggestion.spec.ts b/frontend/app/e2e/browserSuggestion.spec.ts index 4998390c..e13a173d 100644 --- a/frontend/app/e2e/browserSuggestion.spec.ts +++ b/frontend/app/e2e/browserSuggestion.spec.ts @@ -57,13 +57,17 @@ async function mockCdn(page: Page, encoderBytes = ENCODER_BYTES, decoderBytes = // recognise is hard-aborted rather than silently reaching the real CDN — including if // `VITE_MODEL_CDN_BASE_URL` or the manifest layout ever drifts out from under this stub. await page.route("**/models.robomous.ai/**", (route) => route.abort()); + // The real, live manifest shape (fixed in manifest.ts/acquireEfficientSam.ts after a + // production incident): artifacts nest under `artifacts`, and each `path` is a bare + // filename resolved relative to the manifest's own revision directory — never a + // leading-slash, top-level path. await page.route("**/models.robomous.ai/models/efficient-sam-ti/**/manifest.json", (route) => - route.fulfill({ json: { encoder: { path: "/encoder.onnx" }, decoder: { path: "/decoder.onnx" } } }), + route.fulfill({ json: { artifacts: { encoder: { path: "encoder.onnx" }, decoder: { path: "decoder.onnx" } } } }), ); - await page.route("**/models.robomous.ai/encoder.onnx", (route) => + await page.route("**/models.robomous.ai/models/efficient-sam-ti/**/encoder.onnx", (route) => route.fulfill({ body: encoderBytes, contentType: "application/octet-stream" }), ); - await page.route("**/models.robomous.ai/decoder.onnx", (route) => + await page.route("**/models.robomous.ai/models/efficient-sam-ti/**/decoder.onnx", (route) => route.fulfill({ body: decoderBytes, contentType: "application/octet-stream" }), ); } @@ -195,7 +199,7 @@ test.describe("browser suggestion", () => { // mockCdn already ran with the real bytes inside openJobWithBrowserRuntime; re-route // the encoder specifically — Playwright tries the most-recently-added matching // handler first, so this one now answers every encoder.onnx request. - await page.route("**/models.robomous.ai/encoder.onnx", (route) => + await page.route("**/models.robomous.ai/models/efficient-sam-ti/**/encoder.onnx", (route) => route.fulfill({ body: wrongBytes, contentType: "application/octet-stream" }), ); await armSuggestTool(page); From be7bd7a05eafac02134f22c0e3afa7a544c3ca35 Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya <1445792+JArmandoAnaya@users.noreply.github.com> Date: Thu, 17 Sep 2026 03:14:43 -0700 Subject: [PATCH 28/42] fix(app): dedupe the CDN URL prefix, validate manifest shape, and guard artifact paths - Extract EFFICIENT_SAM_TI_BASE_URL so the manifest and artifact URLs share one source for the /models/efficient-sam-ti// prefix instead of restating it. - Strip a trailing slash from MODEL_CDN_BASE_URL to avoid a double slash. - Validate the parsed manifest's artifacts.{encoder,decoder}.path before use, so a future CDN schema drift throws a diagnosable error instead of a bare TypeError. - Reject a manifest artifact path containing a slash before building its URL. - Add tests proving verification never reads the manifest's own bytes/sha256, the path-traversal guard fails closed, and the manifest-shape validation works. --- .../acquireEfficientSam.test.ts | 84 +++++++++++++++++++ .../browserInference/acquireEfficientSam.ts | 19 ++--- .../app/src/data/browserInference/manifest.ts | 31 +++++-- 3 files changed, 119 insertions(+), 15 deletions(-) diff --git a/frontend/app/src/data/browserInference/acquireEfficientSam.test.ts b/frontend/app/src/data/browserInference/acquireEfficientSam.test.ts index 4f3413bb..d0fecc57 100644 --- a/frontend/app/src/data/browserInference/acquireEfficientSam.test.ts +++ b/frontend/app/src/data/browserInference/acquireEfficientSam.test.ts @@ -11,6 +11,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { fetchVerified } from "./acquireEfficientSam.js"; +import { fetchEfficientSamManifest } from "./manifest.js"; // `Uint8Array`, not the bare `Uint8Array` — see the same note in // acquireEfficientSam.ts: TypeScript 6's `lib.dom.d.ts` requires the concrete @@ -106,4 +107,87 @@ describe("acquireEfficientSam", () => { "https://models.robomous.ai/models/efficient-sam-ti/b19782d049c0-843761ca46f4/decoder.onnx", ); }); + + it("never trusts the manifest's own bytes/sha256 — a manifest that lies about both still verifies against the pinned constants", async () => { + const encoderBytes = bytesOf("encoder-fixture"); + const decoderBytes = bytesOf("decoder-fixture"); + const fetchMock = vi.fn(async (url: string) => { + if (url.endsWith("manifest.json")) { + return new Response( + JSON.stringify({ + artifacts: { + // Deliberately wrong `bytes`/`sha256` alongside the real `path` — a + // manifest that lies about its own artifacts' hashes. If acquisition + // ever read these instead of `EFFICIENT_SAM_TI_EXPECTED`, this fixture + // would either reject the correct fixture bytes or accept forged ones. + encoder: { path: "encoder.onnx", bytes: 1, sha256: "0".repeat(64) }, + decoder: { path: "decoder.onnx", bytes: 1, sha256: "0".repeat(64) }, + }, + }), + ); + } + if (url.endsWith("encoder.onnx")) return new Response(encoderBytes); + if (url.endsWith("decoder.onnx")) return new Response(decoderBytes); + throw new Error(`unexpected url ${url}`); + }); + vi.stubGlobal("fetch", fetchMock); + vi.resetModules(); + vi.doMock("./manifest.js", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + EFFICIENT_SAM_TI_EXPECTED: { + encoder: { sha256: await sha256Of(encoderBytes), bytes: encoderBytes.byteLength }, + decoder: { sha256: await sha256Of(decoderBytes), bytes: decoderBytes.byteLength }, + }, + }; + }); + const { acquireEfficientSam } = await import("./acquireEfficientSam.js"); + + const result = await acquireEfficientSam(); + + expect(result.encoder).toEqual(encoderBytes); + expect(result.decoder).toEqual(decoderBytes); + }); + + it("fails closed on a manifest artifact path shaped like a traversal, rather than building whatever URL it names", async () => { + vi.stubGlobal( + "fetch", + vi.fn(async (url: string) => { + if (url.endsWith("manifest.json")) { + return new Response( + JSON.stringify({ artifacts: { encoder: { path: "../secrets.onnx" }, decoder: { path: "decoder.onnx" } } }), + ); + } + throw new Error(`unexpected url ${url}`); + }), + ); + vi.resetModules(); + const { acquireEfficientSam } = await import("./acquireEfficientSam.js"); + + await expect(acquireEfficientSam()).rejects.toThrow(/unexpected manifest artifact path/i); + }); +}); + +describe("fetchEfficientSamManifest", () => { + beforeEach(() => vi.restoreAllMocks()); + + it("throws a diagnosable error, not a bare TypeError, when the CDN's manifest schema has moved", async () => { + vi.stubGlobal("fetch", vi.fn().mockResolvedValue(new Response(JSON.stringify({ artifacts: { encoder: {} } })))); + await expect(fetchEfficientSamManifest()).rejects.toThrow(/unexpected manifest schema/i); + }); + + it("accepts the real, already-deployed manifest shape", async () => { + vi.stubGlobal( + "fetch", + vi.fn().mockResolvedValue( + new Response( + JSON.stringify({ artifacts: { encoder: { path: "encoder.onnx" }, decoder: { path: "decoder.onnx" } } }), + ), + ), + ); + const manifest = await fetchEfficientSamManifest(); + expect(manifest.artifacts.encoder.path).toBe("encoder.onnx"); + expect(manifest.artifacts.decoder.path).toBe("decoder.onnx"); + }); }); diff --git a/frontend/app/src/data/browserInference/acquireEfficientSam.ts b/frontend/app/src/data/browserInference/acquireEfficientSam.ts index 481478bb..1bf87b27 100644 --- a/frontend/app/src/data/browserInference/acquireEfficientSam.ts +++ b/frontend/app/src/data/browserInference/acquireEfficientSam.ts @@ -1,16 +1,15 @@ -import { - EFFICIENT_SAM_TI_EXPECTED, - EFFICIENT_SAM_TI_REVISION, - MODEL_CDN_BASE_URL, - fetchEfficientSamManifest, -} from "./manifest.js"; +import { EFFICIENT_SAM_TI_BASE_URL, EFFICIENT_SAM_TI_EXPECTED, fetchEfficientSamManifest } from "./manifest.js"; // The manifest's `artifacts.*.path` is a bare filename, resolved relative to the -// manifest's own directory — not an absolute path safe to append directly to -// `MODEL_CDN_BASE_URL`. This restates that same directory prefix, which is also how -// `EFFICIENT_SAM_TI_MANIFEST_URL` itself is built. +// manifest's own directory (`EFFICIENT_SAM_TI_BASE_URL`, the same prefix +// `EFFICIENT_SAM_TI_MANIFEST_URL` is built from) — not an absolute path safe to +// append directly to `MODEL_CDN_BASE_URL`. A `path` containing a slash is rejected +// outright: it should always be a bare filename, and a mutated manifest asking to +// climb out of its own directory fails closed here rather than silently building +// whatever URL it names. function artifactUrl(path: string): string { - return `${MODEL_CDN_BASE_URL}/models/efficient-sam-ti/${EFFICIENT_SAM_TI_REVISION}/${path}`; + if (path.includes("/")) throw new Error(`unexpected manifest artifact path: ${path}`); + return `${EFFICIENT_SAM_TI_BASE_URL}/${path}`; } // `Uint8Array`, not the bare `Uint8Array` (which now defaults to the diff --git a/frontend/app/src/data/browserInference/manifest.ts b/frontend/app/src/data/browserInference/manifest.ts index 0ac33b52..65d32d40 100644 --- a/frontend/app/src/data/browserInference/manifest.ts +++ b/frontend/app/src/data/browserInference/manifest.ts @@ -4,13 +4,20 @@ * Self-hosted deployments override VITE_MODEL_CDN_BASE_URL to point at their own mirror * of this same manifest layout. */ -export const MODEL_CDN_BASE_URL: string = - (import.meta.env["VITE_MODEL_CDN_BASE_URL"] as string | undefined) ?? "https://models.robomous.ai"; +export const MODEL_CDN_BASE_URL: string = ( + (import.meta.env["VITE_MODEL_CDN_BASE_URL"] as string | undefined) ?? "https://models.robomous.ai" +).replace(/\/+$/, ""); export const EFFICIENT_SAM_TI_REVISION = "b19782d049c0-843761ca46f4"; -export const EFFICIENT_SAM_TI_MANIFEST_URL = - `${MODEL_CDN_BASE_URL}/models/efficient-sam-ti/${EFFICIENT_SAM_TI_REVISION}/manifest.json`; +/** + * Shared by the manifest URL and every artifact URL, so the CDN's directory layout + * for this revision is stated exactly once — see `acquireEfficientSam.ts`'s + * `artifactUrl`. + */ +export const EFFICIENT_SAM_TI_BASE_URL = `${MODEL_CDN_BASE_URL}/models/efficient-sam-ti/${EFFICIENT_SAM_TI_REVISION}`; + +export const EFFICIENT_SAM_TI_MANIFEST_URL = `${EFFICIENT_SAM_TI_BASE_URL}/manifest.json`; /** Verified 2026-09-16 against the live models.robomous.ai release — see the design doc §5. */ export const EFFICIENT_SAM_TI_EXPECTED = { @@ -31,8 +38,22 @@ export interface EfficientSamManifest { }; } +function assertManifestShape(value: unknown): asserts value is EfficientSamManifest { + const artifacts = (value as { artifacts?: unknown } | null)?.artifacts as + | { encoder?: { path?: unknown }; decoder?: { path?: unknown } } + | undefined; + for (const key of ["encoder", "decoder"] as const) { + const path = artifacts?.[key]?.path; + if (typeof path !== "string" || path.length === 0) { + throw new Error(`unexpected manifest schema: missing artifacts.${key}.path`); + } + } +} + export async function fetchEfficientSamManifest(signal?: AbortSignal): Promise { const response = await fetch(EFFICIENT_SAM_TI_MANIFEST_URL, { signal }); if (!response.ok) throw new Error(`manifest fetch failed: ${response.status} ${response.statusText}`); - return (await response.json()) as EfficientSamManifest; + const parsed: unknown = await response.json(); + assertManifestShape(parsed); + return parsed; } From 27553b2178e93d5bcfde52e9ef2fc1f9fe2b9de1 Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya <1445792+JArmandoAnaya@users.noreply.github.com> Date: Thu, 17 Sep 2026 03:37:15 -0700 Subject: [PATCH 29/42] fix(browser-inference): make the ORT wasm asset path survive bundler URL rewriting Vite's `import.meta.url`-relative asset transform rewrites the literal argument of `new URL("./ort/", import.meta.url)` and drops the trailing slash, so `wasmPaths` arrived as `.../dist/browser/ort`. ORT then resolved its glue module as a *sibling* of `ort` rather than a file inside it; a dev server answers that 404 with its SPA fallback, so the dynamic import received HTML, both execution providers failed to initialize and every session load reported `graph-load-failed`. The trailing slash is now re-applied to the resolved string instead of being trusted to the literal, which holds for any bundler and any host. --- frontend/browser-inference/src/browser/worker.ts | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/frontend/browser-inference/src/browser/worker.ts b/frontend/browser-inference/src/browser/worker.ts index e0072df5..68800a9b 100644 --- a/frontend/browser-inference/src/browser/worker.ts +++ b/frontend/browser-inference/src/browser/worker.ts @@ -103,10 +103,20 @@ function fail(id: OperationId, code: InferenceRuntimeErrorCode, error: unknown): * host serving nothing special. A host that would rather serve them from its own origin * passes `assetBaseUrl`; the main thread never guesses this location, because only the * worker knows where it was loaded from. + * + * The trailing slash is re-applied to the resolved string rather than trusted to the + * literal: a bundler that rewrites `import.meta.url`-relative asset URLs can emit the + * directory without it, and ORT then resolves its `.mjs` glue as a *sibling* of `ort` + * instead of a file inside it — a 404 a dev server answers with an SPA fallback, so the + * dynamic import receives HTML and every execution provider fails to initialize. */ +function withTrailingSlash(base: string): string { + return base.endsWith("/") ? base : `${base}/`; +} + function assetsAt(stated: string | undefined): string { - if (stated !== undefined && stated !== "") return stated.endsWith("/") ? stated : `${stated}/`; - return new URL("./ort/", import.meta.url).href; + if (stated !== undefined && stated !== "") return withTrailingSlash(stated); + return withTrailingSlash(new URL("./ort/", import.meta.url).href); } function toTensor(input: TensorLike): ort.Tensor { From 5e05107c591f72591fbae1bbc43bed35872e1d4b Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya <1445792+JArmandoAnaya@users.noreply.github.com> Date: Thu, 17 Sep 2026 03:37:18 -0700 Subject: [PATCH 30/42] fix(app): build browser-inference before serving the e2e suite The suite resolves `@visionset/browser-inference` through its `dist/`, exactly as it does the three packages already built here, so an unbuilt change there was invisible in the browser and the run silently depended on that `dist/` happening to be current. --- frontend/app/playwright.config.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/frontend/app/playwright.config.ts b/frontend/app/playwright.config.ts index 59ad4d84..2040217b 100644 --- a/frontend/app/playwright.config.ts +++ b/frontend/app/playwright.config.ts @@ -145,6 +145,9 @@ export default defineConfig({ // all, which fails the build outright rather than quietly. "pnpm --filter @visionset/media build && " + "pnpm --filter @visionset/ui-core build && " + + // Unrelated to the three above, but resolved through its `dist/` just as they are, + // so an unbuilt change here is invisible in the browser rather than a compile error. + "pnpm --filter @visionset/browser-inference build && " + `vite --port ${PORT.e2e} --strictPort`, url: `http://localhost:${PORT.e2e}`, reuseExistingServer: !process.env.CI, From de961d04316d7eec1b04e995189d4a8f6fbe83ba Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya <1445792+JArmandoAnaya@users.noreply.github.com> Date: Thu, 17 Sep 2026 03:44:58 -0700 Subject: [PATCH 31/42] test(app): serve a real correctly-sized asset image in browserSuggestion _wireApiStub.ts's /content route always serves a real-but-1x1 PNG, which is fine for annotate.spec.ts's ~150 tests (AnnotatorCanvas lays the picture out at the asset's declared width/height, never its own naturalWidth) but breaks this suite: the browser executor validates click coordinates against the image's actual decoded dimensions, so every click landed "outside" a 1x1 image and was correctly refused. Adds a small dependency-free PNG encoder and overrides just this file's /content route with a real 640x480 image, matching the stub's asset metadata and the coordinate frame the existing click math already assumes. Scoped to this file rather than changing _wireApiStub.ts's shared default. First real green run against the model fixture: 7/7 passed. --- frontend/app/e2e/browserSuggestion.spec.ts | 93 +++++++++++++++++++++- 1 file changed, 91 insertions(+), 2 deletions(-) diff --git a/frontend/app/e2e/browserSuggestion.spec.ts b/frontend/app/e2e/browserSuggestion.spec.ts index e13a173d..763ebe8f 100644 --- a/frontend/app/e2e/browserSuggestion.spec.ts +++ b/frontend/app/e2e/browserSuggestion.spec.ts @@ -18,11 +18,25 @@ * fallback render). The "shown" and "refused" cards replace it entirely, so a * claim about the active tab or the tab list has to be made from idle, never from * mid-suggestion or mid-refusal. + * + * `_wireApiStub.ts`'s `/content` route serves a real but 1×1 PNG — fine for the + * ~150 tests in `annotate.spec.ts`, because `AnnotatorCanvas.tsx` lays the picture + * out at the asset's *declared* width/height and never at its own `naturalWidth` + * ("a picture whose natural size disagrees is a preview"). This suite is the one + * caller that cannot get away with that: the browser executor reads the real + * decoded image's `naturalWidth`/`naturalHeight` (`AnnotationPage.tsx`'s + * `onImageReady`) as the bounds it validates click coordinates against + * (`efficientSam.ts`'s `x < 0 || x > width || ...`). A 1×1 real image makes every + * on-canvas click land "outside" it. `mockAssetImage` below overrides just this + * file's `/content` route with a real, correctly-sized PNG — scoped here rather + * than changing `_wireApiStub.ts`'s shared default, which those ~150 other tests + * may depend on for load speed. */ import { existsSync, readFileSync } from "node:fs"; import path from "node:path"; +import zlib from "node:zlib"; import { expect, test, type Page, type Request } from "@playwright/test"; -import { openJob } from "./_wireApiStub"; +import { JOB, serveApi } from "./_wireApiStub"; const ARTIFACTS_DIR = path.resolve( import.meta.dirname, "..", "..", "browser-inference", "model-artifacts", "efficientsam-ti", @@ -72,13 +86,88 @@ async function mockCdn(page: Page, encoderBytes = ENCODER_BYTES, decoderBytes = ); } +// A minimal PNG encoder (signature + IHDR + one IDAT + IEND), so this file needs +// no image-processing dependency to produce a real, correctly-sized asset. +// Grayscale, 8-bit, one filter byte per row — `zlib.deflateSync` already emits a +// standard zlib stream, which is exactly what an IDAT chunk holds. Verified by +// hand against a real Chromium `Image.decode()` while writing this: a solid +// 640×480 PNG from this function reports `naturalWidth: 640, naturalHeight: 480`. +const CRC_TABLE = ((): Uint32Array => { + const table = new Uint32Array(256); + for (let n = 0; n < 256; n++) { + let c = n; + for (let k = 0; k < 8; k++) c = c & 1 ? 0xedb88320 ^ (c >>> 1) : c >>> 1; + table[n] = c >>> 0; + } + return table; +})(); + +function crc32(buf: Buffer): number { + let c = 0xffffffff; + for (const byte of buf) c = CRC_TABLE[(c ^ byte) & 0xff]! ^ (c >>> 8); + return (c ^ 0xffffffff) >>> 0; +} + +function pngChunk(type: string, data: Buffer): Buffer { + const typeBuf = Buffer.from(type, "ascii"); + const length = Buffer.alloc(4); + length.writeUInt32BE(data.length, 0); + const crc = Buffer.alloc(4); + crc.writeUInt32BE(crc32(Buffer.concat([typeBuf, data])), 0); + return Buffer.concat([length, typeBuf, data, crc]); +} + +function solidPng(width: number, height: number, gray = 128): Buffer { + const signature = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]); + const ihdrData = Buffer.alloc(13); + ihdrData.writeUInt32BE(width, 0); + ihdrData.writeUInt32BE(height, 4); + ihdrData[8] = 8; // bit depth + ihdrData[9] = 0; // color type: grayscale + const raw = Buffer.alloc((width + 1) * height); + for (let y = 0; y < height; y++) { + const rowStart = y * (width + 1); + raw[rowStart] = 0; // filter type: none + raw.fill(gray, rowStart + 1, rowStart + 1 + width); + } + return Buffer.concat([ + signature, + pngChunk("IHDR", ihdrData), + pngChunk("IDAT", zlib.deflateSync(raw)), + pngChunk("IEND", Buffer.alloc(0)), + ]); +} + +/** Matches `_wireApiStub.ts`'s `asset()` metadata — the coordinate frame every click here is in. */ +const ASSET_WIDTH = 640; +const ASSET_HEIGHT = 480; +const REAL_ASSET_IMAGE = solidPng(ASSET_WIDTH, ASSET_HEIGHT); + +/** + * Overrides `/content` with a real, correctly-sized PNG. Must be registered + * *after* `serveApi`'s own `**\/api/**` handler (Playwright tries the + * most-recently-added matching handler first) so this one wins for `/content` + * instead of `_wireApiStub.ts`'s 1×1 `PIXEL` — and before `page.goto`, since the + * asset image is requested as soon as the annotation page mounts. + */ +async function mockAssetImage(page: Page): Promise { + await page.route("**/projects/**/assets/**/content", (route) => + route.fulfill({ contentType: "image/png", body: REAL_ASSET_IMAGE }), + ); +} + async function openJobWithBrowserRuntime( page: Page, sent: Request[], suggestible: boolean, ): Promise { await mockCdn(page); - await openJob(page, sent, undefined, undefined, undefined, undefined, suggestible); + await serveApi(page, sent, undefined, undefined, undefined, undefined, suggestible); + await mockAssetImage(page); + await page.goto(`/jobs/${JOB}`); + await page.getByTestId("token-input").fill("a-token"); + await page.getByTestId("token-submit").click(); + await expect(page.getByTestId("annotation-page")).toBeVisible(); } function suggestCallsOf(sent: Request[]): Request[] { From 1e109376a0107fa6fe5a0f7a8d1818acb7700d64 Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya <1445792+JArmandoAnaya@users.noreply.github.com> Date: Thu, 17 Sep 2026 03:57:37 -0700 Subject: [PATCH 32/42] fix(browser-inference): move the asset-resolution doc-comment to the function it describes It had drifted above `withTrailingSlash`, the helper it only mentions in its last paragraph, and away from `assetsAt`, which is what actually decides where the ORT artifacts are fetched from. --- frontend/browser-inference/src/browser/worker.ts | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/frontend/browser-inference/src/browser/worker.ts b/frontend/browser-inference/src/browser/worker.ts index 68800a9b..1960fa61 100644 --- a/frontend/browser-inference/src/browser/worker.ts +++ b/frontend/browser-inference/src/browser/worker.ts @@ -95,13 +95,18 @@ function fail(id: OperationId, code: InferenceRuntimeErrorCode, error: unknown): }); } +function withTrailingSlash(base: string): string { + return base.endsWith("/") ? base : `${base}/`; +} + /** * Where the ORT WebAssembly artifacts are fetched from. * * Resolved against this module's own URL by default, so the built worker finds the * `ort/` directory the build step put beside it and an installed package works with the * host serving nothing special. A host that would rather serve them from its own origin - * passes `assetBaseUrl`; the main thread never guesses this location, because only the + * — or whose bundler has moved this worker away from that directory — passes + * `assetBaseUrl`; the main thread never guesses this location, because only the * worker knows where it was loaded from. * * The trailing slash is re-applied to the resolved string rather than trusted to the @@ -110,10 +115,6 @@ function fail(id: OperationId, code: InferenceRuntimeErrorCode, error: unknown): * instead of a file inside it — a 404 a dev server answers with an SPA fallback, so the * dynamic import receives HTML and every execution provider fails to initialize. */ -function withTrailingSlash(base: string): string { - return base.endsWith("/") ? base : `${base}/`; -} - function assetsAt(stated: string | undefined): string { if (stated !== undefined && stated !== "") return withTrailingSlash(stated); return withTrailingSlash(new URL("./ort/", import.meta.url).href); From efb2d7cbfe239f824b2f252c0779bbc08e9e665a Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya <1445792+JArmandoAnaya@users.noreply.github.com> Date: Thu, 17 Sep 2026 03:57:44 -0700 Subject: [PATCH 33/42] fix(app): serve browser-inference's ORT assets in production builds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A production `vite build` emitted no `ort/` directory at all. The worker's default resolution — `./ort/` against its own module URL — is right for the package as installed, but rolldown hashes the worker into `assets/`, so the built bundle asked for `/app/assets/ort/ort-wasm-simd-threaded.asyncify.mjs`, which no build writes. The SPA fallback answers that with HTML, the dynamic import of the glue fails, and every execution provider fails to initialize — a feature that works under the dev server and 404s in every real deployment. A vite plugin now copies `@visionset/browser-inference`'s own `dist/browser/ort/` into the build output at `ort/`, and serves the same URL from a dev middleware, so both modes answer the one address the app states through `assetBaseUrl`: `ort/`, absolute against the document, since `BASE_URL` is the only thing that knows the wheel mounts the bundle at `/app/`. The bytes are read from the dependency's build output at build time, so an ONNX Runtime bump needs nothing here. --- .../BrowserInferenceRuntime.ts | 21 +++++- frontend/app/vite.config.ts | 73 ++++++++++++++++++- 2 files changed, 91 insertions(+), 3 deletions(-) diff --git a/frontend/app/src/data/browserInference/BrowserInferenceRuntime.ts b/frontend/app/src/data/browserInference/BrowserInferenceRuntime.ts index 65197b0e..2f6eaa1b 100644 --- a/frontend/app/src/data/browserInference/BrowserInferenceRuntime.ts +++ b/frontend/app/src/data/browserInference/BrowserInferenceRuntime.ts @@ -19,7 +19,26 @@ interface Deps { readonly createRuntime: (artifacts: { encoder: Uint8Array; decoder: Uint8Array }) => PromptableSegmentationRuntime; } -const REAL_DEPS: Deps = { acquire: acquireEfficientSam, createRuntime: createEfficientSamRuntime }; +/** + * Where this deployment serves ONNX Runtime's WebAssembly artifacts. + * + * Stated rather than left to the worker's own default, which resolves `./ort/` against + * the worker's module URL: correct for the package as installed, wrong once vite has + * hashed the worker into `assets/`. `vite.config.ts` puts the directory at `ort/`, + * and `BASE_URL` is the only thing that knows what `` is — `/app/` in a build, + * because the wheel mounts the bundle there, and `/` under the dev server. + * + * Absolute, against the document: the worker resolves a relative `wasmPaths` against + * *its* location, which is the one place the path must not be relative to. + */ +function ortAssetBaseUrl(): string { + return new URL(`${import.meta.env.BASE_URL}ort/`, window.location.href).href; +} + +const REAL_DEPS: Deps = { + acquire: acquireEfficientSam, + createRuntime: (artifacts) => createEfficientSamRuntime({ ...artifacts, assetBaseUrl: ortAssetBaseUrl() }), +}; type State = { readonly kind: "unacquired" } | { readonly kind: "ready"; readonly runtime: PromptableSegmentationRuntime }; diff --git a/frontend/app/vite.config.ts b/frontend/app/vite.config.ts index b9862271..ce23d817 100644 --- a/frontend/app/vite.config.ts +++ b/frontend/app/vite.config.ts @@ -1,9 +1,78 @@ +import { createReadStream, readdirSync } from "node:fs"; +import { readFile } from "node:fs/promises"; +import { basename, dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; + import tailwindcss from "@tailwindcss/vite"; import react from "@vitejs/plugin-react"; -import { defineConfig } from "vite"; +import { defineConfig, type Plugin } from "vite"; + +/** + * Serve `@visionset/browser-inference`'s ONNX Runtime WebAssembly artifacts as part of + * *this* app's static output. + * + * The package ships them inside its own `dist/browser/ort/`, beside the worker that + * fetches them, and the worker's default resolution — `./ort/` against its own module + * URL — is correct for the package used as-installed. It stops being correct the moment + * a bundler owns the worker: rolldown hashes `worker.js` into `assets/`, so the default + * resolves to `assets/ort/`, a directory no build ever writes, and every execution + * provider fails to initialize on a 404 the SPA fallback answers with HTML. + * + * So the directory is copied to `ort/` instead and the app states that location + * through `assetBaseUrl` (see `src/data/browserInference/BrowserInferenceRuntime.ts`). + * The files are read out of the dependency's build output rather than kept in this + * package's `public/`: a 25 MB binary has no business in a source tree, and an ONNX + * Runtime bump must not need a second, hand-updated copy. + * + * The dev server gets the same URL from a middleware rather than a copy, so dev and + * production differ in how the bytes arrive and not in what the running code asks for. + */ +function ortAssets(): Plugin { + // Through the subpath the package exports, not by joining `dist/` onto a resolved + // package root — `exports` deliberately omits `./package.json`, and a hand-written + // `node_modules` path would survive a pnpm store layout change only by accident. + // `import.meta.resolve` rather than `createRequire().resolve`, because the subpath is + // declared under the `import` condition alone and CJS resolution refuses it outright. + const directory = join(dirname(fileURLToPath(import.meta.resolve("@visionset/browser-inference/browser"))), "ort"); + + let files: readonly string[]; + try { + files = readdirSync(directory); + } catch (cause) { + throw new Error( + `browser-inference's ORT assets are missing from ${directory}. ` + + "Run `pnpm --filter @visionset/browser-inference build` first.", + { cause }, + ); + } + if (files.length === 0) { + throw new Error(`browser-inference's ORT asset directory ${directory} is empty.`); + } + + return { + name: "visionset:ort-assets", + configureServer(server) { + server.middlewares.use("/ort", (request, response, next) => { + // `files` is the allowlist, which is also what keeps a `..` out of the join. + const name = basename((request.url ?? "").split("?")[0] ?? ""); + if (!files.includes(name)) { + next(); + return; + } + response.setHeader("Content-Type", name.endsWith(".wasm") ? "application/wasm" : "text/javascript"); + createReadStream(join(directory, name)).pipe(response); + }); + }, + async generateBundle() { + for (const name of files) { + this.emitFile({ type: "asset", fileName: `ort/${name}`, source: await readFile(join(directory, name)) }); + } + }, + }; +} export default defineConfig(({ command }) => ({ - plugins: [react(), tailwindcss()], + plugins: [react(), tailwindcss(), ortAssets()], server: { // The dev proxy, and the reason the server has no CORS middleware. // From 38cefdd520fa9f5ff1b166feccb64dd928812fd1 Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya <1445792+JArmandoAnaya@users.noreply.github.com> Date: Thu, 17 Sep 2026 04:08:29 -0700 Subject: [PATCH 34/42] test(packaging): guard that the ORT runtime travels inside the wheel The production fix shipped with nothing that would notice its removal: delete the `ortAssets()` plugin and tsc, lint, 96 app tests, 129 browser-inference tests and the dev e2e suite all stay green while `/app/` serves an app whose first "this device" suggestion fails inside a worker. The wheel suite already builds the frontend, so the guard is one assertion against output that exists. Counting the wheel's size had to change with it. `_static/ort/` is ~6.4 MB deflated, against a 2 MB ceiling whose stated job is catching the day a `node_modules/` or a fixture video gets swept in. Raising that ceiling past 8 MB would have retired the guard to admit one file, so the ORT runtime is excluded from it and bounded separately instead: the rest of the wheel is still held to 2 MB, and a version bump that doubles ONNX Runtime now reads as a sentence about ONNX Runtime. Also in the plugin, two ways it could fail unhelpfully: an unhandled `error` on the dev middleware's read stream took down the whole dev server, which `tsup`'s `clean: true` makes reachable by rebuilding browser-inference while the server is up; and a subdirectory under `dist/browser/ort/` would have surfaced as an opaque EISDIR from `readFile` rather than as the missing-assets message written for it. --- frontend/app/vite.config.ts | 11 +++++-- tests/packaging/test_wheel.py | 58 ++++++++++++++++++++++++++++++++--- 2 files changed, 63 insertions(+), 6 deletions(-) diff --git a/frontend/app/vite.config.ts b/frontend/app/vite.config.ts index ce23d817..de428d3d 100644 --- a/frontend/app/vite.config.ts +++ b/frontend/app/vite.config.ts @@ -37,7 +37,11 @@ function ortAssets(): Plugin { let files: readonly string[]; try { - files = readdirSync(directory); + // `withFileTypes` so a subdirectory ever appearing here fails as the empty case + // below rather than as an opaque `EISDIR` from `readFile` half a build later. + files = readdirSync(directory, { withFileTypes: true }) + .filter((entry) => entry.isFile()) + .map((entry) => entry.name); } catch (cause) { throw new Error( `browser-inference's ORT assets are missing from ${directory}. ` + @@ -60,7 +64,10 @@ function ortAssets(): Plugin { return; } response.setHeader("Content-Type", name.endsWith(".wasm") ? "application/wasm" : "text/javascript"); - createReadStream(join(directory, name)).pipe(response); + // `tsup` builds with `clean: true`, so rebuilding browser-inference while this + // server is up deletes these files under an in-flight request. Unhandled, that + // stream's `error` event takes down the whole dev server. + createReadStream(join(directory, name)).on("error", next).pipe(response); }); }, async generateBundle() { diff --git a/tests/packaging/test_wheel.py b/tests/packaging/test_wheel.py index e88dca71..42914665 100644 --- a/tests/packaging/test_wheel.py +++ b/tests/packaging/test_wheel.py @@ -68,6 +68,20 @@ #: in a directory listing. MAX_WHEEL_BYTES = 2 * 1024 * 1024 +#: The one deliberately enormous thing inside, measured separately so the guard +#: above keeps its teeth. +#: +#: `_static/ort/` is ONNX Runtime's WebAssembly runtime, ~25 MB uncompressed and +#: ~6.4 MB deflated, and it has to be in the wheel: browser inference runs the +#: model on the user's machine, and the wheel is the only thing that serves the +#: app. Folding it into `MAX_WHEEL_BYTES` would have meant raising that ceiling +#: past 8 MB, which is the same as deleting it — the accidents it exists to catch +#: are megabyte-sized. So the rest of the wheel is still held to 2 MB and this +#: payload is bounded on its own, where a version bump that doubles it is a +#: sentence about ONNX Runtime rather than a mystery about the wheel. +ORT_PREFIX = "visionset/_static/ort/" +MAX_ORT_BYTES = 10 * 1024 * 1024 + #: Nothing matching these may be inside. Each is something that has ended up in #: somebody's wheel: a dependency tree, a test corpus, a virtualenv, a workspace. FORBIDDEN = ( @@ -80,7 +94,8 @@ ) #: Media suffixes. `_static/` legitimately holds none today — the app ships as -#: HTML, CSS and JavaScript — so any of these is something nobody meant to ship. +#: HTML, CSS, JavaScript and one WebAssembly runtime — so any of these is +#: something nobody meant to ship. FORBIDDEN_SUFFIXES = (".mp4", ".mov", ".avi", ".jpg", ".jpeg", ".tiff", ".bmp") #: How long the freshly installed server gets to bind a socket. @@ -141,6 +156,21 @@ def test_the_compiled_app_travels_inside_the_wheel(names: list[str]) -> None: assert any(name.endswith(".css") for name in names) +def test_the_browser_inference_runtime_travels_inside_the_wheel(names: list[str]) -> None: + """The same failure as above, one directory over and with no 404 to read. + + ONNX Runtime's WebAssembly runtime is fetched by the inference worker at the + moment a user first asks this device for a suggestion — not at page load — so a + wheel without it serves an app that looks entirely healthy until that click, + and then fails inside a worker where the SPA fallback has already answered the + 404 with HTML. `frontend/app/vite.config.ts` copies the directory out of + `@visionset/browser-inference`'s build output for exactly this reason, and + nothing else in the suite would notice if that plugin were removed. + """ + assert any(name.startswith(ORT_PREFIX) for name in names) + assert any(name.endswith(".wasm") for name in names) + + def test_the_bundle_was_built_for_the_ui_prefix(names: list[str]) -> None: """The bundle-base trap, and it is invisible once the wheel is built. @@ -202,9 +232,29 @@ def test_nothing_enormous_came_along_for_the_ride(names: list[str]) -> None: def test_the_wheel_stays_under_its_ceiling() -> None: - """A guard, not a budget. See `MAX_WHEEL_BYTES`.""" - size = WHEEL.stat().st_size - assert size < MAX_WHEEL_BYTES, f"{WHEEL.name} is {size} bytes" + """A guard, not a budget. See `MAX_WHEEL_BYTES`. + + Everything but the ORT runtime, which has its own ceiling — see `MAX_ORT_BYTES` + for why the two are counted apart. + """ + with zipfile.ZipFile(WHEEL) as archive: + size = sum( + entry.compress_size + for entry in archive.infolist() + if not entry.filename.startswith(ORT_PREFIX) + ) + assert size < MAX_WHEEL_BYTES, f"{WHEEL.name} is {size} bytes without the ORT runtime" + + +def test_the_ort_runtime_stays_under_its_own_ceiling() -> None: + """See `MAX_ORT_BYTES`.""" + with zipfile.ZipFile(WHEEL) as archive: + size = sum( + entry.compress_size + for entry in archive.infolist() + if entry.filename.startswith(ORT_PREFIX) + ) + assert size < MAX_ORT_BYTES, f"{ORT_PREFIX} is {size} bytes" def test_no_source_maps_ship(names: list[str]) -> None: From 8aa1623a3aec82f7d666ba11e67c709cf87d8b4f Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya <1445792+JArmandoAnaya@users.noreply.github.com> Date: Thu, 17 Sep 2026 04:15:38 -0700 Subject: [PATCH 35/42] docs(packaging): correct MAX_WHEEL_BYTES' stated numbers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The comment still described a ~570 KB wheel with a ~640 KB bundle and "room for the UI to roughly triple", which was stale twice over: the constant now counts everything but the ORT runtime, and the app bundle has since grown to ~1.19 MB uncompressed on its own. The counted part is ~1.45 MB compressed, so the real headroom is about 1.45x — worth saying plainly, because a reader who trusts "roughly triple" will misjudge how close the next UI addition puts them to a ceiling they would then raise without noticing they were the reason. No threshold or behaviour change. --- tests/packaging/test_wheel.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/tests/packaging/test_wheel.py b/tests/packaging/test_wheel.py index 42914665..d7694e9e 100644 --- a/tests/packaging/test_wheel.py +++ b/tests/packaging/test_wheel.py @@ -59,13 +59,17 @@ REPO_ROOT = Path(__file__).resolve().parents[2] -#: The ceiling a wheel may not cross, and it is a guard rather than a budget. +#: The ceiling the counted part of a wheel may not cross, a guard rather than a +#: budget. Everything but `ORT_PREFIX` is counted — see `MAX_ORT_BYTES`. #: -#: The wheel is ~570 KB today, of which ~640 KB uncompressed is the app's one -#: JavaScript bundle. Two megabytes leaves room for the UI to roughly triple and -#: still fails loudly the day `node_modules/`, a fixture video or a `.venv` gets -#: swept in — which is the failure this exists for, and the one that is invisible -#: in a directory listing. +#: That counted part is ~1.45 MB compressed today, of which the app's one +#: JavaScript bundle is ~350 KB compressed and ~1.19 MB uncompressed. So the +#: headroom is about 1.45x, not the "roughly triple" this said while the bundle +#: was ~640 KB uncompressed — the UI has grown into most of it, and the next +#: person to read this is likelier to be raising the number than reassured by it. +#: It still fails loudly the day `node_modules/`, a fixture video or a `.venv` +#: gets swept in, which is the failure this exists for and the one that is +#: invisible in a directory listing. MAX_WHEEL_BYTES = 2 * 1024 * 1024 #: The one deliberately enormous thing inside, measured separately so the guard From 3eccb124c8a03d7693953fc2810b958620c33802 Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya <1445792+JArmandoAnaya@users.noreply.github.com> Date: Thu, 17 Sep 2026 04:49:47 -0700 Subject: [PATCH 36/42] fix(app): hold one executor, and wait for the ORT session before claiming ready MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three things the composition root was getting wrong about its own lifecycle. **One encode per asset.** `createBrowserSuggestionExecutor`'s one-embedding slot is the only encode cache there is, and it lives in that closure. `executorFor` built a fresh executor per call, `AnnotationPage` calls it during render, and it re-renders on every click — so every refinement click re-ran the full encoder pass, silently defeating the design's "one encode per asset, N decodes per N refinements". The executor is now memoized beside the runtime it wraps, which keeps the invariant true whatever ui-core's render behaviour does. **Ready means the session exists.** `createRuntime` returns before the worker has loaded a graph; `ready()` is what waits for that. Claiming ready on the constructor alone listed a target whose every click then refused — and a listed target hides the Download control, so the retry path went with it. `ready()` is now awaited, a rejection leaves the state unacquired and retryable, and the failed runtime is disposed rather than leaked. **No download that can only fail.** `listAcquisitions()` consults the package's own capability check first: a browser with no Worker or no WebAssembly is offered nothing, which is the port's own rule one level down. --- .../BrowserInferenceRuntime.test.ts | 88 +++++++++++++++++-- .../BrowserInferenceRuntime.ts | 64 ++++++++++++-- 2 files changed, 138 insertions(+), 14 deletions(-) diff --git a/frontend/app/src/data/browserInference/BrowserInferenceRuntime.test.ts b/frontend/app/src/data/browserInference/BrowserInferenceRuntime.test.ts index 65e5129a..f9783c9c 100644 --- a/frontend/app/src/data/browserInference/BrowserInferenceRuntime.test.ts +++ b/frontend/app/src/data/browserInference/BrowserInferenceRuntime.test.ts @@ -1,16 +1,23 @@ import { describe, expect, it, vi } from "vitest"; +import type { SuggestionRequest } from "@visionset/ui-core"; import { createOssBrowserInferenceRuntime } from "./BrowserInferenceRuntime.js"; import { EFFICIENT_SAM_TI_REVISION } from "./manifest.js"; -function fakeDeps(overrides?: { acquire?: () => Promise<{ encoder: Uint8Array; decoder: Uint8Array }> }) { +function fakeDeps(overrides?: { + acquire?: () => Promise<{ encoder: Uint8Array; decoder: Uint8Array }>; + ready?: () => Promise; + supported?: () => boolean; +}) { + const prepareImage = vi.fn(async () => ({ width: 4, height: 4 })); + const dispose = vi.fn(); const createRuntime = vi.fn(() => ({ - ready: async () => [], - prepareImage: async () => ({ width: 1, height: 1 }), - suggest: async () => ({ width: 1, height: 1, mask: new Uint8Array(1), confidence: 1 }), - dispose: () => {}, + ready: overrides?.ready ?? (async () => []), + prepareImage, + suggest: async () => ({ width: 4, height: 4, mask: new Uint8Array(16).fill(1), confidence: 1 }), + dispose, })); const acquire = vi.fn(overrides?.acquire ?? (async () => ({ encoder: new Uint8Array(1), decoder: new Uint8Array(1) }))); - return { acquire, createRuntime }; + return { acquire, createRuntime, supported: overrides?.supported ?? ((): boolean => true), prepareImage, dispose }; } describe("createOssBrowserInferenceRuntime", () => { @@ -62,4 +69,73 @@ describe("createOssBrowserInferenceRuntime", () => { const runtime = createOssBrowserInferenceRuntime(fakeDeps()); expect(() => runtime.executorFor("efficient-sam-ti")).toThrow(); }); + + it("offers no acquisition at all where no runtime could exist", async () => { + // A download that can only end in `unsupported-runtime` is worse than no control: + // the port's rule is that a host which cannot honour a control does not offer it. + const deps = fakeDeps({ supported: () => false }); + const runtime = createOssBrowserInferenceRuntime(deps); + expect(runtime.listAcquisitions?.()).toEqual([]); + expect(await runtime.listTargets()).toEqual([]); + expect(deps.acquire).not.toHaveBeenCalled(); + }); + + describe("readiness is the ORT session's, not the constructor's", () => { + it("leaves the model unacquired and retryable when ready() rejects, and disposes it", async () => { + // `createRuntime` returns before the worker has loaded a graph. Setting "ready" on + // it alone lists a target whose every click then refuses — and a listed target hides + // the Download control, so the acquisition UI's own retry path is gone too. + const deps = fakeDeps({ ready: () => Promise.reject(new Error("graph load failed")) }); + const runtime = createOssBrowserInferenceRuntime(deps); + + await expect(runtime.listAcquisitions?.()[0]!.acquire()).rejects.toThrow("graph load failed"); + + expect(await runtime.listTargets()).toEqual([]); + expect(runtime.listAcquisitions?.()).toHaveLength(1); + // The worker behind the failed runtime is let go rather than left running. + expect(deps.dispose).toHaveBeenCalledTimes(1); + }); + }); + + describe("one encode per asset survives composition", () => { + it("hands out the same executor on every executorFor call", async () => { + const runtime = createOssBrowserInferenceRuntime(fakeDeps()); + await runtime.listAcquisitions?.()[0]!.acquire(); + expect(runtime.executorFor("efficient-sam-ti")).toBe(runtime.executorFor("efficient-sam-ti")); + }); + + it("encodes once across two separately obtained executors", async () => { + // The behavioural half, and the one that matters: the embedding cache lives inside + // the executor closure, so a fresh executor per call throws it away. `AnnotationPage` + // calls `executorFor` during render and re-renders on every click, which made each + // refinement click a full encoder pass. Two `executorFor` results, two asks, one + // `prepareImage` — that is the design's "one encode per asset, N decodes per N + // refinements", proved where the composition actually happens. + const deps = fakeDeps(); + const runtime = createOssBrowserInferenceRuntime(deps); + await runtime.listAcquisitions?.()[0]!.acquire(); + + const rgb = new Uint8Array(4 * 4 * 3); + runtime.setActiveAsset?.({ + assetId: "a1", + width: 4, + height: 4, + readRgb: () => ({ width: 4, height: 4, rgb }), + }); + + const request: SuggestionRequest = { + projectId: "p1", + assetId: "a1", + positive: [[1, 1]], + negative: [], + allowedGeometries: ["polygon"], + adjustments: { tolerance: 2 }, + }; + + await runtime.executorFor("efficient-sam-ti").suggest(request); + await runtime.executorFor("efficient-sam-ti").suggest(request); + + expect(deps.prepareImage).toHaveBeenCalledTimes(1); + }); + }); }); diff --git a/frontend/app/src/data/browserInference/BrowserInferenceRuntime.ts b/frontend/app/src/data/browserInference/BrowserInferenceRuntime.ts index 2f6eaa1b..e41bd669 100644 --- a/frontend/app/src/data/browserInference/BrowserInferenceRuntime.ts +++ b/frontend/app/src/data/browserInference/BrowserInferenceRuntime.ts @@ -1,5 +1,5 @@ import type { PromptableSegmentationRuntime } from "@visionset/browser-inference"; -import { createEfficientSamRuntime } from "@visionset/browser-inference/browser"; +import { browserSupports, createEfficientSamRuntime } from "@visionset/browser-inference/browser"; import type { BrowserSuggestionAssetSource, BrowserSuggestionTarget, @@ -17,6 +17,12 @@ const MODEL_REF = `efficient-sam-ti@${EFFICIENT_SAM_TI_REVISION}`; interface Deps { readonly acquire: (signal?: AbortSignal) => Promise<{ encoder: Uint8Array; decoder: Uint8Array }>; readonly createRuntime: (artifacts: { encoder: Uint8Array; decoder: Uint8Array }) => PromptableSegmentationRuntime; + /** + * Whether a runtime can exist in this environment at all — a `Worker` to host ORT and + * WebAssembly to run it. Injected rather than called directly so both answers are + * reachable from a Node test; the real one is the package's own capability check. + */ + readonly supported: () => boolean; } /** @@ -38,9 +44,27 @@ function ortAssetBaseUrl(): string { const REAL_DEPS: Deps = { acquire: acquireEfficientSam, createRuntime: (artifacts) => createEfficientSamRuntime({ ...artifacts, assetBaseUrl: ortAssetBaseUrl() }), + supported: browserSupports, }; -type State = { readonly kind: "unacquired" } | { readonly kind: "ready"; readonly runtime: PromptableSegmentationRuntime }; +/** + * The executor is held *here*, beside the runtime it wraps, rather than built per call. + * + * `createBrowserSuggestionExecutor`'s one-embedding slot is the only encode cache in the + * system, and it lives in that closure — so a fresh executor per `executorFor` call throws + * the embedding away. `AnnotationPage` calls `executorFor` during render, and it re-renders + * on every click, which turned "one encode per asset, N decodes per N refinements" into a + * full ~25 MB encoder pass per refinement click. Memoizing at the composition root keeps the + * invariant true whatever `ui-core`'s render behaviour does, which a `useMemo` over there + * would not. + */ +type State = + | { readonly kind: "unacquired" } + | { + readonly kind: "ready"; + readonly runtime: PromptableSegmentationRuntime; + readonly executor: SuggestionExecutor; + }; export function createOssBrowserInferenceRuntime(deps: Deps = REAL_DEPS): VisionSetBrowserInferenceRuntime { let state: State = { kind: "unacquired" }; @@ -53,6 +77,9 @@ export function createOssBrowserInferenceRuntime(deps: Deps = REAL_DEPS): Vision }, listAcquisitions() { if (state.kind === "ready") return []; + // The port's own convention, one level down: a control that cannot be honoured is not + // offered. A browser with no `Worker` or no WebAssembly can only fail this download. + if (!deps.supported()) return []; return [ { id: MODEL_ID, @@ -65,7 +92,32 @@ export function createOssBrowserInferenceRuntime(deps: Deps = REAL_DEPS): Vision try { const artifacts = await deps.acquire(options?.signal); const runtime = deps.createRuntime(artifacts); - state = { kind: "ready", runtime }; + // `createRuntime` returns before the worker has loaded either graph or + // settled on an execution provider; `ready()` is what waits for that. + // Claiming "ready" on the constructor alone lists a target that every + // later click refuses — and, since a listed target hides the download + // control, refuses with no way back short of a page reload. + try { + await runtime.ready(); + } catch (error) { + try { + runtime.dispose(); + } catch { + // A runtime that could not start may not stop cleanly either; the + // load failure is the one worth reporting, and the worker still has + // to be let go. + } + throw error; + } + state = { + kind: "ready", + runtime, + executor: createBrowserSuggestionExecutor({ + modelRef: MODEL_REF, + runtime, + getActiveSource: () => activeAssetSource, + }), + }; } finally { inFlight = null; } @@ -79,11 +131,7 @@ export function createOssBrowserInferenceRuntime(deps: Deps = REAL_DEPS): Vision if (state.kind !== "ready" || targetId !== MODEL_ID) { throw new Error(`no ready browser target "${targetId}"`); } - return createBrowserSuggestionExecutor({ - modelRef: MODEL_REF, - runtime: state.runtime, - getActiveSource: () => activeAssetSource, - }); + return state.executor; }, setActiveAsset(source: BrowserSuggestionAssetSource | null): void { activeAssetSource = source; From 1d63104b715ee418dd4a70bf2070322a9aac9535 Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya <1445792+JArmandoAnaya@users.noreply.github.com> Date: Thu, 17 Sep 2026 04:49:57 -0700 Subject: [PATCH 37/42] fix(app): stop reporting browser-local failures as "the server could not be reached" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every refusal this executor raised besides the negative-point one was a bare `Error`, and `refusalProse` routes a non-`ApiError` through `asApiError`, which stamps it `NETWORK_ERROR` — whose prose blames a server that was never asked. Three codes, each with prose that says what actually happened on this device: `BROWSER_ASSET_CHANGED` for the missing/mismatched source and both staleness checks, `BROWSER_INFERENCE_UNAVAILABLE` for a runtime that could not start, and `BROWSER_INFERENCE_FAILED` for a run that did not answer. The runtime's own errors are mapped through an exhaustive table over `InferenceRuntimeErrorCode`, so a code added to that package stops compiling here until somebody decides what a person should be told about it. `refusals.test.ts` holds the "never says the server" assertion for all three, which is the regression that would otherwise reopen silently. --- .../BrowserSuggestionExecutor.ts | 83 +++++++++++++++++-- frontend/ui-core/src/data/refusals.test.ts | 14 ++++ frontend/ui-core/src/data/refusals.ts | 10 +++ 3 files changed, 98 insertions(+), 9 deletions(-) diff --git a/frontend/app/src/data/browserInference/BrowserSuggestionExecutor.ts b/frontend/app/src/data/browserInference/BrowserSuggestionExecutor.ts index 7057e11b..81855949 100644 --- a/frontend/app/src/data/browserInference/BrowserSuggestionExecutor.ts +++ b/frontend/app/src/data/browserInference/BrowserSuggestionExecutor.ts @@ -30,7 +30,12 @@ * with the server's Python and pinned to it by a fixture. */ import { shapesFromMask } from "@visionset/annotator"; -import type { PreparedImage, PromptableSegmentationRuntime } from "@visionset/browser-inference"; +import { isInferenceRuntimeError } from "@visionset/browser-inference"; +import type { + InferenceRuntimeErrorCode, + PreparedImage, + PromptableSegmentationRuntime, +} from "@visionset/browser-inference"; import { ApiError } from "@visionset/ui-core"; import type { BrowserSuggestionAssetSource, @@ -47,6 +52,47 @@ interface Deps { readonly getActiveSource: () => BrowserSuggestionAssetSource | null; } +/** + * Every refusal this executor raises is an `ApiError`, because `refusalProse` stamps + * anything else `NETWORK_ERROR` — and "the server could not be reached" is a lie about a + * failure that never left the tab. The codes below are this file's own, matched by + * `REFUSAL_PROSE` entries in `ui-core`. + */ +const ASSET_CHANGED = "BROWSER_ASSET_CHANGED"; +const INFERENCE_UNAVAILABLE = "BROWSER_INFERENCE_UNAVAILABLE"; +const INFERENCE_FAILED = "BROWSER_INFERENCE_FAILED"; + +/** + * Which refusal each of the runtime's own failures becomes. + * + * Exhaustive over the closed `InferenceRuntimeErrorCode` union on purpose: a code added + * to the package stops compiling here until somebody decides what a person should be + * told about it, rather than falling into a generic bucket by default. + * + * The split is between "this device cannot run the model" — a dead end for the session, + * where the honest remedy is Server — and "this ask did not work", which a second click + * may well answer. `image-superseded` is neither: it is the runtime saying the embedding + * it held has been replaced, which is the same fact as the staleness checks below. + */ +const REFUSAL_FOR: Readonly> = { + "unsupported-runtime": INFERENCE_UNAVAILABLE, + "worker-initialization-failed": INFERENCE_UNAVAILABLE, + "worker-crashed": INFERENCE_UNAVAILABLE, + "webgpu-unavailable": INFERENCE_UNAVAILABLE, + "graph-load-failed": INFERENCE_UNAVAILABLE, + disposed: INFERENCE_UNAVAILABLE, + "runtime-execution-failed": INFERENCE_FAILED, + "prompt-rejected": INFERENCE_FAILED, + cancelled: INFERENCE_FAILED, + "image-superseded": ASSET_CHANGED, +}; + +/** The runtime's own error as a refusal, or anything else untouched. */ +function asRefusal(error: unknown): unknown { + if (!isInferenceRuntimeError(error)) return error; + return new ApiError({ code: REFUSAL_FOR[error.code], message: error.message }); +} + export function createBrowserSuggestionExecutor(deps: Deps): SuggestionExecutor { // The runtime's one embedding, and which source leased the pixels behind it. One slot, because // the runtime has one slot; see the note at the top of the file. @@ -74,7 +120,10 @@ export function createBrowserSuggestionExecutor(deps: Deps): SuggestionExecutor const source = deps.getActiveSource(); if (source === null || source.assetId !== request.assetId) { - throw new Error(`no active browser asset source for asset ${request.assetId}`); + throw new ApiError({ + code: ASSET_CHANGED, + message: `no active browser asset source for asset ${request.assetId}`, + }); } let preparing: Promise; @@ -105,17 +154,33 @@ export function createBrowserSuggestionExecutor(deps: Deps): SuggestionExecutor currentPrepared = started; preparing = started; } - const preparedImage = await preparing; + let preparedImage: PreparedImage; + try { + preparedImage = await preparing; + } catch (error) { + throw asRefusal(error); + } if (deps.getActiveSource() !== source) { - throw new Error("the active asset changed while this device was preparing the image"); + throw new ApiError({ + code: ASSET_CHANGED, + message: "the active asset changed while this device was preparing the image", + }); } - const raw = await deps.runtime.suggest(preparedImage, { - positive: request.positive, - negative: [], - }); + let raw; + try { + raw = await deps.runtime.suggest(preparedImage, { + positive: request.positive, + negative: [], + }); + } catch (error) { + throw asRefusal(error); + } if (deps.getActiveSource() !== source) { - throw new Error("the active asset changed while this device was answering"); + throw new ApiError({ + code: ASSET_CHANGED, + message: "the active asset changed while this device was answering", + }); } const shapes = shapesFromMask( diff --git a/frontend/ui-core/src/data/refusals.test.ts b/frontend/ui-core/src/data/refusals.test.ts index 96a7dd42..dfe0a578 100644 --- a/frontend/ui-core/src/data/refusals.test.ts +++ b/frontend/ui-core/src/data/refusals.test.ts @@ -11,4 +11,18 @@ describe("refusalProse — browser target refusals", () => { expect(refusalProse(error)).toMatch(/positive-point/i); expect(refusalProse(error)).not.toMatch(/could not be reached/i); }); + + // Every one of these is raised inside the tab by `BrowserSuggestionExecutor`, with no + // request made. A bare `Error` from there reaches `asApiError`, which stamps it + // `NETWORK_ERROR` — whose prose blames the server. Each code needs both an entry and + // this assertion, or that regression reopens silently. + it.each([ + ["BROWSER_ASSET_CHANGED", /asset changed/i], + ["BROWSER_INFERENCE_UNAVAILABLE", /could not start the model/i], + ["BROWSER_INFERENCE_FAILED", /could not answer/i], + ])("says what happened on this device for %s, never that a server was unreachable", (code, expected) => { + const error = new ApiError({ code, message: "unused — REFUSAL_PROSE wins" }); + expect(refusalProse(error)).toMatch(expected); + expect(refusalProse(error)).not.toMatch(/could not be reached/i); + }); }); diff --git a/frontend/ui-core/src/data/refusals.ts b/frontend/ui-core/src/data/refusals.ts index 337fb9ec..e73775f9 100644 --- a/frontend/ui-core/src/data/refusals.ts +++ b/frontend/ui-core/src/data/refusals.ts @@ -215,6 +215,16 @@ export const REFUSAL_PROSE: Record = { // a person sees is contract-tested at this boundary rather than assumed to propagate. BROWSER_NEGATIVE_POINTS_UNSUPPORTED: "This device supports positive-point refinement only. Choose Server to add a negative point.", + // The other three are the same rule as the entries above and one more: **none of + // these sentences may mention the server**, because nothing about them is a request. + // A browser-target refusal that fell through to `asApiError` would be stamped + // `NETWORK_ERROR` and read "the server could not be reached", which is false about a + // failure that never left the tab. `refusals.test.ts` holds that. + BROWSER_ASSET_CHANGED: + "The displayed asset changed before this device could answer — try again.", + BROWSER_INFERENCE_UNAVAILABLE: + "This device could not start the model. Choose Server, or reload and try again.", + BROWSER_INFERENCE_FAILED: "This device could not answer — try again, or choose Server.", }; /** From 34ae80d57c66a2767c0d08457f04b26a094516b6 Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya <1445792+JArmandoAnaya@users.noreply.github.com> Date: Thu, 17 Sep 2026 04:49:57 -0700 Subject: [PATCH 38/42] ci: wake the browser-models job for everything its e2e suite exercises MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `browser-models` named only `browserSuggestion.spec.ts`, not the code that suite guards. A change to the composition root, the vite or playwright config, or the ui-core inference seam and panels woke only the `frontend` job — where that suite self-skips, having neither the ONNX artifacts nor `VISIONSET_REQUIRE_BROWSER_MODELS`. So the PRs most in need of real-model coverage were the ones least likely to get it. `ci_path_filters.test.mjs` now asserts each of those paths wakes both groups. --- .github/path-filters.yml | 11 +++++++++++ tests/scripts/ci_path_filters.test.mjs | 18 ++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/.github/path-filters.yml b/.github/path-filters.yml index 67a1b368..6e108da2 100644 --- a/.github/path-filters.yml +++ b/.github/path-filters.yml @@ -100,6 +100,17 @@ docs: browser-models: - "frontend/browser-inference/**" - "frontend/app/e2e/browserSuggestion.spec.ts" + # Everything that suite actually exercises. Naming only the spec would have been + # the usual trap: `browserSuggestion.spec.ts` is the one place a real ONNX model + # runs in a real browser, and it self-skips in the `frontend` job (no artifacts, no + # `VISIONSET_REQUIRE_BROWSER_MODELS`), so a change to the composition root or to the + # panel it drives would have been "covered" by a job that ran nothing. + - "frontend/app/src/data/browserInference/**" + - "frontend/app/vite.config.ts" + - "frontend/app/playwright.config.ts" + - "frontend/ui-core/src/inference/**" + - "frontend/ui-core/src/annotator/SuggestPanel.tsx" + - "frontend/ui-core/src/annotator/AnnotationPage.tsx" - "scripts/browser_models/**" - "tests/browser_models/**" - "pyproject.toml" diff --git a/tests/scripts/ci_path_filters.test.mjs b/tests/scripts/ci_path_filters.test.mjs index b59f264e..2785ebd4 100644 --- a/tests/scripts/ci_path_filters.test.mjs +++ b/tests/scripts/ci_path_filters.test.mjs @@ -75,6 +75,24 @@ test("frontend source wakes only the frontend jobs", () => { assert.deepEqual(groupsFor("frontend/app/src/main.tsx"), ["frontend"]); }); +test("what the real-model e2e suite exercises wakes the job that runs it for real", () => { + // `browserSuggestion.spec.ts` self-skips without the ONNX artifacts and the + // `VISIONSET_REQUIRE_BROWSER_MODELS` flag, both of which only the `browser-models` + // job supplies. So every file that suite drives has to wake that job by name: a + // change reaching only `frontend` is a change whose one real-browser proof ran + // nothing and still reported green. + for (const file of [ + "frontend/app/src/data/browserInference/BrowserInferenceRuntime.ts", + "frontend/app/vite.config.ts", + "frontend/app/playwright.config.ts", + "frontend/ui-core/src/inference/browserPort.ts", + "frontend/ui-core/src/annotator/SuggestPanel.tsx", + "frontend/ui-core/src/annotator/AnnotationPage.tsx", + ]) { + assert.deepEqual(groupsFor(file), ["browser-models", "frontend"], file); + } +}); + test("documentation wakes only the docs site", () => { assert.deepEqual(groupsFor("docs/content/install.md"), ["docs"]); }); From 89b196fb847191c2aa72fa714daa5fed5c60cae5 Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya <1445792+JArmandoAnaya@users.noreply.github.com> Date: Thu, 17 Sep 2026 04:50:08 -0700 Subject: [PATCH 39/42] refactor(ui-core): unpublish computeSuggestBlocker, and stop inviting a dead click MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `computeSuggestBlocker` was exported from the package root and consumed by nobody outside it — `AnnotationPage` imports it relatively. An internal helper on the public surface is a contract nobody asked for. And with "This device" selected before any download, the idle card still read "Click the thing you want" over a click that is a silent no-op, since there is no executor for an unready browser target. That one state now points at the Download button instead. Every other state's copy is untouched. --- .../ui-core/src/annotator/SuggestPanel.tsx | 40 ++++++++++++++----- .../src/annotator/suggestPanel.test.tsx | 18 +++++++++ frontend/ui-core/src/index.ts | 1 - 3 files changed, 47 insertions(+), 12 deletions(-) diff --git a/frontend/ui-core/src/annotator/SuggestPanel.tsx b/frontend/ui-core/src/annotator/SuggestPanel.tsx index 48ef5336..a6a3bb92 100644 --- a/frontend/ui-core/src/annotator/SuggestPanel.tsx +++ b/frontend/ui-core/src/annotator/SuggestPanel.tsx @@ -428,19 +428,37 @@ export function SuggestPanel({ blocker !== null && blocker !== undefined; + // "This device" is the active target and has no model to answer with yet. The click + // this card would otherwise invite is a silent no-op — `AnnotationPage` holds no + // executor for an unready browser target — so the invitation is replaced by the one + // thing that is actually available: the Download control the tab below already draws. + const browserTabUnacquired = + runtimeWired && activeTarget?.kind === "browser" && blocker === "not-ready"; + return ( }> - {!serverTabBlocked && ( - <> -

- Click the thing you want -

-

- One click proposes a shape for “{session.labelClass}”. Alt-click marks something - that is not part of it. -

- - )} + {!serverTabBlocked && + (browserTabUnacquired ? ( + <> +

+ Download the model first +

+

+ “This device” has nothing to answer with yet, so a click does nothing. Download + it below, or switch back to Server. +

+ + ) : ( + <> +

+ Click the thing you want +

+

+ One click proposes a shape for “{session.labelClass}”. Alt-click marks something + that is not part of it. +

+ + ))} {/* Here and in no other reading. This is the state where nothing is in flight and nothing is waiting to be accepted, so it is the only one where diff --git a/frontend/ui-core/src/annotator/suggestPanel.test.tsx b/frontend/ui-core/src/annotator/suggestPanel.test.tsx index 1a62807a..00dd7621 100644 --- a/frontend/ui-core/src/annotator/suggestPanel.test.tsx +++ b/frontend/ui-core/src/annotator/suggestPanel.test.tsx @@ -845,6 +845,24 @@ describe("this device, once a browser runtime is wired", () => { expect(screen.getByTestId("suggest-idle")).toBeTruthy(); }); + it("points at Download instead of inviting a click the unacquired browser tab cannot answer", () => { + // "Click the thing you want" is a promise, and with "This device" selected before + // any download it is a false one: `AnnotationPage` holds no executor for an unready + // browser target, so the click is a silent no-op. + render( + mount({ + browserTargets: [], + browserAcquisitions: [acquisition()], + activeTarget: { kind: "browser", targetId: "efficient-sam-ti" }, + onChooseTarget: vi.fn(), + blocker: "not-ready", + }), + ); + + expect(screen.queryByTestId("suggest-idle")).toBeNull(); + expect(screen.getByTestId("suggest-idle-unacquired").textContent).toMatch(/download/i); + }); + it("draws the warn icon inline with a warn-tone blocker on the server tab", () => { render( mount({ diff --git a/frontend/ui-core/src/index.ts b/frontend/ui-core/src/index.ts index fd9d1f56..abc11b12 100644 --- a/frontend/ui-core/src/index.ts +++ b/frontend/ui-core/src/index.ts @@ -129,7 +129,6 @@ export type { BrowserSuggestionTarget, VisionSetBrowserInferenceRuntime, } from "./inference/browserPort.js"; -export { computeSuggestBlocker } from "./inference/targetBlocker.js"; export { useServerSuggestionExecutor, type SuggestionExecutor, From 890b49730de8c4107d95851d70a13e316173179a Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya <1445792+JArmandoAnaya@users.noreply.github.com> Date: Thu, 17 Sep 2026 04:50:08 -0700 Subject: [PATCH 40/42] docs: catch up with a browser inference port that now has a host MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four passages in `architecture/frontend/ui-core.md` had gone false. No host supplied a runtime (`frontend/app` now does, unconditionally); the port was two members wide (it is four — two required, two optional and additive); no capability let a host ask for local inference (one does); and no consumer of the pixel lease had needed to reconcile browser-canvas-vs-Pillow decode divergence. That last one is the important one, and it is now stated as a live limitation rather than a future concern: the browser suggestion executor feeds `readRgb` straight to a model, so Server and "This device" can legitimately return different masks for the same asset and the same click — and for an asset carrying EXIF orientation or an AdobeRGB profile the difference is not marginal. `ui.md`'s suggest section described only the server path. It now covers the two tabs, the explicit ~41 MB download, that the download does not survive a reload, and that Alt-click needs Server. --- docs/content/architecture/frontend/ui-core.md | 47 ++++++++++++------- docs/content/ui.md | 26 ++++++++-- 2 files changed, 54 insertions(+), 19 deletions(-) diff --git a/docs/content/architecture/frontend/ui-core.md b/docs/content/architecture/frontend/ui-core.md index 023b1945..d160ca90 100644 --- a/docs/content/architecture/frontend/ui-core.md +++ b/docs/content/architecture/frontend/ui-core.md @@ -74,16 +74,22 @@ concrete adapter, and it discovers none either. Absence is the ordinary case, so the hook answers `null` and never throws - the rule stated twice above, for a third capability. **A host that offers browser inference composes the -runtime at the host boundary; a host that does not offer it supplies nothing.** No host in this -repository supplies one, so every build behaves as it did before the port existed. - -The port is two members wide on purpose, and it is an *execution* contract rather than a -catalog: finding and obtaining a model is a question about things that cannot answer yet, and -this is not where it is answered. Nor is the narrowness a promise that growth is free - the -interface is published, so adding a **required** member to it would break every host -implementing the older one, and +runtime at the host boundary; a host that does not offer it supplies nothing.** `frontend/app` +now supplies one: `data/browserInference/BrowserInferenceRuntime.ts` composes a real +`VisionSetBrowserInferenceRuntime` over `@visionset/browser-inference` and passes it through +`OssSession.tsx` unconditionally. A host that omitted it would still behave exactly as it did +before the port existed. + +The port is four members wide, and it is an *execution* contract rather than a catalog: the two +required members - `listTargets` and `executorFor` - are only about asking a model that can +already answer. The two added since are optional, which is what let them arrive without +breaking a host implementing the older interface: `listAcquisitions?()`, which names models this +device could run once their bytes are fetched, and `setActiveAsset?()`, which tells the runtime +which asset is on screen so an executor's staleness checks have something to check against. That +is exactly how [browser inference is host-injected](../decisions/browser-inference-is-host-injected.md) says -how it is grown instead. +the interface is grown - the published shape means a new **required** member would break every +host implementing the older one, so growth is additive and optional. > An `InferenceConnection` is a model the VisionSet server has. A browser inference runtime is > something this browser can do. They are not two spellings of one idea, and neither is @@ -108,9 +114,10 @@ reference for one asset from reading pixels of the next asset either through a r through the detached one a host's real asset-switch (unmount the old canvas, mount a fresh one) leaves behind. -This is resource plumbing only. It neither selects a browser model nor exposes an inference -target — no capability in this phase lets a host ask for local inference at all; composing pixels -with a browser runtime remains a later host decision. +This is resource plumbing only: it neither selects a browser model nor exposes an inference +target. Composing the two is the host's decision, and `frontend/app` now makes it — the lease +reaches the browser runtime through the port's `setActiveAsset`, and the executor behind +`executorFor` is what reads `readRgb`. **What is proved, and what is not claimed.** `e2e/assetPixels.spec.ts` drives a real Chromium against a tiny runtime-generated image and confirms, in that browser, on that image: one content @@ -126,10 +133,18 @@ rounding loss on partially-transparent pixels, EXIF-orientation auto-rotation (a image's EXIF orientation when decoding to canvas; the server's direct-bytes decode path does not), and AdobeRGB ICC-profile color management (a browser color-manages a tagged profile toward sRGB on decode; the server's path does not). None of these are exact figures worth repeating here — they -are a known limitation, not a benchmark — and no consumer of this lease has yet needed to reconcile -them, since nothing in this phase reads pixels for inference. A future phase that feeds this lease's -`readRgb` output to a model must account for these divergences before treating browser-decoded -pixels as equivalent to the server's own decode of the same asset. +are a known limitation, not a benchmark. + +**This is now a live limitation, not a future one.** The browser suggestion executor feeds this +lease's `readRgb` output straight to a model, so the pixels "This device" segments are the +browser's decode and the pixels the server segments are Pillow's. **Server and "This device" can +therefore return different masks for the same asset and the same click**, and for an asset +carrying EXIF orientation or an AdobeRGB profile the difference can be large rather than +marginal — an auto-rotated decode is not a variation on the same picture. Nothing reconciles +the two, and nothing in the editor tells a person which decode answered. Treat a browser +suggestion as this browser's answer about this browser's decode; it is not a claim about what +the server would have said. Closing the gap means changing a decode, not adding a tolerance, +and neither side has been changed here. ## Asking for a suggestion is not sending one diff --git a/docs/content/ui.md b/docs/content/ui.md index 0ddcd7cd..b25b44c4 100644 --- a/docs/content/ui.md +++ b/docs/content/ui.md @@ -599,9 +599,10 @@ object list is how a lane is selected, which is a real affordance rather than a The sparkles button - hotkey `S` - arms the **suggest tool**: click the thing you want and a segmentation model proposes its shape, which you can then adjust -before accepting. It runs through a model -connection (`docs/content/inference.md`), and the server side of it is -`POST /inference/suggest`. +before accepting. It answers from one of two places - a model connection on the +server, or a model running in this browser - and the panel carries that choice. +Through a connection it is `POST /inference/suggest` +(`docs/content/inference.md`). **It runs through a connection that can answer a click**, which is a narrower set than "the ones that are ready": only those declaring `point_suggest`. A workspace @@ -619,6 +620,25 @@ there is no control at all, only a line naming what is answering. The picker appears on the idle card alone: changing which model answers while a proposal is on screen would leave a shape nothing on the card explains. +**Server, or this device.** The panel's two tabs are where a click goes. **Server** +is the connection described above. **This device** runs a smaller segmentation model +in this browser, which answers without the picture or the click leaving the machine +and without a connection being configured at all - useful where the workspace has no +model that can answer a click, or where the server is slow to reach. + +It has to be downloaded first, and the panel says so: about 41 MB, on an explicit +press, never on its own. **The download does not survive a page reload.** It is held +for the session only, so reopening the editor tomorrow - or reloading today - means +downloading it again before "This device" can answer. Until then the tab shows the +Download button rather than inviting a click that would do nothing, and the choice +falls back to Server. + +**Alt-click needs Server.** The model running here takes only points that are *on* +the object; a point marking something that is not part of it is refused rather than +answered with something that is not an exclusion. Switch to Server to refine that +way. Everything else - the first click, refining with more points, the tolerance, +`↵` and `Esc` - works the same on both. + The gesture: | Press | What it does | From 3dbb98d958f8bad080f66e00ed825adbf625ff12 Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya <1445792+JArmandoAnaya@users.noreply.github.com> Date: Thu, 17 Sep 2026 08:31:33 -0700 Subject: [PATCH 41/42] ci(app): build browser-inference before the clean-clone cycle suite frontend/app now imports @visionset/browser-inference and its /browser subpath, both resolved through its generated dist/. The cycle suite's webServer build sequence built every other workspace dependency first but never this one, so app's build failed with TS2307 on a clean clone (GitHub Actions run 1261) even though it passed locally wherever an earlier, unrelated command had already built browser-inference. --- frontend/app/playwright.cycle.config.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/frontend/app/playwright.cycle.config.ts b/frontend/app/playwright.cycle.config.ts index edad93fd..16b0bea4 100644 --- a/frontend/app/playwright.cycle.config.ts +++ b/frontend/app/playwright.cycle.config.ts @@ -93,6 +93,11 @@ export default defineConfig({ "pnpm --filter @visionset/media build", "pnpm --filter @visionset/annotator build", "pnpm --filter @visionset/ui-core build", + // `@visionset/browser-inference` has no workspace dependencies of its own, + // so its place in the order is free — but `app` imports it and its + // `/browser` subpath directly, both resolved through its `dist/`, so it + // still has to be built before `app` on the same clean clone. + "pnpm --filter @visionset/browser-inference build", "pnpm --filter @visionset/app build", "pnpm bundle:static", // `uv run`, which is what puts the virtualenv's `bin/` on PATH — so From c49ea8517ad818576a95c5458e63709a367308f1 Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya <1445792+JArmandoAnaya@users.noreply.github.com> Date: Thu, 17 Sep 2026 08:33:49 -0700 Subject: [PATCH 42/42] fix(ui-core): hand the browser executor the descriptor's frame, not the decode's MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `onImageReady` built the `BrowserSuggestionAssetSource` from the decoded ``'s `naturalWidth`/`naturalHeight`. Everything that source meets is in the *descriptor's* pixels instead: the click points `suggestAt` sends, the geometry `shapesFromMask` returns, and the annotations already on the frame. `AnnotatorCanvas` has always laid the picture out that way — this is the same rule one layer on. A decode that disagrees — EXIF orientation transposing the axes, a preview served in place of the original — therefore had the executor bound-check clicks and extract pixels against a second, private frame, and answer with suggestions that are individually plausible and uniformly wrong. Nothing crashes and nothing is logged, which is what makes it worth a test. The regression test makes the two frames disagree by a transposition, because a fixture where they coincide cannot tell them apart — which is exactly how this survived the suite that shipped it. It claims the extent the executor bound-checks against *and* what `readRgb` asks the decoder for; mutating either one alone fails it. The e2e header's account of why it serves a real, correctly-sized asset image was written against the old behaviour and is corrected here: the bounds now agree either way, and what a 1x1 `/content` still gets wrong is the pixels. --- frontend/app/e2e/browserSuggestion.spec.ts | 19 +++--- .../ui-core/src/annotator/AnnotationPage.tsx | 20 +++++- .../src/inference/browserRuntime.test.tsx | 64 ++++++++++++++++++- 3 files changed, 90 insertions(+), 13 deletions(-) diff --git a/frontend/app/e2e/browserSuggestion.spec.ts b/frontend/app/e2e/browserSuggestion.spec.ts index 763ebe8f..26fec77c 100644 --- a/frontend/app/e2e/browserSuggestion.spec.ts +++ b/frontend/app/e2e/browserSuggestion.spec.ts @@ -22,15 +22,16 @@ * `_wireApiStub.ts`'s `/content` route serves a real but 1×1 PNG — fine for the * ~150 tests in `annotate.spec.ts`, because `AnnotatorCanvas.tsx` lays the picture * out at the asset's *declared* width/height and never at its own `naturalWidth` - * ("a picture whose natural size disagrees is a preview"). This suite is the one - * caller that cannot get away with that: the browser executor reads the real - * decoded image's `naturalWidth`/`naturalHeight` (`AnnotationPage.tsx`'s - * `onImageReady`) as the bounds it validates click coordinates against - * (`efficientSam.ts`'s `x < 0 || x > width || ...`). A 1×1 real image makes every - * on-canvas click land "outside" it. `mockAssetImage` below overrides just this - * file's `/content` route with a real, correctly-sized PNG — scoped here rather - * than changing `_wireApiStub.ts`'s shared default, which those ~150 other tests - * may depend on for load speed. + * ("a picture whose natural size disagrees is a preview") — and `AnnotationPage.tsx`'s + * `onImageReady` now hands the browser executor that same declared frame, so the + * bounds it validates click coordinates against (`efficientSam.ts`'s + * `x < 0 || x > width || ...`) agree with the canvas a click was made on. What a 1×1 + * `/content` still leaves wrong here is the *pixels*: `readRgb` would stretch one + * sample across the whole declared frame, so this suite would be asking a real + * EfficientSAM to segment an image it never saw. `mockAssetImage` below overrides + * just this file's `/content` route with a real, correctly-sized PNG — scoped here + * rather than changing `_wireApiStub.ts`'s shared default, which those ~150 other + * tests may depend on for load speed. */ import { existsSync, readFileSync } from "node:fs"; import path from "node:path"; diff --git a/frontend/ui-core/src/annotator/AnnotationPage.tsx b/frontend/ui-core/src/annotator/AnnotationPage.tsx index 075c7b8e..d21c2930 100644 --- a/frontend/ui-core/src/annotator/AnnotationPage.tsx +++ b/frontend/ui-core/src/annotator/AnnotationPage.tsx @@ -2667,8 +2667,24 @@ function Workspace({ clipboard={clipboard} onHostAction={hostAction} onImageReady={(image) => { - const width = image.image.naturalWidth; - const height = image.image.naturalHeight; + /* + The **descriptor's** frame, never the decoded image's + `naturalWidth`/`naturalHeight` — the same rule + `AnnotatorCanvas` lays the picture out under, one layer on. + + Everything this source meets is already in the descriptor's + pixels: the click points `suggestAt` sends, the geometry + `shapesFromMask` hands back, and the annotations already on + the frame. A decode that disagrees — EXIF orientation + swapping the axes, a preview served in place of the + original — would have the executor bound-check clicks and + extract pixels against a second, private frame, and produce + suggestions that are individually plausible and uniformly + wrong. `readRgb` scales the decode into the frame asked for, + so naming the descriptor here is also what makes a + disagreeing decode harmless rather than silent. + */ + const { width, height } = store.document.asset; const source: BrowserSuggestionAssetSource = { assetId: asset.id, width, diff --git a/frontend/ui-core/src/inference/browserRuntime.test.tsx b/frontend/ui-core/src/inference/browserRuntime.test.tsx index 9008eb94..a439a789 100644 --- a/frontend/ui-core/src/inference/browserRuntime.test.tsx +++ b/frontend/ui-core/src/inference/browserRuntime.test.tsx @@ -124,14 +124,22 @@ function connectionRow(): Record { }; } +/** + * The asset's declared frame — what the wire says this asset measures, and so what + * `documentFromWire` puts in the document's `AssetDescriptor`. Per-test rather than + * a constant only so the descriptor-frame test below can pick numbers no decoded + * `` in this file reports; `beforeEach` puts it back. + */ +let assetExtent = { width: 640, height: 480 }; + function assetRow(id: string, hash: string): Record { return { id, project_id: PROJECT, modality: "image", content_hash: hash.padEnd(64, "0"), - width: 640, - height: 480, + width: assetExtent.width, + height: assetExtent.height, format: "png", thumbnail_hash: null, frame_index: null, @@ -194,6 +202,7 @@ function answer(path: string): unknown { beforeEach(() => { sent.length = 0; clearPrefs(); + assetExtent = { width: 640, height: 480 }; connections = [connectionRow()]; suggestion = { model_ref: MODEL_REF, @@ -461,4 +470,55 @@ describe("BrowserSuggestionAssetSource", () => { expect(source.assetId).toBe(ASSET); expect(typeof source.readRgb).toBe("function"); }); + + it("carries the asset descriptor's frame, not the decoded image's natural size", async () => { + /* + The two frames are made to disagree — and to disagree by a *transposition*, + the shape an EXIF-rotated decode actually takes — because a fixture where + they coincide cannot tell them apart. That is exactly how reading + `naturalWidth`/`naturalHeight` here survived the suite that shipped it. + + The descriptor is what every coordinate this source meets is expressed in: + the click points, the shapes `shapesFromMask` returns, the annotations + already on the frame. So the claim is made twice — on the extent the + executor bound-checks clicks against, and on what `readRgb` actually asks + the decoder for, which is the one a plausible "fix" to the first alone + would leave wrong. + */ + assetExtent = { width: 7, height: 5 }; + const setActiveAsset = vi.fn(); + const runtime: VisionSetBrowserInferenceRuntime = { + listTargets: async () => [], + executorFor: () => ({ + suggest: async () => { + throw new Error("unused"); + }, + }), + setActiveAsset, + }; + + await open(runtime); + + const image = screen.getByTestId("annotator-image") as HTMLImageElement; + Object.defineProperty(image, "naturalWidth", { value: 5, configurable: true }); + Object.defineProperty(image, "naturalHeight", { value: 7, configurable: true }); + fireEvent.load(image); + + await waitFor(() => expect(setActiveAsset).toHaveBeenCalledTimes(1)); + const [source] = setActiveAsset.mock.calls[0] as [BrowserSuggestionAssetSource]; + expect({ width: source.width, height: source.height }).toEqual({ width: 7, height: 5 }); + + const drawImage = vi.fn(); + const getImageData = vi.fn(() => ({ data: new Uint8ClampedArray(7 * 5 * 4) })); + const getContext = vi + .spyOn(HTMLCanvasElement.prototype, "getContext") + .mockReturnValue({ drawImage, getImageData } as unknown as CanvasRenderingContext2D); + try { + const pixels = source.readRgb(); + expect({ width: pixels.width, height: pixels.height }).toEqual({ width: 7, height: 5 }); + } finally { + getContext.mockRestore(); + } + expect(drawImage).toHaveBeenCalledWith(image, 0, 0, 7, 5); + }); });