fix(inference): editing a connection's model sends it back for a download - #491
Merged
Conversation
…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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A local connection stayed
readywhen its model reference was edited, overweights that belong to the model it no longer names.
setup_stateanswers arethe 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_statewere only displayed. It is what twoother answers are derived from, so the error propagated into places that look
independent of it:
allowed_actions.check_integrityre-reads a snapshot, and it stayeddeclared over a reference whose snapshot was never fetched — an action whose
honest result is not a verdict at all.
with_familieslooks 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 thevalue kept distinct from
Noneprecisely 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
capabilitiesempty, so #486's filter skipped the row. It was protected bya 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.
useUpdateConnectiontakes the wholeConnectionInputandbodyOf(input)sendsevery field, so a rename PATCHes
model_idwith 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_changewas red before thefix for exactly that reason.
What the reset does not do
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_familywalks that round trip.
httpconnections are untouched. They hold no weights here, andreadyfor 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_upwouldoffer a remedy it cannot perform.
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
readyrow with no family" by editing the model. The fix makes thatmanufacture 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.
allowed_actionsnameuseUpdateConnectionstops invalidating the listReadyFound, not fixed
test_configuring_a_connection_reaches_no_model_runtimeasserts a process-globalsys.modulesfact, so it is order-dependent across the whole suite: collectingtests/serverandtests/inferencein the same process imports the runtimebefore it runs. Carried over from #486, reproduced on unmodified
mainat themerge-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.
pytest tests/architecturepytest tests/clipytest tests/examplespytest tests/formatspytest tests/inferencepytest tests/jobspytest tests/kernelpytest tests/mcppytest tests/packagingpytest tests/serverpytest tests/test_versioning.pyruff check ./ruff format --check .mypy src/visionsetlint-importspnpm -r build/pnpm -r lintpnpm testscripts/check.sh generatedscripts/check.sh browserPlus the two runs that match CI rather than this machine:
uv syncwith noextras; torch, transformers and huggingface_hub all genuinely absent) —
3089 passed, 9 skipped;
inference-smokejob, run as it runs it — the extra installed,VISIONSET_REQUIRE_LOCAL_INFERENCE=1,HF_HUB_OFFLINE=1— 274 passed,7 skipped, the skips all missing-GPU.
openapi.jsonand the generated client are unchanged: no wire shape moved, onlythe value a row reports.
Rebased onto
7fb01eaafter #490 landed mid-flight; no conflicts.Closes #487