feat(inference): a weight download reports its bytes, and the connection carries it - #493
Merged
Merged
Conversation
…ion carries it The download job reported `1 of 1`, once, at the end — a placeholder rather than progress for a transfer measured in gigabytes. And the only view of it was a job id held in a component, so a reload, a second tab or a return visit lost a running download and showed `Not set up` beside a button somebody had pressed. The handler now measures bytes. `snapshot_download` exposes no byte callback — its one injection point counts files, over a repository that is typically one multi-gigabyte checkpoint beside ten small JSON files — so the number comes off the disk, from the blobs a transfer is filling, `.incomplete` parts included. The total is read from the hub's file listing before the first byte, and a size that cannot be read leaves the total null rather than cancelling a download that would have run. `ConnectionOut` carries the connection's most recent weight download: the job, its state, and both byte counts. That is what makes a transfer observable by somebody who did not start it, and it is the read a screen can poll while one is live. The unit is named at the one boundary that knows the job type, so no client has to know that a job row's `processed` counts bytes here and files for the integrity check. `ConnectionSetupState` stays two-valued. A `downloading` member would reopen the half-fetched window that ordering closes, and would strand a connection there whenever a worker died; a job settles itself, including through the orphan sweep.
This was referenced Aug 10, 2026
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.
Backend and wire for #492. The Inference screen follows in a second PR, on top of
the regenerated client —
openapi.jsonandfrontend/ui-core/src/generated/area shared surface, so they land alone.
What changed
The handler reports bytes. It reported
1 of 1, once, at the end. For atransfer measured in gigabytes that is a placeholder, and the microline it fed
said so.
huggingface_hubexposes no byte-level callback for a snapshot:tqdm_classishanded to the
thread_mapover files, and the per-file byte bars come fromhttp_get, which builds its own and takes none from the caller. Mapping filecounts onto bytes is not an alternative either — a repository is typically one
multi-gigabyte
.safetensorsbeside ten small JSON files, so "10 of 11 files" is1% of the transfer. So the number is measured off the disk, from the blobs the
transfer is filling,
.incompleteparts included, about once a second from athread that lives exactly as long as the download. The repository's path is asked
of
scan_cache_dirrather than assembled — the rulecached_filealreadyfollows; its own
size_on_diskis not the number wanted, because it counts onlyblobs a snapshot already points at and therefore reads zero for the whole of a
first download.
The total is read before the first byte, from the same
measure()the formalready asks. A size that cannot be read leaves
bytes_totalnull rather thancancelling a download that would have run: sizing reaches the hub's listing and
the transfer reaches its files, and the two fail independently.
ConnectionOutcarries the connection's most recent weight download — jobid, state,
bytes_done,bytes_total,error. This is the part that makes atransfer observable by somebody who did not start it: a download outlives the
request that launched it and the page that asked, so a reload, a second tab or
another machine read the same progress from the resource they were listing
anyway. The client holds no job id, which is why recovery in PR 2 is automatic
rather than implemented.
The latest rather than only a live one, because both questions get asked —
is something running and what happened last time. Dropping the record when a
job settles would leave a transfer that failed while nobody was watching sitting
at
not_set_upwith nothing saying why.JobQueue.listgained atypesfilter so a connection can find its owntransfers without reading every ingest the workspace has ever queued.
Answers to the issue's step-0 questions, as shipped
JobRunneris a dispatcher thread plus aProcessPoolExecutor(spawn)owned by the application lifespan; a requestenqueues and returns
202. The only early stops are an explicitrequest_canceland server shutdown.test_a_download_finishes_with_nobody_polling_itholds it: the run completes and lands on the row with no client reading
/background-jobs/{id}at any point.downloadingsetup state was not added, and the reasoning is inWeightDownload's docstring.ConnectionSetupStateis two-valued because theflip is the last statement of
fetch_weights— a third member would reopenthe half-fetched window that ordering closes, and would strand a connection
there whenever a worker died. Liveness is the job's, and a job settles itself,
including through
sweep_orphans.measure()sums everysibling of the revision;
download()callssnapshot_downloadwith noallow/ignore patterns.
DownloadSizesis a process-wide LRU keyed onmodel_id@revision, so the worker's lookup is usually a hit behind the form.Decisions worth arguing with
Storage is the job row's existing
processed/total, named as bytes at oneboundary. Those fields are an absolute count of whatever unit a handler works
in — files for the integrity check, bytes for a transfer — which is all they ever
claimed to be.
WeightDownload.ofis the single place that says which, so aclient reads
bytes_doneand formats bytes rather than readingprocessedandlooking up a job type to find out what it counted.
Rejected: two new columns on
JobRow— a migration, a second reporter path,and two permanent nulls carried by every other job type, to hold a number the row
already holds. That is the second encoding the projection exists to avoid.
The write path is the existing
SqliteProgressReporter, not a new narrow portmethod. The issue's
set_asset_progressprecedent is about a guarded write to acontended datum; for job progress that write already exists, is already throttled
to 0.5 s, and already tolerates a busy store by dropping a report rather than the
run. A second path would be a second encoding. The sampler adds its own 1 s bound
on top, so the filesystem is not walked faster than a bar can move.
downloadis a required field,nullwhere nothing was ever asked for. Everyother field on
ConnectionOutis required and this is the convention that keeps aclient from having to know which reads it may believe. The three ui-core
connection stubs gained
download: nullin this PR rather than the next.The CLI publishes it too.
visionset inference list --jsonandshow --jsoncarry the same key from the same projection, so a terminal can watch a transfer
the server's worker is running. A surface that carried it as permanently null
would be two surfaces disagreeing about a concept while
tests/cli/test_json_contract.pycalled them identical.Test plan
New:
tests/kernel/test_weight_downloads.py— the projection: bytes named once, anull total, the clamp, the type refusing a non-download, and what a connection
reports queued / running / settled / twice / never.
tests/kernel/test_job_queue.py— thetypesfilter, alone and withstates.tests/inference/test_weights.py— the reported sequence (0 of Nbefore thefirst byte, the sampler's word, the whole of it at the end), a size that cannot
be read not stopping the transfer, a sample above the total held at it, and the
sampler itself against a real cache directory.
tests/server/test_inference.py—nullwhere nothing was asked; the downloadvisible on the row from the
202and before a worker touches it; a runfinishing with nobody polling; a failure staying readable with its sentence; a
second attempt replacing the first; an edit answering what the listing would;
and the integrity check not being read as a download.
tests/cli/test_inference_commands.py— a terminal watching a transfer theserver is running, and
nullfor a connection nobody downloaded.Five of the new tests skip locally and run in CI.
_bytes_on_diskand_watching_bytescallscan_cache_dir, so they are gated on the runtime throughrequire_local_inference— the base development environment deliberately has noextra. They run in the
inference-smokejob, whereVISIONSET_REQUIRE_LOCAL_INFERENCE=1turns a missing runtime into an error rather than a skip (cf. #488). Their
behaviour was also verified by hand here against
huggingface_hub1.27: 1000bytes counted across a complete blob and an
.incompleteone, 0 for an absentcache,
[100, 100, 100, 200, …, 300]while a cache grew, and monotone at 300through a file being unlinked underneath the sampler.
Every existing fake of
weights_module.downloadwas widened, and every modulethat fakes it now fakes
download_sizetoo — five files. Left alone, the newsize lookup would reach the hub in tests that stub only the transfer, and only on
a machine carrying the extra, which is the worst kind of intermittent.
Found, not fixed
useWeightsRunpolls ajob id in component state for both runs; this PR gives the download a
wire-derived view and leaves the check exactly as it was. It has the same
coupling — a reload loses a check in flight — and it is a different question
over the same files with its own state vocabulary, so it is not this issue's.
JobQueue.liststill reads every row before filtering. Both filters areapplied in Python over
uow.jobs.list(), which is what the method already didfor
states; thetypesnarrowing makes the answer small, not the read. Aworkspace with a very long job history pays for that on the connection listing.
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— 238 e2e passed, 1 cycle passedtests/scriptsexits 5 by design: nothing pytest-shaped lives there, it isnode --testand runs undercheck.sh generated.cf. #434, #454, #470, #471, #488.