Skip to content

feat(inference): ModelProvider port, local adapter, weight download (#418 slice 2) - #434

Merged
JArmandoAnaya merged 1 commit into
mainfrom
feat/model-provider
Aug 8, 2026
Merged

feat(inference): ModelProvider port, local adapter, weight download (#418 slice 2)#434
JArmandoAnaya merged 1 commit into
mainfrom
feat/model-provider

Conversation

@JArmandoAnaya

Copy link
Copy Markdown
Contributor

The ModelProvider port re-shaped from a placeholder into a designed one, the first adapter that
implements it, and the weight download that takes a local connection from not_set_up to ready.

cf. #418 (the port), cf. #421 (the connections it is instantiated from), cf. #424 (the first
consumer). This is slice 2 of #418 and does not complete it — the hosted adapter is a later slice.

What changed

  • The port (kernel/ports/model_provider.py) — was
    predict(asset, schema) -> Sequence[Annotation], declared to fix a name and documented as a
    placeholder. It failed ModelProvider: declare the kernel port for autolabeling — local and cloud adapters both ordinary #418's dual test (must hold for a local runner and a hosted service) in
    three ways, and each is answered: per asset → per batch, so a hosted provider pays one round
    trip for a chunk rather than one per image; a materialised sequence → an iterator, so a slow
    provider has somewhere to be and a caller reports progress between yields without the port naming
    a reporter, a queue or a future; an Annotation → an AssetPrediction, because a model
    produces a claim and not a schema-validated label.
  • New domain shapes (kernel/domain/prediction.py) — PredictionRequest / PredictionTarget /
    TextPrompt / PointPrompt / AssetPrediction / PredictedRegion. A target carries bytes,
    not a path: handing over an Asset and letting the provider read the file is precisely the
    spelling a remote adapter cannot implement.
  • visionset.inference (new package, sibling of formats / wire / jobs) —
    LocalTransformersProvider, cross-box NMS, the fp16 shims, the weight download, and
    provider_for(), the composition root that resolves a connection by kind.
  • The local-inference extra — torch, transformers, accelerate, huggingface_hub. Every one is
    reached from inside a function, and an architecture test proves the base import graph stays
    clear of all four in a fresh interpreter.
  • Capabilitydownload_weights joins ConnectionAction, legal iff the kind is local and
    the state is not_set_up, in the same change as the route, the command and the job that perform
    it. test is still absent.
  • SurfacesPOST /inference/connections/{id}/download (202 + Location, the export route's
    contract) and visionset inference download <name|id> with --json. openapi.json and the
    generated TS client are regenerated and committed.
  • Docsdocs/inference.md gains the extra, the download flow, the cache location, the CPU
    fallback and six new error rows; docs/install.md gains a section; docs/cli.md and
    docs/README.md follow.

No annotation is written anywhere in this slice, no HTTP adapter, no test action, and no UI.

The two spike findings, as code with tests

Both are from #418's findings comment, and both have a test that fails if the code is removed.

  1. fp16 needs shims. torch.autocast around the forward plus a grid_sample that casts its
    sampling grid to the input's dtype — autocast does not cover grid_sample, which is why the
    spike hit it inside deformable attention. Scoped to the forward and restored in a finally,
    unlike the spike's permanent module-level swap. Proved twice: tests/inference/test_fp16.py
    drives the rule with stand-ins that carry a dtype and a to (runs everywhere, including with
    no torch installed), and
    test_a_half_precision_grid_sample_survives_a_float32_grid_on_cuda reproduces the original
    failure against real half-precision CUDA tensors — skipped without a GPU, so it does not run in
    CI
    .
  2. Raw output needs cross-box NMS. On by default, threshold configurable, applied before
    anything leaves the adapter. Cross-box rather than per-label, because a multi-phrase prompt
    finds one animal and calls it two things.

Design decisions worth review

The port names no entry-point group, and that is the answer rather than an omission. #418 asks
for one or an argument against. A format is a plugin — a third party ships one and the product must
discover it. A provider is not: the recorded decision is that adapters are instantiated from
user-created connections, so InferenceConnection is the registry, and a provider discovered by
entry point would have nothing to be instantiated from. Worse, a workspace could acquire the ability
to predict through an unrelated pip install, which is what "VisionSet never downloads a model on
its own" exists to prevent. Resolution is a match on ConnectionType in the composition root.

PointPrompt ships unserved. The port takes a discriminated prompt — words or a place on the
image — and this adapter is a detector, so it refuses points with UNSUPPORTED_PROMPT. #424 is the
next issue and is point-prompted; designing the payload for one way of asking would mean reshaping
the port for it. Neither variant encodes a model's API: the model-specific knobs (per-head
thresholds, tokenizer punctuation) stay in the adapter, and the port carries only
minimum_confidence, which every model can honour.

retrying on the download gate. sweep_orphans re-enqueues an idempotent orphan as a new
job, so a crash between the state flip committing and the row settling produces a second run against
a connection that is now ready. Refusing it would fail a job whose work is done. retrying=True
relaxes the state half of the gate and only that half — a kind with no weights of its own has
none on the second attempt either — and it is passed by the job handler and by nobody else.

Two 5xx errors, on MediaToolUnavailable's licence. LOCAL_INFERENCE_UNAVAILABLE and
INFERENCE_CONNECTION_NOT_RUNNABLE are conditions of the installation, not of the request: no state
change and no retry makes either succeed, so 409 would promise something it cannot keep and 503
would promise transience. Both expose their message because the message is the remedy — the exact
pip install for the first, and which kind this build has no adapter for in the second.

download_weights stays declared on a machine without the extra. Whether torch is installed is
not a fact about the connection, and no pure function over domain values can see it. The action is
offered and the request is refused with the install command, which is #421's stated design and
design principle 9 — never a bare disabled control.

Deliberate exclusions

Mutation verification

Both rules were broken deliberately, with the anchor asserted present-exactly-once beforehand, and
reverted by exact recorded diff (git apply -R). The tree was committed before the first mutation
and verified clean after each revert.

Rule broken Mutation Test that went red
Cross-box NMS dropped the suppressed(...) call from regions_from tests/inference/test_provider.py::test_duplicate_detections_are_suppressed_before_they_leave_the_adapter
The download_weights gate widened CONNECTION_GATES[DOWNLOAD_WEIGHTS] to EVERY_SETUP_STATE tests/kernel/test_capabilities.py::test_download_weights_is_declared_on_exactly_one_square

Test plan — staged gate, exit codes verbatim

Run in stages because the harness kills long commands; pytest stages derived from ls tests/ at run
time. Toolchain: Node 24.19.0 + pnpm 10.30.2 (the pinned pair).

Stage Exit
ruff check . 0
ruff format --check . 0
mypy src/visionset 0
lint-imports (4 contracts kept, 0 broken) 0
pytest tests/architecture tests/fixtures tests/formats tests/inference tests/jobs tests/packaging tests/scripts tests/test_versioning.py 0 (283 passed, 11 skipped)
pytest kernel part A (annotation … capabilities) 0 (311 passed, 3 skipped)
pytest kernel part B (concurrency … ingest_service) 0 (345 passed, 12 skipped)
pytest kernel part C (job_concurrency … project_service) 0 (245 passed)
pytest kernel part D (release … workspace_service) 1 — pre-existing only, see below
pytest tests/cli 0 (319 passed, 1 skipped)
pytest tests/server 1 — pre-existing only, see below
pytest tests/mcp 0 (244 passed, 1 skipped)
pytest tests/examples 0 (27 passed, 2 skipped)
export_openapi.py + git diff --exit-code openapi.json 0
export_mcp_tools.py --check 0
pnpm generate:client:check 0
pnpm version:check 0
pnpm -r build 0
pnpm test 0 (77 tests — includes the docs-link gate over the new sections)
pnpm -r lint 0
CI=1 playwright test 0 (227 passed)
CI=1 playwright test -c playwright.cycle.config.ts 0 (1 passed)

The two non-zero stages are the same pre-existing environment gap and are not caused by this
branch: ffmpeg/ffprobe are not installed on this machine
, so every failure is
MediaToolUnavailable. Ten failures in total — 9 in tests/kernel/test_video_processor.py and
tests/kernel/test_source_service.py, 1 in tests/server/test_sources.py. Identical set, identical
test ids, to the ones recorded on #430 against clean origin/main at 08e1483. CI has ffmpeg and
is the arbiter.

One flake, named because a reader of a transcript would otherwise see it. Kernel part B first
came back with 91 errors, all FileNotFoundError: /tmp/pytest-of-anaya/pytest-60 — pytest's own
numbered-tmpdir garbage collection removing the running session's root, not a code failure. Re-run
alone: 345 passed, exit 0. That is the number in the table.

The lockfile

uv.lock gains eight packages (accelerate, hf-xet, huggingface-hub, regex, safetensors, tokenizers,
tqdm, transformers) and no incidental upgrades. torch and numpy were already locked as
transitive dependencies of the yolo group.

The cool-down was applied and it caught something: bash scripts/cooldown.sh uv lock selected
huggingface-hub 1.26.0, while an unrestricted resolve took 1.27.0, published 2026-08-07 —
inside the three-day window. The lock carries 1.26.0.

Worth knowing for the next person: uv lock under the wrapper writes exclude-newer into the
lockfile itself
, and uv sync --locked then refuses it with "Ignoring existing lockfile due to
removal of global exclude newer"
— which is what CI runs. So the sequence used here was: lock under
the cutoff to learn which versions the rule permits, lock without it, then uv lock -P "huggingface-hub==1.26.0" to hold the one version that differed. The result satisfies the cool-down
and uv sync --locked passes. Every added package's newest wheel upload-time in the committed
lock is older than the cutoff; this was checked rather than assumed.

Found, not fixed

  • ffmpeg/ffprobe are absent on this development machine, which makes 10 media tests unrunnable
    locally. An environment gap rather than a defect, recorded here because a reader of this gate
    record should not have to rediscover why two stages are non-zero. Unchanged since feat(inference): InferenceConnection entity, persistence, wire, REST, CLI (#421 slice 1) #430.
  • scripts/cooldown.sh documents uv lock as a supported wrapped command, but a lockfile produced
    that way is rejected by uv sync --locked. The workaround above is in this PR; the script's
    guidance is not touched, because fixing it is not this slice's scope.

…418 slice 2)

The port re-shaped from a placeholder into a designed one, the first adapter
that implements it, and the download that takes a local connection from
not_set_up to ready.

The port was predict(asset, schema) -> Sequence[Annotation], which failed the
dual test #418 sets in three ways. Each is answered: per asset -> per batch, so
a hosted provider pays one round trip for a chunk rather than one per image; a
materialised sequence -> an iterator, so a slow provider has somewhere to be and
a caller reports progress between yields without the port naming a reporter, a
queue or a future; an Annotation -> an AssetPrediction, because a model produces
a claim rather than a schema-validated label. A target carries bytes, not a
path: handing over an Asset and letting the provider read the file is exactly
the spelling a remote adapter cannot implement.

visionset.inference is a new sibling of formats, wire and jobs, and every heavy
import lives inside a function so a base install still starts a server and runs
a worker. The two Phase 0 findings ship as code with tests: fp16 needs autocast
plus a grid_sample dtype cast, and raw output needs cross-box NMS.

download_weights joins ConnectionAction in the same change as the route, the
command and the job that perform it, gated on local + not_set_up. There is no
test action and no annotation writing anywhere in this slice.

cf. #418, #421, #424
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant