diff --git a/docs/api.md b/docs/api.md index 9730e4e0..e032f095 100644 --- a/docs/api.md +++ b/docs/api.md @@ -208,6 +208,24 @@ set. `tests/architecture/test_capability_reachability.py` is what now measures i of `BatchAction` is resolved against the published paths and the MCP tool listing, computed from the enum rather than from a list somebody maintains. +**What a resource *is* is a second declaration, and not the same question.** `ConnectionOut` +carries `capabilities` beside its `allowed_actions`: an action is something you may do **to** the +connection and is decided by its state, a capability is what its model **answers** and is decided +by the weights. Offering a tool needs both — a connection being `ready` says its files are here, +not that they are the right kind of model — and either list may be empty without the other being. + +``` +GET /inference/connections/{id} → { "setup_state": "ready", + "allowed_actions": ["download_weights", …], + "capabilities": ["point_suggest"], … } +``` + +An empty `capabilities` is not a refusal to act on: the server judges every request on its own +either way. It says only that nothing can yet rely on this connection for a particular tool — +because its weights never arrived, because its config declared a model type this build has no +adapter for, or because it is an `http` connection, which declares nothing until the remote +contract says how an endpoint states what it can do. + **A client renders these; it never computes them.** Re-deriving the rules from `state` and `progress` is what the browser used to do, and its copy drifted by dropping the batch-state dimension — which is why skipping a frame was offered on a batch the kernel refused every write diff --git a/docs/architecture/backend/inference.md b/docs/architecture/backend/inference.md index 8300ce61..bf97cf6d 100644 --- a/docs/architecture/backend/inference.md +++ b/docs/architecture/backend/inference.md @@ -48,6 +48,36 @@ A family this build does not serve is refused rather than guessed at — a fallb answers in the wrong adapter's vocabulary, which is a confident sentence about a model the user does not have. +## One fact, two readings, one module + +`families.py` holds the family sets **and** the map from a family to what a +connection may be asked for, because they are the same fact read twice: which +adapter can run this model, and which prompts a caller may send it. The map is +*derived* from the sets rather than listed beside them, so an adapter and its +declaration are one edit — a family added to `SEGMENTER_FAMILIES` and forgotten in +a hand-written map would run fine and declare nothing, and every client that +filters on the declaration would stop offering it. + +The vocabulary itself is the kernel's (`ModelCapability`) and the mapping is not: +what a tool can ask for is a domain word, while which `model_type` values this +build serves is a fact about an optional runtime that the kernel has no view of. + +## The family is recorded, not only resolved + +Resolution used to happen on every provider build and be thrown away. A connection +now stores what its config declared, written when the download finishes — the +first moment the answer exists without reaching a network, and the reason nothing +is read at connection *creation*. + +**The backfill for older rows is on the read path, and that is a layering fact.** +A migration would be the natural home and cannot be one: migrations run inside the +kernel, and the kernel may not import this package or address the model cache the +answer lives in. So `with_families` fills a row in on the first read of it, from +files already on the disk, once — a row that has an answer is never asked again, +including when the answer is "the config declared nothing". A build without the +optional runtime records nothing rather than recording that it found nothing, +because a build that cannot look has not looked. + ## Importing this package imports nothing heavy Every reference to torch, transformers, accelerate and huggingface_hub is inside a diff --git a/docs/architecture/backend/wire.md b/docs/architecture/backend/wire.md index 22dd1e17..9e70bbaf 100644 --- a/docs/architecture/backend/wire.md +++ b/docs/architecture/backend/wire.md @@ -52,6 +52,13 @@ Leaf encoding is explicit throughout: UUIDs as strings, enums as `.value`, paths as strings, timestamps in pydantic's format so the parity gate compares like with like. +Almost everything a projection reads is a domain model. The one exception is +`capabilities_of` from `visionset.inference`, and it is here rather than spelled +out because which model families this build serves is a fact this package has no +way to know — a second copy of that map would be exactly the drift every other +rule in this file prevents. The direction is the usual one: a sibling below the +surfaces, importing nothing from here. + ## Where it sits The `Kernel purity` contract forbids `visionset.kernel` importing diff --git a/docs/inference.md b/docs/inference.md index 5bde2dcd..b6f1f56e 100644 --- a/docs/inference.md +++ b/docs/inference.md @@ -213,10 +213,44 @@ on a machine that has one; `precision` is `fp16` or `fp32`, and `float16`, `half members. What this closes is a gap rather than a freedom: `gpu` used to be accepted and then resolved onto the CPU, so the connection described a run that never happened. +## What a connection can be asked for + +A connection row says where a model runs and whether its weights are here. Neither answers the +question a caller has to settle *before* asking: does this model take the kind of prompt I am +about to send? So a connection also declares what it can be asked for. + +```json +{ "setup_state": "ready", "capabilities": ["point_suggest"], … } +``` + +| Capability | Means | Families | +| --- | --- | --- | +| `point_suggest` | Give me the thing under these points | the SAM 2 family | +| `text_detect` | Find everything these words name | the grounding-dino family | + +**Read from the model, never from its name.** The value comes from the `model_type` the +downloaded config declares — the same fact that decides which adapter runs it, so a model that +runs and a model that declares are the same list. Matching on a model id would answer confidently +for every model this build has never heard of, and the wrongness would only surface as a refusal +deep in a request. + +**Empty means nothing is known yet**, which happens four ways: the weights were never fetched, so +nothing has read a config; the config declared no model type; it declared one this build has no +adapter for; or it is an `http` connection, whose model runs elsewhere and which declares nothing +until the remote contract says how an endpoint states what it can do. Empty is not a refusal — +the server still judges every request on its own. It says only that no tool can rely on this +connection. + +**It is recorded when the weights arrive**, because that is the first moment it is knowable +without reaching a network. Editing a connection to point at another model or revision clears it +again: nothing has read the new one, and a stale answer reads exactly like a fresh one. A +connection created before this shipped acquires its answer the first time something reads it, +from files already on your disk. + ## Suggesting a shape from a click -A connection whose model answers *places* rather than *words* can propose a shape for whatever -sits under a point. One call, one asset, one set of points: +A connection whose model answers *places* rather than *words* — one declaring `point_suggest` — +can propose a shape for whatever sits under a point. One call, one asset, one set of points: ```http POST /inference/suggest @@ -278,6 +312,10 @@ problem. A connection whose weights are not here yet is `INFERENCE_CONNECTION_NO names `download` as the remedy; one whose model answers words rather than places is `UNSUPPORTED_PROMPT`. +That last one is still the law and is still enforced on every call. It is simply no longer how a +person finds out: a client with `capabilities` in hand can decline to ask, which is why the +editor now says so once on the panel instead of collecting one refusal per click. + ## What a connection is not It is **not a credential store**, yet. An HTTP connection carries no secret today, and the field @@ -356,6 +394,11 @@ about that flow forces you out of the editor or loses work: the panel is an expl door, and the door is optional — a host that wires no destination gets the explanation and no control. +*No usable connection* covers one case more than it reads: a workspace can hold a connection that +is configured, downloaded and running, and still have nothing that can answer a click, because +the model it holds answers words. The panel says which of the two it is — nothing set up, nothing +downloaded, or nothing of the right kind — and each names a different thing to do. + ## At a terminal ```bash diff --git a/docs/persistence.md b/docs/persistence.md index 660c9d5e..b6362853 100644 --- a/docs/persistence.md +++ b/docs/persistence.md @@ -155,8 +155,10 @@ MIGRATIONS: list[Migration] = [ Migration(version=3, name="annotation_provenance", upgrade=_add_annotation_provenance), Migration(version=4, name="job_queue", upgrade=_add_job_queue), Migration(version=5, name="schema_provenance", upgrade=_add_schema_provenance), + Migration(version=6, name="inference_connections", upgrade=_add_inference_connections), + Migration(version=7, name="model_family", upgrade=_add_model_family), ] -FORMAT_VERSION: int = MIGRATIONS[-1].version # 5 +FORMAT_VERSION: int = MIGRATIONS[-1].version # 7 ``` **Generation 1 is the baseline, and everything after it is an ordinary migration.** A long @@ -172,9 +174,16 @@ force again for every entry appended after the baseline. `tests/kernel/test_migrations.py` that builds an old-looking file. The failure is the silent kind: a column left in place makes its own migration find the column already there and return early, so `test_a_fresh_database_and_a_migrated_one_have_the_same_schema` compares a file -against itself and passes while proving nothing. Migration 4 is the standing exception — it -creates a *table*, and dropping that in the helper would exercise SQLite rather than this -module. +against itself and passes while proving nothing. The table-creating migrations — 4 and 6 — are +the standing exception: dropping a whole table in the helper would exercise SQLite rather than +this module. + +**A migration cannot always backfill what it adds, and saying which is which is part of adding +one.** Migration 3 could attribute an annotation because the file already recorded enough to +answer it; migration 7 cannot fill in a connection's model family at all, because that answer +lives in a model cache the kernel is forbidden to reach. Where the value is unknowable here, the +column arrives NULL and something outside the kernel fills it in later — and the column's own +docstring says which, so a reader does not mistake an honest absence for a forgotten step. **There are no downgrade paths, deliberately.** Nothing walks a file backwards and the tests no longer do either. A downgrade is a compatibility promise and a promise is owed diff --git a/docs/ui.md b/docs/ui.md index 51c9aa92..27405024 100644 --- a/docs/ui.md +++ b/docs/ui.md @@ -418,6 +418,22 @@ want and a segmentation model proposes its shape. It runs through a model connection (`docs/inference.md`), and the server side of it is `POST /inference/suggest`. +**It runs through a connection that can answer a click**, which is a narrower set +than "the ones that are ready": only those declaring `point_suggest`. A workspace +whose only downloaded model answers text prompts gets a panel saying so, and no +request is sent — the server would refuse each one truthfully, which is a correct +answer to a question the editor should not have asked. The panel tells that case +apart from having nothing configured and from having nothing downloaded, because +each is a different thing to go and do. + +Where more than one connection can answer, the panel carries a picker naming the +model under each, and the choice is remembered **per project** — it is a +preference about this browser, so it survives leaving the editor and does not +become a workspace setting that everybody annotating shares. With one candidate +there is no control at all, only a line naming what is answering. The picker +appears on the idle card alone: changing which model answers while a proposal is +on screen would leave a shape nothing on the card explains. + The gesture: | Press | What it does | diff --git a/frontend/ui-core/src/annotator/AnnotationPage.tsx b/frontend/ui-core/src/annotator/AnnotationPage.tsx index bd4c0291..5092e075 100644 --- a/frontend/ui-core/src/annotator/AnnotationPage.tsx +++ b/frontend/ui-core/src/annotator/AnnotationPage.tsx @@ -199,6 +199,19 @@ import { AddClassDialog, runAddClass } from "./AddClassDialog"; import { FrameGallery } from "./FrameGallery"; import { SuggestPanel } from "./SuggestPanel"; import { useConnections, useSuggestRegion, usableConnection } from "../data/inferenceQueries"; +import { readPref, writePref } from "../data/prefs"; + +/** + * Where a project's suggest-through choice is remembered. + * + * Keyed by project rather than globally: two projects can hold different + * schemas and different work, and the model that suits one is not the model that + * suits the other. Keyed by project rather than by *job* for the opposite + * reason — nobody wants to re-pick a model per batch. + */ +function preferredConnectionKey(projectId: string): string { + return `suggest.connection.${projectId}`; +} import { PROGRESS_LABEL, outstandingWork, progressDotClass, progressTone } from "../screens/batchState"; import type { LabelClassBody, SchemaDiff, SchemaVersion } from "../screens/queries"; import { @@ -799,9 +812,34 @@ function Workspace({ * than leaving a click to vanish into it. */ const connections = useConnections(session !== null); - const { connection, blocker } = usableConnection(connections.data?.items); + /** + * Which model this project suggests through, when there is more than one. + * + * **A preference, per project, and never a constraint.** It survives leaving + * the editor and coming back — which is the whole point of remembering it — + * and `usableConnection` falls back to the first candidate whenever the + * remembered one is gone, renamed away from capability, or not downloaded any + * more. So a deleted connection cannot leave a project unable to suggest. + * + * `readPref`/`writePref` rather than server state: it is a view preference in + * `prefs.ts`'s own sense — a choice about this browser, not a fact about the + * workspace, and one that must not turn into a write every annotator on a + * shared workspace fights over. + */ + const [preferredConnection, setPreferredConnection] = useState(() => + readPref(preferredConnectionKey(projectId)), + ); + const { connection, candidates, blocker } = usableConnection( + connections.data?.items, + preferredConnection, + ); const suggestRegion = useSuggestRegion(); + function chooseConnection(connectionId: string): void { + setPreferredConnection(connectionId); + writePref(preferredConnectionKey(projectId), connectionId); + } + /** * Arming and disarming — and arming activates a class, exactly as every other * button on the strip does. @@ -2430,6 +2468,9 @@ function Workspace({ heldClass={activeClass} blocker={blocker} refusal={suggesting.refusal} + candidates={candidates} + connectionId={connection?.id ?? null} + onChooseConnection={chooseConnection} onAccept={acceptSuggestion} onDiscard={discardSuggestion} {...(onConfigureInference === undefined diff --git a/frontend/ui-core/src/annotator/SuggestPanel.tsx b/frontend/ui-core/src/annotator/SuggestPanel.tsx index fa90c1dd..9e362707 100644 --- a/frontend/ui-core/src/annotator/SuggestPanel.tsx +++ b/frontend/ui-core/src/annotator/SuggestPanel.tsx @@ -30,6 +30,14 @@ * because it is, and because a person who has just been told a capability does * not apply will otherwise assume they have to turn it back on. * + * ## And one choice, in the one state where it is safe to make + * + * Where a workspace has more than one model that can answer a click, the idle + * card carries the picker for it — never the asking or the accept card, because + * changing which model answers while an answer is on screen would leave a + * proposal nothing on the card explains. With a single candidate there is no + * control at all, only the line naming it. + * * Principle 9, *never disable without explanation*, is what makes the refusal * cases carry a remedy rather than a state: "not configured" names the thing to * make, "not ready" names the download, and a server refusal is quoted **as the @@ -57,7 +65,14 @@ import type { JSX, ReactNode } from "react"; import { EditorNotice } from "./EditorNotice"; import { Button } from "../primitives/Button"; -import type { SuggestBlocker } from "../data/inferenceQueries"; +import { + Select, + SelectContent, + SelectItem, + SelectTrigger, + SelectValue, +} from "../primitives/Select"; +import type { Connection, SuggestBlocker } from "../data/inferenceQueries"; export interface SuggestPanelProps { /** The session, whose status decides which sentence this card carries. */ @@ -85,6 +100,19 @@ export interface SuggestPanelProps { * throw that away. `refusalProse` is what turns the rest into prose. */ readonly refusal: string | null; + /** + * Every connection a click *could* go through, in the list's own order. + * + * One is the common case and renders as a line naming it rather than as a + * control: a picker over a single option is a decision nobody has. Two or more + * render a picker, because at that point which model answers is a choice, and + * one made per project rather than per click. + */ + readonly candidates?: readonly Connection[]; + /** Which of them a click goes through now. */ + readonly connectionId?: string | null; + /** Absent leaves the choice unrendered, on `onConfigure`'s rule. */ + readonly onChooseConnection?: (connectionId: string) => void; /** Where a person goes to make or finish a connection, if the host has one. */ readonly onConfigure?: () => void; readonly onAccept: () => void; @@ -92,10 +120,10 @@ export interface SuggestPanelProps { } /** - * What the two blockers say, and what each one's action means. + * What each blocker says, and what its action means. * - * A record rather than a ternary chain so the pair is readable as a table and a - * third blocker cannot be added to the copy without an entry — the same reason + * A record rather than a ternary chain so the set is readable as a table and a + * further blocker cannot be added to the union without an entry — the same reason * `ToolPalette`'s `TOOL_LABELS` is total over what `drawableGeometry` answers. */ const BLOCKER_COPY: Readonly< @@ -130,6 +158,15 @@ const BLOCKER_COPY: Readonly< action: "Finish setting it up", tone: "warn", }, + // Ranked *below* `not-ready` by `usableConnection`, and the copy relies on it: + // this sentence is only ever read where something is downloaded and running, + // so "the model you have" is a model that is genuinely here. + "not-capable": { + title: "That model answers a different question", + body: "Suggesting a shape needs a model that takes points — the SAM 2 family. The connections that are ready here answer something else, so a click would come back refused.", + action: "Set up a connection", + tone: "warn", + }, }; export function SuggestPanel({ @@ -137,6 +174,9 @@ export function SuggestPanel({ heldClass, blocker, refusal, + candidates = [], + connectionId = null, + onChooseConnection, onConfigure, onAccept, onDiscard, @@ -297,11 +337,71 @@ export function SuggestPanel({ One click proposes a shape for “{session.labelClass}”. Alt-click marks something that is not part of it.

+ {/* + Here and in no other reading. This is the state where nothing is in + flight and nothing is waiting to be accepted, so it is the only one where + changing which model answers cannot pull the ground out from under + something already on screen. + */} + {hasPending(session) && } ); } +/** + * Which connection a click goes through: a line, or a picker where there is a choice. + * + * **One candidate renders no control**, which is the common case and the whole + * of the friction argument: a select over a single option is a decision nobody + * has, and it would sit in the editor asking to be read on every job. What stays + * is the sentence naming the model, because a person who is about to spend + * clicks on a suggestion should be able to see what is answering them. + * + * A host with no `onChoose` gets the sentence too, on the panel's standing rule: + * an explanation with no control beats a control that does nothing. + */ +function Through({ + candidates, + connectionId, + onChoose, +}: { + readonly candidates: readonly Connection[]; + readonly connectionId: string | null; + readonly onChoose?: (connectionId: string) => void; +}): JSX.Element | null { + const active = candidates.find((one) => one.id === connectionId) ?? candidates[0]; + if (active === undefined) return null; + if (candidates.length === 1 || onChoose === undefined) { + return ( +

+ Through “{active.name}” +

+ ); + } + return ( + + ); +} + /** The take-back, where a state has something to take back and nothing to accept. */ function Discard({ onDiscard }: { readonly onDiscard: () => void }): JSX.Element { return ( diff --git a/frontend/ui-core/src/annotator/suggestFlow.test.tsx b/frontend/ui-core/src/annotator/suggestFlow.test.tsx index 00974f65..ed3bd3d3 100644 --- a/frontend/ui-core/src/annotator/suggestFlow.test.tsx +++ b/frontend/ui-core/src/annotator/suggestFlow.test.tsx @@ -24,6 +24,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import type { JSX, ReactNode } from "react"; import { ApiProvider } from "../data/ApiProvider"; +import { clearPrefs, writePref } from "../data/prefs"; import { writeToken } from "../data/session"; import { AnnotationPage } from "./AnnotationPage"; import { TooltipProvider } from "../primitives/Menu"; @@ -37,6 +38,8 @@ const ASSET = "44444444-4444-4444-8444-444444444444"; /** The second frame, so "switching assets discards" has somewhere to switch to. */ const ASSET_TWO = "55555555-5555-4555-8555-555555555555"; const CONNECTION = "66666666-6666-4666-8666-666666666666"; +/** A second capable connection, so "which one" is a question worth asking. */ +const OTHER_CONNECTION = "88888888-8888-4888-8888-888888888888"; const MODEL_REF = "facebook/sam2-hiera-base-plus@main"; const SCHEMA = { @@ -70,7 +73,10 @@ let connections: readonly Record[] = []; let suggestion: Record | null = null; let suggestRefusal: { status: number; code: string; message: string } | null = null; -function connectionRow(setup: "ready" | "not_set_up"): Record { +function connectionRow( + setup: "ready" | "not_set_up", + overrides: Record = {}, +): Record { return { id: CONNECTION, name: "local sam", @@ -82,11 +88,35 @@ function connectionRow(setup: "ready" | "not_set_up"): Record { endpoint_url: null, setup_state: setup, allowed_actions: [], + // What the server resolved from this model's own config. A row that has + // never been downloaded declares nothing, which is why the default only + // makes sense beside `setup`. + capabilities: setup === "ready" ? ["point_suggest"] : [], created_at: "2026-08-08T00:00:00Z", updated_at: "2026-08-08T00:00:00Z", + ...overrides, }; } +/** A second connection this tool could equally go through. */ +function theOtherSam(): Record { + return connectionRow("ready", { + id: OTHER_CONNECTION, + name: "the big one", + model_id: "facebook/sam2-hiera-large", + }); +} + +/** The workspace as it was observed: one ready connection, and it answers words. */ +function aDetector(): Record { + return connectionRow("ready", { + id: "77777777-7777-4777-8777-777777777777", + name: "grounding dino", + model_id: "IDEA-Research/grounding-dino-tiny", + capabilities: ["text_detect"], + }); +} + function assetRow(id: string, hash: string): Record { return { id, @@ -150,6 +180,9 @@ function answer(path: string): unknown { beforeEach(() => { sent.length = 0; + // The remembered model choice is a preference like any other, and a test + // that inherited the last one's would pass alone and fail in a suite. + clearPrefs(); connections = [connectionRow("ready")]; suggestion = { model_ref: MODEL_REF, @@ -554,6 +587,75 @@ describe("when there is nothing to suggest through (D6)", () => { await screen.findByTestId("suggest-not-ready"); }); + it("says why it cannot run over a workspace whose model answers words", async () => { + // The reproduction, exactly as observed: `grounding-dino-tiny` is the only + // ready connection. Every click used to round-trip and come back with a + // truthful `UnsupportedPrompt` refusal — the tool was offered where it could + // never work, and the server said so one click at a time. + connections = [aDetector()]; + await open(); + await arm(); + await screen.findByTestId("suggest-not-capable"); + }); + + it("sends no request at all over one, which is the half the server cannot fix", async () => { + connections = [aDetector()]; + await open(); + await arm(); + await screen.findByTestId("suggest-not-capable"); + clickCanvas(); + + expect(asks()).toHaveLength(0); + }); + + it("says the weights are missing before it says the model is the wrong kind", async () => { + // An undownloaded connection has no capability *yet* — nothing has read its + // config. Ranking capability first would tell somebody their SAM connection + // answers the wrong question when the truth is that it has not arrived. + connections = [connectionRow("not_set_up")]; + await open(); + await arm(); + await screen.findByTestId("suggest-not-ready"); + expect(screen.queryByTestId("suggest-not-capable")).toBeNull(); + }); + + it("suggests through the capable connection when a workspace holds both kinds", async () => { + connections = [aDetector(), connectionRow("ready")]; + await open(); + await arm(); + clickCanvas(); + + await waitFor(() => expect(asks()).toHaveLength(1)); + expect(asks()[0]?.["connection_id"]).toBe(CONNECTION); + }); + + it("remembers, per project, which of several models a click goes through", async () => { + // Seeded before the page mounts, which is the claim: the choice survives + // leaving the editor and coming back, because it is read at mount rather + // than held in the session. + connections = [connectionRow("ready"), theOtherSam()]; + writePref(`suggest.connection.${PROJECT}`, OTHER_CONNECTION); + await open(); + await arm(); + clickCanvas(); + + await waitFor(() => expect(asks()).toHaveLength(1)); + expect(asks()[0]?.["connection_id"]).toBe(OTHER_CONNECTION); + }); + + it("does not carry one project's choice into another", async () => { + // The key names the project, so a workspace whose two projects want + // different models does not have them fighting over one setting. + connections = [connectionRow("ready"), theOtherSam()]; + writePref("suggest.connection.99999999-9999-4999-8999-999999999999", OTHER_CONNECTION); + await open(); + await arm(); + clickCanvas(); + + await waitFor(() => expect(asks()).toHaveLength(1)); + expect(asks()[0]?.["connection_id"]).toBe(CONNECTION); + }); + it("sends nothing while the tool is blocked", async () => { connections = []; await open(); diff --git a/frontend/ui-core/src/annotator/suggestPanel.test.tsx b/frontend/ui-core/src/annotator/suggestPanel.test.tsx index b4da98b5..c7dd0f89 100644 --- a/frontend/ui-core/src/annotator/suggestPanel.test.tsx +++ b/frontend/ui-core/src/annotator/suggestPanel.test.tsx @@ -49,8 +49,11 @@ function mount(overrides: Partial[0]> = {}): JSX ); } -/** A connection row, in whichever setup state a case needs. */ -function connection(setup: Connection["setup_state"]): Connection { +/** A connection row, in whichever setup state and capability a case needs. */ +function connection( + setup: Connection["setup_state"], + overrides: Partial = {}, +): Connection { return { id: "c1", name: "local sam", @@ -62,34 +65,89 @@ function connection(setup: Connection["setup_state"]): Connection { endpoint_url: null, setup_state: setup, allowed_actions: [], + // Resolved by the server from the model's own config, and empty until + // something has read one — which is why a row that never downloaded + // declares nothing. + capabilities: setup === "ready" ? ["point_suggest"] : [], created_at: "2026-08-08T00:00:00Z", updated_at: "2026-08-08T00:00:00Z", + ...overrides, } as Connection; } +/** The one the reproduction had: ready, and it answers words rather than points. */ +function aDetector(id = "d1"): Connection { + return connection("ready", { + id, + name: "grounding dino", + model_id: "IDEA-Research/grounding-dino-tiny", + capabilities: ["text_detect"], + }); +} + describe("which connection a click goes through", () => { it("is none, and says why, when the workspace has configured none", () => { - expect(usableConnection([])).toEqual({ connection: null, blocker: "no-connections" }); + expect(usableConnection([])).toEqual({ + connection: null, + candidates: [], + blocker: "no-connections", + }); }); it("is none, and says why, when one exists but its weights are not here", () => { expect(usableConnection([connection("not_set_up")])).toEqual({ connection: null, + candidates: [], blocker: "not-ready", }); }); - it("is the first ready one, in the list's own order", () => { - const ready = connection("ready"); + it("is the first capable one, in the list's own order", () => { + const ready = connection("ready", { id: "c2" }); const answer = usableConnection([connection("not_set_up"), ready, connection("ready")]); expect(answer.connection).toBe(ready); expect(answer.blocker).toBe(null); }); + it("never picks a ready connection whose model answers a different question", () => { + // The bug this whole slice exists for. `find(setup_state === "ready")` sent + // every point-prompt click to whatever was ready, and a workspace holding + // only a text-prompt detector got a truthful refusal per click. + const answer = usableConnection([aDetector()]); + expect(answer.connection).toBe(null); + expect(answer.blocker).toBe("not-capable"); + }); + + it("looks past one to a connection that can answer", () => { + const sam = connection("ready", { id: "c2" }); + const answer = usableConnection([aDetector(), sam]); + expect(answer.connection).toBe(sam); + expect(answer.candidates).toEqual([sam]); + }); + + it("says the weights are missing before it says the model is the wrong kind", () => { + // An undownloaded connection has no capability *yet*, so ranking capability + // first would answer "wrong kind of model" where the truth is "not here". + expect(usableConnection([connection("not_set_up")]).blocker).toBe("not-ready"); + }); + + it("honours a remembered choice, and falls back rather than blocking on a stale one", () => { + const first = connection("ready", { id: "c1" }); + const second = connection("ready", { id: "c2", name: "the other sam" }); + + expect(usableConnection([first, second], "c2").connection).toBe(second); + // A connection somebody deleted must not leave a project unable to suggest. + expect(usableConnection([first, second], "gone").connection).toBe(first); + }); + it("names the loading window rather than pretending it is a working tool", () => { // The list is only fetched once the tool is armed, so this window is real — // and a click landing in it must be told something rather than vanishing. - expect(usableConnection(undefined)).toEqual({ connection: null, blocker: "checking" }); + expect(usableConnection(undefined)).toEqual({ + connection: null, + candidates: [], + blocker: "checking", + }); }); }); @@ -244,3 +302,82 @@ describe("a refusal", () => { expect(screen.queryByTestId("suggest-accept")).toBeNull(); }); }); + +describe("the wrong-kind panel", () => { + it("says what the model answers instead, and offers the way out", async () => { + const onConfigure = vi.fn(); + render(mount({ blocker: "not-capable", onConfigure })); + + const card = screen.getByTestId("suggest-not-capable"); + expect(card.textContent).toContain("different question"); + expect(screen.getByTestId("suggest-panel").getAttribute("data-tone")).toBe("warn"); + await userEvent.click(screen.getByTestId("suggest-configure")); + expect(onConfigure).toHaveBeenCalledTimes(1); + }); + + it("is a different sentence from having nothing configured or nothing downloaded", () => { + render(mount({ blocker: "not-capable" })); + expect(screen.queryByTestId("suggest-no-connections")).toBeNull(); + expect(screen.queryByTestId("suggest-not-ready")).toBeNull(); + }); +}); + +describe("which connection a click goes through, on the card", () => { + const SAM = connection("ready", { id: "c1", name: "local sam" }); + const OTHER = connection("ready", { + id: "c2", + name: "the big one", + model_id: "facebook/sam2-hiera-large", + }); + + it("names the one there is, with no control to press", () => { + render(mount({ candidates: [SAM], connectionId: "c1", onChooseConnection: vi.fn() })); + + expect(screen.getByTestId("suggest-connection").textContent).toContain("local sam"); + // A picker over a single option is a decision nobody has, and it would sit + // in the editor asking to be read on every job. + expect(screen.queryByTestId("suggest-connection-select")).toBeNull(); + }); + + it("offers a picker once there is a choice, showing the model under the name", () => { + render(mount({ candidates: [SAM, OTHER], connectionId: "c2", onChooseConnection: vi.fn() })); + + const trigger = screen.getByTestId("suggest-connection-select"); + expect(trigger.textContent).toContain("the big one"); + expect(trigger.textContent).toContain("facebook/sam2-hiera-large"); + expect(screen.queryByTestId("suggest-connection")).toBeNull(); + }); + + it("names the active one rather than a dead control when the host cannot honour a choice", () => { + // `onConfigure`'s standing rule, applied to the second control this card + // grew: an explanation with no control beats a control that does nothing. + render(mount({ candidates: [SAM, OTHER], connectionId: "c2" })); + expect(screen.getByTestId("suggest-connection").textContent).toContain("the big one"); + expect(screen.queryByTestId("suggest-connection-select")).toBeNull(); + }); + + it("is absent while something is in flight or waiting to be accepted", () => { + // Changing which model answers while an answer is on screen would leave a + // proposal that nothing on the card explains. + for (const session of [asked(), shown()]) { + const view = render( + mount({ session, candidates: [SAM, OTHER], connectionId: "c1", onChooseConnection: vi.fn() }), + ); + expect(screen.queryByTestId("suggest-connection-select")).toBeNull(); + expect(screen.queryByTestId("suggest-connection")).toBeNull(); + view.unmount(); + } + }); + + it("is absent while the tool is blocked, which has nothing to choose between", () => { + render( + mount({ + blocker: "not-ready", + candidates: [SAM, OTHER], + connectionId: "c1", + onChooseConnection: vi.fn(), + }), + ); + expect(screen.queryByTestId("suggest-connection-select")).toBeNull(); + }); +}); diff --git a/frontend/ui-core/src/data/inferenceQueries.ts b/frontend/ui-core/src/data/inferenceQueries.ts index da7853a1..527d7ad5 100644 --- a/frontend/ui-core/src/data/inferenceQueries.ts +++ b/frontend/ui-core/src/data/inferenceQueries.ts @@ -359,24 +359,71 @@ export function useSuggestRegion() { * `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 + * must be told something rather than vanishing. Four states, one union, so the * panel's copy is total over them. */ -export type SuggestBlocker = "checking" | "no-connections" | "not-ready"; +export type SuggestBlocker = "checking" | "no-connections" | "not-ready" | "not-capable"; + +/** + * The capability a click needs, which is the whole of what this tool is. + * + * Read off `capabilities` rather than guessed from a model id or inferred from a + * setup state: the server resolves it from the model's own config, and a client + * that re-derived it would be guessing about weights it has never seen. + */ +export const SUGGEST_CAPABILITY = "point_suggest" as const; + +/** The connection a click goes through, the alternatives, and why there is none. */ +export interface UsableConnection { + /** Where to send a click, or `null` when there is nowhere to send one. */ + readonly connection: Connection | null; + /** + * Every connection this tool *could* go through, in the list's own order. + * + * What a chooser renders. One candidate is the common case and needs no + * control at all; the array is still returned so the caller decides that, + * rather than this function deciding it by returning `null`. + */ + readonly candidates: readonly Connection[]; + readonly blocker: SuggestBlocker | null; +} /** * 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. + * + * ## Ready is not enough, and that was a shipped bug + * + * This used to be `find(row => row.setup_state === "ready")`. A workspace whose + * one ready connection answers text prompts therefore sent every point-prompt + * click to it, and the server refused each one truthfully — a tool offered where + * it could never work, one refusal at a time. Being ready says the files are + * here; it says nothing about what kind of model they are. + * + * ## The order of the two refusals + * + * `not-ready` outranks `not-capable`, because an undownloaded connection has no + * capability *yet* — nothing has read its config. Asking about capability first + * would tell somebody their SAM connection is the wrong kind of model when the + * truth is that its weights have not arrived. + * + * `preferredId` is a preference and never a constraint: a remembered choice that + * is no longer a candidate falls back to the first one rather than blocking the + * tool over a connection somebody deleted. */ -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 }; +export function usableConnection( + connections: readonly Connection[] | undefined, + preferredId?: string | null, +): UsableConnection { + if (connections === undefined) return { connection: null, candidates: [], blocker: "checking" }; + if (connections.length === 0) + return { connection: null, candidates: [], blocker: "no-connections" }; + const ready = connections.filter((row) => row.setup_state === "ready"); + if (ready.length === 0) return { connection: null, candidates: [], blocker: "not-ready" }; + const candidates = ready.filter((row) => row.capabilities.includes(SUGGEST_CAPABILITY)); + if (candidates.length === 0) return { connection: null, candidates: [], blocker: "not-capable" }; + const preferred = candidates.find((row) => row.id === preferredId); + return { connection: preferred ?? candidates[0], candidates, blocker: null }; } diff --git a/frontend/ui-core/src/generated/api.ts b/frontend/ui-core/src/generated/api.ts index bca7584a..0484b946 100644 --- a/frontend/ui-core/src/generated/api.ts +++ b/frontend/ui-core/src/generated/api.ts @@ -704,6 +704,12 @@ export interface paths { /** * List Inference Connections * @description Every configured connection in this workspace, in the order they were made. + * + * A set-up connection that has never been asked what kind of model it holds is + * asked here, once, from files already on this disk — see + * ``visionset.inference.weights.with_families``. It is the backfill for rows + * written before a connection recorded that, and it is on the read path because + * the kernel cannot reach a model cache and a migration runs in the kernel. */ get: operations["list_inference_connections"]; put?: never; @@ -728,6 +734,9 @@ export interface paths { /** * Get Inference Connection * @description The connection with that id. + * + * Carries the same backfill the listing does, so that reading one connection + * and reading the list never disagree about what it can be asked for. */ get: operations["get_inference_connection"]; put?: never; @@ -2648,6 +2657,8 @@ export interface components { ConnectionOut: { /** Allowed Actions */ allowed_actions: components["schemas"]["ConnectionAction"][]; + /** Capabilities */ + capabilities: components["schemas"]["ModelCapability"][]; connection_type: components["schemas"]["ConnectionType"]; /** * Created At @@ -3095,6 +3106,12 @@ export interface components { /** Name */ name: string; }; + /** + * ModelCapability + * @description What a connection's model can be asked for: the kind of prompt it takes. + * @enum {string} + */ + ModelCapability: "point_suggest" | "text_detect"; /** * PolygonBody * @description A closed polygon of at least three points. The closing edge is implicit. diff --git a/frontend/ui-core/src/generated/checks.ts b/frontend/ui-core/src/generated/checks.ts index e670ac0b..084d5093 100644 --- a/frontend/ui-core/src/generated/checks.ts +++ b/frontend/ui-core/src/generated/checks.ts @@ -119,11 +119,14 @@ export const checkConnectionSetupState: Check = export const checkConnectionType: Check = /*#__PURE__*/ oneOf(["local", "http"] as const); +export const checkModelCapability: Check = + /*#__PURE__*/ oneOf(["point_suggest", "text_detect"] as const); + export const checkPrecision: Check = /*#__PURE__*/ oneOf(["fp16", "fp32"] as const); export const checkConnectionOut: Check = - /*#__PURE__*/ object({ "allowed_actions": [true, arrayOf(checkConnectionAction)], "connection_type": [true, checkConnectionType], "created_at": [true, isString], "device": [true, either([isString, isNull] as const)], "endpoint_url": [true, either([isString, isNull] as const)], "id": [true, isString], "model_id": [true, isString], "model_revision": [true, isString], "name": [true, isString], "precision": [true, either([checkPrecision, isNull] as const)], "setup_state": [true, checkConnectionSetupState], "updated_at": [true, isString] } as const); + /*#__PURE__*/ object({ "allowed_actions": [true, arrayOf(checkConnectionAction)], "capabilities": [true, arrayOf(checkModelCapability)], "connection_type": [true, checkConnectionType], "created_at": [true, isString], "device": [true, either([isString, isNull] as const)], "endpoint_url": [true, either([isString, isNull] as const)], "id": [true, isString], "model_id": [true, isString], "model_revision": [true, isString], "name": [true, isString], "precision": [true, either([checkPrecision, isNull] as const)], "setup_state": [true, checkConnectionSetupState], "updated_at": [true, isString] } as const); export const checkConnectionPage: Check = /*#__PURE__*/ object({ "items": [true, arrayOf(checkConnectionOut)], "total": [true, isInteger] } as const); diff --git a/frontend/ui-core/src/screens/inference.test.tsx b/frontend/ui-core/src/screens/inference.test.tsx index 4c38bb4e..0686b4e2 100644 --- a/frontend/ui-core/src/screens/inference.test.tsx +++ b/frontend/ui-core/src/screens/inference.test.tsx @@ -97,6 +97,10 @@ function connection(overrides: Partial = {}): Connection { endpoint_url: null, setup_state: "not_set_up", allowed_actions: ["download_weights", "update", "delete"], + // Not optional on the wire, so not optional here: the generated runtime + // check refuses a response missing it, and a stub that omitted one rendered + // this screen's error card in every case — which reads as a component bug. + capabilities: [], created_at: "2026-08-08T00:00:00Z", updated_at: "2026-08-08T00:00:00Z", ...overrides, diff --git a/openapi.json b/openapi.json index eb5df0ff..40d2cd38 100644 --- a/openapi.json +++ b/openapi.json @@ -1573,6 +1573,13 @@ "title": "Allowed Actions", "type": "array" }, + "capabilities": { + "items": { + "$ref": "#/components/schemas/ModelCapability" + }, + "title": "Capabilities", + "type": "array" + }, "connection_type": { "$ref": "#/components/schemas/ConnectionType" }, @@ -1650,6 +1657,7 @@ "endpoint_url", "setup_state", "allowed_actions", + "capabilities", "created_at", "updated_at" ], @@ -2464,6 +2472,15 @@ "title": "LabelClassBody", "type": "object" }, + "ModelCapability": { + "description": "What a connection's model can be asked for: the kind of prompt it takes.", + "enum": [ + "point_suggest", + "text_detect" + ], + "title": "ModelCapability", + "type": "string" + }, "PolygonBody": { "additionalProperties": false, "description": "A closed polygon of at least three points. The closing edge is implicit.", @@ -5945,7 +5962,7 @@ }, "/inference/connections": { "get": { - "description": "Every configured connection in this workspace, in the order they were made.", + "description": "Every configured connection in this workspace, in the order they were made.\n\nA set-up connection that has never been asked what kind of model it holds is\nasked here, once, from files already on this disk \u2014 see\n``visionset.inference.weights.with_families``. It is the backfill for rows\nwritten before a connection recorded that, and it is on the read path because\nthe kernel cannot reach a model cache and a migration runs in the kernel.", "operationId": "list_inference_connections", "responses": { "200": { @@ -6177,7 +6194,7 @@ ] }, "get": { - "description": "The connection with that id.", + "description": "The connection with that id.\n\nCarries the same backfill the listing does, so that reading one connection\nand reading the list never disagree about what it can be asked for.", "operationId": "get_inference_connection", "parameters": [ { diff --git a/src/visionset/inference/__init__.py b/src/visionset/inference/__init__.py index 3ec7380f..78a933ff 100644 --- a/src/visionset/inference/__init__.py +++ b/src/visionset/inference/__init__.py @@ -47,6 +47,14 @@ DEFAULT_PROVIDER_CAPACITY, BoundedCache, ) +from visionset.inference.families import ( + CAPABILITY_BY_FAMILY, + DETECTOR_FAMILIES, + SEGMENTER_FAMILIES, + SUPPORTED_FAMILIES, + capabilities_of, + family_of, +) from visionset.inference.integrity import ( READ_CHUNK, Digest, @@ -59,15 +67,7 @@ ) from visionset.inference.masks import DEFAULT_DETAIL, narrowed, polygon_from from visionset.inference.nms import DEFAULT_IOU_THRESHOLD, suppressed -from visionset.inference.providers import ( - DETECTOR_FAMILIES, - SEGMENTER_FAMILIES, - SUPPORTED_FAMILIES, - ProviderPool, - family_of, - provider_for, - resident, -) +from visionset.inference.providers import ProviderPool, provider_for, resident from visionset.inference.sam_provider import LocalSamProvider from visionset.inference.suggestions import suggest from visionset.inference.transformers_provider import LocalTransformersProvider @@ -81,6 +81,7 @@ fetch_weights, known_sizes, measure, + with_families, ) __all__ = [ @@ -88,6 +89,7 @@ "DEFAULT_EMBEDDING_CAPACITY", "DEFAULT_IOU_THRESHOLD", "DEFAULT_PROVIDER_CAPACITY", + "CAPABILITY_BY_FAMILY", "DEFAULT_SIZE_CAPACITY", "DETECTOR_FAMILIES", "EXTRA", @@ -102,6 +104,7 @@ "LocalTransformersProvider", "ProviderPool", "cache_root", + "capabilities_of", "check_integrity", "digest_of", "download", @@ -123,4 +126,5 @@ "resident", "suggest", "suppressed", + "with_families", ] diff --git a/src/visionset/inference/families.py b/src/visionset/inference/families.py new file mode 100644 index 00000000..77e2d5f4 --- /dev/null +++ b/src/visionset/inference/families.py @@ -0,0 +1,138 @@ +# usage: from visionset.inference import family_of, capabilities_of +"""What kind of model a connection points at, and what that lets it be asked. + +Two questions with one answer between them. A model's family — the ``model_type`` +its own config declares — decides which adapter can run it, and the *same* fact +decides what a caller may ask it for. Keeping both in one module is what stops +those two readings from drifting apart: the day a family is added to a set below, +it acquires an adapter and a declared capability in the same edit, because the +second is derived from the first rather than written beside it. + +**The family is read from the model, never guessed from its name.** A model id is +something somebody typed; the config is something the publisher wrote. Matching on +the id gives a confident answer for every model this build has never heard of, +and the wrongness is invisible until an adapter fails somewhere inside a forward +pass. + +**And the capability vocabulary is the kernel's, while the mapping is here.** +``ModelCapability`` is a domain word because whether a model takes points or words +is a fact about the product's tools; which ``model_type`` values *this build* can +serve is a fact about an optional runtime, and the kernel has no view of one. So +the enum is declared over there and this is the only place the two meet. +""" + +from __future__ import annotations + +from collections.abc import Mapping +from pathlib import Path +from typing import Final + +from visionset.inference._extra import imported +from visionset.kernel.domain import InferenceConnection, ModelCapability + +SEGMENTER_FAMILIES: Final[frozenset[str]] = frozenset({"sam2", "sam2_video"}) +"""``model_type`` values this build serves with the point-prompted adapter. + +**Two spellings of one architecture, and the second is not a door held open for +later.** The published SAM 2 checkpoints — including the one the connection form +suggests — declare ``sam2_video``, and ``transformers`` loads such a checkpoint +into the image model deliberately, saying so as it does: *"loading a +``sam2_video`` checkpoint into ``Sam2Model``"*. Naming only ``sam2`` sends the +commonest point-prompt model in the product to the detector adapter, which then +refuses a click with a sentence about text prompts. + +Whole models only. The locked ``transformers`` also registers +``sam2_vision_model`` and ``sam2_hiera_det_model``, which are the encoder halves +a full config nests rather than checkpoints anything can prompt. A connection +naming one of those is refused by ``provider_for``, not handed to an adapter that +would look for a mask decoder and find none. +""" + +DETECTOR_FAMILIES: Final[frozenset[str]] = frozenset({"grounding-dino", "mm-grounding-dino"}) +"""``model_type`` values this build serves with the text-prompted adapter. + +Narrower than "everything ``AutoModelForZeroShotObjectDetection`` accepts", and +measured rather than assumed. ``transformers_provider`` post-processes with +``post_process_grounded_object_detection(outputs, input_ids, …, text_threshold=…)`` +— this family's signature. The other zero-shot detectors the locked +``transformers`` registers take a different one, with no ``input_ids`` and no +``text_threshold``, so listing them here would claim a support that fails inside +a post-processor instead of in a refusal a reader can act on. +""" + +SUPPORTED_FAMILIES: Final[frozenset[str]] = SEGMENTER_FAMILIES | DETECTOR_FAMILIES +"""Every ``model_type`` this build has an adapter for, and what a refusal lists. + +Derived rather than written a third time, so a family added to one set above +cannot be missing from the sentence that tells somebody what they may use. +""" + +CAPABILITY_BY_FAMILY: Final[Mapping[str, ModelCapability]] = { + **dict.fromkeys(SEGMENTER_FAMILIES, ModelCapability.POINT_SUGGEST), + **dict.fromkeys(DETECTOR_FAMILIES, ModelCapability.TEXT_DETECT), +} +"""Which prompt each family takes — derived from the sets, never listed again. + +**The derivation is the guarantee.** Writing this out by hand would make it +possible to add a family to :data:`SEGMENTER_FAMILIES`, ship the adapter, and +leave the capability behind — and a model that runs but declares nothing is +invisible to every client that filters on the declaration. Deriving it means the +adapter and the declaration are the same edit. + +It is a one-to-one map rather than a judgement because the split already exists +and is exactly this one: the two sets *are* "answers points" and "answers words", +which is why each has its own adapter. +""" + + +def capabilities_of(model_family: str | None) -> list[ModelCapability]: + """What a model of that family can be asked for. Empty when nothing is known. + + Three inputs collapse to the empty list, and they are genuinely the same + answer to a caller: ``None`` (nobody has read this connection's config), + ``""`` (somebody read it and it declared nothing), and a family this build + has no adapter for. In every one of them there is no request a client could + make with any confidence, so there is no capability to declare. What + separates them is the *remedy*, and a remedy belongs to the surface that has + room for a sentence — not to a vocabulary a client switches on. + + A list rather than a member, for the wire's sake and for honesty: nothing + says a family answers only one kind of prompt forever, and a client written + against a list on the day one does will not have to change. + """ + capability = CAPABILITY_BY_FAMILY.get(model_family or "") + return [] if capability is None else [capability] + + +def family_of(connection: InferenceConnection, *, cache_dir: Path) -> str: + """The ``model_type`` the downloaded config declares, or ``""`` if it cannot say. + + Read from the cache rather than from the network — ``local_files_only`` — for + the same reason every other load in this package is: this product downloads + weights when somebody asks it to and at no other time. + + An unreadable config answers ``""`` rather than raising: reading the files + and deciding what to do about them are separate jobs, and this one only + reports. ``""`` is not a family, so ``provider_for`` refuses it — the same + answer it gives a type nobody here serves, because "the config says nothing" + and "the config says something unknown" leave the resolver equally unable to + pick an adapter honestly. + + Raises: + LocalInferenceUnavailable: the optional runtime is not installed, so + nothing here can read a config at all. Deliberately *not* folded into + the ``""`` above: a build that cannot look has not looked, and a + caller recording an answer must be able to tell that apart from a + config that answered nothing. + """ + transformers = imported("transformers") + try: + config = transformers.AutoConfig.from_pretrained( + connection.model_id, + revision=connection.model_revision, + cache_dir=str(cache_dir), + local_files_only=True, + ) + except Exception: # noqa: BLE001 — see the docstring: this is a fallback, not a handler + return "" + return str(getattr(config, "model_type", "") or "") diff --git a/src/visionset/inference/providers.py b/src/visionset/inference/providers.py index f7943ebc..fab85c83 100644 --- a/src/visionset/inference/providers.py +++ b/src/visionset/inference/providers.py @@ -5,7 +5,9 @@ the first being cheap to ask. **Resolution is by the model's own declared family, not by the connection's -kind.** ``ConnectionType`` says *where* a model runs — here or elsewhere — and +kind**, and the families themselves live in ``families.py`` — one module over, +because the same fact also decides what a connection may be *asked* for and the +two readings must not drift. ``ConnectionType`` says *where* a model runs — here or elsewhere — and that is the only thing it says. It cannot say whether the weights behind a local connection are a detector or a segmenter, and those answer different questions: one takes words and one takes places. So the family is read from the model's own @@ -15,7 +17,7 @@ somewhere inside a forward pass on a shape mismatch. **And a family this build does not serve is refused rather than guessed at.** -The two sets below are the whole of what resolves; there is no fallback. A +``SUPPORTED_FAMILIES`` is the whole of what resolves; there is no fallback. A resolver with one guesses on every model it has not been told about, and the guess is invisible until the wrong adapter refuses the request in its own vocabulary — a sentence that describes a model the user does not have. @@ -38,8 +40,14 @@ from pathlib import Path from typing import Any, Final -from visionset.inference._extra import imported, require +from visionset.inference._extra import require from visionset.inference.cache import DEFAULT_PROVIDER_CAPACITY, BoundedCache +from visionset.inference.families import ( + DETECTOR_FAMILIES, + SEGMENTER_FAMILIES, + SUPPORTED_FAMILIES, + family_of, +) from visionset.inference.sam_provider import LocalSamProvider from visionset.inference.transformers_provider import LocalTransformersProvider from visionset.inference.weights import cache_root @@ -50,43 +58,6 @@ ) from visionset.kernel.ports import ModelProvider -SEGMENTER_FAMILIES: Final[frozenset[str]] = frozenset({"sam2", "sam2_video"}) -"""``model_type`` values this build serves with the point-prompted adapter. - -**Two spellings of one architecture, and the second is not a door held open for -later.** The published SAM 2 checkpoints — including the one the connection form -suggests — declare ``sam2_video``, and ``transformers`` loads such a checkpoint -into the image model deliberately, saying so as it does: *"loading a -``sam2_video`` checkpoint into ``Sam2Model``"*. Naming only ``sam2`` sends the -commonest point-prompt model in the product to the detector adapter, which then -refuses a click with a sentence about text prompts. - -Whole models only. The locked ``transformers`` also registers -``sam2_vision_model`` and ``sam2_hiera_det_model``, which are the encoder halves -a full config nests rather than checkpoints anything can prompt. A connection -naming one of those is refused below, not handed to an adapter that would look -for a mask decoder and find none. -""" - -DETECTOR_FAMILIES: Final[frozenset[str]] = frozenset({"grounding-dino", "mm-grounding-dino"}) -"""``model_type`` values this build serves with the text-prompted adapter. - -Narrower than "everything ``AutoModelForZeroShotObjectDetection`` accepts", and -measured rather than assumed. ``transformers_provider`` post-processes with -``post_process_grounded_object_detection(outputs, input_ids, …, text_threshold=…)`` -— this family's signature. The other zero-shot detectors the locked -``transformers`` registers take a different one, with no ``input_ids`` and no -``text_threshold``, so listing them here would claim a support that fails inside -a post-processor instead of in a refusal a reader can act on. -""" - -SUPPORTED_FAMILIES: Final[frozenset[str]] = SEGMENTER_FAMILIES | DETECTOR_FAMILIES -"""Every ``model_type`` this build has an adapter for, and what a refusal lists. - -Derived rather than written a third time, so a family added to one set above -cannot be missing from the sentence that tells somebody what they may use. -""" - _Key = tuple[str, str] @@ -236,30 +207,3 @@ def _no_adapter_for(connection: InferenceConnection, family: str) -> str: f"model type {family!r}, and this build has no adapter for that model type; it supports " f"{supported} — point the connection at a model of one of those types" ) - - -def family_of(connection: InferenceConnection, *, cache_dir: Path) -> str: - """The ``model_type`` the downloaded config declares, or ``""`` if it cannot say. - - Read from the cache rather than from the network — ``local_files_only`` — for - the same reason every other load in this package is: this product downloads - weights when somebody asks it to and at no other time. - - An unreadable config answers ``""`` rather than raising: reading the files - and deciding what to do about them are separate jobs, and this one only - reports. ``""`` is not a family, so :func:`_local` refuses it — the same - answer it gives a type nobody here serves, because "the config says nothing" - and "the config says something unknown" leave the resolver equally unable to - pick an adapter honestly. - """ - transformers = imported("transformers") - try: - config = transformers.AutoConfig.from_pretrained( - connection.model_id, - revision=connection.model_revision, - cache_dir=str(cache_dir), - local_files_only=True, - ) - except Exception: # noqa: BLE001 — see the docstring: this is a fallback, not a handler - return "" - return str(getattr(config, "model_type", "") or "") diff --git a/src/visionset/inference/weights.py b/src/visionset/inference/weights.py index daa4d192..1a32e3ba 100644 --- a/src/visionset/inference/weights.py +++ b/src/visionset/inference/weights.py @@ -22,6 +22,13 @@ state meaning "some of it arrived". That is an ordering rather than a guard, which is why nothing in the domain has to encode it. +**What arrives is also read, once.** A download ends by reading the model's own +config out of the cache it just filled and recording the family it declares, so +that a client can be told what this connection may be asked for. That is the +first moment the answer exists without a network call, which is why it happens +here rather than at connection creation — and :func:`with_families` is the same +read, late, for rows written before the column existed. + **Idempotent, and two callers need it to be.** A connection already ``ready`` is re-checked — every file the revision names is looked for, and anything missing is fetched — and then left alone. That is not only a convenience for people typing @@ -43,14 +50,20 @@ from __future__ import annotations import logging -from collections.abc import Callable +from collections.abc import Callable, Sequence from pathlib import Path from typing import Final from uuid import UUID from visionset.inference._extra import imported from visionset.inference.cache import BoundedCache -from visionset.kernel.domain import ConnectionType, DownloadSize, InferenceConnection +from visionset.inference.families import family_of +from visionset.kernel.domain import ( + ConnectionSetupState, + ConnectionType, + DownloadSize, + InferenceConnection, +) from visionset.kernel.errors import LocalInferenceUnavailable from visionset.kernel.services import InferenceConnectionService, WorkspaceService @@ -118,11 +131,104 @@ def fetch_weights( connections = InferenceConnectionService(workspace) connection = connections.require_downloadable(connection_id) say = on_progress or (lambda _: None) + cache = cache_root(workspace.root) say(f"fetching {connection.model_id} at {connection.model_revision}") - download(connection, into=cache_root(workspace.root)) + download(connection, into=cache) + # Only knowable now, and knowable without a network only now: the config + # that says what kind of model this is arrived with the weights. Reading it + # here is what lets a client be told what this connection can be asked for + # instead of finding out one refusal at a time — see ``families``. + say("reading what kind of model arrived") + family = _family_if_it_can_be_read(connection, cache_dir=cache) say("recording the connection as ready") - return connections.record_weights_ready(connection.id) + return connections.record_weights_ready(connection.id, model_family=family) + + +def _family_if_it_can_be_read(connection: InferenceConnection, *, cache_dir: Path) -> str | None: + """What the config declares, or ``None`` where nothing here could read it. + + **A download that worked must not be undone by a question about it.** The + bytes are on the disk by the time this is asked; letting the read's refusal + out would leave the connection ``not_set_up`` beside a full cache, and the + remedy on offer would be the transfer that already happened. So a build that + cannot read a config records that it does not know, which is recoverable — + the next read of the connection asks again (:func:`with_families`). + + Only :class:`LocalInferenceUnavailable` is caught, and only that. Anything + the read itself could not survive is already ``""`` by ``family_of``'s own + contract, so what is left here is exactly one condition: nothing on this + machine can parse a config at all. + """ + try: + return family_of(connection, cache_dir=cache_dir) + except LocalInferenceUnavailable: + _logger.info("no runtime here to read %s's config", connection.name) + return None + + +def with_families( + workspace: WorkspaceService, connections: Sequence[InferenceConnection] +) -> list[InferenceConnection]: + """Those connections, with any missing family filled in from the cache. + + The backfill for every row written before a connection recorded what kind of + model it points at. It is deliberately *not* a migration: the answer lives in + a model config inside this workspace's cache, the kernel is forbidden from + reaching that cache, and a migration runs inside the kernel — so the only + place this can happen is out here, where the resolver already lives. + + **A read that writes, and the bound is what makes it honest.** It touches + only a ``local`` connection that is ``ready`` and has never been asked, it + reads a small JSON file already on this disk, and it reaches no network. Once + a row has an answer — *any* answer, the empty string included — it is never + considered again, so the whole cost is one config read per pre-existing row, + once, ever. A row created after this shipped arrives with its family already + recorded by the download and never enters the loop at all. + + **What it will not do is invent one.** A build without the optional runtime + cannot read a config, and a build that cannot look has not looked: the + connection is returned exactly as it was, still NULL, so a machine that later + installs the runtime resolves it then. Writing the empty string there would + record "this model declares nothing" on the strength of never having + checked — and every client filtering on the declaration would believe it. + That is :func:`_family_if_it_can_be_read`'s rule, shared with the download so + there is one answer to "what does a build that cannot look record". + """ + service = InferenceConnectionService(workspace) + cache = cache_root(workspace.root) + resolved: list[InferenceConnection] = [] + for connection in connections: + if not _awaiting_a_family(connection): + resolved.append(connection) + continue + family = _family_if_it_can_be_read(connection, cache_dir=cache) + if family is None: + resolved.append(connection) + continue + _logger.info("resolved %s as model type %r", connection.name, family) + # The same write the download makes, made late — one encoding of "the + # weights are here and this is what they turned out to be", rather than a + # second path that could disagree with it. The state half is already + # true, so what this commits is the family. + resolved.append(service.record_weights_ready(connection.id, model_family=family)) + return resolved + + +def _awaiting_a_family(connection: InferenceConnection) -> bool: + """Whether reading this connection's config is a question worth asking. + + Three conditions, each ruling out a different kind of nonsense: an ``http`` + connection keeps its model somewhere else, so there is no config here to + read and the vocabulary for what a remote endpoint declares does not exist + yet; a connection that is not ``ready`` has no files at all; and one that + already carries an answer has been asked. + """ + return ( + connection.connection_type is ConnectionType.LOCAL + and connection.setup_state is ConnectionSetupState.READY + and connection.model_family is None + ) def download(connection: InferenceConnection, *, into: Path) -> Path: diff --git a/src/visionset/kernel/adapters/_mappers.py b/src/visionset/kernel/adapters/_mappers.py index e52baf98..80e7aeee 100644 --- a/src/visionset/kernel/adapters/_mappers.py +++ b/src/visionset/kernel/adapters/_mappers.py @@ -417,6 +417,7 @@ def _connection_to_row(entity: InferenceConnection) -> t.Base: setup_state=entity.setup_state.value, created_at=entity.created_at.isoformat(), updated_at=entity.updated_at.isoformat(), + model_family=entity.model_family, ) @@ -433,6 +434,9 @@ def _connection_to_domain(_: Session, row: Any) -> InferenceConnection: setup_state=ConnectionSetupState(row.setup_state), created_at=datetime.fromisoformat(row.created_at), updated_at=datetime.fromisoformat(row.updated_at), + # Read straight through, empty string included: a row that was looked at + # and declared nothing must not come back as one nobody has looked at. + model_family=row.model_family, ) diff --git a/src/visionset/kernel/adapters/_tables.py b/src/visionset/kernel/adapters/_tables.py index a484bcbb..fb0eecd5 100644 --- a/src/visionset/kernel/adapters/_tables.py +++ b/src/visionset/kernel/adapters/_tables.py @@ -711,6 +711,16 @@ class InferenceConnectionRow(Base): #: ISO-8601 with offset, never SQLite ``DATETIME``. See the module docstring. created_at: Mapped[str] = mapped_column(String, nullable=False) updated_at: Mapped[str] = mapped_column(String, nullable=False) + #: Last on the class because it arrives by ``ALTER TABLE``, which SQLite + #: appends — the module docstring's rule, and the same pair + #: ``BatchRow.parent_batch_id`` and ``AnnotationSchemaRow.provenance`` + #: already stand in. + #: + #: Nullable in the schema *and* meaningful when empty: NULL is "nobody has + #: read this connection's config yet", the empty string is "somebody read it + #: and it declared nothing". ``InferenceConnection.model_family`` carries the + #: reason those are worth telling apart. + model_family: Mapped[str | None] = mapped_column(String, nullable=True) #: Connection names are unique in the workspace, case-insensitively. diff --git a/src/visionset/kernel/adapters/migrations.py b/src/visionset/kernel/adapters/migrations.py index 169dc7c5..6e7cbf09 100644 --- a/src/visionset/kernel/adapters/migrations.py +++ b/src/visionset/kernel/adapters/migrations.py @@ -227,6 +227,27 @@ def _add_inference_connections(connection: Connection) -> None: Base.metadata.create_all(connection, tables=[Base.metadata.tables["inference_connection"]]) +def _add_model_family(connection: Connection) -> None: + """``inference_connection.model_family``: what kind of model this one is. + + **The backfill cannot happen here, and that is a layering fact rather than a + shortcut.** The value is read from a model's own config, which sits in a + cache under the workspace root that only ``visionset.inference`` knows how to + address — and this module is in the kernel, which is forbidden from importing + it. So every existing row starts NULL, meaning *nobody has looked yet*, and + the look happens where the resolver already lives: at the next download, and + on the first read of a connection that is set up (see + ``visionset.inference.weights.with_families``). + + NULL is therefore the honest starting value rather than a gap. It is + distinguishable from the empty string, which the resolver writes when it did + look and the config declared nothing — so a row that has been asked and + answered nothing is never asked again, and a row that has never been asked + still will be. + """ + _add_column(connection, "inference_connection", "model_family") + + MIGRATIONS: list[Migration] = [ Migration(version=1, name="baseline_schema", upgrade=_create_baseline_schema), Migration(version=2, name="batch_lineage", upgrade=_add_batch_lineage), @@ -234,6 +255,7 @@ def _add_inference_connections(connection: Connection) -> None: Migration(version=4, name="job_queue", upgrade=_add_job_queue), Migration(version=5, name="schema_provenance", upgrade=_add_schema_provenance), Migration(version=6, name="inference_connections", upgrade=_add_inference_connections), + Migration(version=7, name="model_family", upgrade=_add_model_family), ] FORMAT_VERSION: int = MIGRATIONS[-1].version diff --git a/src/visionset/kernel/domain/__init__.py b/src/visionset/kernel/domain/__init__.py index 8902738a..77e617bb 100644 --- a/src/visionset/kernel/domain/__init__.py +++ b/src/visionset/kernel/domain/__init__.py @@ -78,6 +78,7 @@ ConnectionType, DownloadSize, InferenceConnection, + ModelCapability, Precision, precisions_for, ) @@ -296,6 +297,7 @@ "Manifest", "ManifestAnnotation", "ManifestAsset", + "ModelCapability", "MembershipChange", "Partition", "PointPrompt", diff --git a/src/visionset/kernel/domain/inference.py b/src/visionset/kernel/domain/inference.py index 52cc4bc1..6c63b29d 100644 --- a/src/visionset/kernel/domain/inference.py +++ b/src/visionset/kernel/domain/inference.py @@ -70,6 +70,36 @@ class ConnectionSetupState(StrEnum): READY = "ready" +# A vocabulary of its own because neither of the other two answers the question. +# `ConnectionType` says where a model runs and `ConnectionSetupState` says whether +# its weights arrived; both are silent about whether this model answers the +# question a caller is about to put to it. A tool offered without that check is a +# tool that works by being lucky, and the editor shipped exactly that: it picked +# the first `ready` connection, sent point prompts to a text-prompted detector, +# and let the server refuse them one click at a time. +# +# **The prompt, not the answer.** A region comes back either way, so naming the +# output would collapse the two members and lose the only distinction that decides +# whether a request can be made at all. +# +# Declared here and **mapped to model families outside**: which `model_type` +# values a build can serve is a fact about that build's optional runtime, and the +# kernel has no view of one. `visionset.inference.families` owns the mapping, +# beside the family sets it reads. +# +# The reasoning is a comment because this enum is *published*: FastAPI copies a +# docstring verbatim into `openapi.json`, where RST markup ships as literal +# backticks and internal rationale ships as API documentation. The docstring is +# the sentence a client should read, on `ConnectionAction`'s terms. +class ModelCapability(StrEnum): + """What a connection's model can be asked for: the kind of prompt it takes.""" + + #: Give me the thing under these points. + POINT_SUGGEST = "point_suggest" + #: Find everything these words name. + TEXT_DETECT = "text_detect" + + class Precision(StrEnum): """The numeric precision a local connection asks its weights to be loaded in. @@ -289,6 +319,22 @@ class InferenceConnection(BaseModel): #: changes it, never null — "never edited" is honestly expressed as "last #: changed when it was made". updated_at: datetime = Field(default_factory=lambda: datetime.now(UTC)) + #: The ``model_type`` this connection's own downloaded config declares — + #: recorded when the weights arrive, because that is the first moment it is + #: knowable without reaching a network. Opaque to the kernel, like + #: ``model_id``: what a family *means* is + #: ``visionset.inference.families``' to say. + #: + #: **Three states, and the third is the useful one.** ``None`` is *nobody has + #: looked*, which is where every row written before this column existed + #: starts. ``""`` is *somebody looked and the config did not say* — the + #: answer ``family_of`` already gave to an unreadable config, kept rather + #: than folded into ``None`` so that a look which found nothing is not + #: repeated on every read. Anything else is the family itself. + #: + #: Never derived from the model id. A name is not a declaration, and + #: matching on one is the guessing this product removed from the resolver. + model_family: str | None = None @field_validator("name", "model_id", "model_revision") @classmethod diff --git a/src/visionset/kernel/services/inference_connection_service.py b/src/visionset/kernel/services/inference_connection_service.py index ac10e00b..2a5211cd 100644 --- a/src/visionset/kernel/services/inference_connection_service.py +++ b/src/visionset/kernel/services/inference_connection_service.py @@ -145,6 +145,10 @@ def update( """Edit a connection in place. Every argument is optional; ``None`` means *leave this alone*. + Pointing a connection at a different model or revision **forgets what + kind of model it was**, because that answer was read out of the old + model's config and nothing has read the new one. + The kind is deliberately not editable. Changing ``local`` to ``http`` would empty every parameter the row carries and keep only its name, which is a new connection wearing an old id — and an id that already travelled @@ -172,6 +176,13 @@ def update( ): if value is not None: changes[field] = value + # A different model is a different config, and nobody has read + # the new one. Keeping the old family would leave the row + # declaring what its *previous* weights could be asked for — + # a stale answer that reads exactly like a fresh one. Forgetting + # is what sends it back through the resolver. + if "model_id" in changes or "model_revision" in changes: + changes["model_family"] = None # Rebuilt rather than mutated, so the cross-field rule runs on the # result: ``model_copy`` does not validate, which is the whole # reason ``Source`` had to turn on ``validate_assignment``. @@ -215,7 +226,9 @@ def require_downloadable(self, connection_id: UUID) -> InferenceConnection: return connection raise InferenceConnectionNotDownloadable(_why_not_downloadable(connection)) - def record_weights_ready(self, connection_id: UUID) -> InferenceConnection: + def record_weights_ready( + self, connection_id: UUID, *, model_family: str | None = None + ) -> InferenceConnection: """Mark the weights present. Called **after** they are, never before. The one edge in this entity's short life, and it is written last on @@ -227,30 +240,32 @@ def record_weights_ready(self, connection_id: UUID) -> InferenceConnection: idempotent, so a crash after this commit and before the row settled means a retry arrives at a connection that is already ``ready``; answering that with a refusal would fail a job whose work is done. Recording something - already true returns it unchanged. - - Deliberately narrow. It takes no state to move *to*, because there is - only one, and it accepts no other field — the caller is a job handler - that has just written bytes to a cache, and the only thing it has - learned is that they are there. + already true returns it unchanged — and *unchanged* is now decided by + comparing the fields rather than by the state alone, because a re-run + over an already-``ready`` connection is exactly how a row that predates + ``model_family`` gets one. + + ``model_family`` is the second thing the caller has learned, and it can + only be learned here. What model type a connection points at is written + in the model's own downloaded config, so the first moment it is knowable + without reaching a network is the moment the download finishes — and the + caller that just finished one is holding the answer. ``None`` means *I + did not find out*, which leaves whatever the row already had; the empty + string means *I looked and it declared nothing*, which is a finding and + is recorded as one. Raises: InferenceConnectionNotFound: no such connection in this workspace. """ with self._workspace.unit_of_work() as uow: current = self.require_connection(uow, connection_id) - if current.setup_state is ConnectionSetupState.READY: + changes: dict[str, object] = {"setup_state": ConnectionSetupState.READY} + if model_family is not None: + changes["model_family"] = model_family + if all(getattr(current, field) == value for field, value in changes.items()): return current return uow.inference_connections.update( - _built( - **( - current.model_dump() - | { - "setup_state": ConnectionSetupState.READY, - "updated_at": datetime.now(UTC), - } - ) - ) + _built(**(current.model_dump() | changes | {"updated_at": datetime.now(UTC)})) ) def require_checkable(self, connection_id: UUID) -> InferenceConnection: diff --git a/src/visionset/server/models.py b/src/visionset/server/models.py index 043c1095..b5045bc7 100644 --- a/src/visionset/server/models.py +++ b/src/visionset/server/models.py @@ -45,6 +45,7 @@ from fastapi import Query from pydantic import BaseModel, ConfigDict, Field, JsonValue, model_validator +from visionset.inference import capabilities_of from visionset.kernel.domain import ( Annotation, AnnotationJob, @@ -87,6 +88,7 @@ JobAction, LabelClass, MembershipChange, + ModelCapability, Partition, PolygonGeometry, PolylineGeometry, @@ -1634,6 +1636,23 @@ class ConnectionOut(BaseModel): endpoint_url: str | None setup_state: ConnectionSetupState allowed_actions: list[ConnectionAction] + #: What this connection's model can be asked for, and empty where nothing is + #: known yet: a connection whose weights have never been fetched, one whose + #: config declared no model type, one of a type this build cannot run, and an + #: ``http`` connection — which declares nothing until the remote contract + #: says how an endpoint states what it can do. + #: + #: **Not the same question as ``allowed_actions``, and both are needed.** An + #: action is something to do *to* this connection and is decided by its state; + #: a capability is what its model answers and is decided by the weights. A + #: client offering a tool wants a connection that is ``ready`` **and** + #: declares the capability the tool needs — being ready says the files are + #: here, not that they are the right kind of model. + #: + #: Empty is not a refusal to act on. It says only that this connection cannot + #: be relied on for a particular tool; the server still judges every request + #: on its own. + capabilities: list[ModelCapability] created_at: datetime updated_at: datetime @@ -1652,6 +1671,7 @@ def of(cls, connection: InferenceConnection) -> Self: allowed_actions=connection_actions( connection.setup_state, connection_type=connection.connection_type ), + capabilities=capabilities_of(connection.model_family), created_at=connection.created_at, updated_at=connection.updated_at, ) diff --git a/src/visionset/server/routes/inference.py b/src/visionset/server/routes/inference.py index 1b5dc631..16f71527 100644 --- a/src/visionset/server/routes/inference.py +++ b/src/visionset/server/routes/inference.py @@ -26,7 +26,7 @@ from fastapi import Response, status -from visionset.inference import download_size, suggest +from visionset.inference import download_size, suggest, with_families from visionset.inference import require as require_local_inference from visionset.jobs.integrity import JOB_TYPE as integrity_job_type from visionset.jobs.integrity import payload_for as integrity_payload_for @@ -61,8 +61,15 @@ @router.get("") def list_inference_connections(workspace: WorkspaceDep) -> ConnectionPage: - """Every configured connection in this workspace, in the order they were made.""" - connections = InferenceConnectionService(workspace).list() + """Every configured connection in this workspace, in the order they were made. + + A set-up connection that has never been asked what kind of model it holds is + asked here, once, from files already on this disk — see + ``visionset.inference.weights.with_families``. It is the backfill for rows + written before a connection recorded that, and it is on the read path because + the kernel cannot reach a model cache and a migration runs in the kernel. + """ + connections = with_families(workspace, InferenceConnectionService(workspace).list()) items = [ConnectionOut.of(one) for one in connections] return ConnectionPage(items=items, total=len(items)) @@ -85,8 +92,14 @@ def create_inference_connection(workspace: WorkspaceDep, body: ConnectionCreate) @router.get("/{connection_id}", responses=documented(404)) def get_inference_connection(workspace: WorkspaceDep, connection_id: UUID) -> ConnectionOut: - """The connection with that id.""" - return ConnectionOut.of(InferenceConnectionService(workspace).get(connection_id)) + """The connection with that id. + + Carries the same backfill the listing does, so that reading one connection + and reading the list never disagree about what it can be asked for. + """ + connection = InferenceConnectionService(workspace).get(connection_id) + (resolved,) = with_families(workspace, [connection]) + return ConnectionOut.of(resolved) @router.patch("/{connection_id}", responses=documented(404, 409, 422)) diff --git a/src/visionset/wire/__init__.py b/src/visionset/wire/__init__.py index 702dbcb1..044e1307 100644 --- a/src/visionset/wire/__init__.py +++ b/src/visionset/wire/__init__.py @@ -52,6 +52,13 @@ from typing import Any from uuid import UUID +# The one import here that is not the kernel's, and it is the same direction as +# everything else in this package: `visionset.inference` is a sibling below the +# surfaces, it imports nothing from here, and what it owns is the fact this +# module has no way to know — which model families this build can serve. The +# alternative is spelling that mapping a second time, which is what every other +# rule in this file exists to prevent. +from visionset.inference import capabilities_of from visionset.kernel.domain import ( Annotation, AnnotationJob, @@ -599,6 +606,10 @@ def connection(value: InferenceConnection) -> dict[str, Any]: a.value for a in connection_actions(value.setup_state, connection_type=value.connection_type) ], + # What its model answers, where its actions are what may be done *to* it. + # Empty until something has read the model's own config — see + # ``InferenceConnection.model_family``. + "capabilities": [c.value for c in capabilities_of(value.model_family)], "created_at": _moment(value.created_at), "updated_at": _moment(value.updated_at), } diff --git a/tests/inference/test_families.py b/tests/inference/test_families.py new file mode 100644 index 00000000..0ecfcfd4 --- /dev/null +++ b/tests/inference/test_families.py @@ -0,0 +1,172 @@ +"""What kind of model a connection points at, and what that lets it be asked. + +Two claims live here, and the second is the one that had never been made before: +that the family is *read* rather than guessed, and that what a client is told a +connection can do is derived from the same sets the adapters are chosen from. A +capability written out by hand beside those sets would be a second encoding, and +the day it fell behind, a model that runs would declare nothing and no client +would offer it. +""" + +from __future__ import annotations + +from collections.abc import Iterator +from pathlib import Path +from typing import Any + +import pytest + +from visionset.inference import families as families_module +from visionset.inference.families import ( + CAPABILITY_BY_FAMILY, + DETECTOR_FAMILIES, + SEGMENTER_FAMILIES, + SUPPORTED_FAMILIES, + capabilities_of, + family_of, +) +from visionset.kernel.domain import ConnectionType, ModelCapability +from visionset.kernel.services import InferenceConnectionService, WorkspaceService + + +@pytest.fixture() +def workspace(tmp_path: Path) -> Iterator[WorkspaceService]: + made = WorkspaceService.init(tmp_path / "ws", name="families") + try: + yield made + finally: + made.close() + + +@pytest.fixture() +def connections(workspace: WorkspaceService) -> InferenceConnectionService: + return InferenceConnectionService(workspace) + + +def a_local(connections: InferenceConnectionService) -> Any: + return connections.create( + "local", + connection_type=ConnectionType.LOCAL, + model_id="some/segmenter", + model_revision="abc123", + device="cuda", + precision="fp16", + ) + + +# --- the sets themselves ------------------------------------------------------ + + +def test_both_spellings_of_the_one_architecture_are_named() -> None: + """A set rather than a string, and both members are load-bearing today.""" + assert {"sam2", "sam2_video"} <= SEGMENTER_FAMILIES + + +def test_the_two_families_are_disjoint_and_are_the_whole_of_what_is_supported() -> None: + """What the refusal lists is derived, so a family cannot be added to one set + and forgotten in the message.""" + assert not SEGMENTER_FAMILIES & DETECTOR_FAMILIES + assert SUPPORTED_FAMILIES == SEGMENTER_FAMILIES | DETECTOR_FAMILIES + + +# --- reading the family ------------------------------------------------------- + + +def test_an_unreadable_config_answers_empty_rather_than_raising( + connections: InferenceConnectionService, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Reading the config and deciding what to do about it stay separate. + + This function's job is to report what the files say; the refusal for "they + say nothing" is the resolver's, one level up. + """ + + class Broken: + class AutoConfig: + @staticmethod + def from_pretrained(*_: Any, **__: Any) -> Any: + raise OSError("nothing in the cache") + + monkeypatch.setattr(families_module, "imported", lambda _: Broken()) + assert family_of(a_local(connections), cache_dir=tmp_path) == "" + + +def test_the_family_comes_from_the_config_and_never_from_the_model_id( + connections: InferenceConnectionService, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A model id is something somebody typed; a config is what the publisher wrote. + + The connection here is named ``some/segmenter`` and its config declares a + detector. Any resolver that read the name would answer the opposite of the + truth, confidently, and pick the adapter that cannot run it. + """ + + class Detector: + class AutoConfig: + @staticmethod + def from_pretrained(*_: Any, **__: Any) -> Any: + return type("Config", (), {"model_type": "grounding-dino"})() + + monkeypatch.setattr(families_module, "imported", lambda _: Detector()) + assert family_of(a_local(connections), cache_dir=tmp_path) == "grounding-dino" + + +def test_a_build_without_the_runtime_cannot_look_and_says_so( + connections: InferenceConnectionService, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Not folded into the empty string, and the difference is what a caller records. + + "I looked and it declared nothing" is a finding. "I cannot look at all" is + not, and recording it as one would let a machine that later installs the + runtime go on believing an answer nobody ever produced. + """ + from visionset.kernel.errors import LocalInferenceUnavailable + + def _absent(_: str) -> Any: + raise LocalInferenceUnavailable("no runtime here") + + monkeypatch.setattr(families_module, "imported", _absent) + with pytest.raises(LocalInferenceUnavailable): + family_of(a_local(connections), cache_dir=tmp_path) + + +# --- what a family lets a connection be asked for ----------------------------- + + +@pytest.mark.parametrize("family", sorted(SEGMENTER_FAMILIES)) +def test_every_segmenter_family_answers_points(family: str) -> None: + assert capabilities_of(family) == [ModelCapability.POINT_SUGGEST] + + +@pytest.mark.parametrize("family", sorted(DETECTOR_FAMILIES)) +def test_every_detector_family_answers_words(family: str) -> None: + assert capabilities_of(family) == [ModelCapability.TEXT_DETECT] + + +def test_the_mapping_covers_exactly_what_this_build_can_run() -> None: + """The derivation, asserted rather than trusted. + + A family that has an adapter and no capability is invisible to every client + that filters on the declaration — the model runs and nothing offers it. A + capability for a family with no adapter is the opposite lie. Deriving the map + from the two sets makes both impossible; this is what says so out loud. + """ + assert set(CAPABILITY_BY_FAMILY) == SUPPORTED_FAMILIES + + +@pytest.mark.parametrize( + ("model_family", "why"), + [ + (None, "nobody has read this connection's config yet"), + ("", "somebody read it and it declared nothing"), + ("totally-unknown-net", "this build has no adapter for that type"), + ], +) +def test_nothing_is_declared_where_nothing_is_known(model_family: str | None, why: str) -> None: + """Three ways to know nothing, and one answer: declare nothing. + + They are the same answer to a *client*, which is why they collapse here. What + separates them is the remedy, and a remedy belongs where there is room for a + sentence rather than in a vocabulary something switches on. + """ + assert capabilities_of(model_family) == [], why diff --git a/tests/inference/test_providers.py b/tests/inference/test_providers.py index b7d1523e..3877ed75 100644 --- a/tests/inference/test_providers.py +++ b/tests/inference/test_providers.py @@ -9,20 +9,12 @@ from collections.abc import Iterator from pathlib import Path -from typing import Any import pytest from visionset.inference import providers as providers_module -from visionset.inference.providers import ( - DETECTOR_FAMILIES, - SEGMENTER_FAMILIES, - SUPPORTED_FAMILIES, - ProviderPool, - family_of, - provider_for, - resident, -) +from visionset.inference.families import SUPPORTED_FAMILIES +from visionset.inference.providers import ProviderPool, provider_for, resident from visionset.inference.sam_provider import LocalSamProvider from visionset.inference.transformers_provider import LocalTransformersProvider from visionset.kernel.domain import ConnectionType, InferenceConnection @@ -183,37 +175,6 @@ def test_a_config_that_declares_no_type_is_refused_too( assert all(family in message for family in SUPPORTED_FAMILIES) -def test_an_unreadable_config_answers_empty_rather_than_raising( - connections: InferenceConnectionService, tmp_path: Path, monkeypatch: pytest.MonkeyPatch -) -> None: - """Reading the config and deciding what to do about it stay separate. - - This function's job is to report what the files say; the refusal for "they - say nothing" is the resolver's, one level up. - """ - - class Broken: - class AutoConfig: - @staticmethod - def from_pretrained(*_: Any, **__: Any) -> Any: - raise OSError("nothing in the cache") - - monkeypatch.setattr(providers_module, "imported", lambda _: Broken()) - assert family_of(a_local(connections), cache_dir=tmp_path) == "" - - -def test_both_spellings_of_the_one_architecture_are_named() -> None: - """A set rather than a string, and both members are load-bearing today.""" - assert {"sam2", "sam2_video"} <= SEGMENTER_FAMILIES - - -def test_the_two_families_are_disjoint_and_are_the_whole_of_what_is_supported() -> None: - """What the refusal lists is derived, so a family cannot be added to one set - and forgotten in the message.""" - assert not SEGMENTER_FAMILIES & DETECTOR_FAMILIES - assert SUPPORTED_FAMILIES == SEGMENTER_FAMILIES | DETECTOR_FAMILIES - - def test_an_unsupported_model_leaves_nothing_behind_for_the_next_request( connections: InferenceConnectionService, tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: diff --git a/tests/inference/test_weights.py b/tests/inference/test_weights.py index 82c2c4ea..fa68e94e 100644 --- a/tests/inference/test_weights.py +++ b/tests/inference/test_weights.py @@ -21,7 +21,7 @@ import pytest -from visionset.inference import MODULES, cache_root, fetch_weights +from visionset.inference import MODULES, cache_root, fetch_weights, with_families from visionset.inference import weights as weights_module from visionset.inference.weights import MODELS_DIRNAME, download from visionset.kernel.domain import ( @@ -74,9 +74,21 @@ def an_http(connections: InferenceConnectionService, name: str = "remote") -> An ) +#: What the faked config declares. A real family rather than a placeholder, +#: because the point of recording it is that a client can act on it. +DOWNLOADED_FAMILY = "sam2" + + @pytest.fixture() def fetched(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> list[tuple[str, Path]]: - """Record the download's arguments and write nothing.""" + """Record the download's arguments and write nothing. + + The config read that follows the download is faked here too, and not only + for speed: the real one imports ``transformers``, and + ``test_configuring_a_connection_reaches_no_model_runtime`` asserts that a + full-suite process has not imported it. A fixture that dragged it in would + fail a test three directories away, in a run whose order decided it. + """ seen: list[tuple[str, Path]] = [] def _download(connection: InferenceConnection, *, into: Path) -> Path: @@ -84,6 +96,7 @@ def _download(connection: InferenceConnection, *, into: Path) -> Path: return tmp_path / "snapshot" monkeypatch.setattr(weights_module, "download", _download) + monkeypatch.setattr(weights_module, "family_of", lambda *_, **__: DOWNLOADED_FAMILY) return seen @@ -128,6 +141,77 @@ def test_a_finished_download_leaves_the_connection_ready( assert fetched == [("some/model@abc123", cache_root(workspace.root))] +def test_a_finished_download_records_what_kind_of_model_arrived( + connections: InferenceConnectionService, workspace: WorkspaceService, fetched: list +) -> None: + """The first moment the answer exists without reaching a network. + + A connection is born knowing a model id and nothing about the model. The + config that says what it is arrives with the weights, so this is where it + becomes knowable — and reading it here is what lets a client be told what + this connection can be asked for instead of finding out one refusal at a + time. + """ + made = a_local(connections) + assert made.model_family is None + + assert fetch_weights(workspace, made.id).model_family == DOWNLOADED_FAMILY + assert connections.get(made.id).model_family == DOWNLOADED_FAMILY + + +def test_a_build_that_cannot_read_a_config_still_finishes_the_download( + connections: InferenceConnectionService, + workspace: WorkspaceService, + fetched: list, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The bytes are here; a question about them must not undo the transfer. + + Letting the config read's refusal out left the connection `not_set_up` beside + a full cache — and the remedy on offer was the download that had just + succeeded. It shipped green locally and failed on CI, where the optional + runtime genuinely is absent: every test that downloads through a faked + `download` reached the *real* config read. + + Not knowing is recoverable. The row records nothing rather than nothing-found, + and the next read of the connection asks again. + """ + + def _no_runtime(*_: object, **__: object) -> str: + raise LocalInferenceUnavailable("'transformers' is not installed here") + + monkeypatch.setattr(weights_module, "family_of", _no_runtime) + made = a_local(connections) + + ready = fetch_weights(workspace, made.id) + assert ready.setup_state is ConnectionSetupState.READY + assert ready.model_family is None + assert connections.get(made.id).setup_state is ConnectionSetupState.READY + + +def test_a_re_download_records_a_family_a_row_was_missing( + connections: InferenceConnectionService, + workspace: WorkspaceService, + fetched: list, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The remedy for a row that predates the column, and it is the ordinary action. + + ``download_weights`` is legal at ``ready``, and a run there used to be a + no-op on the row. It is still idempotent — but *idempotent* now means "the + row ends up saying what is true", not "the row is never written", which is + why the early return compares the fields rather than the state. + """ + made = a_local(connections) + fetch_weights(workspace, made.id) + # A row written before a connection recorded any of this. + monkeypatch.setattr(weights_module, "family_of", lambda *_, **__: "") + assert fetch_weights(workspace, made.id).model_family == "" + + monkeypatch.setattr(weights_module, "family_of", lambda *_, **__: DOWNLOADED_FAMILY) + assert fetch_weights(workspace, made.id).model_family == DOWNLOADED_FAMILY + + def test_the_pinned_revision_is_what_is_asked_for( connections: InferenceConnectionService, workspace: WorkspaceService, fetched: list ) -> None: @@ -175,7 +259,11 @@ def test_the_phases_are_reported_in_order( a number that looks like progress without being any.""" said: list[str] = [] fetch_weights(workspace, a_local(connections).id, on_progress=said.append) - assert said == ["fetching some/model at abc123", "recording the connection as ready"] + assert said == [ + "fetching some/model at abc123", + "reading what kind of model arrived", + "recording the connection as ready", + ] def test_reporting_is_optional( @@ -342,3 +430,129 @@ def snapshot_download(**_: object) -> str: ) assert download(made, into=into) == into / "snapshot" assert into.is_dir() + + +# --- the backfill ------------------------------------------------------------- +# +# Every row written before a connection recorded what kind of model it holds has +# to acquire one somewhere, and a migration cannot do it: the answer is in a +# model cache under the workspace, and the kernel — where migrations run — is +# forbidden from reaching one. So it happens here, on the read path, and these +# are the bounds that make that defensible. + + +class _Resolver: + """A stand-in for reading a config, counting how often it is asked.""" + + def __init__(self, answer: str | Exception = "sam2") -> None: + self.answer = answer + self.calls = 0 + + def __call__(self, *_: object, **__: object) -> str: + self.calls += 1 + if isinstance(self.answer, Exception): + raise self.answer + return self.answer + + +def test_a_set_up_row_with_no_family_acquires_one_and_keeps_it( + connections: InferenceConnectionService, + workspace: WorkspaceService, + fetched: list, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The backfill itself, and the bound that makes it cost once.""" + made = a_local(connections) + fetch_weights(workspace, made.id) + _forget_the_family(connections, made.id) + + resolver = _Resolver("sam2") + monkeypatch.setattr(weights_module, "family_of", resolver) + (resolved,) = with_families(workspace, [connections.get(made.id)]) + assert resolved.model_family == "sam2" + assert connections.get(made.id).model_family == "sam2" + assert resolver.calls == 1 + + # Read again: the row has an answer, so nothing looks a second time. + with_families(workspace, [connections.get(made.id)]) + assert resolver.calls == 1 + + +def test_a_config_that_declared_nothing_is_recorded_and_not_asked_again( + connections: InferenceConnectionService, + workspace: WorkspaceService, + fetched: list, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The empty string is a finding, which is why it is worth storing. + + Folding it into NULL would make every read of that connection re-open a + config that has already answered — for the life of the workspace. + """ + made = a_local(connections) + fetch_weights(workspace, made.id) + _forget_the_family(connections, made.id) + + resolver = _Resolver("") + monkeypatch.setattr(weights_module, "family_of", resolver) + with_families(workspace, [connections.get(made.id)]) + assert connections.get(made.id).model_family == "" + + with_families(workspace, [connections.get(made.id)]) + assert resolver.calls == 1 + + +def test_a_build_that_cannot_look_records_nothing( + connections: InferenceConnectionService, + workspace: WorkspaceService, + fetched: list, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A build without the optional runtime has not looked, and must not say it did. + + Recording the empty string here would put "this model declares nothing" on + the row on the strength of never having checked — and a machine that later + installs the runtime would go on believing it, because a row with an answer + is never asked again. + """ + made = a_local(connections) + fetch_weights(workspace, made.id) + _forget_the_family(connections, made.id) + + monkeypatch.setattr( + weights_module, "family_of", _Resolver(LocalInferenceUnavailable("no runtime")) + ) + (resolved,) = with_families(workspace, [connections.get(made.id)]) + assert resolved.model_family is None + assert connections.get(made.id).model_family is None + + +def test_nothing_that_has_no_config_here_is_ever_asked( + connections: InferenceConnectionService, + workspace: WorkspaceService, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """An http connection keeps its model elsewhere; an unfetched one has no files. + + Both would send the resolver looking for a config that cannot be there, and + the empty string it came back with would be recorded as a finding about a + model nobody has ever read. + """ + resolver = _Resolver("sam2") + monkeypatch.setattr(weights_module, "family_of", resolver) + rows = [an_http(connections), a_local(connections)] + + assert [one.model_family for one in with_families(workspace, rows)] == [None, None] + assert resolver.calls == 0 + + +def _forget_the_family(connections: InferenceConnectionService, connection_id: Any) -> None: + """Put a row back the way one written before the column looked. + + Through the service rather than by editing the row, so the state this starts + from is one the shipped code can actually produce: pointing a connection at a + different model forgets what kind of model it was. + """ + current = connections.get(connection_id) + connections.update(connection_id, model_id=current.model_id + "-again") + assert connections.get(connection_id).model_family is None diff --git a/tests/jobs/test_weights_job.py b/tests/jobs/test_weights_job.py index 9096af6f..70fb7862 100644 --- a/tests/jobs/test_weights_job.py +++ b/tests/jobs/test_weights_job.py @@ -95,6 +95,22 @@ def _download(connection: object, *, into: Path) -> Path: return seen +#: What the faked config declares, where a test needs one. +DOWNLOADED_FAMILY = "sam2" + + +@pytest.fixture(autouse=True) +def _the_config_read_is_faked(monkeypatch: pytest.MonkeyPatch) -> None: + """A finished download reads the model's config; here it does not. + + Nothing in this file is about what a config says, and the real read imports + ``transformers`` — which ``test_configuring_a_connection_reaches_no_model_runtime`` + asserts a full-suite process has not done. An unfaked read would fail that + test, in another directory, in a run whose order decided it. + """ + monkeypatch.setattr(weights_module, "family_of", lambda *_, **__: DOWNLOADED_FAMILY) + + # --- registration ------------------------------------------------------------- diff --git a/tests/kernel/test_inference_connections.py b/tests/kernel/test_inference_connections.py index 79671489..d4206bb2 100644 --- a/tests/kernel/test_inference_connections.py +++ b/tests/kernel/test_inference_connections.py @@ -361,3 +361,82 @@ def test_configuring_a_connection_reaches_no_model_runtime() -> None: # module is what a caller does before creating a connection, so this is the # "creating a connection downloads nothing" claim at its narrowest. assert not {"torch", "transformers", "huggingface_hub"} & set(sys.modules) + + +# --- what kind of model a connection points at -------------------------------- + + +def test_a_new_connection_knows_nothing_about_its_model_yet(connections) -> None: # noqa: ANN001 + """Nothing is fetched at creation, so nothing has been read at creation. + + NULL here is "nobody has looked", which is exactly true of a connection + nobody has downloaded — and telling it apart from "looked and found nothing" + is what stops the look from being repeated forever. + """ + assert connections.create("local", **LOCAL).model_family is None + + +def test_recording_the_weights_ready_records_what_they_turned_out_to_be( + connections, # noqa: ANN001 +) -> None: + """The two things the caller has learned, written together. + + The state and the family are one commit because they are one finding: the + weights are here, and this is what they are. + """ + made = connections.create("local", **LOCAL) + ready = connections.record_weights_ready(made.id, model_family="sam2") + assert ready.setup_state is ConnectionSetupState.READY + assert ready.model_family == "sam2" + + +def test_a_caller_that_did_not_find_out_leaves_the_answer_alone(connections) -> None: # noqa: ANN001 + """`None` means *I did not find out*, on `update`'s convention. + + A caller that cannot read a config must not be able to erase an answer + somebody else read — silence is not a finding. + """ + made = connections.create("local", **LOCAL) + connections.record_weights_ready(made.id, model_family="sam2") + assert connections.record_weights_ready(made.id).model_family == "sam2" + + +def test_recording_a_family_on_an_already_ready_connection_writes_it( + connections, # noqa: ANN001 +) -> None: + """The idempotent early return compares the fields, never the state alone. + + This is the whole backfill path: a row written before the column existed is + `ready` already, so a guard that returned on the state would silently drop + the one thing the caller came to record. + """ + made = connections.create("local", **LOCAL) + connections.record_weights_ready(made.id) + assert connections.record_weights_ready(made.id, model_family="sam2").model_family == "sam2" + assert connections.get(made.id).model_family == "sam2" + + +def test_pointing_a_connection_at_another_model_forgets_what_kind_it_was( + connections, # noqa: ANN001 +) -> None: + """A stale family reads exactly like a fresh one, which is why it is dropped. + + The answer was read out of the old model's config; nothing has read the new + one. Keeping it would leave the row declaring what its *previous* weights + could be asked for, and every client filtering on that declaration would + believe it. + """ + made = connections.create("local", **LOCAL) + connections.record_weights_ready(made.id, model_family="sam2") + + assert connections.update(made.id, model_id="other/model").model_family is None + connections.record_weights_ready(made.id, model_family="grounding-dino") + assert connections.update(made.id, model_revision="beef1234").model_family is None + + +def test_editing_anything_else_keeps_the_family(connections) -> None: # noqa: ANN001 + """Renaming a connection or moving it to another device changes no weights.""" + made = connections.create("local", **LOCAL) + connections.record_weights_ready(made.id, model_family="sam2") + assert connections.update(made.id, name="renamed").model_family == "sam2" + assert connections.update(made.id, device="cpu", precision="fp32").model_family == "sam2" diff --git a/tests/kernel/test_migrations.py b/tests/kernel/test_migrations.py index 143709dc..ca8b4841 100644 --- a/tests/kernel/test_migrations.py +++ b/tests/kernel/test_migrations.py @@ -178,6 +178,7 @@ def _at_generation_one(path: Path) -> None: connection.execute(text("ALTER TABLE batch DROP COLUMN parent_batch_id")) connection.execute(text("ALTER TABLE annotation DROP COLUMN job_id")) connection.execute(text("ALTER TABLE annotation_schema DROP COLUMN provenance")) + connection.execute(text("ALTER TABLE inference_connection DROP COLUMN model_family")) connection.execute(text(f"UPDATE {META_TABLE} SET format_version = 1")) store.close() diff --git a/tests/server/test_inference.py b/tests/server/test_inference.py index 794ffdd8..c9265a00 100644 --- a/tests/server/test_inference.py +++ b/tests/server/test_inference.py @@ -80,6 +80,22 @@ def _download(connection: Any, *, into: Path) -> Path: return seen +#: What the faked config declares, where a test needs one. +DOWNLOADED_FAMILY = "sam2" + + +@pytest.fixture(autouse=True) +def _the_config_read_is_faked(monkeypatch: pytest.MonkeyPatch) -> None: + """A finished download reads the model's config; here it does not. + + Nothing in this file is about what a config says, and the real read imports + ``transformers`` — which ``test_configuring_a_connection_reaches_no_model_runtime`` + asserts a full-suite process has not done. An unfaked read would fail that + test, in another directory, in a run whose order decided it. + """ + monkeypatch.setattr(weights_module, "family_of", lambda *_, **__: DOWNLOADED_FAMILY) + + def created(client: TestClient, body: dict[str, Any]) -> dict[str, Any]: response = client.post("/inference/connections", json=body) assert response.status_code == 201, response.text @@ -703,3 +719,82 @@ def test_a_size_without_the_runtime_carries_the_install_command(client: TestClie body = response.json() assert body["code"] == "LOCAL_INFERENCE_UNAVAILABLE" assert 'pip install "visionset[local-inference]"' in body["message"] + + +# --- what its model can be asked for ------------------------------------------ +# +# A second declaration beside `allowed_actions`, answering a different question: +# an action is something to do *to* this connection, a capability is what its +# model answers. A client offering a tool needs both, and being ready says the +# files are here rather than that they are the right kind of model. + + +def test_a_connection_declares_what_its_model_can_be_asked_for( + tmp_path: Path, runtime_present: None, fetched: list[str] +) -> None: + """Read off the wire, and only after a download has read the config.""" + with api_client(tmp_path / "ws", dispatcher=InlineDispatcher()) as client: + assert _made_ready(client)["capabilities"] == ["point_suggest"] + + +def test_a_connection_whose_weights_never_arrived_declares_nothing( + client: TestClient, +) -> None: + """Nothing is fetched at creation, so nothing has been read at creation. + + The empty list is not a refusal: the server still judges every request on + its own. It says only that no client can rely on this connection for a + particular tool yet. + """ + assert created(client, LOCAL)["capabilities"] == [] + + +def test_an_http_connection_declares_nothing_yet(client: TestClient) -> None: + """`ready` on arrival and still capable of nothing a client may rely on. + + An HTTP connection's model runs elsewhere, and how a remote endpoint states + what it can do is the remote-contract slice's question. Until it is + answered, the honest declaration is the empty one — the two states this + resource can be in are not the two questions being asked. + """ + made = created(client, HTTP) + assert made["setup_state"] == "ready" + assert made["capabilities"] == [] + + +def test_a_row_written_before_the_column_is_resolved_on_its_first_read( + tmp_path: Path, runtime_present: None, fetched: list[str], monkeypatch: pytest.MonkeyPatch +) -> None: + """The backfill, over HTTP, with its bound: once per row, then never again. + + Reached through the listing because that is the client that needs it — the + editor reads the list to decide which connection a click can go through, and + a row that predates the column would otherwise be invisible to every tool + for the life of the workspace. + """ + with api_client(tmp_path / "ws", dispatcher=InlineDispatcher()) as client: + made = _made_ready(client) + # A row the way one written before the column looked: still `ready`, + # with nothing recorded about what kind of model it holds. The edit's own + # answer is the honest empty one — it is a write, and nothing has read + # the new model's config. + patched = client.patch( + f"/inference/connections/{made['id']}", json={"model_id": "other/model"} + ) + assert patched.json()["capabilities"] == [] + + reads: list[str] = [] + + def _read(connection: Any, **_: Any) -> str: + reads.append(connection.model_id) + return "grounding-dino" + + monkeypatch.setattr(weights_module, "family_of", _read) + listed = client.get("/inference/connections").json()["items"] + assert [one["capabilities"] for one in listed] == [["text_detect"]] + assert reads == ["other/model"] + + # And the answer is now on the row, so no read of it looks again. + client.get("/inference/connections") + client.get(f"/inference/connections/{made['id']}") + assert reads == ["other/model"]