Skip to content

feat(inference): the integrity check joins the download on the wire - #496

Merged
JArmandoAnaya merged 1 commit into
mainfrom
feat/integrity-on-the-wire
Aug 10, 2026
Merged

feat(inference): the integrity check joins the download on the wire#496
JArmandoAnaya merged 1 commit into
mainfrom
feat/integrity-on-the-wire

Conversation

@JArmandoAnaya

Copy link
Copy Markdown
Contributor

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

useIntegrityRun kept a job id in component state, set from the 202 it had
itself received. Only the mount that pressed the menu item could see a check
reading gigabytes: a reload showed Ready and the item offered again, and a check
started 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_check sits beside download, carrying job_id,
state, files_read, files_total and error.

The same discovery, not a copy of it. ConnectionJob is the base both shapes
share: one type check, one payload read, one of() body. CONNECTION_JOB_KEY and
connection_job_payload are now shared by both handlers, so the key that says
which 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 one
queue 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.py was already reporting on_file(done, total), so the check's bar is determinate with no new accounting — the issue's
prose-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 ConnectionSetupState member, for #494's reason
verbatim — 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_GATES is 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_actions is a function of
setup state and connection type; a job moves neither. So a connection with a check
in flight is still ready, still declares check_integrity and
download_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_checkable passes
at ready precisely so a re-queued orphan and a person asking twice take one
path. 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_actions a third dimension and change what
CONNECTION_GATES is. That is a design decision, not a consequence of putting a
check on the wire, so test_a_live_check_does_not_change_what_the_connection_declares
pins 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_download faked
IntegrityReport(files=3, …) where the field is files_checked. The fake raised
TypeError, the job failed, and the assertion — that a check leaves the download
record 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 -R on its
recorded diff with the anchor asserted unique before and present after:

Mutation What went red
the check's live sourced from mutation.isPending instead of the wire all three browser tests that read a check off the row, including the reload
isBusy narrowed back to the download alone polls while a check is live, and stops when it settles
IntegrityCheck._counts returning zeroes three kernel tests and the wire's file-count test

New, backend: tests/kernel/test_connection_jobs.py (renamed from
test_weight_downloads.py, since it is now about both kinds) gains the check
reading 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.py gains a
never-checked connection reporting null; a queued check visible with nothing
polling the job
; the allowed_actions pin above; a failure that happened while
nobody 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.py gains a terminal
watching a check the server is running.

New, frontend: inference.test.tsx gains a check nobody on the page started, the
queued 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 surviving page.reload(), its bar
moving 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, per
tests/scripts/e2e_discipline and #494.

Found, not fixed

  • download-bar is now download-progress-bar. The container, bar and prose
    test 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.list still reads every row before filtering, as recorded in feat(inference): a weight download reports its bytes, and the connection carries it #493.
    The types narrowing now covers two types instead of one; the read is still
    whole-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:

Stage Exit
pytest tests/architecture 0
pytest tests/cli 0
pytest tests/examples 0
pytest tests/fixtures 0
pytest tests/formats 0
pytest tests/inference 0
pytest tests/jobs 0
pytest tests/kernel 0
pytest tests/mcp 0
pytest tests/packaging 0
pytest tests/scripts 5
pytest tests/server 0
pytest tests/test_versioning.py 0
ruff check . 0
ruff format --check . 0
mypy src/visionset/kernel 0
lint-imports 0 — 4 contracts kept
check.sh frontend generated 0
check.sh browser 0 — 247 e2e passed, 1 cycle passed

tests/scripts exits 5 by design: nothing pytest-shaped lives there, it is
node --test and runs under check.sh generated.

cf. #475, #486, #492, #493, #494.

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.
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.

Integrity check is observable on the wire and survives reload

1 participant