feat(inference): the integrity check joins the download on the wire - #496
Merged
Conversation
The check still held a client-side job id, so a reload lost one in flight and a check started from a terminal was invisible to every browser — the failure #492 removed from the download, still present on the other action over the same files. The connection now carries `integrity_check` beside `download`, through the same discovery rather than a copy of it: one job type constant and one payload key in the domain, one `ConnectionJob.of` body that identifies a run, and one queue read that answers both kinds for every row on a screen's poll path. The two shapes name different units because they count different things. A transfer measures bytes off the disk; a check owns its loop and knows how many files the revision holds before it opens the first one, so it reports files — determinate, from the counts `jobs/integrity.py` was already reporting. Neither borrows the other's name at any layer, which is why the wire carries two shapes rather than one with a discriminator. No new `ConnectionSetupState` member, for the download's reason verbatim: the state flip is the last statement, so a third member reopens the crash window that ordering closes. #475's semantics are untouched — purge-first stands, and a hub that cannot be reached is still a failure with prose, no verdict, no purge and no state change. What changes is that the sentence outlives the request. `allowed_actions` is unchanged and pinned. A connection with a check in flight is still `ready`, so it still declares `check_integrity` and `download_weights`; nothing in the kernel refuses a second concurrent run. Making the declaration job-aware would give `connection_actions` a third dimension, which is a design decision rather than a consequence of this one.
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.
Closes #495.
One PR, not two. The wire diff is one field of one existing model on one
already-shared route, and the frontend change is the one that gives it a reader.
#493/#494 split because the download's backend half was a new sampler thread, a
new size lookup and a new port filter — a diff worth landing and looking at alone.
This is the shape that PR built being used a second time, and splitting it would
mean a merged commit whose only effect is a field nothing reads.
What was wrong
useIntegrityRunkept a job id in component state, set from the202it haditself received. Only the mount that pressed the menu item could see a check
reading gigabytes: a reload showed
Readyand the item offered again, and a checkstarted from a terminal was invisible to every browser. It is the failure #492
removed from the download, still present on the other action over the same files.
What it does now
ConnectionOut.integrity_checksits besidedownload, carryingjob_id,state,files_read,files_totalanderror.The same discovery, not a copy of it.
ConnectionJobis the base both shapesshare: one type check, one payload read, one
of()body.CONNECTION_JOB_KEYandconnection_job_payloadare now shared by both handlers, so the key that sayswhich connection a job is about has one spelling for both kinds — the half that
would actually bite, since a mismatch produces a job that runs perfectly and is
invisible to everything watching for it.
InferenceConnectionService.downloads()became
connection_jobs(), which answers both kinds for every row from onequeue read, because the caller is a listing that polls.
Two shapes, because they count two things. A transfer measures bytes off the
disk; a check owns its loop and knows how many files the revision names before it
opens the first one.
jobs/integrity.pywas already reportingon_file(done, total), so the check's bar is determinate with no new accounting — the issue'sprose-only fallback was not needed. Neither borrows the other's name at any
layer, which is why the wire carries two shapes rather than one with a
discriminator: a field whose meaning depended on a sibling field would put that
lookup in every client.
Invariants untouched. No new
ConnectionSetupStatemember, for #494's reasonverbatim — the state flip is the last statement, so a third member reopens the
window ordering closes and strands a connection there when a worker dies.
Purge-first is not touched. A hub that cannot be reached is still a job failure
carrying prose with no verdict, no purge and no state change; what changed is that
the sentence now outlives the request, and there is a test for exactly that.
CONNECTION_GATESis unmodified.The question this pins rather than answers
A live check does not change what a connection declares, and that is today's
answer written down rather than a new one.
connection_actionsis a function ofsetup state and connection type; a job moves neither. So a connection with a check
in flight is still
ready, still declarescheck_integrityanddownload_weights, and the routes accept a second request for either.Defensible as it stands — the check job is registered idempotent, a second run
reads the same files and reaches the same verdict, and
require_checkablepassesat
readyprecisely so a re-queued orphan and a person asking twice take onepath. Also wasteful, and the overlap of a download and a check over one cache is a
question nobody has answered.
Refusing either would give
connection_actionsa third dimension and change whatCONNECTION_GATESis. That is a design decision, not a consequence of putting acheck on the wire, so
test_a_live_check_does_not_change_what_the_connection_declarespins the current behaviour and says in its docstring that whoever changes it has
to come there and say so. This is the stop-and-flag from the dispatch: if a
concurrent check or a download-during-check should be refused, that is a separate
issue and yours to decide. The screen still disables its own control while its
own run is live, which is rendering state and not computing legality.
A test I shipped in #493 that was passing for the wrong reason
test_the_integrity_check_is_not_read_as_a_downloadfakedIntegrityReport(files=3, …)where the field isfiles_checked. The fake raisedTypeError, the job failed, and the assertion — that a check leaves the downloadrecord untouched — held because a check that never succeeded leaves it untouched
too. Two more of the new fakes here would have had the same defect. All three now
spell the field correctly, so the test exercises a passing check, which is what it
claims to.
Test plan
Mutation-verified, three mutations, each reverted by
git apply -Ron itsrecorded diff with the anchor asserted unique before and present after:
livesourced frommutation.isPendinginstead of the wireisBusynarrowed back to the download alonepolls while a check is live, and stops when it settlesIntegrityCheck._countsreturning zeroesNew, backend:
tests/kernel/test_connection_jobs.py(renamed fromtest_weight_downloads.py, since it is now about both kinds) gains the checkreading as files, a download refused as a check, the null-total window, the clamp,
a running check's counts, a failed check's sentence, both kinds from one read, and
neither kind read as the other.
tests/server/test_inference.pygains anever-checked connection reporting
null; a queued check visible with nothingpolling the job; the
allowed_actionspin above; a failure that happened whilenobody watched still saying why; the unreachable-hub case keeping #475's
semantics; a finished check's file counts; and the two runs as separate records
with disjoint field sets.
tests/cli/test_inference_commands.pygains a terminalwatching a check the server is running.
New, frontend:
inference.test.tsxgains a check nobody on the page started, thequeued and listing-read renderings (no bar in either), a passed check showing
nothing, and the poll starting and stopping for a check. Two existing integrity
tests moved off the retired job-id path.
New, browser (
inference.spec.ts): a check survivingpage.reload(), its barmoving on the poll alone and going when it passes, a damaged verdict readable
after the fact, and a transfer and a re-read as two records on one row. No
waitForTimeout— the counting claim stays in jsdom, pertests/scripts/e2e_disciplineand #494.Found, not fixed
download-baris nowdownload-progress-bar. The container, bar and prosetest ids were
download-progress/download-bar/download-progress-prose— inconsistent, and I wrote them yesterday. They are derived from the container
now so the check's pair is symmetric. A test-id rename, no product change.
JobQueue.liststill reads every row before filtering, as recorded in feat(inference): a weight download reports its bytes, and the connection carries it #493.The
typesnarrowing now covers two types instead of one; the read is stillwhole-table.
Local gate
Full run, staged against this box's ~10-minute command ceiling, pytest split by
directory derived from
ls tests/at run time. Every stage's exit code:pytest tests/architecture0pytest tests/cli0pytest tests/examples0pytest tests/fixtures0pytest tests/formats0pytest tests/inference0pytest tests/jobs0pytest tests/kernel0pytest tests/mcp0pytest tests/packaging0pytest tests/scripts5pytest tests/server0pytest tests/test_versioning.py0ruff check .0ruff format --check .0mypy src/visionset/kernel0lint-imports0— 4 contracts keptcheck.sh frontend generated0check.sh browser0— 247 e2e passed, 1 cycle passedtests/scriptsexits 5 by design: nothing pytest-shaped lives there, it isnode --testand runs undercheck.sh generated.cf. #475, #486, #492, #493, #494.