Skip to content

fix(inference): editing a connection's model sends it back for a download - #491

Merged
JArmandoAnaya merged 2 commits into
mainfrom
fix/edit-resets-setup
Aug 10, 2026
Merged

fix(inference): editing a connection's model sends it back for a download#491
JArmandoAnaya merged 2 commits into
mainfrom
fix/edit-resets-setup

Conversation

@JArmandoAnaya

Copy link
Copy Markdown
Contributor

A local connection stayed ready when its model reference was edited, over
weights that belong to the model it no longer names. setup_state answers are
the weights here
, so the stored answer was describing the wrong question.

Two commits: the fix and its tests, then the doc.

The wrong value did not stay put

It would be a stale badge if setup_state were only displayed. It is what two
other answers are derived from, so the error propagated into places that look
independent of it:

  • allowed_actions. check_integrity re-reads a snapshot, and it stayed
    declared over a reference whose snapshot was never fetched — an action whose
    honest result is not a verdict at all.
  • The family backfill's bound. with_families looks at local + ready +
    never-asked, and an edited row satisfied all three because feat(inference): connections declare what their model can be asked for #486 correctly
    forgets the family. It resolved the new model id against a cache that could not
    hold it and persisted ""looked and it declared nothing — which is the
    value kept distinct from None precisely so a fruitless look is not repeated.
    A later download repaired it, which made the window quiet rather than harmless.

The suggest tool was already safe, and only by accident: the forgotten family
left capabilities empty, so #486's filter skipped the row. It was protected by
a fact about a different column, not by this one being right.

Compared, not counted — and this is the part I did not expect

The obvious rule is "the model reference was supplied, so reset". That would have
shipped a worse bug than the one being fixed.

useUpdateConnection takes the whole ConnectionInput and bodyOf(input) sends
every field, so a rename PATCHes model_id with the value it already had.
Under the obvious rule, renaming a set-up connection would send it back for a
download of weights that never left — from the one client that exists, on its
commonest edit.

So the reset compares values. That also repairs the same over-reach in #486's
family reset, which until now dropped the family on every edit the form made.
It self-healed on the next read, which is why nothing caught it.

test_resupplying_the_same_model_reference_is_not_a_change was red before the
fix for exactly that reason.

What the reset does not do

  • The previous model's blobs stay in the cache. It is keyed by model and
    shared, another connection may reference the same one, and pointing a
    connection back at something it used to name then costs a cache hit rather
    than a second transfer. test_editing_back_and_downloading_again_restores_the_family
    walks that round trip.
  • http connections are untouched. They hold no weights here, and ready
    for that kind has always meant there is nothing to set up on this machine
    a fact about the kind, not about a download. Sending one to not_set_up would
    offer a remedy it cannot perform.
  • No UI change. The mutation already invalidated the list, which is what
    carries the new state across; the test below is what proves it does.

Two tests changed rather than broke

_forget_the_family (inference) and the server backfill test both manufactured
"a ready row with no family" by editing the model. The fix makes that
manufacture impossible, which is the point — after it, the only producers of that
state are a migration over a row that predates the column, and a machine whose
optional runtime cannot read a config, which downloads successfully and records
that it could not look.

Both now use the second, which is reachable from a test and lands on the same
row. The helper says so, because the next reader will otherwise reach for the
edit again.

Mutation verification

Each applied to the committed tree, anchor asserted present-once before and gone
after, reverted by its exact patch.

Mutation Result
the state reset removed, family reset kept 4 red — both kernel reproductions, the round trip, and the wire's allowed_actions
the reset widened to any supplied field, including name 4 red — the two non-model-edit tests, the resupply test, and the rename over HTTP
useUpdateConnection stops invalidating the list 1 red — the edited row keeps showing Ready

Found, not fixed

test_configuring_a_connection_reaches_no_model_runtime asserts a process-global
sys.modules fact, so it is order-dependent across the whole suite: collecting
tests/server and tests/inference in the same process imports the runtime
before it runs. Carried over from #486, reproduced on unmodified main at the
merge-base with the identical command, and it does not reach CI, which runs the
directories separately. cf. #486.

Test plan

Full local gate, staged for the harness ceiling, every stage exit 0.

Stage Result
pytest tests/architecture 21 passed
pytest tests/cli 331 passed, 2 skipped
pytest tests/examples 41 passed
pytest tests/formats 151 passed, 2 skipped
pytest tests/inference 171 passed, 3 skipped
pytest tests/jobs 30 passed
pytest tests/kernel 1421 passed, 4 skipped
pytest tests/mcp 246 passed
pytest tests/packaging 3 passed, 1 skipped
pytest tests/server 644 passed, 2 skipped
pytest tests/test_versioning.py 3 passed
ruff check . / ruff format --check . passed / 361 files formatted
mypy src/visionset 153 files, no issues
lint-imports 4 kept, 0 broken
pnpm -r build / pnpm -r lint passed
pnpm test annotator 914, ui-core 876, node gates 77
scripts/check.sh generated passed
scripts/check.sh browser both suites passed, 109s

Plus the two runs that match CI rather than this machine:

  • the whole suite in one process with no optional runtime (uv sync with no
    extras; torch, transformers and huggingface_hub all genuinely absent) —
    3089 passed, 9 skipped;
  • test(inference): a CI job runs the inference suite with the runtime installed #490's new inference-smoke job, run as it runs it — the extra installed,
    VISIONSET_REQUIRE_LOCAL_INFERENCE=1, HF_HUB_OFFLINE=1274 passed,
    7 skipped
    , the skips all missing-GPU.

openapi.json and the generated client are unchanged: no wire shape moved, only
the value a row reports.

Rebased onto 7fb01ea after #490 landed mid-flight; no conflicts.

Closes #487

…load

`setup_state` answers *are the weights here*, and the weights on disk belong to
the model reference the connection was pointing at. Editing `model_id` or
`model_revision` left the row `ready` over files nothing had ever fetched.

The wrong value did not stay put: `allowed_actions` is derived from it, so a
connection with no snapshot went on offering `check_integrity`; and the family
backfill is bounded on it, so an edited row was eligible to have its *new* model
resolved against a cache that could not hold it, persisting "looked and found
nothing" as a finding nobody was in a position to make.

A model-reference edit now returns a weight-holding connection to `not_set_up`
alongside the family it already forgot. Downloading again is the remedy and was
already among the actions such a row offers, and the previous model's blobs stay
in the cache — it is keyed by model, so pointing a connection back at something
it used to name costs a cache hit rather than a transfer.

Compared rather than merely supplied. The app's edit form PATCHes the whole
shape, so a rename arrives carrying the model id it already had; reading that as
a move would have sent a set-up connection back for a download of weights that
never left. That also repairs the same over-reach in the family reset, which
until now dropped the family on every edit the form made.

An `http` connection holds no weights here and is unaffected.
…nload

The behaviour has a user-visible remedy and a rule about what does *not*
trigger it, so it gets a section of its own rather than a clause: what resets,
why the previous model's files are left in the cache, and that a name, a device
or an unchanged reference resets nothing. The capability paragraph and the
screen's editing paragraph point at it instead of restating it.
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.

Editing a ready connection's model reference leaves it ready with absent weights

1 participant