diff --git a/.agents/skills/frontend/information-architecture/SKILL.md b/.agents/skills/frontend/information-architecture/SKILL.md index 7e2aaeec..ff2ed188 100644 --- a/.agents/skills/frontend/information-architecture/SKILL.md +++ b/.agents/skills/frontend/information-architecture/SKILL.md @@ -11,6 +11,7 @@ Navigation maps 1:1 to domain objects. This is the target structure; if implemen ``` /projects Projects list +/inference Inference — model connections (workspace-scoped) /projects/:id Project — tabs, in this order: ?tab=overview Overview (dashboard, see below) ?tab=schema Schema (contract) @@ -40,13 +41,14 @@ Rules derived from the 2026-08 audit (§6): - **"Schema history" is not a sibling tab.** Version history lives inside the Schema tab, below the editor and beside the `VersionNavigator` seam. The two overlap on purpose: the navigator is the *reader* (one version, with what it changed), the history is the *ledger* (every version at once). `?tab=versions` remains as a redirect; it does not appear in the tab bar. - **The 4-step checklist is onboarding, not navigation.** It retires itself twice over: when the journey is finished (`hasReleases` makes `done` derivable) and when somebody dismisses it. Dismissal is **per project** and persisted — finishing one project does not teach you the pipeline for the next. It gates nothing and is never the sole path to a screen. `hasReleases` is derived in `useProjectReadiness` from the two-hop read (project → dataset → releases) rather than added to the project-stats wire model: the Overview dashboard already makes both requests for its own cards, so a third spelling of the fact on the server would be the drift this audit was about. +- **Inference is a rail destination, not a project tab.** Model connections carry no `project_id`: one workspace is one SQLite file, every project uses the same connections, and navigation maps 1:1 to domain objects — so a project tab would state a scope the object does not have. The decision is recorded on #421 (2026-08-08) and **supersedes #58's rail rule** ("logo, collapse toggle, Home, Projects, account avatar — nothing else"); the rail now carries Home, Projects, Inference and the account control, and `DESIGN.md` carries the same membership in both places it states it. What earns a rail entry is a workspace-level object with nowhere else to live, never mere frequency of use. - **Overview is a dashboard**: pipeline state of batches, trunk size, latest release, active schema version — each card links to its tab (`StatCard`'s `onGo`, which renders the card as a **button** so it is keyboard-reachable and announced as an action). Overview never duplicates a tab's full function: every number on it is a *pointer* at the section that owns it, and a section with nothing yet says so in words rather than showing a zero. ## Structural invariants - **Single route definition site**: `frontend/app/src/routes.tsx`. No routes defined elsewhere. - **`ui-core` stays router-free.** Screens receive navigation as callback props (`routes.tsx:113-121` pattern). Never import a router in `ui-core`. -- **Back-links are declared** in the routes parent map (`PARENT` in `routes.tsx`) and must point to the contextual parent: the gallery's back is the Batches tab. **A tab has no back-link** — its way out is the tab bar, and one inside a panel would be a second, contradictory answer to "where am I". That is why `DatasetScreen` takes `onBack` as optional and the tab mount passes none. +- **Back-links are declared** in the routes parent map (`PARENT` in `routes.tsx`) and must point to the contextual parent: the gallery's back is the Batches tab. **A tab has no back-link** — its way out is the tab bar, and one inside a panel would be a second, contradictory answer to "where am I". That is why `DatasetScreen` takes `onBack` as optional and the tab mount passes none. **A rail destination has no back-link either**, for the same reason and with the rail in the tab bar's place: `InferenceScreen` takes no `onBack`, and `PARENT.inference` exists as the address other screens send people *to* (#424's D6 panel is the first) rather than as a parent anything returns from. - Tab state lives in `?tab=` with `replace: true`; unknown values fall back to `overview` silently. ## Process rule diff --git a/DESIGN.md b/DESIGN.md index c5ddf125..562fd27e 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -289,9 +289,14 @@ base**, line-height 1.6. One scale — reuse it, don't invent sizes: ## Layout -- **App shell**: dark left rail (logo, collapse toggle, Home, Projects, account avatar at - the bottom — nothing else; #58), bright content area. Rail width 240px, 60px collapsed, - 280px mobile — a single source of truth. +- **App shell**: dark left rail (logo, collapse toggle, Home, Projects, Inference, account + avatar at the bottom — nothing else; #58, and #421's decision of 2026-08-08 which + supersedes it by adding Inference), bright content area. Rail width 240px, 60px + collapsed, 280px mobile — a single source of truth. + **What earns a rail entry**: a workspace-level object every project uses, which has + nowhere else to live. Model connections carry no project id, so a project tab would + state a scope the object does not have. A destination that belongs to one project does + not qualify, however often it is visited. - **Page widths**: lists/dashboards/detail `max-w-7xl`; forms/settings `max-w-3xl`; centered, `px-4 md:px-6 py-6`. - **Page header**: title + subtitle left, actions right, `border-b` below, `mb-8`. @@ -329,7 +334,9 @@ from being rediscovered one screen at a time. to send anybody renders no control rather than a dead one. - **The rail is for top-level destinations only.** Per-screen return navigation never lives on it — that is what lets it name where it goes, and what keeps the rail the - four things `## Layout` gives it. + five things `## Layout` gives it. A rail destination therefore has no back-link of + its own, for the reason a tab has none: the rail *is* its way out, and a second + answer to "where am I" inside the pane would contradict it. - **The browser's Back button stays correct, and is never the only way out.** Nothing here replaces it; a `replace` navigation is still right where a change is a view of the same resource rather than a place (#171's tabs). diff --git a/docs/inference.md b/docs/inference.md index 4c474a1a..db6a9836 100644 --- a/docs/inference.md +++ b/docs/inference.md @@ -42,6 +42,8 @@ with WorkspaceService.open("./road-signs") as workspace: print(one.name, one.connection_type.value, one.setup_state.value) ``` +**In the browser:** the **Inference** entry in the rail. See *The Inference section* below. + **Over HTTP:** `GET`/`POST /inference/connections`, `GET`/`PATCH`/`DELETE /inference/connections/{connection_id}`, and `POST /inference/connections/{connection_id}/download`. `PATCH` edits in place and leaves out what @@ -76,6 +78,32 @@ The action stays **offered** on a machine that lacks the extra, deliberately. Wh installation has torch is not a fact about your connection, and a control that quietly vanished would leave the install command with nowhere to be shown. +## Knowing what a download costs, before agreeing to it + +A decision you cannot see the price of is not a decision, so the size is readable on its own, +ahead of anything being fetched: + +```bash +visionset inference size facebook/sam2-hiera-base-plus --revision main +``` + +It prints the byte count on stdout and the file count on stderr, and `--json` gives the document +the API answers with. Over HTTP it is +`GET /inference/download-size?model_id=…&model_revision=…`. + +This reads the publishing hub's **file listing** — names and byte counts — and downloads nothing. +It takes a model and a revision rather than a connection, because the moment the number is wanted +is usually the moment before a connection exists; the same call answers for a connection that +already has one. + +**Every file in the revision is counted**, because fetching takes the whole snapshot. A repository +publishing two serialisations of the same tensors really does cost both, and a figure counting one +of them would understate what lands on your disk. A revision the listing cannot fully size is +refused rather than estimated. + +It needs the `local-inference` extra, because the size is read with the same client that would do +the fetching. Without it you get `LOCAL_INFERENCE_UNAVAILABLE` and the install command. + ## Fetching weights Nothing arrives on your behalf: not at install, not at startup, not on the way to anything else. @@ -214,9 +242,50 @@ provenance, and only this configuration is removed. That is also why a connection has no lifecycle to speak of — it is a form somebody filled in, and the remedy for a wrong one is to edit it or make another. +## The Inference section + +Connections live behind **Inference** in the rail, beside Home and Projects. It is a top-level +destination rather than something inside a project because a connection belongs to the +*workspace*: it carries no project id, and every project uses the same ones. + +A workspace with none says so and offers one thing — **Add connection**. Creating one is two +steps, because the two kinds share almost no fields: first where the model runs, then that kind's +form. + +- **Local** opens pre-filled with the suggested model, `facebook/sam2-hiera-base-plus` at `main`, + a `cpu` device and `fp16` precision. Every one of those is a starting point you can type over. + Underneath the fields is what fetching that revision would cost — the size described above, read + while you are still deciding. If this machine has no `local-inference` extra the size cannot be + read, and the form says so, in the server's own words, with the install command. **It stays + usable**: creating a connection downloads nothing, so not knowing the size is information rather + than a barrier. +- **HTTP** asks for the endpoint URL. There is no credential field; where a secret would live is + still open (`cf. #421`), and a field added ahead of that answer would be answering it. + +Each row shows its name, its kind, `model @ revision`, and its status as a word — **Ready** or +**Not set up** — beside a colour, never as a colour alone. A local row that is not set up carries +**Download weights**, which launches the background job described above and reports its progress +in place; the row becomes **Ready** when the job finishes. A machine without the extra still shows +the control, and pressing it answers with the install command — a control that vanished would take +the remedy with it. + +Editing does not offer to change the kind, because the kind is not editable. Deleting asks once +and says exactly what it destroys: *annotations keep their model provenance; only this +configuration is removed.* + +Above twenty rows the list grows a filter, which matches a name substring and keeps saying how +many it hid. + +**Reached from the editor, too.** Arming the editor's suggest tool with no usable connection shows +a panel naming what is missing and offering **Set up a connection**, which lands here. Nothing +about that flow forces you out of the editor or loses work: the panel is an explanation with a +door, and the door is optional — a host that wires no destination gets the explanation and no +control. + ## At a terminal ```bash +visionset inference size some/model --revision abc123 visionset inference create local-detector \ --type local --model some/model --revision abc123 --device cuda --precision fp16 visionset inference list @@ -226,6 +295,9 @@ visionset inference download local-detector visionset inference delete local-detector --yes ``` +`size` is the one command here that opens no workspace: it asks about a published model rather +than about a configured row, so it takes no `--workspace`. + `create` prints the new id on stdout alone. `list` leads with the id, so `awk '{print $1}'` is stable even for a name holding internal whitespace. Every command takes `--json`, and the document it prints is the same shape the REST API answers with. diff --git a/docs/ui.md b/docs/ui.md index 41ff7092..e8feb5f4 100644 --- a/docs/ui.md +++ b/docs/ui.md @@ -38,6 +38,7 @@ handler is not an operation. | --- | --- | --- | | `/` | Home | yes | | `/projects`, `/projects/:id` (`?tab=schema\|batches\|versions`), `/projects/:id/ingest`, `/projects/:id/batches/:id`, `/projects/:id/dataset` | the product | yes | +| `/inference` | model connections, workspace-scoped | yes | | `/jobs/:jobId` (`?asset=`) | the annotation page | yes | | `/demo` | the annotator showcase (`?scene=bench` for #49's benchmark) | **no** | | `/styleguide` | the rendered design system | **no** | @@ -62,9 +63,11 @@ prefix the wheel serves under. A **reload** on a client route is a real request path no file backs; [`api.md`](api.md#where-the-ui-lives) describes the server-side fallback that answers it. -The rail is the whole shell: logo, collapse toggle, Home, Projects, sign out. Anything -richer growing on it is what the thin-app audit exists to catch — a capability in -`app/` is one the future enterprise UI cannot reuse. +The rail is the whole shell: logo, collapse toggle, Home, Projects, Inference, sign +out. Anything richer growing on it is what the thin-app audit exists to catch — a +capability in `app/` is one the future enterprise UI cannot reuse. `Inference` joined by +the decision recorded on #421 (2026-08-08): what earns an entry is a workspace-level +object every project uses and that has nowhere else to live, never frequency of use. ### Two panes, and which one a route gets diff --git a/frontend/app/e2e/annotate.spec.ts b/frontend/app/e2e/annotate.spec.ts index 708bb1bc..5730f313 100644 --- a/frontend/app/e2e/annotate.spec.ts +++ b/frontend/app/e2e/annotate.spec.ts @@ -325,6 +325,12 @@ async function serveApi( return route.fulfill({ contentType: "image/png", body: PIXEL }); } if (path === "/projects") return route.fulfill({ json: { items: [], total: 0 } }); + // The suggest tool's own read (#424 D5/D6). Empty is the interesting answer + // here: it is the state D6's panel exists for, and it is what a workspace + // that has never been to the Inference section is in. + if (path === "/inference/connections") { + return route.fulfill({ json: { items: [], total: 0 } }); + } return route.fulfill({ status: 500, json: { code: "NO_STUB", message: path } }); }); } @@ -2690,3 +2696,21 @@ test("Escape closes the canvas picker and leaves the object alone", async ({ pag await expect(page.getByTestId("canvas-reclass-pedestrian")).toHaveCount(0); await expect(page.getByTestId("object-row-0")).toContainText("1. vehicle"); }); + +test("the no-connection panel now has somewhere to send you (#424 D6)", async ({ page }) => { + const sent: Request[] = []; + await openJob(page, sent); + + // Arming the tool is what makes the editor ask whether a model is reachable — + // a job nobody suggests on makes no inference request at all. + await page.getByTestId("tool-suggest").click(); + await expect(page.getByTestId("suggest-no-connections")).toBeVisible(); + + // The half that did not exist until this slice. `ui-core` imports no router, + // so the panel's action is a callback and `routes.tsx` is the only place that + // can name a destination for it — which is why this is asserted here and not + // in a component test. + await page.getByTestId("suggest-configure").click(); + await expect(page).toHaveURL(/\/inference$/); + await expect(page.getByTestId("inference-screen")).toBeVisible(); +}); diff --git a/frontend/app/e2e/shell.spec.ts b/frontend/app/e2e/shell.spec.ts index 2a5fc172..b81ad847 100644 --- a/frontend/app/e2e/shell.spec.ts +++ b/frontend/app/e2e/shell.spec.ts @@ -35,6 +35,13 @@ async function serveApi(page: Page, { session = false } = {}): Promise { // because the gate and the sign-out button are what it is about and both are // only reachable when the server declines to sign the browser in by itself. await page.route("**/api/session", (route) => route.fulfill({ json: { issued: session } })); + // The Inference section's one read. Empty, because this suite is about the + // rail and the router: what the screen does with rows is `ui-core`'s + // `inference.test.tsx`, and an unrouted request here would leave the page + // waiting on a network that is not there. + await page.route("**/api/inference/**", (route) => + route.fulfill({ status: 200, json: { items: [], total: 0 } }), + ); await page.route("**/api/projects**", (route) => { const path = new URL(route.request().url()).pathname; if (path.endsWith("/schema")) { @@ -154,13 +161,32 @@ test("the rail carries exactly what the design gives it", async ({ page }) => { await signIn(page); const rail = page.getByTestId("app-rail"); - await expect(rail.getByRole("link")).toHaveCount(2); + // Three, not two: `Inference` joined by the decision recorded on #421 + // (2026-08-08), which supersedes #58's rule. The count is the assertion — a + // fourth destination arriving without that decision fails here first. + await expect(rail.getByRole("link")).toHaveCount(3); await expect(page.getByTestId("rail-home")).toBeVisible(); await expect(page.getByTestId("rail-projects")).toBeVisible(); + await expect(page.getByTestId("rail-inference")).toBeVisible(); await expect(page.getByTestId("rail-collapse")).toBeVisible(); await expect(page.getByTestId("rail-sign-out")).toBeVisible(); }); +test("the Inference entry goes to the section, and is current once you are on it", async ({ + page, +}) => { + await signIn(page); + await expect(page.getByTestId("rail-inference")).toHaveAttribute("href", "/inference"); + + await page.getByTestId("rail-inference").click(); + await expect(page).toHaveURL(/\/inference$/); + await expect(page.getByTestId("rail-inference")).toHaveAttribute("aria-current", "page"); + await expect(page.getByTestId("rail-projects")).not.toHaveAttribute("aria-current", "page"); + // A rail destination has no back-link: the rail is its way out, and a second + // answer to "where am I" inside the pane would contradict it. + await expect(page.getByTestId("back-link")).toHaveCount(0); +}); + test("navigation is real links, and the active one is the one you are on", async ({ page }) => { await signIn(page); diff --git a/frontend/app/src/routes.tsx b/frontend/app/src/routes.tsx index b5e2415d..0ea57c65 100644 --- a/frontend/app/src/routes.tsx +++ b/frontend/app/src/routes.tsx @@ -45,6 +45,7 @@ import { AnnotationPage, assetParamFor, GalleryScreen, + InferenceScreen, resolveProjectTab, IngestScreen, ProjectScreen, @@ -74,6 +75,13 @@ export function AppRoutes(): JSX.Element { redirect keeps one screen rather than two that drift. */} } /> } /> + {/* + A top-level destination rather than a project route, per the + decision recorded on #421: a connection carries no project id and + every project uses the same ones, so nesting it under a project + would put a workspace-scoped object inside one project's URL. + */} + } /> } /> } /> } /> @@ -155,6 +163,10 @@ function Projects(): JSX.Element { */ const PARENT = { projects: "/projects", + //: A rail destination, so nothing declares it as a parent — it is here because + //: this table is the route map's own index, and an entry point missing from it + //: is the drift #199 was about. Its own way out is the rail. + inference: "/inference", project: (projectId: string) => `/projects/${projectId}`, batches: (projectId: string) => `/projects/${projectId}?tab=batches`, dataset: (projectId: string) => `/projects/${projectId}?tab=dataset`, @@ -313,6 +325,10 @@ function Annotate(): JSX.Element { const next = assetParamFor(showing, query.get("asset")); if (next !== null) setQuery({ asset: next }, { replace: true }); }} + // #424's D6: the editor's no-connection panel had no destination until + // there was an Inference section to send somebody to. `ui-core` renders no + // control when this callback is absent, which is what it did until now. + onConfigureInference={() => void navigate(PARENT.inference)} /> ); } diff --git a/frontend/app/src/shell/AppShell.tsx b/frontend/app/src/shell/AppShell.tsx index d3322e0e..24b59522 100644 --- a/frontend/app/src/shell/AppShell.tsx +++ b/frontend/app/src/shell/AppShell.tsx @@ -3,7 +3,11 @@ * routed page. * * `DESIGN.md` is explicit about what is on it — logo, collapse toggle, Home, - * Projects, and the account control at the bottom — and about what is not. + * Projects, Inference, and the account control at the bottom — and about what is + * not. `Inference` joined it by the decision recorded on #421 (2026-08-08), which + * supersedes #58's rule: model connections are workspace infrastructure that every + * project uses, so they cannot live inside any one project, and the rail is the + * only workspace-level surface there is. * Anything richer growing here is exactly what the **thin-app audit** exists to * catch: this file is composition and identity, and a capability that lands in it * is one the future enterprise UI cannot reuse. @@ -66,7 +70,7 @@ */ import { readRailCollapsed, useApiSession, writeRailCollapsed } from "@visionset/ui-core"; -import { FolderGit2, Home, LogOut, PanelLeftClose, PanelLeftOpen } from "lucide-react"; +import { Cpu, FolderGit2, Home, LogOut, PanelLeftClose, PanelLeftOpen } from "lucide-react"; import { useState, type JSX, type ReactNode } from "react"; import { NavLink, Outlet } from "react-router"; @@ -120,6 +124,9 @@ export function AppShell(): JSX.Element { + +
diff --git a/frontend/ui-core/src/annotator/AnnotationPage.tsx b/frontend/ui-core/src/annotator/AnnotationPage.tsx index 302f9aec..274e146a 100644 --- a/frontend/ui-core/src/annotator/AnnotationPage.tsx +++ b/frontend/ui-core/src/annotator/AnnotationPage.tsx @@ -195,7 +195,7 @@ import { import { AddClassDialog, runAddClass } from "./AddClassDialog"; import { FrameGallery } from "./FrameGallery"; import { SuggestPanel } from "./SuggestPanel"; -import { useInferenceConnections, useSuggestRegion, usableConnection } from "./inferenceQueries"; +import { useConnections, useSuggestRegion, usableConnection } from "../data/inferenceQueries"; import { PROGRESS_LABEL, outstandingWork, progressDotClass, progressTone } from "../screens/batchState"; import type { LabelClassBody, SchemaDiff, SchemaVersion } from "../screens/queries"; import { @@ -793,8 +793,8 @@ function Workspace({ * answer is not known yet, and `usableConnection` names it (`checking`) rather * than leaving a click to vanish into it. */ - const connections = useInferenceConnections(session !== null); - const { connection, blocker } = usableConnection(connections.data); + const connections = useConnections(session !== null); + const { connection, blocker } = usableConnection(connections.data?.items); const suggestRegion = useSuggestRegion(); /** diff --git a/frontend/ui-core/src/annotator/SuggestPanel.tsx b/frontend/ui-core/src/annotator/SuggestPanel.tsx index 30944c4f..305e0b9a 100644 --- a/frontend/ui-core/src/annotator/SuggestPanel.tsx +++ b/frontend/ui-core/src/annotator/SuggestPanel.tsx @@ -44,7 +44,7 @@ import { Check, Loader2, Sparkles, TriangleAlert, X } from "lucide-react"; import type { JSX, ReactNode } from "react"; import { Button } from "../primitives/Button"; -import type { SuggestBlocker } from "./inferenceQueries"; +import type { SuggestBlocker } from "../data/inferenceQueries"; export interface SuggestPanelProps { /** The session, whose status decides which sentence this card carries. */ @@ -289,7 +289,19 @@ function Card({ data-testid={testId} data-tone={tone} role="status" - className={`absolute bottom-2 right-2 flex max-w-80 gap-2 rounded-lg border p-3 text-meta shadow-lg ${ + /* + Above the zoom widget, not beside it. Both are bottom-right overlays on + the same stage, and at `bottom-2` this card sat *under* `ZoomWidget`'s + `bottom-3` box — which does not merely look wrong: the widget's subtree + intercepts the pointer, so the panel's own action could not be clicked at + all. It shipped that way in #451 because nothing had a destination to + click through to yet, and the e2e that gave it one is what found it. + + `bottom-16` clears the widget's 44px row and its gutter. The two never + overlap now, so no z-index is needed and neither has to know about the + other beyond this line. + */ + className={`absolute bottom-16 right-3 flex max-w-80 gap-2 rounded-lg border p-3 text-meta shadow-lg ${ tone === "warn" ? "border-destructive/40 bg-destructive/5" : "border-border bg-card" diff --git a/frontend/ui-core/src/annotator/inferenceQueries.ts b/frontend/ui-core/src/annotator/inferenceQueries.ts deleted file mode 100644 index e2f08a28..00000000 --- a/frontend/ui-core/src/annotator/inferenceQueries.ts +++ /dev/null @@ -1,163 +0,0 @@ -/** - * The two calls the suggest tool makes: which connections exist, and what the - * model proposes for a click (#424, slice 3b). - * - * ## Why the connection list is a *read* the editor does - * - * D5 puts the model behind a connection and D6 says the editor must explain - * itself when there is no usable one — and "usable" is three different states - * (`setup_state`, and whether any row exists at all) that only the list can - * answer. So the panel's copy is derived from the same read the request uses to - * pick a connection, rather than from a refusal discovered after the click. - * - * The list is **workspace-scoped**, not project-scoped: `/inference/connections` - * takes no project, because a model is a machine's capability and not a - * project's property. That is why the key below sits at the root. - * - * ## Which connection, and the limit that is stated rather than hidden - * - * The **first `ready` one, in the list's own order**. There is no picker, because - * the surface that would hold one is #421's and waits on its open rail question — - * so this is a deliberate limit rather than a design: a workspace with two ready - * connections always suggests through the older of them, and choosing is what - * #421 adds. - * - * It is not a hand-mirrored capability table. `setup_state` is the wire's own - * field and this reads it; the *legality* of the call is the server's answer, and - * a connection this picks that the server then refuses renders its refusal like - * any other. `ui-capabilities`' rule is that the client may not compute what is - * legal, and picking which of several offered rows to send is not that. - * - * ## The suggestion is a mutation, and it writes nothing - * - * `useMutation` over `useQuery` for a POST that has no cache to hold: the same - * points sent twice answer the same way, but the ask is an *event* — somebody - * clicked — and caching it would make a refine click on the same pixel silently - * skip the request. Nothing here invalidates anything, because nothing is - * written: acceptance is a separate, ordinary annotation create. - */ - -import type { GeometryType } from "@visionset/annotator"; -import { useMutation, useQuery, type UseQueryResult } from "@tanstack/react-query"; - -import { useApiClient } from "../data/ApiProvider"; -import { unwrap } from "../data/errors"; -import { checkListInferenceConnections, checkSuggestRegion } from "../generated/checks"; -import type { components } from "../generated/api"; - -export type Connection = components["schemas"]["ConnectionOut"]; - -/** - * The suggest route's answer, declared structurally rather than imported. - * - * `WireAnnotation`'s precedent, one route along and for the identical reason: the - * spec spells a polygon's points with `prefixItems`, `openapi-typescript` honours - * it as a **tuple**, and the value `openapi-fetch` hands back widens to - * `number[][]`. The two are the same JSON and TypeScript will not unify them, so - * naming the shape this module actually reads is the honest answer. - * - * `geometry` stays `unknown` on top of that, because it is about to go through - * `parseGeometry` — the annotator's *"unknown in, typed out"* door — and a type - * the caller then re-narrows anyway would be a second mirror of a kernel shape, - * which `annotator-core` forbids in so many words. - */ -export interface SuggestedRegion { - readonly geometry: unknown; - readonly confidence: number | null; -} - -export interface SuggestionOut { - readonly model_ref: string; - readonly region?: SuggestedRegion | null; -} - -export const inferenceKeys = { - connections: () => ["inference", "connections"] as const, -}; - -/** - * Every connection this workspace has, `ready` or not. - * - * Unfiltered on purpose: "none configured" and "one configured but its weights - * are not here" are different sentences with different remedies, and a filtered - * list would make them look identical. - */ -export function useInferenceConnections( - enabled = true, -): UseQueryResult { - const client = useApiClient(); - return useQuery({ - queryKey: inferenceKeys.connections(), - enabled, - queryFn: async () => - unwrap(await client.GET("/inference/connections", {}), checkListInferenceConnections) - .items, - }); -} - -/** What a suggest call needs beyond the points: whose asset, and through what. */ -export interface SuggestInput { - readonly projectId: string; - readonly assetId: string; - readonly connectionId: string; - /** Every positive click so far, in placement order. */ - readonly positive: readonly (readonly [number, number])[]; - readonly negative: readonly (readonly [number, number])[]; - /** - * The kinds the active class can hold — the caller's schema, not a preference. - * - * The route's own docstring is emphatic about this: an answer in a kind the - * schema would refuse is a suggestion that cannot be accepted, so the server - * narrows or answers nothing rather than proposing something unusable. - */ - readonly allowedGeometries: readonly GeometryType[]; -} - -/** Ask the model. Nothing is written and nothing is remembered. */ -export function useSuggestRegion() { - const client = useApiClient(); - return useMutation({ - mutationFn: async (input: SuggestInput): Promise => - unwrap( - await client.POST("/inference/suggest", { - body: { - project_id: input.projectId, - asset_id: input.assetId, - connection_id: input.connectionId, - positive: input.positive.map(([x, y]) => ({ x, y })), - negative: input.negative.map(([x, y]) => ({ x, y })), - allowed_geometries: [...input.allowedGeometries], - } as never, - }), - checkSuggestRegion, - ), - }); -} - -/** - * Why the suggest tool cannot run *yet*, when it cannot. `null` when it can. - * - * `checking` is one of them deliberately. The list is only fetched once the tool - * is armed — a job that never suggests makes no inference request at all — so - * there is a real moment where the answer is not known, and a click landing in it - * must be told something rather than vanishing. Three states, one union, so the - * panel's copy is total over them. - */ -export type SuggestBlocker = "checking" | "no-connections" | "not-ready"; - -/** - * The connection a click should go through, and why there is none. - * - * One function rather than two, because the answers are exclusive and the panel - * needs whichever it is: a `connection` to send to, or a `blocker` to explain. - */ -export function usableConnection(connections: readonly Connection[] | undefined): { - readonly connection: Connection | null; - readonly blocker: SuggestBlocker | null; -} { - if (connections === undefined) return { connection: null, blocker: "checking" }; - if (connections.length === 0) return { connection: null, blocker: "no-connections" }; - const ready = connections.find((row) => row.setup_state === "ready"); - if (ready === undefined) return { connection: null, blocker: "not-ready" }; - return { connection: ready, blocker: null }; -} diff --git a/frontend/ui-core/src/annotator/suggestPanel.test.tsx b/frontend/ui-core/src/annotator/suggestPanel.test.tsx index 46b6a195..26f0cd13 100644 --- a/frontend/ui-core/src/annotator/suggestPanel.test.tsx +++ b/frontend/ui-core/src/annotator/suggestPanel.test.tsx @@ -18,7 +18,7 @@ import { answered, armed, refused, withPoint } from "@visionset/annotator"; import type { Suggestion, SuggestionState } from "@visionset/annotator"; import { SuggestPanel } from "./SuggestPanel"; -import { usableConnection, type Connection } from "./inferenceQueries"; +import { usableConnection, type Connection } from "../data/inferenceQueries"; const A_BOX = { type: "bbox", x: 10, y: 20, width: 30, height: 40 } as const; diff --git a/frontend/ui-core/src/data/inferenceQueries.ts b/frontend/ui-core/src/data/inferenceQueries.ts new file mode 100644 index 00000000..e45f2173 --- /dev/null +++ b/frontend/ui-core/src/data/inferenceQueries.ts @@ -0,0 +1,334 @@ +/** + * Everything the product asks about inference: which connections exist, how to + * configure one, what a model would cost to fetch, and what it proposes for a + * click. + * + * **In `data/` rather than in `annotator/`, because two surfaces read it.** The + * suggest tool asks which connections exist so it can explain itself (#424 D6); + * the Inference section is where they are made and set up (#421). The list, its + * key and its invalidation are one fact, and a second copy under the screen that + * happens to have been written second is how two callers come to disagree about + * what "ready" means. + * + * ## Why the connection list is a *read* the editor does + * + * D5 puts the model behind a connection and D6 says the editor must explain + * itself when there is no usable one — and "usable" is three different states + * (`setup_state`, and whether any row exists at all) that only the list can + * answer. So the panel's copy is derived from the same read the request uses to + * pick a connection, rather than from a refusal discovered after the click. + * + * The list is **workspace-scoped**, not project-scoped: `/inference/connections` + * takes no project, because a model is a machine's capability and not a + * project's property. That is why the key below sits at the root. + * + * ## Which connection, and the limit that is stated rather than hidden + * + * The **first `ready` one, in the list's own order**. There is no picker, because + * the surface that would hold one is #421's and waits on its open rail question — + * so this is a deliberate limit rather than a design: a workspace with two ready + * connections always suggests through the older of them, and choosing is what + * #421 adds. + * + * It is not a hand-mirrored capability table. `setup_state` is the wire's own + * field and this reads it; the *legality* of the call is the server's answer, and + * a connection this picks that the server then refuses renders its refusal like + * any other. `ui-capabilities`' rule is that the client may not compute what is + * legal, and picking which of several offered rows to send is not that. + * + * ## The suggestion is a mutation, and it writes nothing + * + * `useMutation` over `useQuery` for a POST that has no cache to hold: the same + * points sent twice answer the same way, but the ask is an *event* — somebody + * clicked — and caching it would make a refine click on the same pixel silently + * skip the request. Nothing here invalidates anything, because nothing is + * written: acceptance is a separate, ordinary annotation create. + */ + +import type { GeometryType } from "@visionset/annotator"; +import { + useMutation, + useQuery, + useQueryClient, + type UseQueryResult, +} from "@tanstack/react-query"; + +import { useApiClient } from "./ApiProvider"; +import { unwrap } from "./errors"; +import { + checkCreateInferenceConnection, + checkDeleteInferenceConnection, + checkDownloadConnectionWeights, + checkInferenceDownloadSize, + checkListInferenceConnections, + checkSuggestRegion, + checkUpdateInferenceConnection, +} from "../generated/checks"; +import type { components } from "../generated/api"; + +export type Connection = components["schemas"]["ConnectionOut"]; +export type ConnectionPage = components["schemas"]["ConnectionPage"]; + +/** + * The suggest route's answer, declared structurally rather than imported. + * + * `WireAnnotation`'s precedent, one route along and for the identical reason: the + * spec spells a polygon's points with `prefixItems`, `openapi-typescript` honours + * it as a **tuple**, and the value `openapi-fetch` hands back widens to + * `number[][]`. The two are the same JSON and TypeScript will not unify them, so + * naming the shape this module actually reads is the honest answer. + * + * `geometry` stays `unknown` on top of that, because it is about to go through + * `parseGeometry` — the annotator's *"unknown in, typed out"* door — and a type + * the caller then re-narrows anyway would be a second mirror of a kernel shape, + * which `annotator-core` forbids in so many words. + */ +export interface SuggestedRegion { + readonly geometry: unknown; + readonly confidence: number | null; +} + +export interface SuggestionOut { + readonly model_ref: string; + readonly region?: SuggestedRegion | null; +} + +export type ConnectionAction = components["schemas"]["ConnectionAction"]; +export type ConnectionType = components["schemas"]["ConnectionType"]; +export type ConnectionSetupState = components["schemas"]["ConnectionSetupState"]; +export type DownloadSizeOut = components["schemas"]["DownloadSizeOut"]; + +export const inferenceKeys = { + connections: () => ["inference", "connections"] as const, + /** + * A published revision's size, keyed on the pair that identifies it. + * + * Under the same root as the connections so that one prefix clears everything + * inference-shaped, and keyed on both halves because a size is a fact about a + * revision — a key naming only the model would serve one revision's number + * under another's name. + */ + size: (modelId: string, revision: string) => ["inference", "size", modelId, revision] as const, +}; + +/** + * Every connection this workspace has, `ready` or not. + * + * Unfiltered on purpose: "none configured" and "one configured but its weights + * are not here" are different sentences with different remedies, and a filtered + * list would make them look identical. + */ +export function useConnections(enabled = true): UseQueryResult { + const client = useApiClient(); + return useQuery({ + queryKey: inferenceKeys.connections(), + enabled, + queryFn: async () => + unwrap(await client.GET("/inference/connections", {}), checkListInferenceConnections), + }); +} + +/** What configuring a connection needs. The kind decides which half is filled. */ +export interface ConnectionInput { + readonly name: string; + readonly connectionType: ConnectionType; + readonly modelId: string; + readonly modelRevision: string; + readonly device?: string | null; + readonly precision?: string | null; + readonly endpointUrl?: string | null; +} + +/** Configure a connection. Nothing is downloaded and nothing is contacted. */ +export function useCreateConnection() { + const client = useApiClient(); + const queries = useQueryClient(); + return useMutation({ + mutationFn: async (input: ConnectionInput): Promise => + unwrap( + await client.POST("/inference/connections", { body: bodyOf(input) }), + checkCreateInferenceConnection, + ), + onSuccess: () => queries.invalidateQueries({ queryKey: inferenceKeys.connections() }), + }); +} + +/** Edit one. Omitted fields are left alone; the kind cannot change. */ +export function useUpdateConnection() { + const client = useApiClient(); + const queries = useQueryClient(); + return useMutation({ + mutationFn: async (input: ConnectionInput & { readonly id: string }): Promise => + unwrap( + await client.PATCH("/inference/connections/{connection_id}", { + params: { path: { connection_id: input.id } }, + body: bodyOf(input), + }), + checkUpdateInferenceConnection, + ), + onSuccess: () => queries.invalidateQueries({ queryKey: inferenceKeys.connections() }), + }); +} + +/** Remove one. Annotations keep the model provenance they recorded. */ +export function useDeleteConnection() { + const client = useApiClient(); + const queries = useQueryClient(); + return useMutation({ + mutationFn: async (connectionId: string) => + unwrap( + await client.DELETE("/inference/connections/{connection_id}", { + params: { path: { connection_id: connectionId } }, + }), + checkDeleteInferenceConnection, + ), + onSuccess: () => queries.invalidateQueries({ queryKey: inferenceKeys.connections() }), + }); +} + +/** + * Fetch a local connection's weights, and answer at once with the job to poll. + * + * The list is invalidated on success even though nothing has finished yet: what + * the 202 changes immediately is the connection's *declaration* — the download + * is now running — and a stale `allowed_actions` is the cache-side twin of the + * hand-mirror `ui-capabilities` bans. The row reaches `ready` when the job does, + * which the screen observes by polling the job. + */ +export function useDownloadWeights() { + const client = useApiClient(); + const queries = useQueryClient(); + return useMutation({ + mutationFn: async (connectionId: string) => + unwrap( + await client.POST("/inference/connections/{connection_id}/download", { + params: { path: { connection_id: connectionId } }, + }), + checkDownloadConnectionWeights, + ), + onSuccess: () => queries.invalidateQueries({ queryKey: inferenceKeys.connections() }), + }); +} + +/** + * What fetching that revision would cost, read before anybody agrees to it. + * + * D1 on #424 requires the number on screen *before* the confirm, which is why + * this is a query the form makes rather than something the create response + * carries: by the time a connection exists the decision has been taken. + * + * `staleTime: Infinity` because a pinned revision is a fixed set of files, so + * there is no event that could change the answer and nothing to invalidate it + * against. `retry: false` because the two ways this fails — the extra is not + * installed, the revision does not resolve — are both answered by *reading the + * refusal*, and retrying a missing install three times only delays the sentence + * that says what to do about it. + */ +export function useDownloadSize( + modelId: string, + revision: string, + enabled = true, +): UseQueryResult { + const client = useApiClient(); + return useQuery({ + queryKey: inferenceKeys.size(modelId, revision), + enabled: enabled && modelId.trim() !== "" && revision.trim() !== "", + staleTime: Infinity, + retry: false, + queryFn: async () => + unwrap( + await client.GET("/inference/download-size", { + params: { query: { model_id: modelId, model_revision: revision } }, + }), + checkInferenceDownloadSize, + ), + }); +} + +/** + * One shape into the wire's two, with the kind deciding which fields travel. + * + * The domain refuses a `local` connection carrying an `endpoint_url` and an + * `http` one carrying a device — both halves, not just the required one — so + * sending everything the form holds would turn a filled-in field somebody + * switched away from into a 422 they cannot see the cause of. + */ +function bodyOf(input: ConnectionInput) { + const local = input.connectionType === "local"; + return { + name: input.name, + connection_type: input.connectionType, + model_id: input.modelId, + model_revision: input.modelRevision, + device: local ? (input.device ?? null) : null, + precision: local ? (input.precision ?? null) : null, + endpoint_url: local ? null : (input.endpointUrl ?? null), + }; +} + +/** What a suggest call needs beyond the points: whose asset, and through what. */ +export interface SuggestInput { + readonly projectId: string; + readonly assetId: string; + readonly connectionId: string; + /** Every positive click so far, in placement order. */ + readonly positive: readonly (readonly [number, number])[]; + readonly negative: readonly (readonly [number, number])[]; + /** + * The kinds the active class can hold — the caller's schema, not a preference. + * + * The route's own docstring is emphatic about this: an answer in a kind the + * schema would refuse is a suggestion that cannot be accepted, so the server + * narrows or answers nothing rather than proposing something unusable. + */ + readonly allowedGeometries: readonly GeometryType[]; +} + +/** Ask the model. Nothing is written and nothing is remembered. */ +export function useSuggestRegion() { + const client = useApiClient(); + return useMutation({ + mutationFn: async (input: SuggestInput): Promise => + unwrap( + await client.POST("/inference/suggest", { + body: { + project_id: input.projectId, + asset_id: input.assetId, + connection_id: input.connectionId, + positive: input.positive.map(([x, y]) => ({ x, y })), + negative: input.negative.map(([x, y]) => ({ x, y })), + allowed_geometries: [...input.allowedGeometries], + } as never, + }), + checkSuggestRegion, + ), + }); +} + +/** + * Why the suggest tool cannot run *yet*, when it cannot. `null` when it can. + * + * `checking` is one of them deliberately. The list is only fetched once the tool + * is armed — a job that never suggests makes no inference request at all — so + * there is a real moment where the answer is not known, and a click landing in it + * must be told something rather than vanishing. Three states, one union, so the + * panel's copy is total over them. + */ +export type SuggestBlocker = "checking" | "no-connections" | "not-ready"; + +/** + * The connection a click should go through, and why there is none. + * + * One function rather than two, because the answers are exclusive and the panel + * needs whichever it is: a `connection` to send to, or a `blocker` to explain. + */ +export function usableConnection(connections: readonly Connection[] | undefined): { + readonly connection: Connection | null; + readonly blocker: SuggestBlocker | null; +} { + if (connections === undefined) return { connection: null, blocker: "checking" }; + if (connections.length === 0) return { connection: null, blocker: "no-connections" }; + const ready = connections.find((row) => row.setup_state === "ready"); + if (ready === undefined) return { connection: null, blocker: "not-ready" }; + return { connection: ready, blocker: null }; +} diff --git a/frontend/ui-core/src/generated/api.ts b/frontend/ui-core/src/generated/api.ts index e8da2a02..02787fd2 100644 --- a/frontend/ui-core/src/generated/api.ts +++ b/frontend/ui-core/src/generated/api.ts @@ -796,6 +796,46 @@ export interface paths { patch?: never; trace?: never; }; + "/inference/download-size": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + /** + * Inference Download Size + * @description How big fetching that model's weights would be, before anybody fetches them. + * + * What the local-connection form shows beside its confirm control, so the + * decision recorded on #418 — that VisionSet downloads nothing on its own — is + * one somebody can actually make (`cf. #421`, `#424`). + * + * **This downloads nothing.** It reads the publishing hub's file listing, which + * is the one question answerable before the download it describes. The number + * covers every file in the revision, because that is what the download fetches. + * + * Query parameters rather than a path, because a model id contains a slash + * (`facebook/sam2-hiera-base-plus`) and a segment that has to be escaped to be + * written is a URL people get wrong by hand. + * + * **Not a connection route**, and it takes no connection id: the moment the + * number is needed is the moment before the connection exists. Asking about a + * connection that already exists is the same pair of values, asked the same way. + * + * Refused with the install command when the local runtime is absent — the size + * is read with the same client that would do the fetching — and refused rather + * than guessed when the hub cannot size every file in the revision. + */ + get: operations["inference_download_size"]; + put?: never; + post?: never; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; "/inference/suggest": { parameters: { query?: never; @@ -2731,6 +2771,25 @@ export interface components { */ dataset_id: string; }; + /** + * DownloadSizeOut + * @description What fetching a model's weights would cost, before anybody fetches them. + * + * Answered from the publishing hub's file listing, so asking costs a metadata + * request and never a download. The pair is echoed back for ``SuggestionOut``'s + * reason: a form that had to remember which model it asked about would be + * keeping a second copy of something the response can simply state. + */ + DownloadSizeOut: { + /** File Count */ + file_count: number; + /** Model Id */ + model_id: string; + /** Model Revision */ + model_revision: string; + /** Total Bytes */ + total_bytes: number; + }; /** * ErrorBody * @description The one error shape this API emits, at every status. @@ -5707,6 +5766,65 @@ export interface operations { }; }; }; + inference_download_size: { + parameters: { + query: { + model_id: string; + model_revision: string; + }; + header?: never; + path?: never; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description Successful Response */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["DownloadSizeOut"]; + }; + }; + /** @description Missing or invalid bearer token */ + 401: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["ErrorBody"]; + }; + }; + /** @description The request payload is not processable */ + 422: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["ErrorBody"]; + }; + }; + /** @description Unhandled server error, with an incident id */ + 500: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["ErrorBody"]; + }; + }; + /** @description The workspace is busy; retry after the header says */ + 503: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["ErrorBody"]; + }; + }; + }; + }; suggest_region: { parameters: { query?: never; diff --git a/frontend/ui-core/src/generated/checks.ts b/frontend/ui-core/src/generated/checks.ts index 959cc2da..792cd6aa 100644 --- a/frontend/ui-core/src/generated/checks.ts +++ b/frontend/ui-core/src/generated/checks.ts @@ -140,6 +140,9 @@ export const checkClassCountOut: Check = export const checkDatasetStatsOut: Check = /*#__PURE__*/ object({ "annotated_asset_count": [true, isInteger], "annotation_count": [true, isInteger], "asset_count": [true, isInteger], "classes": [true, arrayOf(checkClassCountOut)], "dataset_id": [true, isString] } as const); +export const checkDownloadSizeOut: Check = + /*#__PURE__*/ object({ "file_count": [true, isInteger], "model_id": [true, isString], "model_revision": [true, isString], "total_bytes": [true, isInteger] } as const); + export const checkClassExportStatus: Check = /*#__PURE__*/ oneOf(["supported", "degraded", "dropped"] as const); @@ -308,6 +311,7 @@ export const checkGetSchemaVersion = checkSchemaVersionOut; export const checkGetSource = checkSourceOut; export const checkHealth: Check = /*#__PURE__*/ mapOf(isString); +export const checkInferenceDownloadSize = checkDownloadSizeOut; export const checkListAssetAnnotations = checkAnnotationPage; export const checkListAssetBatches = checkBatchPage; export const checkListBackgroundJobs = checkBackgroundJobPage; diff --git a/frontend/ui-core/src/index.ts b/frontend/ui-core/src/index.ts index fc0d6c62..b4040b05 100644 --- a/frontend/ui-core/src/index.ts +++ b/frontend/ui-core/src/index.ts @@ -252,6 +252,7 @@ export { type ErrorBoundaryProps, } from "./patterns/ErrorBoundary.js"; export { DatasetScreen, type DatasetScreenProps } from "./screens/DatasetScreen.js"; +export { InferenceScreen } from "./screens/InferenceScreen.js"; export { saveBlob } from "./screens/download.js"; export { AssetThumbnail, type AssetThumbnailProps } from "./screens/AssetThumbnail.js"; @@ -327,14 +328,24 @@ export { SuggestPanel, type SuggestPanelProps } from "./annotator/SuggestPanel.j export { inferenceKeys, usableConnection, - useInferenceConnections, + useConnections, + useCreateConnection, + useDeleteConnection, + useDownloadSize, + useDownloadWeights, useSuggestRegion, + useUpdateConnection, type Connection, + type ConnectionInput, + type ConnectionPage, + type ConnectionSetupState, + type ConnectionType, + type DownloadSizeOut, type SuggestBlocker, type SuggestInput, type SuggestionOut, type SuggestedRegion, -} from "./annotator/inferenceQueries.js"; +} from "./data/inferenceQueries.js"; export { queryKeys, useActiveSchema, diff --git a/frontend/ui-core/src/screens/InferenceScreen.tsx b/frontend/ui-core/src/screens/InferenceScreen.tsx new file mode 100644 index 00000000..c9d984a9 --- /dev/null +++ b/frontend/ui-core/src/screens/InferenceScreen.tsx @@ -0,0 +1,628 @@ +/** + * The Inference section: where model connections are made, set up and removed. + * + * A top-level destination rather than a project tab, per the decision recorded on + * #421 (2026-08-08): a connection carries no project id, every project uses the + * same ones, and navigation maps 1:1 to domain objects — so a project tab would + * state a scope the object does not have. That decision supersedes #58's rail + * rule, and `DESIGN.md` carries the new membership. + * + * ## Nothing here decides what is legal + * + * Every row action is rendered from `allowed_actions` on `ConnectionOut` and from + * nothing else. `download_weights` is declared for a local connection whose + * weights are not here yet — including on a machine with no local runtime + * installed, deliberately, because whether *this* machine has the extra is not a + * fact about the connection and hiding the control would leave the install + * command with nowhere to be shown. So the refusal arrives from the request and + * renders as prose (design principle 9, `ui-capabilities`). + * + * ## The status column has two values, not three + * + * `#421`'s journey lists `Ready` / `Not set up` / `Unreachable`. The wire has two: + * `setup_state` is deliberately **not** a reachability answer — whether an + * endpoint responds has a fresh answer every time it is asked, so it belongs to a + * test call and its result rather than to a stored row that would start lying the + * moment the network moved. The test action ships with the HTTP endpoint contract + * (`cf. #421`); until then there is no third value to render and no control that + * would produce one. + * + * ## The size is asked for before the connection exists + * + * D1 on #424 requires the local form to show what a download would cost *before* + * somebody confirms. That is a query the form makes about a published revision, + * not something the create response could carry — by the time a connection exists + * the decision has already been taken. The same query is what surfaces a missing + * local runtime, which is why the form stays usable and shows the install command + * instead of disabling itself. + */ + +import { Download, Filter, MoreHorizontal, Pencil, Plug, Trash2 } from "lucide-react"; +import { useEffect, useState, type FormEvent, type JSX } from "react"; + +import { Async } from "../data/Async"; +import { asApiError } from "../data/errors"; +import { + useConnections, + useCreateConnection, + useDeleteConnection, + useDownloadSize, + useDownloadWeights, + useUpdateConnection, + type Connection, + type ConnectionType, +} from "../data/inferenceQueries"; +import { Badge } from "../primitives/Badge"; +import { Button } from "../primitives/Button"; +import { + Dialog, + DialogContent, + DialogDescription, + DialogFooter, + DialogTitle, +} from "../primitives/Dialog"; +import { + DropdownMenu, + DropdownMenuContent, + DropdownMenuItem, + DropdownMenuTrigger, +} from "../primitives/Menu"; +import { FieldError, FieldHint, Input, Label } from "../primitives/Input"; +import { Table, TableBody, TableCell, TableHead, TableHeader, TableRow } from "../primitives/Table"; +import { useBackgroundJob } from "./queries"; + +/** + * The model D1 suggests, and the revision it is pinned at. + * + * "Suggested default" means exactly that: it fills the form in, and anybody may + * type over it. Nothing is bundled and nothing is fetched until somebody presses + * the action that fetches it. + */ +export const SUGGESTED_MODEL = "facebook/sam2-hiera-base-plus"; +export const SUGGESTED_REVISION = "main"; + +/** Above this many rows a list carries a filter input (`DESIGN.md`). */ +const FILTER_ABOVE = 20; + +export function InferenceScreen(): JSX.Element { + const connections = useConnections(); + const [creating, setCreating] = useState(false); + const [editing, setEditing] = useState(null); + const [doomed, setDoomed] = useState(null); + const [needle, setNeedle] = useState(""); + + return ( +
+
+
+

Inference

+

+ Where a model may be asked to predict. Connections are shared by every project + in this workspace. +

+
+ +
+ + setCreating(true)}> + Add connection + + ), + }} + > + {(page) => { + const shown = matching(page.items, needle); + return ( +
+ {page.items.length > FILTER_ABOVE && ( +
+
+ )} + + + + Name + Type + Model + Status + + + + + {shown.map((row) => ( + setEditing(row)} + onDelete={() => setDoomed(row)} + /> + ))} + +
+
+ ); + }} + + + setCreating(false)} /> + setEditing(null)} + {...(editing === null ? {} : { editing })} + /> + setDoomed(null)} /> +
+ ); +} + +/** Case-insensitive name substring, the shape every filter in this product has. */ +function matching(rows: readonly Connection[], needle: string): readonly Connection[] { + const wanted = needle.trim().toLowerCase(); + if (wanted === "") return rows; + return rows.filter((row) => row.name.toLowerCase().includes(wanted)); +} + +function ConnectionRow({ + connection, + onEdit, + onDelete, +}: { + readonly connection: Connection; + readonly onEdit: () => void; + readonly onDelete: () => void; +}): JSX.Element { + const can = new Set(connection.allowed_actions); + const ready = connection.setup_state === "ready"; + return ( + + {connection.name} + + + {connection.connection_type === "local" ? "Local" : "HTTP"} + + + + {/* One column, the way a person reads them — the CLI's listing agrees. */} + {connection.model_id} @ {connection.model_revision} + + + {/* + Semantic token **and** text, never colour alone: the word is what a + screen reader announces and what somebody who cannot tell the two + desaturated chips apart reads. + */} + + {ready ? "Ready" : "Not set up"} + + + +
+ {can.has("download_weights") && } + {(can.has("update") || can.has("delete")) && ( + + + + + + {can.has("update") && ( + + + )} + {can.has("delete") && ( + + + )} + + + )} +
+ + + ); +} + +/** + * The `download_weights` action, and the job it launches. + * + * 202 and poll, the contract the export route uses: the button hands off to + * `useBackgroundJob` and reports the phase the job is in. A failed run leaves the + * connection exactly as it was — the state flip is the download's last statement + * — so there is nothing to undo and the button simply comes back. + */ +function DownloadWeights({ connection }: { readonly connection: Connection }): JSX.Element { + const download = useDownloadWeights(); + const [jobId, setJobId] = useState(null); + const job = useBackgroundJob(jobId); + const state = job.data?.state; + const running = download.isPending || state === "queued" || state === "running"; + + // Stop polling once the work settles, and let the invalidated list carry the + // outcome — the row itself is what says `Ready`, so a second announcement here + // would be the same fact twice. + useEffect(() => { + if (state === "succeeded" || state === "cancelled") setJobId(null); + }, [state]); + + const failure = download.isError + ? asApiError(download.error) + : state === "failed" + ? { code: "DOWNLOAD_FAILED", message: job.data?.error ?? "The download did not finish." } + : null; + + return ( +
+ + {running && job.data !== undefined && ( + + {job.data.processed} + {job.data.total === null ? "" : ` of ${job.data.total}`} + + )} + {failure !== null && ( + + {failure.code} {failure.message} + + )} +
+ ); +} + +/** + * Two steps on the way in, one on the way back. + * + * Creating picks a kind and then fills in that kind's form, because the two + * kinds share almost no fields and a single form holding both would be mostly + * disabled whichever was chosen. Editing skips step one: the kind is not + * editable, so offering it would be offering something the server refuses. + */ +function ConnectionDialog({ + open, + onClose, + editing, +}: { + readonly open: boolean; + readonly onClose: () => void; + readonly editing?: Connection; +}): JSX.Element { + const create = useCreateConnection(); + const update = useUpdateConnection(); + const [kind, setKind] = useState(null); + const [name, setName] = useState(""); + const [modelId, setModelId] = useState(""); + const [revision, setRevision] = useState(""); + const [device, setDevice] = useState(""); + const [precision, setPrecision] = useState(""); + const [endpoint, setEndpoint] = useState(""); + + // Fill the form from whatever the dialog was opened for. An edit arrives with a + // row; a create arrives with nothing and, once a local kind is chosen, with + // D1's suggestion already in it. + useEffect(() => { + if (!open) return; + if (editing !== undefined) { + setKind(editing.connection_type); + setName(editing.name); + setModelId(editing.model_id); + setRevision(editing.model_revision); + setDevice(editing.device ?? ""); + setPrecision(editing.precision ?? ""); + setEndpoint(editing.endpoint_url ?? ""); + return; + } + setKind(null); + setName(""); + setModelId(""); + setRevision(""); + setDevice("cpu"); + setPrecision("fp16"); + setEndpoint(""); + }, [open, editing]); + + function choose(next: ConnectionType): void { + setKind(next); + if (next === "local") { + setModelId(SUGGESTED_MODEL); + setRevision(SUGGESTED_REVISION); + } + } + + const local = kind === "local"; + const pending = create.isPending || update.isPending; + const complete = + name.trim() !== "" && + modelId.trim() !== "" && + revision.trim() !== "" && + (local ? device.trim() !== "" && precision.trim() !== "" : endpoint.trim() !== ""); + + function submit(event: FormEvent): void { + event.preventDefault(); + if (kind === null) return; + const input = { + name: name.trim(), + connectionType: kind, + modelId: modelId.trim(), + modelRevision: revision.trim(), + device: device.trim(), + precision: precision.trim(), + endpointUrl: endpoint.trim(), + }; + // Only on success: a refusal leaves the dialog open with what was typed + // still in it. + if (editing === undefined) create.mutate(input, { onSuccess: onClose }); + else update.mutate({ ...input, id: editing.id }, { onSuccess: onClose }); + } + + const failure = create.isError ? create.error : update.isError ? update.error : null; + + return ( + !next && onClose()}> + + {editing === undefined ? "Add connection" : `Edit ${editing.name}`} + {kind === null ? ( + <> + + Where does this model run? Creating a connection downloads nothing. + +
+ + +
+ + ) : ( + <> + + {local + ? "Nothing is fetched until you ask for it, from the row this creates." + : "The endpoint answers this project's own inference contract."} + +
+
+ + setName(event.target.value)} + autoFocus + /> + Unique in this workspace, ignoring case. +
+
+ + setModelId(event.target.value)} + /> +
+
+ + setRevision(event.target.value)} + /> + Pinned. A moving pointer is not a provenance. +
+ {local ? ( + <> +
+
+ + setDevice(event.target.value)} + /> +
+
+ + setPrecision(event.target.value)} + /> +
+
+ + + ) : ( +
+ + setEndpoint(event.target.value)} + /> +
+ )} + {failure !== null && ( + + {asApiError(failure).code}{" "} + {asApiError(failure).message} + + )} + + + + + + + )} +
+ + ); +} + +/** + * What this revision would cost to fetch, beside the control that confirms it. + * + * D1's "download size shown before confirming", and the one place a missing local + * runtime becomes visible in this form. The refusal is rendered **as the server + * wrote it** — `LOCAL_INFERENCE_UNAVAILABLE` is one of the four codes that opt + * out of the opaque body precisely so the install command reaches a person, and a + * sentence written here would throw it away. + * + * The form stays usable throughout (principle 9): not knowing the size does not + * stop somebody configuring a connection, because creating one downloads nothing. + */ +function DownloadSizeLine({ + modelId, + revision, +}: { + readonly modelId: string; + readonly revision: string; +}): JSX.Element { + const size = useDownloadSize(modelId, revision); + if (modelId === "" || revision === "") return <>; + if (size.isPending) { + return ( +

+ Reading the download size… +

+ ); + } + if (size.isError) { + const failure = asApiError(size.error); + return ( +

+ {failure.code} {failure.message} +

+ ); + } + return ( +

+ Downloads {bytes(size.data.total_bytes)} across {size.data.file_count} files when you + ask for it. +

+ ); +} + +/** + * Bytes as somebody reads them. + * + * Decimal units, because a download size is what a network moves and what a + * publisher quotes; binary units would put a different number on screen from the + * one the model's own page shows. + */ +export function bytes(count: number): string { + const units = ["B", "kB", "MB", "GB", "TB"]; + let value = count; + let unit = 0; + while (value >= 1000 && unit < units.length - 1) { + value /= 1000; + unit += 1; + } + return `${unit === 0 ? value : value.toFixed(1)} ${units[unit]}`; +} + +function DeleteConnectionDialog({ + connection, + onClose, +}: { + readonly connection: Connection | null; + readonly onClose: () => void; +}): JSX.Element { + const remove = useDeleteConnection(); + return ( + !next && onClose()}> + + Delete {connection?.name}? + + Annotations keep their model provenance; only this configuration is removed. + + {remove.isError && ( + + {asApiError(remove.error).code}{" "} + {asApiError(remove.error).message} + + )} + + + + + + + ); +} diff --git a/frontend/ui-core/src/screens/inference.test.tsx b/frontend/ui-core/src/screens/inference.test.tsx new file mode 100644 index 00000000..efabc1d4 --- /dev/null +++ b/frontend/ui-core/src/screens/inference.test.tsx @@ -0,0 +1,409 @@ +/** + * The Inference section: what it offers, what it refuses to offer, and why. + * + * Three claims here that nothing else in the suite makes: + * + * 1. **Availability is the wire's.** Every row below declares its own + * `allowed_actions`, and the tests that matter are the ones where a declared + * action is missing: `Download weights` is not a fact this screen derives from + * `setup_state`, it is a fact the server states. A screen that computed it + * would be the hand-mirror `ui-capabilities` bans. + * 2. **The size is read before anything is fetched**, and a machine that cannot + * read it says so *in the server's own words* — the install command reaches a + * person only if the client stops rewriting the refusal. + * 3. **The form stays usable when the size is unknown** (design principle 9). + * Creating a connection downloads nothing, so not knowing what a download + * would cost is not a reason to prevent one being configured. + * + * The requests are stubbed, never the questions: every mutation goes out on the + * path that reaches it, and the refusals come back from the stub. + */ + +import { QueryClient } from "@tanstack/react-query"; +import { render, screen, waitFor } from "@testing-library/react"; +import { userEvent } from "@testing-library/user-event"; +import { afterEach, beforeEach, expect, it, vi } from "vitest"; +import type { JSX, ReactNode } from "react"; + +import { ApiProvider } from "../data/ApiProvider"; +import { InferenceScreen, SUGGESTED_MODEL, SUGGESTED_REVISION, bytes } from "./InferenceScreen"; +import type { Connection } from "../data/inferenceQueries"; + +const API = "http://visionset.test"; + +type Answer = { status: number; body?: unknown }; +let handlers: ((request: Request) => Answer | undefined)[] = []; +const sent: Request[] = []; + +beforeEach(() => { + handlers = []; + sent.length = 0; + vi.stubGlobal("fetch", (input: RequestInfo | URL, init?: RequestInit) => { + const request = input instanceof Request ? input : new Request(String(input), init); + sent.push(request); + for (const handler of handlers) { + const answer = handler(request); + if (answer !== undefined) { + return Promise.resolve( + new Response(answer.status === 204 ? null : JSON.stringify(answer.body ?? null), { + status: answer.status, + headers: { "content-type": "application/json" }, + }), + ); + } + } + return Promise.resolve( + new Response(JSON.stringify({ code: "NO_STUB", message: request.url }), { + status: 500, + headers: { "content-type": "application/json" }, + }), + ); + }); +}); + +afterEach(() => vi.unstubAllGlobals()); + +function value(node: HTMLElement): string { + return (node as HTMLInputElement).value; +} + +function on(method: string, pattern: RegExp, answer: Answer): void { + handlers.push((request) => + request.method === method && pattern.test(new URL(request.url).pathname) ? answer : undefined, + ); +} + +function mount(node: ReactNode): JSX.Element { + return ( + + {node} + + ); +} + +function connection(overrides: Partial = {}): Connection { + return { + id: "11111111-1111-4111-8111-111111111111", + name: "sam2-local", + connection_type: "local", + model_id: SUGGESTED_MODEL, + model_revision: SUGGESTED_REVISION, + device: "cuda", + precision: "fp16", + endpoint_url: null, + setup_state: "not_set_up", + allowed_actions: ["download_weights", "update", "delete"], + created_at: "2026-08-08T00:00:00Z", + updated_at: "2026-08-08T00:00:00Z", + ...overrides, + } as Connection; +} + +function listing(rows: readonly Connection[]): void { + on("GET", /^\/inference\/connections$/, { + status: 200, + body: { items: rows, total: rows.length }, + }); +} + +function job(state: string, processed = 0, total: number | null = null): unknown { + return { + id: "job-1", + type: "download_weights", + state, + processed, + total, + failures: [], + error: null, + result: {}, + cancel_requested: false, + attempt: 1, + created_at: "2026-08-08T00:00:00Z", + started_at: null, + finished_at: null, + }; +} + +function sizeIs(totalBytes: number, fileCount = 3): void { + on("GET", /^\/inference\/download-size$/, { + status: 200, + body: { + model_id: SUGGESTED_MODEL, + model_revision: SUGGESTED_REVISION, + total_bytes: totalBytes, + file_count: fileCount, + }, + }); +} + +// --- the list ------------------------------------------------------------------ + +it("invites a first connection rather than apologising for having none", async () => { + listing([]); + render(mount()); + expect(await screen.findByText("Connect a model to enable auto-labeling")).not.toBeNull(); + expect( + screen.getByText( + "VisionSet never downloads models on its own — you choose what runs and where.", + ), + ).not.toBeNull(); +}); + +it("renders a failed listing as a refusal rather than as an empty list", async () => { + on("GET", /^\/inference\/connections$/, { + status: 503, + body: { code: "SERVICE_UNAVAILABLE", message: "The workspace is busy." }, + }); + render(mount()); + await waitFor(() => expect(screen.queryByTestId("connections-table")).toBeNull()); + expect(screen.queryByText("Connect a model to enable auto-labeling")).toBeNull(); +}); + +it("says the status in words as well as in a token", async () => { + listing([connection(), connection({ id: "b", name: "remote", setup_state: "ready" })]); + render(mount()); + const rows = await screen.findAllByTestId("connection-status"); + expect(rows[0].textContent).toContain("Not set up"); + expect(rows[1].textContent).toContain("Ready"); +}); + +it("shows the model and its revision the way a person reads them", async () => { + listing([connection()]); + render(mount()); + expect( + await screen.findByText(`${SUGGESTED_MODEL} @ ${SUGGESTED_REVISION}`), + ).not.toBeNull(); +}); + +it("carries no filter until a list could be long enough to need one", async () => { + listing([connection()]); + render(mount()); + await screen.findByTestId("connections-table"); + expect(screen.queryByTestId("connection-filter")).toBeNull(); +}); + +it("filters by name and keeps saying how many it hid", async () => { + const many = Array.from({ length: 24 }, (_, index) => + connection({ id: `id-${index}`, name: index === 3 ? "needle" : `hay-${index}` }), + ); + listing(many); + render(mount()); + await userEvent.type(await screen.findByTestId("connection-filter"), "need"); + expect(screen.getByTestId("filter-count").textContent).toContain("1 of 24"); + expect(screen.getByTestId("connection-needle")).not.toBeNull(); +}); + +// --- what the wire declares, and only that ------------------------------------- + +it("offers Download weights only where the wire declares it", async () => { + listing([connection()]); + render(mount()); + expect(await screen.findByTestId("download-weights")).not.toBeNull(); +}); + +it("does not offer Download weights when the wire withholds it", async () => { + // The identical `setup_state`, so a screen deriving the action from the row's + // state would still render the button here. Only reading `allowed_actions` + // gets this right. + listing([connection({ allowed_actions: ["update", "delete"] })]); + render(mount()); + await screen.findByTestId("connections-table"); + expect(screen.queryByTestId("download-weights")).toBeNull(); +}); + +it("offers no overflow at all when neither edit nor delete is declared", async () => { + listing([connection({ allowed_actions: [] })]); + render(mount()); + await screen.findByTestId("connections-table"); + expect(screen.queryByTestId("actions-sam2-local")).toBeNull(); +}); + +it("renders a refused download as prose carrying the install command", async () => { + listing([connection()]); + on("POST", /\/download$/, { + status: 500, + body: { + code: "LOCAL_INFERENCE_UNAVAILABLE", + message: + 'running a model locally needs the local-inference extra. Install it with: pip install "visionset[local-inference]"', + }, + }); + render(mount()); + await userEvent.click(await screen.findByTestId("download-weights")); + const shown = await screen.findByTestId("download-error"); + expect(shown.textContent).toContain("LOCAL_INFERENCE_UNAVAILABLE"); + expect(shown.textContent).toContain('pip install "visionset[local-inference]"'); +}); + +it("watches the job a download hands back", async () => { + listing([connection()]); + on("POST", /\/download$/, { status: 202, body: job("queued") }); + on("GET", /^\/background-jobs\/job-1$/, { status: 200, body: job("running", 2, 5) }); + render(mount()); + await userEvent.click(await screen.findByTestId("download-weights")); + expect((await screen.findByTestId("download-progress")).textContent).toContain("2 of 5"); +}); + +// --- creating ------------------------------------------------------------------ + +it("asks where the model runs before asking anything else", async () => { + listing([]); + render(mount()); + await userEvent.click(await screen.findByTestId("new-connection")); + expect(await screen.findByTestId("choose-type")).not.toBeNull(); + expect(screen.queryByTestId("connection-name")).toBeNull(); +}); + +it("pre-fills the local form with the suggested model", async () => { + listing([]); + sizeIs(1_200_000_000); + render(mount()); + await userEvent.click(await screen.findByTestId("new-connection")); + await userEvent.click(await screen.findByTestId("choose-local")); + expect(value(await screen.findByTestId("connection-model"))).toBe(SUGGESTED_MODEL); + expect(value(screen.getByTestId("connection-revision"))).toBe(SUGGESTED_REVISION); +}); + +it("shows the download size before anything is confirmed", async () => { + listing([]); + sizeIs(1_200_000_000, 4); + render(mount()); + await userEvent.click(await screen.findByTestId("new-connection")); + await userEvent.click(await screen.findByTestId("choose-local")); + expect((await screen.findByTestId("size-known")).textContent).toContain("1.2 GB"); + expect(screen.getByTestId("size-known").textContent).toContain("4 files"); + // Nothing has been created and nothing has been fetched at this point. + expect(sent.some((one) => one.method === "POST")).toBe(false); +}); + +it("keeps the local form usable when the size cannot be read, and quotes the refusal", async () => { + listing([]); + on("GET", /^\/inference\/download-size$/, { + status: 500, + body: { + code: "LOCAL_INFERENCE_UNAVAILABLE", + message: + 'running a model locally needs the local-inference extra. Install it with: pip install "visionset[local-inference]"', + }, + }); + render(mount()); + await userEvent.click(await screen.findByTestId("new-connection")); + await userEvent.click(await screen.findByTestId("choose-local")); + const shown = await screen.findByTestId("size-unavailable"); + expect(shown.textContent).toContain('pip install "visionset[local-inference]"'); + // Principle 9: the form is not disabled by not knowing. Creating downloads + // nothing, so the unknown size is information rather than a gate. + await userEvent.type(screen.getByTestId("connection-name"), "sam2"); + expect((screen.getByTestId("connection-submit") as HTMLButtonElement).disabled).toBe(false); +}); + +it("asks for no size at all for an http connection", async () => { + listing([]); + render(mount()); + await userEvent.click(await screen.findByTestId("new-connection")); + await userEvent.click(await screen.findByTestId("choose-http")); + await screen.findByTestId("connection-endpoint"); + expect(sent.some((one) => one.url.includes("download-size"))).toBe(false); +}); + +it("sends only the fields the chosen kind carries", async () => { + listing([]); + const bodies: unknown[] = []; + handlers.push((request) => { + if (request.method !== "POST" || !request.url.endsWith("/inference/connections")) return; + return { status: 201, body: connection() }; + }); + on("GET", /^\/inference\/download-size$/, { status: 200, body: null }); + render(mount()); + await userEvent.click(await screen.findByTestId("new-connection")); + await userEvent.click(await screen.findByTestId("choose-http")); + await userEvent.type(await screen.findByTestId("connection-name"), "remote"); + await userEvent.type(screen.getByTestId("connection-model"), "some/model"); + await userEvent.type(screen.getByTestId("connection-revision"), "abc123"); + await userEvent.type(screen.getByTestId("connection-endpoint"), "https://example.invalid"); + await userEvent.click(screen.getByTestId("connection-submit")); + await waitFor(() => + expect(sent.filter((one) => one.method === "POST").length).toBeGreaterThan(0), + ); + const posted = sent.find((one) => one.method === "POST"); + const body = JSON.parse(await posted!.clone().text()) as Record; + bodies.push(body); + // The domain refuses an http connection carrying a device, so a form that sent + // everything it held would turn a field somebody switched away from into a 422. + expect(body.device).toBeNull(); + expect(body.precision).toBeNull(); + expect(body.endpoint_url).toBe("https://example.invalid"); +}); + +it("keeps what was typed when a create is refused", async () => { + listing([]); + on("POST", /^\/inference\/connections$/, { + status: 409, + body: { code: "ENTITY_ALREADY_EXISTS", message: "That name is taken." }, + }); + render(mount()); + await userEvent.click(await screen.findByTestId("new-connection")); + await userEvent.click(await screen.findByTestId("choose-http")); + await userEvent.type(await screen.findByTestId("connection-name"), "remote"); + await userEvent.type(screen.getByTestId("connection-model"), "some/model"); + await userEvent.type(screen.getByTestId("connection-revision"), "abc123"); + await userEvent.type(screen.getByTestId("connection-endpoint"), "https://example.invalid"); + await userEvent.click(screen.getByTestId("connection-submit")); + expect((await screen.findByTestId("connection-error")).textContent).toContain("That name is taken."); + expect(value(screen.getByTestId("connection-name"))).toBe("remote"); +}); + +it("has no credential field, because where a secret lives is still open", async () => { + listing([]); + render(mount()); + await userEvent.click(await screen.findByTestId("new-connection")); + await userEvent.click(await screen.findByTestId("choose-http")); + await screen.findByTestId("connection-endpoint"); + expect(screen.queryByLabelText(/credential|token|api key|secret/i)).toBeNull(); +}); + +// --- editing and deleting ------------------------------------------------------ + +it("edits without offering to change the kind", async () => { + listing([connection({ setup_state: "ready", allowed_actions: ["update", "delete"] })]); + render(mount()); + await userEvent.click(await screen.findByTestId("actions-sam2-local")); + await userEvent.click(await screen.findByTestId("action-edit")); + expect(value(await screen.findByTestId("connection-name"))).toBe("sam2-local"); + expect(screen.queryByTestId("choose-type")).toBeNull(); +}); + +it("states the blast radius of a delete accurately", async () => { + listing([connection()]); + render(mount()); + await userEvent.click(await screen.findByTestId("actions-sam2-local")); + await userEvent.click(await screen.findByTestId("action-delete")); + expect(await screen.findByText("Annotations keep their model provenance; only this configuration is removed.")).not.toBeNull(); +}); + +it("renders a refused delete rather than closing over it", async () => { + listing([connection()]); + on("DELETE", /\/inference\/connections\//, { + status: 404, + body: { code: "INFERENCE_CONNECTION_NOT_FOUND", message: "It is already gone." }, + }); + render(mount()); + await userEvent.click(await screen.findByTestId("actions-sam2-local")); + await userEvent.click(await screen.findByTestId("action-delete")); + await userEvent.click(await screen.findByTestId("delete-connection-submit")); + expect((await screen.findByTestId("delete-connection-error")).textContent).toContain("It is already gone."); +}); + +// --- the number itself ---------------------------------------------------------- + +it("says bytes the way a publisher quotes them", () => { + // Decimal, so the figure matches the one on the model's own page rather than + // being the same download described with a smaller number. + expect(bytes(0)).toBe("0 B"); + expect(bytes(999)).toBe("999 B"); + expect(bytes(1_000)).toBe("1.0 kB"); + expect(bytes(1_200_000_000)).toBe("1.2 GB"); +}); diff --git a/openapi.json b/openapi.json index 8814f860..7df691cc 100644 --- a/openapi.json +++ b/openapi.json @@ -1921,6 +1921,35 @@ "title": "DatasetStatsOut", "type": "object" }, + "DownloadSizeOut": { + "description": "What fetching a model's weights would cost, before anybody fetches them.\n\nAnswered from the publishing hub's file listing, so asking costs a metadata\nrequest and never a download. The pair is echoed back for ``SuggestionOut``'s\nreason: a form that had to remember which model it asked about would be\nkeeping a second copy of something the response can simply state.", + "properties": { + "file_count": { + "title": "File Count", + "type": "integer" + }, + "model_id": { + "title": "Model Id", + "type": "string" + }, + "model_revision": { + "title": "Model Revision", + "type": "string" + }, + "total_bytes": { + "title": "Total Bytes", + "type": "integer" + } + }, + "required": [ + "model_id", + "model_revision", + "total_bytes", + "file_count" + ], + "title": "DownloadSizeOut", + "type": "object" + }, "ErrorBody": { "description": "The one error shape this API emits, at every status.", "properties": { @@ -6422,6 +6451,93 @@ ] } }, + "/inference/download-size": { + "get": { + "description": "How big fetching that model's weights would be, before anybody fetches them.\n\nWhat the local-connection form shows beside its confirm control, so the\ndecision recorded on #418 \u2014 that VisionSet downloads nothing on its own \u2014 is\none somebody can actually make (`cf. #421`, `#424`).\n\n**This downloads nothing.** It reads the publishing hub's file listing, which\nis the one question answerable before the download it describes. The number\ncovers every file in the revision, because that is what the download fetches.\n\nQuery parameters rather than a path, because a model id contains a slash\n(`facebook/sam2-hiera-base-plus`) and a segment that has to be escaped to be\nwritten is a URL people get wrong by hand.\n\n**Not a connection route**, and it takes no connection id: the moment the\nnumber is needed is the moment before the connection exists. Asking about a\nconnection that already exists is the same pair of values, asked the same way.\n\nRefused with the install command when the local runtime is absent \u2014 the size\nis read with the same client that would do the fetching \u2014 and refused rather\nthan guessed when the hub cannot size every file in the revision.", + "operationId": "inference_download_size", + "parameters": [ + { + "in": "query", + "name": "model_id", + "required": true, + "schema": { + "title": "Model Id", + "type": "string" + } + }, + { + "in": "query", + "name": "model_revision", + "required": true, + "schema": { + "title": "Model Revision", + "type": "string" + } + } + ], + "responses": { + "200": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/DownloadSizeOut" + } + } + }, + "description": "Successful Response" + }, + "401": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ErrorBody" + } + } + }, + "description": "Missing or invalid bearer token" + }, + "422": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ErrorBody" + } + } + }, + "description": "The request payload is not processable" + }, + "500": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ErrorBody" + } + } + }, + "description": "Unhandled server error, with an incident id" + }, + "503": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ErrorBody" + } + } + }, + "description": "The workspace is busy; retry after the header says" + } + }, + "security": [ + { + "HTTPBearer": [] + } + ], + "summary": "Inference Download Size", + "tags": [ + "inference" + ] + } + }, "/inference/suggest": { "post": { "description": "Propose a shape for the thing under those points.\n\nThe server side of the editor's suggest gesture (`cf. #424`). One asset, one\nprompt set, one answer \u2014 batch prediction is a separate path and is not this\none.\n\n**Nothing is written and nothing is remembered.** A suggestion is a proposal:\naccepting it is a later, ordinary annotation write carrying `provenance:\nmodel`, this response's `model_ref`, and its `confidence`. Discarding it\ncosts a request that already finished. The only thing that outlives the call\nis a cached image embedding, which is an optimisation rather than a record \u2014\nso the same points sent twice answer the same way, and a restart changes\nnothing but the latency of the first click.\n\n**The first click on an asset is the slow one.** A segmenter reads the whole\nimage once and then answers any number of clicks from that reading almost for\nfree, which is what makes refining by adding points practical. Sending the\naccumulated points \u2014 rather than a diff \u2014 is what keeps this stateless.\n\n**`allowed_geometries` is the caller's schema, not a preference.** The answer\nis produced in one of the kinds named or not at all: a class that admits\npolygons gets the outline, a class that admits only boxes gets its extent,\nand a class that admits neither gets `region: null`. Answering in a kind the\nschema would refuse would produce a suggestion that cannot be accepted.\n\nA null `region` is a successful answer with nothing to propose. Refusals are\nreserved for things the caller can act on: an unknown project, asset or\nconnection is 404; a connection whose weights are not here yet, or whose kind\nthis build cannot run, is 409 and names what to do; a connection whose model\nanswers words rather than places is 422.", diff --git a/src/visionset/cli/inference.py b/src/visionset/cli/inference.py index 6b3b5db7..41cdff8e 100644 --- a/src/visionset/cli/inference.py +++ b/src/visionset/cli/inference.py @@ -1,10 +1,14 @@ # usage: from visionset.cli.inference import inference_app """``visionset inference`` — configuring where a model may be asked to predict. -Six commands over one service, so a workspace can be made ready for -auto-labeling without a browser — including the one operation that reaches a -network, ``download``. Contacting an endpoint is still absent (`cf. #421`): a -command that cannot work is worse than one that is not there yet. +Seven commands, so a workspace can be made ready for auto-labeling without a +browser. Six are one call to ``InferenceConnectionService``; ``size`` is the +exception and says so — it is about a published model rather than about a +configured row, so it opens no workspace at all. Two reach a network: +``download``, which fetches, and ``size``, which reads a listing so that +``download`` can be an informed decision. Contacting a configured endpoint is +still absent (`cf. #421`): a command that cannot work is worse than one that is +not there yet. **``download`` blocks, and that is ``ingest``'s pattern rather than a shortcut.** The API queues the same work because it has a dispatcher to run it; a terminal @@ -23,9 +27,10 @@ import typer from visionset import wire +from visionset.cli._errors import domain_errors from visionset.cli._output import JsonOption, document, note, table from visionset.cli._workspace import WorkspaceOption, opened_workspace -from visionset.inference import fetch_weights +from visionset.inference import download_size, fetch_weights from visionset.kernel.domain import ConnectionType, InferenceConnection from visionset.kernel.services import InferenceConnectionService @@ -142,6 +147,36 @@ def inference_update( typer.echo(str(edited.id)) +@inference_app.command("size") +def inference_size( + model_id: Annotated[str, typer.Argument(help="Which model, at its source.")], + model_revision: Annotated[ + str, typer.Option("--revision", help="Pinned. A size is a fact about one revision.") + ], + json_out: JsonOption = False, +) -> None: + """How big fetching that model's weights would be. Nothing is downloaded. + + The number to look at *before* running ``download``, read from the publishing + hub's file listing rather than from the files. It covers every file in the + revision, because that is what the download fetches. + + Takes a model and a revision rather than a connection, and opens no + workspace: the moment the number is wanted is usually the moment before a + connection exists. ``domain_errors`` is therefore explicit here, where every + other command in this file inherits it from ``opened_workspace`` — a missing + extra is still a refusal and must still be a sentence rather than a + traceback. + """ + with domain_errors(): + size = download_size(model_id, model_revision) + if json_out: + document(wire.download_size(size)) + return + note(f"{size.file_count} files in {size.model_id} at {size.model_revision}.") + typer.echo(str(size.total_bytes)) + + @inference_app.command("download") def inference_download( connection: ConnectionArgument, diff --git a/src/visionset/inference/__init__.py b/src/visionset/inference/__init__.py index 5d604da9..5ce45933 100644 --- a/src/visionset/inference/__init__.py +++ b/src/visionset/inference/__init__.py @@ -57,26 +57,41 @@ from visionset.inference.sam_provider import LocalSamProvider from visionset.inference.suggestions import suggest from visionset.inference.transformers_provider import LocalTransformersProvider -from visionset.inference.weights import MODELS_DIRNAME, cache_root, download, fetch_weights +from visionset.inference.weights import ( + DEFAULT_SIZE_CAPACITY, + MODELS_DIRNAME, + DownloadSizes, + cache_root, + download, + download_size, + fetch_weights, + known_sizes, + measure, +) __all__ = [ "DEFAULT_DETAIL", "DEFAULT_EMBEDDING_CAPACITY", "DEFAULT_IOU_THRESHOLD", "DEFAULT_PROVIDER_CAPACITY", + "DEFAULT_SIZE_CAPACITY", "EXTRA", "INSTALL_COMMAND", "MODELS_DIRNAME", "MODULES", "SEGMENTER_FAMILIES", "BoundedCache", + "DownloadSizes", "LocalSamProvider", "LocalTransformersProvider", "ProviderPool", "cache_root", "download", + "download_size", "family_of", "fetch_weights", + "known_sizes", + "measure", "narrowed", "polygon_from", "provider_for", diff --git a/src/visionset/inference/weights.py b/src/visionset/inference/weights.py index 47b8a293..21641d97 100644 --- a/src/visionset/inference/weights.py +++ b/src/visionset/inference/weights.py @@ -38,12 +38,21 @@ from uuid import UUID from visionset.inference._extra import imported -from visionset.kernel.domain import ConnectionType, InferenceConnection +from visionset.inference.cache import BoundedCache +from visionset.kernel.domain import ConnectionType, DownloadSize, InferenceConnection from visionset.kernel.errors import LocalInferenceUnavailable from visionset.kernel.services import InferenceConnectionService, WorkspaceService _logger: Final = logging.getLogger(__name__) +DEFAULT_SIZE_CAPACITY: Final = 32 +"""How many ``model_id@revision`` sizes to remember. + +Small because the working set is: the models offered in a form, plus whatever +somebody typed while deciding. Each entry is two integers and two short strings, +so the bound is about not growing without limit rather than about memory. +""" + MODELS_DIRNAME: Final = "models" """Where weights land, under the workspace root. @@ -157,3 +166,126 @@ def download(connection: InferenceConnection, *, into: Path) -> Path: raise LocalInferenceUnavailable( f"could not fetch {connection.model_id} at {connection.model_revision}: {exc}" ) from exc + + +def measure(model_id: str, model_revision: str) -> DownloadSize: + """How big that revision is, asked of the hub's metadata and nothing else. + + **Nothing is downloaded here, and that is the whole point of the function.** + ``model_info`` reads the repository's file listing — names and byte counts — + which is the one question that can be answered before somebody has agreed to + pay for the answer. Reaching :func:`download` to find out how big a download + would be is the shape this exists to avoid, and + ``test_measuring_a_size_downloads_nothing`` is what holds it. + + Every file in the revision is counted, because :func:`download` fetches every + file in the revision. The two numbers describe the same set on purpose: a + figure that counted only the weights would understate what lands on the disk + for any repository that also publishes a tokenizer, a processor config, or a + second serialisation of the same tensors. + + A file the listing does not size is refused rather than skipped. Silently + dropping it would answer with a number smaller than the truth, which is worse + than no number at all when the number's whole job is to inform a decision. + + Raises: + LocalInferenceUnavailable: ``huggingface_hub`` is not installed, the + revision could not be read, or the listing did not size every file. + """ + hub = imported("huggingface_hub") + _logger.debug("reading the size of %s at %s", model_id, model_revision) + try: + info = hub.model_info(model_id, revision=model_revision, files_metadata=True) + except Exception as exc: # noqa: BLE001 — ``download``'s reason, one call earlier + raise LocalInferenceUnavailable( + f"could not read the size of {model_id} at {model_revision}: {exc}" + ) from exc + files = tuple(getattr(info, "siblings", None) or ()) + if not files: + raise LocalInferenceUnavailable( + f"the hub listed no files for {model_id} at {model_revision}, so there is no " + "size to show; check the model id and the revision" + ) + total = 0 + unsized: list[str] = [] + for one in files: + size = getattr(one, "size", None) + if size is None: + unsized.append(str(getattr(one, "rfilename", "?"))) + continue + total += int(size) + if unsized: + raise LocalInferenceUnavailable( + f"the hub did not report a size for {len(unsized)} of {len(files)} files in " + f"{model_id} at {model_revision} (for example {unsized[0]!r}), so the download " + "size cannot be stated" + ) + return DownloadSize( + model_id=model_id, + model_revision=model_revision, + total_bytes=total, + file_count=len(files), + ) + + +class DownloadSizes: + """Sizes already looked up, bounded and least-recently-used. + + Instantiable rather than only module-level for ``ProviderPool``'s reason: a + test holds its own and asserts on it without reaching into process state. + + **A size is immutable per revision**, which is what makes caching it correct + rather than merely fast: a pinned revision is a fixed set of files, so the + answer cannot go stale. That is also why nothing invalidates this — there is + no event that could change what it holds. A moving pointer like ``main`` is + the one case where the pin is not a pin, and it is cached anyway: a + connection is required to carry a real revision (`domain/inference.py`), so + the only caller that can reach one is a form somebody is still typing into. + """ + + def __init__(self, capacity: int = DEFAULT_SIZE_CAPACITY) -> None: + self._held: BoundedCache[str, DownloadSize] = BoundedCache(capacity) + self._lookups = 0 + + @property + def lookups(self) -> int: + """How many times this has actually reached the hub. + + The counter that separates a working cache from one that asks every time + — both answer correctly, and only this tells them apart. + """ + return self._lookups + + def get(self, model_id: str, model_revision: str) -> DownloadSize: + """That revision's size, read once and kept.""" + key = f"{model_id}@{model_revision}" + held = self._held.get(key) + if held is not None: + return held + measured = measure(model_id, model_revision) + self._lookups += 1 + return self._held.put(key, measured) + + def clear(self) -> None: + """Forget everything. What a test does between cases.""" + self._held.clear() + + def __len__(self) -> int: + return len(self._held) + + +_KNOWN: Final = DownloadSizes() + + +def known_sizes() -> DownloadSizes: + """The process-wide size cache, on ``resident``'s terms.""" + return _KNOWN + + +def download_size(model_id: str, model_revision: str) -> DownloadSize: + """What fetching that revision would cost, from the cache or from the hub. + + The one surfaces call: a route before a form confirms, and the CLI when + somebody wants the number before typing ``download``. + """ + return known_sizes().get(model_id, model_revision) diff --git a/src/visionset/kernel/domain/__init__.py b/src/visionset/kernel/domain/__init__.py index 26eb60c7..93ba50ba 100644 --- a/src/visionset/kernel/domain/__init__.py +++ b/src/visionset/kernel/domain/__init__.py @@ -70,6 +70,7 @@ EVERY_SETUP_STATE, ConnectionSetupState, ConnectionType, + DownloadSize, InferenceConnection, ) from visionset.kernel.domain.ingest import ( @@ -257,6 +258,7 @@ "DatasetOperation", "DatasetStats", "DomainEvent", + "DownloadSize", "ClassCompatibility", "ClassExportStatus", "ExportCompatibility", diff --git a/src/visionset/kernel/domain/inference.py b/src/visionset/kernel/domain/inference.py index d6a4ae4a..e0e0f6ed 100644 --- a/src/visionset/kernel/domain/inference.py +++ b/src/visionset/kernel/domain/inference.py @@ -34,7 +34,7 @@ from typing import Final from uuid import UUID, uuid4 -from pydantic import BaseModel, Field, field_validator, model_validator +from pydantic import BaseModel, ConfigDict, Field, field_validator, model_validator class ConnectionType(StrEnum): @@ -93,6 +93,43 @@ class ConnectionSetupState(StrEnum): """ +class DownloadSize(BaseModel): + """What fetching a model's weights would cost, before anybody fetches them. + + The answer to the question a setup form has to ask on somebody's behalf: the + decision recorded on #418 is that VisionSet downloads nothing on its own, and + a person can only make that decision if the size is on screen **before** they + confirm. So this is read separately from the download and ahead of it. + + **A pair, not a connection.** It is keyed on a model id and a revision rather + than on an :class:`InferenceConnection`, because the moment it is needed is + the moment before a connection exists — the form is being filled in. A + connection that already exists is the same pair, asked the same way. + + **Every file, because the download fetches every file.** ``total_bytes`` is + the whole revision rather than the weights alone: what gets fetched is a + snapshot, so a number counting only ``.safetensors`` would be a smaller + number than the thing it claims to describe. A repository publishing both a + ``.bin`` and a ``.safetensors`` copy of the same tensors is therefore + reported at the sum of the two, which is what will actually land on the disk. + + Not persisted anywhere. A size is a fact about a published revision, so it is + the same answer on every machine and there is nothing about it worth storing + in a workspace. + """ + + model_config = ConfigDict(frozen=True, extra="forbid") + + model_id: str + model_revision: str + #: A revision whose size could not be established is a refusal, never a zero + #: here: a form showing "0 B" would be inviting somebody to confirm a download + #: it knows nothing about. The bound admits zero because the type should not + #: encode a lookup's policy, not because anything answers with it. + total_bytes: int = Field(ge=0) + file_count: int = Field(ge=0) + + class InferenceConnection(BaseModel): """One configured place a model can be asked to predict. diff --git a/src/visionset/server/models.py b/src/visionset/server/models.py index e3e6e15c..4274ab4f 100644 --- a/src/visionset/server/models.py +++ b/src/visionset/server/models.py @@ -73,6 +73,7 @@ Dataset, DatasetChange, DatasetStats, + DownloadSize, ExportCompatibility, Geometry, GeometryType, @@ -1679,6 +1680,33 @@ class ConnectionUpdate(BaseModel): endpoint_url: str | None = None +class DownloadSizeOut(BaseModel): + """What fetching a model's weights would cost, before anybody fetches them. + + Answered from the publishing hub's file listing, so asking costs a metadata + request and never a download. The pair is echoed back for ``SuggestionOut``'s + reason: a form that had to remember which model it asked about would be + keeping a second copy of something the response can simply state. + """ + + model_id: str + model_revision: str + #: Every file in the revision, because the download fetches every file in the + #: revision. Bytes rather than a formatted string: how to say "2.3 GB" is a + #: question about a locale and a screen width, and neither is the server's. + total_bytes: int + file_count: int + + @classmethod + def of(cls, size: DownloadSize) -> Self: + return cls( + model_id=size.model_id, + model_revision=size.model_revision, + total_bytes=size.total_bytes, + file_count=size.file_count, + ) + + class SuggestPoint(BaseModel): """One click, in the asset's own pixel coordinates. diff --git a/src/visionset/server/routes/__init__.py b/src/visionset/server/routes/__init__.py index d02e809f..43eac900 100644 --- a/src/visionset/server/routes/__init__.py +++ b/src/visionset/server/routes/__init__.py @@ -66,7 +66,7 @@ # a stage of the data's life but a piece of this workspace's configuration, # which the pipeline reads rather than produces. inference.router, - inference.suggestions, + inference.beside_connections, # Last, and outside the pipeline order above on purpose: a background job is # not a stage of the data's life, it is how some of those stages run. Reading # it into the sequence would suggest a place it does not have. diff --git a/src/visionset/server/routes/inference.py b/src/visionset/server/routes/inference.py index 80e39211..c896da24 100644 --- a/src/visionset/server/routes/inference.py +++ b/src/visionset/server/routes/inference.py @@ -6,13 +6,17 @@ handlers ``create_app()`` installed turn it into an ``ErrorBody`` with a stable code. -**Nothing in this file runs a model or contacts an endpoint.** The weight -download is queued rather than performed — it answers 202 and points at a +**Nothing in this file runs a model or contacts a configured endpoint.** The +weight download is queued rather than performed — it answers 202 and points at a background job, the contract the export route already uses — and a reachability ``test`` is still absent rather than stubbed, so ``allowed_actions`` does not name it and no client is told about a control that does not exist yet (`cf. #418`, `#421`). +The one network call made here is ``download-size``, and it reads a file +listing rather than files: the number has to be on screen *before* somebody +agrees to a download, so it cannot be a by-product of one. + Handlers are ``def`` rather than ``async def``, on ``projects``' terms: every kernel call underneath is a blocking SQLite call, and a coroutine would run it on the event loop. @@ -22,7 +26,7 @@ from fastapi import Response, status -from visionset.inference import DEFAULT_DETAIL, suggest +from visionset.inference import DEFAULT_DETAIL, download_size, suggest from visionset.inference import require as require_local_inference from visionset.jobs.weights import JOB_TYPE as download_job_type from visionset.jobs.weights import payload_for as download_payload_for @@ -36,6 +40,7 @@ ConnectionOut, ConnectionPage, ConnectionUpdate, + DownloadSizeOut, SuggestedRegion, SuggestionOut, SuggestRequest, @@ -43,12 +48,13 @@ router = protected_router(prefix="/inference/connections", tags=["inference"]) -#: A second router because the path is a sibling of ``connections`` rather than a -#: child of one: a suggestion is made *through* a connection, not *on* it, and +#: A second router for the paths that are siblings of ``connections`` rather than +#: children of one. A suggestion is made *through* a connection, not *on* it, and #: nesting it under ``/inference/connections/{id}/suggest`` would put the asset — #: the thing the call is actually about — in the body under a URL claiming the -#: connection owns it. -suggestions = protected_router(prefix="/inference", tags=["inference"]) +#: connection owns it. A download size is not about a connection at all: it is +#: asked while a form is being filled in, before there is a row to hang it on. +beside_connections = protected_router(prefix="/inference", tags=["inference"]) @router.get("") @@ -156,7 +162,34 @@ def download_connection_weights( return BackgroundJobOut.of(job) -@suggestions.post("/suggest", responses=documented(404, 409, 422)) +@beside_connections.get("/download-size", responses=documented(422)) +def inference_download_size(model_id: str, model_revision: str) -> DownloadSizeOut: + """How big fetching that model's weights would be, before anybody fetches them. + + What the local-connection form shows beside its confirm control, so the + decision recorded on #418 — that VisionSet downloads nothing on its own — is + one somebody can actually make (`cf. #421`, `#424`). + + **This downloads nothing.** It reads the publishing hub's file listing, which + is the one question answerable before the download it describes. The number + covers every file in the revision, because that is what the download fetches. + + Query parameters rather than a path, because a model id contains a slash + (`facebook/sam2-hiera-base-plus`) and a segment that has to be escaped to be + written is a URL people get wrong by hand. + + **Not a connection route**, and it takes no connection id: the moment the + number is needed is the moment before the connection exists. Asking about a + connection that already exists is the same pair of values, asked the same way. + + Refused with the install command when the local runtime is absent — the size + is read with the same client that would do the fetching — and refused rather + than guessed when the hub cannot size every file in the revision. + """ + return DownloadSizeOut.of(download_size(model_id, model_revision)) + + +@beside_connections.post("/suggest", responses=documented(404, 409, 422)) def suggest_region(workspace: WorkspaceDep, body: SuggestRequest) -> SuggestionOut: """Propose a shape for the thing under those points. diff --git a/src/visionset/wire/__init__.py b/src/visionset/wire/__init__.py index 29319dbb..6597342f 100644 --- a/src/visionset/wire/__init__.py +++ b/src/visionset/wire/__init__.py @@ -68,6 +68,7 @@ ClassificationGeometry, Dataset, DatasetStats, + DownloadSize, ExportCompatibility, ExportResult, Geometry, @@ -591,3 +592,18 @@ def connection(value: InferenceConnection) -> dict[str, Any]: "created_at": _moment(value.created_at), "updated_at": _moment(value.updated_at), } + + +def download_size(value: DownloadSize) -> dict[str, Any]: + """What fetching a model's weights would cost, before anybody fetches them. + + Bytes rather than a formatted string, on ``export_result``'s terms: how to + say "2.3 GB" is a question about a locale and a screen width, and a machine + reading this wants the integer either way. + """ + return { + "model_id": value.model_id, + "model_revision": value.model_revision, + "total_bytes": value.total_bytes, + "file_count": value.file_count, + } diff --git a/tests/cli/test_inference_commands.py b/tests/cli/test_inference_commands.py index 926da11b..0feffa0b 100644 --- a/tests/cli/test_inference_commands.py +++ b/tests/cli/test_inference_commands.py @@ -13,8 +13,10 @@ from typing import Any import pytest -from tests.cli._flow import ok, payload, run, workspace +from click.testing import Result +from tests.cli._flow import ok, payload, run, runner, workspace +from visionset.cli.main import app from visionset.inference import MODULES from visionset.inference import weights as weights_module from visionset.kernel.services import ( @@ -343,3 +345,99 @@ def test_a_missing_local_runtime_exits_one_with_the_install_command(root: Path) assert result.exit_code == 1, result.output assert 'pip install "visionset[local-inference]"' in result.stderr assert "Traceback" not in result.stderr + + +# --- ``size``, the one command here that opens no workspace -------------------- + + +def sized(*argv: str) -> Result: + """Invoke ``size`` without ``--workspace``. + + ``_flow.run`` appends the flag to everything, and this command genuinely does + not take one: it asks about a published model, not about a configured row. + That is the assertion as much as the helper — a ``size`` that had grown a + workspace option would fail here rather than quietly acquiring a dependency + on state it does not read. + """ + return runner.invoke(app, list(argv)) + + +class _Sibling: + def __init__(self, rfilename: str, size: int) -> None: + self.rfilename = rfilename + self.size = size + + +class _Listing: + """A hub that lists files and fails the test if asked to fetch one.""" + + siblings = [_Sibling("config.json", 24), _Sibling("model.safetensors", 1_000)] + + @classmethod + def model_info(cls, _repo_id: str, **_: object) -> type[_Listing]: + return cls + + @staticmethod + def snapshot_download(**_: object) -> str: + raise AssertionError("reading a size must not download anything") + + +def test_the_size_command_reads_a_listing_and_downloads_nothing( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The number to look at before running ``download``. + + ``snapshot_download`` raises, so this reds if the command ever answers by + fetching the files it is measuring. + """ + monkeypatch.setattr(weights_module, "imported", lambda _name: _Listing) + weights_module.known_sizes().clear() + result = sized("inference", "size", "some/model", "--revision", "abc123") + assert result.exit_code == 0, result.output + assert result.stdout.strip() == "1024" + assert "2 files" in result.stderr + + +def test_the_size_command_needs_no_workspace( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """The moment the number is wanted is usually the moment before anything exists. + + With no workspace flag and no environment variable, a command that opened one + would refuse here — which is exactly what a first-time setup would hit. + """ + monkeypatch.setattr(weights_module, "imported", lambda _name: _Listing) + monkeypatch.delenv(WORKSPACE_ENV_VAR, raising=False) + monkeypatch.chdir(tmp_path) + weights_module.known_sizes().clear() + result = sized("inference", "size", "some/model", "--revision", "abc123") + assert result.exit_code == 0, result.output + + +def test_the_size_command_prints_the_document_on_json(monkeypatch: pytest.MonkeyPatch) -> None: + """Bytes as an integer: how to say "2.3 GB" is a question about a screen.""" + monkeypatch.setattr(weights_module, "imported", lambda _name: _Listing) + weights_module.known_sizes().clear() + result = sized("inference", "size", "some/model", "--revision", "abc123", "--json") + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == { + "model_id": "some/model", + "model_revision": "abc123", + "total_bytes": 1_024, + "file_count": 2, + } + + +@pytest.mark.skipif(_extra_is_installed(), reason="the local runtime is installed here") +def test_a_size_without_the_runtime_exits_one_with_the_install_command() -> None: + """``size`` opens no workspace, so it carries ``domain_errors`` itself. + + Without it a missing extra would reach a terminal as a traceback rather than + as the one line naming what to install — the translation every other command + in this file inherits from ``opened_workspace``. + """ + weights_module.known_sizes().clear() + result = sized("inference", "size", "some/model", "--revision", "abc123") + assert result.exit_code == 1, result.output + assert 'pip install "visionset[local-inference]"' in result.stderr + assert "Traceback" not in result.stderr diff --git a/tests/inference/test_download_size.py b/tests/inference/test_download_size.py new file mode 100644 index 00000000..35eb2eaf --- /dev/null +++ b/tests/inference/test_download_size.py @@ -0,0 +1,258 @@ +"""Reading how big a download would be, without doing the download. + +The number the local-connection form shows before somebody confirms (#424 D1). +Its whole reason to exist is that the decision recorded on #418 — VisionSet +downloads nothing on its own — is only a real decision if the cost is on screen +first, so the one property worth proving is negative: **asking never fetches**. + +The hub client is faked, as it is in `test_weights.py`, because a real call is a +network round trip. What is not faked is the code that decides what to count, +what to refuse, and what to remember. +""" + +from __future__ import annotations + +import importlib.util +from dataclasses import dataclass +from typing import Any + +import pytest + +from visionset.inference import MODULES, DownloadSizes, download_size, known_sizes, measure +from visionset.inference import weights as weights_module +from visionset.kernel.errors import LocalInferenceUnavailable + +EXTRA_INSTALLED = all(importlib.util.find_spec(name) is not None for name in MODULES) + + +@dataclass(frozen=True) +class FakeSibling: + """One row of a repository's file listing, shaped like the hub client's.""" + + rfilename: str + size: int | None + + +class FakeInfo: + def __init__(self, siblings: list[FakeSibling]) -> None: + self.siblings = siblings + + +class FakeHub: + """A hub client that lists files and refuses to be used for anything else. + + ``snapshot_download`` raises rather than being absent: a missing attribute + would fail the same test with an ``AttributeError`` that reads like a typo, + while this one names the rule that was broken. + """ + + def __init__(self, siblings: list[FakeSibling]) -> None: + self._siblings = siblings + self.calls = 0 + self.asked: list[tuple[str, str | None, bool]] = [] + + def model_info(self, repo_id: str, **kwargs: Any) -> FakeInfo: + self.calls += 1 + self.asked.append((repo_id, kwargs.get("revision"), bool(kwargs.get("files_metadata")))) + return FakeInfo(self._siblings) + + @staticmethod + def snapshot_download(**_: object) -> str: + raise AssertionError("reading a size must not download anything") + + +def hub_of(monkeypatch: pytest.MonkeyPatch, siblings: list[FakeSibling]) -> FakeHub: + fake = FakeHub(siblings) + monkeypatch.setattr(weights_module, "imported", lambda _name: fake) + return fake + + +A_LISTING = [ + FakeSibling("config.json", 1_024), + FakeSibling("model.safetensors", 300_000_000), + FakeSibling("README.md", 2_048), +] + + +def test_measuring_a_size_downloads_nothing(monkeypatch: pytest.MonkeyPatch) -> None: + """The property the whole surface exists for. + + A size read by fetching the files it is measuring would be a download + somebody never agreed to, wearing the name of the control that was supposed + to ask them. `FakeHub.snapshot_download` fails the test rather than returning, + so this reds the moment the implementation reaches for the download path. + """ + hub = hub_of(monkeypatch, A_LISTING) + measured = measure("some/model", "abc123") + assert measured.total_bytes == 300_003_072 + assert hub.calls == 1 + + +def test_the_lookup_asks_for_file_metadata_at_the_pinned_revision( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Sizes arrive only when they are asked for, and a size is per revision. + + Without ``files_metadata`` the listing comes back with every ``size`` null, + which this code would then correctly refuse — a green suite reporting a + refusal nobody meant. + """ + hub = hub_of(monkeypatch, A_LISTING) + measure("some/model", "v1.2") + assert hub.asked == [("some/model", "v1.2", True)] + + +def test_every_file_counts_because_the_download_fetches_every_file( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Not the weights alone. + + ``download`` takes a snapshot with no patterns, so a repository publishing + both serialisations of the same tensors really does cost both. A number + counting one of them would understate what lands on the disk. + """ + hub_of( + monkeypatch, + [ + FakeSibling("model.safetensors", 100), + FakeSibling("pytorch_model.bin", 100), + FakeSibling("tokenizer.json", 7), + ], + ) + measured = measure("some/model", "abc123") + assert measured.total_bytes == 207 + assert measured.file_count == 3 + + +def test_the_pair_is_echoed_back(monkeypatch: pytest.MonkeyPatch) -> None: + """A form that had to remember what it asked about would hold a second copy.""" + hub_of(monkeypatch, A_LISTING) + measured = measure("facebook/sam2-hiera-base-plus", "main") + assert measured.model_id == "facebook/sam2-hiera-base-plus" + assert measured.model_revision == "main" + + +def test_a_file_the_hub_did_not_size_is_refused_rather_than_skipped( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Skipping it would answer with a number smaller than the truth. + + Which is worse than no number, because the number's only job is to inform a + decision somebody is about to make. + """ + hub_of( + monkeypatch, + [FakeSibling("config.json", 1_024), FakeSibling("model.safetensors", None)], + ) + with pytest.raises(LocalInferenceUnavailable) as raised: + measure("some/model", "abc123") + assert "did not report a size for 1 of 2 files" in str(raised.value) + assert "model.safetensors" in str(raised.value) + + +def test_an_empty_listing_is_refused_rather_than_reported_as_zero( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """ "0 B" would invite somebody to confirm a download nothing is known about.""" + hub_of(monkeypatch, []) + with pytest.raises(LocalInferenceUnavailable) as raised: + measure("some/model", "abc123") + assert "listed no files" in str(raised.value) + + +def test_a_failed_lookup_arrives_in_the_kernels_vocabulary( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """``download``'s translation, one call earlier: no library exception escapes.""" + + class Broken: + @staticmethod + def model_info(*_: object, **__: object) -> FakeInfo: + raise ValueError("404 Client Error: Repository Not Found") + + monkeypatch.setattr(weights_module, "imported", lambda _name: Broken) + with pytest.raises(LocalInferenceUnavailable) as raised: + measure("some/model", "abc123") + assert "could not read the size of some/model at abc123" in str(raised.value) + assert "Repository Not Found" in str(raised.value) + + +@pytest.mark.skipif(EXTRA_INSTALLED, reason="the local runtime is installed here") +def test_a_missing_hub_client_names_the_install_command() -> None: + """Unstubbed: the size is read with the client that would do the fetching, so + a machine without the extra is refused here too — with the remedy.""" + with pytest.raises(LocalInferenceUnavailable) as raised: + measure("some/model", "abc123") + assert 'pip install "visionset[local-inference]"' in str(raised.value) + + +def test_a_size_is_read_once_per_revision(monkeypatch: pytest.MonkeyPatch) -> None: + """A pinned revision is a fixed set of files, so the answer cannot go stale. + + ``lookups`` is what separates a working cache from one that asks every time: + both answer correctly, and only the counter tells them apart. + """ + hub = hub_of(monkeypatch, A_LISTING) + sizes = DownloadSizes() + first = sizes.get("some/model", "abc123") + second = sizes.get("some/model", "abc123") + assert first == second + assert sizes.lookups == 1 + assert hub.calls == 1 + + +def test_two_revisions_of_one_model_are_two_answers(monkeypatch: pytest.MonkeyPatch) -> None: + """The key is the pair. A cache keyed on the model id alone would report one + revision's size under another's name.""" + hub = hub_of(monkeypatch, A_LISTING) + sizes = DownloadSizes() + sizes.get("some/model", "v1") + sizes.get("some/model", "v2") + assert sizes.lookups == 2 + assert hub.calls == 2 + assert len(sizes) == 2 + + +def test_a_refusal_is_not_cached_as_an_answer(monkeypatch: pytest.MonkeyPatch) -> None: + """A revision that could not be read is asked again next time. + + Caching the failure would make one bad moment on the network permanent for + the life of the process, with no way for anybody to retry. + """ + hub_of(monkeypatch, []) + sizes = DownloadSizes() + for _ in range(2): + with pytest.raises(LocalInferenceUnavailable): + sizes.get("some/model", "abc123") + assert len(sizes) == 0 + assert sizes.lookups == 0 + + +def test_the_cache_is_bounded(monkeypatch: pytest.MonkeyPatch) -> None: + """It holds a working set, not a log of everything anybody ever typed.""" + hub_of(monkeypatch, A_LISTING) + sizes = DownloadSizes(capacity=2) + for revision in ("v1", "v2", "v3"): + sizes.get("some/model", revision) + assert len(sizes) == 2 + + +def test_the_process_wide_cache_is_one_object() -> None: + """``known_sizes`` is a function so that importing this module does not read + as taking a handle on shared state.""" + assert known_sizes() is known_sizes() + + +def test_the_module_level_entry_point_goes_through_the_cache( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """``download_size`` is what surfaces call, and it must not bypass the cache + the surfaces are the reason for.""" + hub = hub_of(monkeypatch, A_LISTING) + known_sizes().clear() + try: + download_size("some/model", "cached-once") + download_size("some/model", "cached-once") + assert hub.calls == 1 + finally: + known_sizes().clear() diff --git a/tests/server/test_inference.py b/tests/server/test_inference.py index 21bd4b73..17a1b1ff 100644 --- a/tests/server/test_inference.py +++ b/tests/server/test_inference.py @@ -407,3 +407,114 @@ def test_a_refused_download_creates_no_job(client: TestClient, runtime_present: assert client.post(f"/inference/connections/{made['id']}/download").status_code == 409 assert client.get("/background-jobs").json()["total"] == 0 assert UUID(made["id"]) + + +# --- the download size, read before anybody agrees to a download --------------- + + +@pytest.fixture() +def listing(monkeypatch: pytest.MonkeyPatch) -> list[tuple[str, str | None]]: + """A hub that lists files and refuses to fetch any. + + Patched at ``visionset.inference.weights.imported`` — the one door the + optional runtime arrives through — so everything above it is the shipped + code: the counting, the refusals, and the cache. + """ + asked: list[tuple[str, str | None]] = [] + + class Sibling: + def __init__(self, rfilename: str, size: int) -> None: + self.rfilename = rfilename + self.size = size + + class Info: + siblings = [Sibling("config.json", 1_024), Sibling("model.safetensors", 300_000_000)] + + class Hub: + @staticmethod + def model_info(repo_id: str, **kwargs: Any) -> type[Info]: + asked.append((repo_id, kwargs.get("revision"))) + return Info + + @staticmethod + def snapshot_download(**_: object) -> str: + raise AssertionError("reading a size must not download anything") + + monkeypatch.setattr(weights_module, "imported", lambda _name: Hub) + weights_module.known_sizes().clear() + return asked + + +def test_the_download_size_is_answered_from_the_listing( + client: TestClient, listing: list[tuple[str, str | None]] +) -> None: + """The number D1 requires on screen before somebody confirms a download. + + `Hub.snapshot_download` raises, so this reds if the route ever answers by + fetching what it is measuring. + """ + response = client.get( + "/inference/download-size", + params={"model_id": "facebook/sam2-hiera-base-plus", "model_revision": "main"}, + ) + assert response.status_code == 200, response.text + assert response.json() == { + "model_id": "facebook/sam2-hiera-base-plus", + "model_revision": "main", + "total_bytes": 300_001_024, + "file_count": 2, + } + assert listing == [("facebook/sam2-hiera-base-plus", "main")] + + +def test_the_size_route_names_no_connection( + client: TestClient, listing: list[tuple[str, str | None]] +) -> None: + """It is asked while a form is being filled in, so there is no row yet. + + A workspace with no connections at all answers it, which is the state every + first-time setup is in. + """ + assert client.get("/inference/connections").json()["total"] == 0 + response = client.get( + "/inference/download-size", + params={"model_id": "some/model", "model_revision": "abc123"}, + ) + assert response.status_code == 200, response.text + + +def test_a_size_asked_for_twice_is_read_once( + client: TestClient, listing: list[tuple[str, str | None]] +) -> None: + """A pinned revision is a fixed set of files, so the answer cannot go stale.""" + for _ in range(2): + client.get( + "/inference/download-size", + params={"model_id": "some/model", "model_revision": "abc123"}, + ) + assert listing == [("some/model", "abc123")] + + +def test_the_size_route_wants_both_halves_of_the_pair(client: TestClient) -> None: + """A size is a fact about one revision, so the revision is not optional.""" + response = client.get("/inference/download-size", params={"model_id": "some/model"}) + assert response.status_code == 422, response.text + + +@pytest.mark.skipif(_extra_is_installed(), reason="the local runtime is installed here") +def test_a_size_without_the_runtime_carries_the_install_command(client: TestClient) -> None: + """Unstubbed, and the same refusal the download gives. + + The size is read with the client that would do the fetching, so a machine + without the extra cannot answer — and says what to install rather than + failing opaquely (design principle 9). + """ + weights_module.known_sizes().clear() + response = client.get( + "/inference/download-size", + params={"model_id": "some/model", "model_revision": "abc123"}, + ) + assert response.status_code == 500, response.text + body = response.json() + assert body["code"] == "LOCAL_INFERENCE_UNAVAILABLE" + assert 'pip install "visionset[local-inference]"' in body["message"]