From d9123d85a58a8ef3778bb568dabe6a7cf2016fc0 Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya Date: Sun, 9 Aug 2026 20:57:04 -0700 Subject: [PATCH 1/4] feat(inference): a connection declares what its model can be asked for MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A connection row said where a model runs and whether its weights had arrived, and nothing anywhere said what it could be asked to do. The family is already resolved from the model's own config on every provider build, used to pick an adapter, and then thrown away — so no surface could publish it and no client could filter on it. - `InferenceConnection` records `model_family`, written when it becomes knowable: at download completion, from the config that arrived with the weights. Three states, and the third earns its place — NULL is "nobody has looked", "" is "looked and it declared nothing", so a look that found nothing is not repeated on every read. - `families.py` holds the family sets and the family-to-capability map, derived from those sets rather than listed again: an adapter and its declaration are now one edit. - `ConnectionOut` and `wire.connection` publish `capabilities`, empty wherever nothing is known — including an `http` connection, which declares nothing until the remote contract says how an endpoint states what it can do. - Migration 7 adds the column. It cannot backfill: the answer lives in a model cache the kernel is forbidden to reach. So the read path fills it in once per pre-existing row, from files already on this disk, and a build without the optional runtime records nothing rather than recording that it found nothing. - Editing a connection's model or revision forgets the family. A stale answer about the previous weights reads exactly like a fresh one. --- frontend/ui-core/src/generated/api.ts | 17 ++ frontend/ui-core/src/generated/checks.ts | 5 +- openapi.json | 21 +- src/visionset/inference/__init__.py | 22 +- src/visionset/inference/families.py | 138 +++++++++++++ src/visionset/inference/providers.py | 78 +------ src/visionset/inference/weights.py | 91 ++++++++- src/visionset/kernel/adapters/_mappers.py | 4 + src/visionset/kernel/adapters/_tables.py | 10 + src/visionset/kernel/adapters/migrations.py | 22 ++ src/visionset/kernel/domain/__init__.py | 2 + src/visionset/kernel/domain/inference.py | 46 +++++ .../services/inference_connection_service.py | 49 +++-- src/visionset/server/models.py | 20 ++ src/visionset/server/routes/inference.py | 23 ++- src/visionset/wire/__init__.py | 11 + tests/inference/test_families.py | 172 ++++++++++++++++ tests/inference/test_providers.py | 43 +--- tests/inference/test_weights.py | 190 +++++++++++++++++- tests/jobs/test_weights_job.py | 16 ++ tests/kernel/test_inference_connections.py | 79 ++++++++ tests/kernel/test_migrations.py | 1 + tests/server/test_inference.py | 95 +++++++++ 23 files changed, 1006 insertions(+), 149 deletions(-) create mode 100644 src/visionset/inference/families.py create mode 100644 tests/inference/test_families.py 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/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..5598b0f3 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,81 @@ 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_of(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 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. + """ + 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 + try: + family = family_of(connection, cache_dir=cache) + except LocalInferenceUnavailable: + 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..9d6c2eaa 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,47 @@ 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_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 +229,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 +400,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"] From 122ec6879c223150dd1f37119188f67f159ffbb7 Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya Date: Sun, 9 Aug 2026 21:04:49 -0700 Subject: [PATCH 2/4] fix(annotator): the suggest tool picks a model that can answer, and lets you choose MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `usableConnection` was `find(row => row.setup_state === "ready")`. With a text-prompt detector as the only ready connection, every click round-tripped and came back with a truthful `UnsupportedPrompt` refusal — the tool was offered where it could never work, one click at a time. - Candidates are the ready connections declaring `point_suggest`. The declaration comes off the wire; nothing here re-derives it from a model id. - A fourth blocker says so when connections are ready and none can answer, ranked *below* `not-ready` — an undownloaded connection has no capability yet, and "wrong kind of model" would be the wrong sentence for one whose weights simply have not arrived. - Where more than one can answer, the idle card carries a picker, remembered per project in `prefs`. One candidate renders the line naming it and no control; a remembered choice that is no longer a candidate falls back rather than blocking the tool over a connection somebody deleted. - The picker is on the idle card only. Changing which model answers while an answer is on screen would leave a proposal nothing on the card explains. The request still carries an explicit `connection_id` and the server still refuses a wrong-family one. The filter is convenience; the kernel is the law. --- .../ui-core/src/annotator/AnnotationPage.tsx | 43 ++++- .../ui-core/src/annotator/SuggestPanel.tsx | 108 ++++++++++++- .../src/annotator/suggestFlow.test.tsx | 104 +++++++++++- .../src/annotator/suggestPanel.test.tsx | 149 +++++++++++++++++- frontend/ui-core/src/data/inferenceQueries.ts | 69 ++++++-- .../ui-core/src/screens/inference.test.tsx | 4 + 6 files changed, 454 insertions(+), 23 deletions(-) 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/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, From 9477f076075579961493a3fd1894ef5c612d6be9 Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya Date: Sun, 9 Aug 2026 21:20:00 -0700 Subject: [PATCH 3/4] docs: a connection declares what it can be asked for MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - `inference.md` gains the capability vocabulary, where the value comes from (the model's own config, never its name), what empty means and when it is recorded — plus the editor case that reads oddly without it: a workspace can have a connection configured, downloaded and running and still have nothing that can answer a click. - `api.md` separates the two declarations. An action is something to do *to* a resource and follows its state; a capability is what its model answers and follows the weights. Offering a tool needs both. - `ui.md` describes the suggest tool's connection: the narrower candidate set, the picker where there is a choice, and why the choice is per project and per browser rather than a workspace setting. - `persistence.md`'s migration list was two generations behind. Correcting it also corrects the table-creating exception, which is migrations 4 *and* 6, and records the rule migration 7 is the first case of: where a value is unknowable inside the kernel, the column arrives NULL and says who fills it in. - `architecture/backend/inference.md` records why the family sets and the capability map share a module, and why the backfill is on the read path. - `architecture/backend/wire.md` names its one non-kernel import. --- docs/api.md | 18 ++++++++++ docs/architecture/backend/inference.md | 30 ++++++++++++++++ docs/architecture/backend/wire.md | 7 ++++ docs/inference.md | 47 ++++++++++++++++++++++++-- docs/persistence.md | 17 +++++++--- docs/ui.md | 16 +++++++++ 6 files changed, 129 insertions(+), 6 deletions(-) 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 | From 5ae71a80f841e515cb7b8e8819dd82ba8f974300 Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya Date: Sun, 9 Aug 2026 21:44:25 -0700 Subject: [PATCH 4/4] fix(inference): a download that worked is not undone by a question about it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI runs without the `local-inference` extra, so the config read that follows a download raised `LocalInferenceUnavailable` — and took the download with it. The connection landed back at `not_set_up` beside a full cache, offering as its remedy the transfer that had just succeeded. Fifteen tests, all of them downloading through a faked `download` into the real config read. Not knowing is recoverable and is now what gets recorded: `None` rather than a refusal, so the next read of the connection asks again. It is the same rule `with_families` already followed for a build that cannot look, and both now go through one function so there is a single answer to what such a build records. --- src/visionset/inference/weights.py | 31 ++++++++++++++++++++++++++---- tests/inference/test_weights.py | 30 +++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 4 deletions(-) diff --git a/src/visionset/inference/weights.py b/src/visionset/inference/weights.py index 5598b0f3..1a32e3ba 100644 --- a/src/visionset/inference/weights.py +++ b/src/visionset/inference/weights.py @@ -140,11 +140,33 @@ def fetch_weights( # 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_of(connection, cache_dir=cache) + family = _family_if_it_can_be_read(connection, cache_dir=cache) say("recording the connection as ready") 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]: @@ -170,6 +192,8 @@ def with_families( 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) @@ -178,9 +202,8 @@ def with_families( if not _awaiting_a_family(connection): resolved.append(connection) continue - try: - family = family_of(connection, cache_dir=cache) - except LocalInferenceUnavailable: + 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) diff --git a/tests/inference/test_weights.py b/tests/inference/test_weights.py index 9d6c2eaa..fa68e94e 100644 --- a/tests/inference/test_weights.py +++ b/tests/inference/test_weights.py @@ -159,6 +159,36 @@ def test_a_finished_download_records_what_kind_of_model_arrived( 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,