unsloth/studio/backend/tests/test_model_cache_snapshot.py
Daniel Han 68408d06e0
Stop a finished download reporting 0 B and offering Retry (#8160)
* Studio: derive the Hub state cache scope from one canonicalization

cache_scope_name normalized a hub cache path with normcase alone while
download_manifest._canonical_hub_cache resolved it first, so a caller that
reached state_dir with its own spelling filed state under a different
cache-<digest> than every reader that had gone through the manifest helpers.
Windows makes the two spellings diverge routinely (junctions, OneDrive
redirects, mapped drives, 8.3 short names, drive-letter case), which is a
completed download whose manifest can never be found again and so can never
report complete; POSIX resolves to itself and stayed consistent, which is why
this was only ever seen on Windows.

Both now go through state_dir.normalize_hub_cache. Because that changes the
digest for any caller that was passing a raw path, reads and deletes also probe
the pre-resolve digest, and fall back to a payload-verified sweep of the
sibling scopes for state whose write-time spelling is no longer reachable at
all. purge_all_state_for_repo and iter_variant_manifests sweep both spellings
too, so nothing recoverable outlives a delete.

The GGUF progress resolver read manifests only from the active cache while
snapshot_progress read them per scanned cache entry; it now asks the entries on
disk, so the two sites cannot disagree about whether a manifest exists.

* Studio: an unresolved variant file set is not zero download progress

metadata_resolver returns an empty hash set both for "this revision expects no
blobs" and for "the expected file set could not be determined", and the blob
walk treated them the same. For a GGUF variant that meant every blob was
filtered out, so a fully downloaded 33 GB quant reported 0 bytes against the
caller's catalog hint -- literally "0 B of 33 GB" -- and, never observing
completion, kept the job active with Retry/Resume on the card. One model_info
failure is enough to get there and is negatively cached, so a 401 on a gated
repo whose token was dropped, or a poll taken offline, holds it for the TTL.

The two cases are now distinguished. When a variant's file set is unknown the
reading falls back to the variant's own files in the snapshot dir: the manifest
declares them exactly when there is one, otherwise the caller's matcher picks
them out by name. Counting the shared blobs/ dir wholesale instead would bring
back the sibling-quant misattribution ("instant ~900 MB", and the ~99% -> ~78%
dip), which the snapshot dir cannot have since its entries are named per file.

Completion no longer requires a manifest to exist. When HF metadata named every
expected blob and all of them are on disk finalized at their declared sizes,
that is the evidence a manifest verify collects, so the snapshot settles
terminal instead of staying partial forever because the manifest was never
written, was deleted, or sits in a scope this reader cannot name. Still gated
on metadata: a caller's catalog hint is never grounds for calling a download
complete, and a missing expected blob keeps it partial no matter the byte total.

* Studio: keep download byte counters monotonic, not just the fraction

resolveProgressUpdate trusted the backend's byte counters verbatim for any GGUF
variant job reporting a total, while keeping the fraction monotonic for exactly
the reason the counters needed it too: the backend recomputes both from the
shared per-repo blobs/ dir, so one poll that cannot resolve the variant's
expected files reports zero downloaded against the catalog-hinted total. That
single reading permanently rewrote a finished card to "0 B of 33 GB", and
because completion is observed by completedBytes reaching expectedBytes the job
never left ACTIVE_STATES and kept its Retry/Resume controls.

Both counters are now high-water marks for every job kind. The total stays
backend-owned, so a re-resolved variant size still lands, and resetMonotonic
still drops the mark for a new generation (XET redownload, restart,
re-adoption). The pure part of the reconciliation moves to progress-reconcile.ts
so it can be covered by import instead of by reading poll-loop's source; the
poll loop re-exports it unchanged.

* Studio: a running download reads progress from the active cache only

force_active exists so a job in flight is measured where it is actually
writing, but hf_cache_root declines a root that is not a directory yet -- which
is exactly the first download into a freshly configured cache -- and the lookup
then fell through to every remembered cache. A previous cache's completed copy
read as this run's progress and finalized a job that had not written a byte. It
now names the directory the run will create.

Also documents why a Windows-separator path in a manifest is counted missing by
verify_against_disk rather than folded onto its posix spelling: the same guard
fronts resolved_dataset_snapshot_file, which splits on PurePosixPath, where
"a\b" is one component and accepting it would pass a traversal through on the
platform that reads it as two. No writer here emits one, and one that reached a
payload would already have failed the identical check in _manifest_from_payload.

* Studio: the no-manifest completion must see the snapshot, not just the blobs

Finalized blobs alone were being taken as proof a download finished, but HF
writes a blob and then links it into the snapshot dir, so a run killed between
the two leaves bytes nothing points at. With a manifest, verify_against_disk
catches that; the new manifest-less path would have called the unloadable
snapshot complete. It now also requires the snapshot dir to present at least
the expected bytes -- scoped through the caller's variant matcher, since a
variant shares that directory with its siblings, and refused outright for a
variant with no matcher to measure with.

* Studio: flatten the scope-probe conditional in _iter_variant_state_files

* Studio: pin the reported Windows download-card symptom end to end

One test that puts every Windows-shaped condition together, since none of them
reproduce on Linux alone: a cache reached through a redirect so its resolved and
unresolved spellings differ, a snapshot of copies rather than symlinks, a
manifest filed under the pre-resolve scope digest, and a model_info failure
holding the expected blob hash set empty. Asserts the reading the QA report
described as wrong -- 0 bytes against a catalog-hinted total, never terminal --
now comes back complete.

* Studio: hold the last download reading through a poll that measured nothing

The first cut of this made the byte counters high-water marks, matching the
fraction. Review found that wrong: unlike the fraction, bytes have legitimate
reasons to fall inside one generation, and a floor pins the card through both
of them. An XET run that falls back to HTTP re-claims with the SAME generation
(download_lifecycle re-reads current_generation and passes it back) and a
freshly computed completed_baseline_bytes that the progress reading subtracts,
and prepare_cache_for_transport purges the partial outright on an XET resume.
A floored card would sit near full for the whole retry and hand the
rolling-window estimator a flat series, so no rate and no ETA either.

The rule is now "ignore a zero", not "never go down". That is exactly the
reading the bug produces -- the backend's empty reply is all zeros against the
caller's catalog-hinted total, and the model_info failure behind it is
negatively cached so every poll for the whole TTL repeats it -- while every
real change, up or down, still lands on the next poll.

Two follow-ons. A held reading cannot manufacture a completion: completeOnDisk
is never held over, and the backend only sets it on a reading whose own
completed_bytes already cleared the bar. And hydration's gone/active probe now
classifies on the raw reading, since whether anything is on disk is a question
about the cache; holding a zero there would leave a wiped cache's persisted job
adopting as a phantom download, blocking a fresh start for the repo until the
sixty-second idle-evict grace expired.

* Studio: drop the payload-verified scope sweep, keep the legacy digest probe

Review of the branch found the sweep was over-engineering that bought a case
that cannot actually arise and cost real consistency. Writers always
canonicalize before they file state, and legacy_cache_scope_name of an already
canonical path is the canonical digest, so the digest under which existing
manifests sit did not move: nothing was orphaned that the cheap pre-resolve
probe does not already recover.

Against that, the sweep only ran in _state_read_path and _owned_state_paths,
which left the one-pass inventory index and the per-repo variant enumeration
answering a different question from the per-triple read. A variant cancel
marker only the read side could see would have the list view and the detail
view disagree about the same quant, and would survive purge_all_state_for_repo
to resurrect a re-downloaded quant as partial -- the exact invariant the sweep's
own comment claimed to uphold. It also put a directory listing on every state
miss, which is the inventory scanner's hot path, and made markers discoverable
that no pre-upgrade clear could ever have removed.

Every enumerator now derives its scope directories from cache_scope_names
alone, so they cannot disagree by construction, and tests pin that across all
four: read, enumerate, index, delete.

* Studio: judge a manifest-less completion against the metadata file list

The first cut proved completion from a byte total taken over the snapshot dir,
scoped by the caller's variant predicate. Review broke it: that predicate
accepts every mmproj and every drafter in the repo, while a plan fetches one of
each, and extract_quant_label attributes any .gguf inside a quant-named
directory to that quant. A leftover mmproj-F32, an opt-in dspark/ drafter or a
stray file in Q4_K_M/ therefore covers for a shard that never landed, and the
snapshot reports 100% on a model that cannot load. Reproduced directly: 5 KB of
unrelated companion satisfied a 100-byte expected total with the main shard
absent from the snapshot dir entirely.

Completion is now verified the same way a manifest-backed one is, against the
exact paths and declared sizes HF gave for the target, supplied by the caller
because only it knows what a variant consists of. Same evidence, no predicate,
and no recursive walk on the completion path. Where no such list is available
the reading simply stays partial, as it did before.

Also from review: the byte fallback is clamped to the expected total, so the
over-matching that remains there can produce a pinned bar but never "44 GB of
33 GB"; the snapshot dir and the metadata list are resolved lazily, so a
mid-download poll no longer pays a snapshots/ listing it cannot use; the
active-cache entry is skipped when scanning caches for a manifest, since the
call above it already probed exactly that scope; and _configured_hub_cache
wraps its result in Path, so a str-typed setting cannot turn the force_active
lookup into a TypeError that surfaces as the very empty card being fixed.

* Studio: make the resolve-failed spelling match the probe that recovers it

normalize_hub_cache degrades to the caller's spelling when resolve refuses, and
legacy_cache_scope_name is documented as the read-side counterpart that
recovers state written in that state. They did not agree: the degraded branch
skipped expanduser while the probe applied it, so a "~"-spelled cache filed
state under a digest no reader could rebuild. Nothing else could attribute it
either, since the ownership it records is not absolute and _payload_cache_path
rejects that. Both now expand, and both tolerate an expanduser that itself
fails -- legacy_cache_scope_name is fed raw spellings now, so a homeless "~"
would otherwise have escaped a plain read_manifest as a RuntimeError.

Also drops a Path.resolve per candidate file in _iter_variant_state_files: the
canonical spelling it builds there is only ever compared by basename, which is
identical under every cache scope, so it can be asked for unscoped.

* Studio: never net a download reading down to "0 B of 0 B"

A variant that was already on disk when its job was claimed carries a
completed_baseline_bytes equal to its whole size. Subtracting that leaves
nothing on either side of the bar, which the existing code avoided only through
the complete_on_disk clause -- and an unresolvable expected file set is
precisely what takes verified completion away. With the snapshot-dir fallback
now supplying real bytes on that path, the subtraction had a reading to cancel
against for the first time, turning "0 B of 33 GB" into "0 B of 0 B": no total
for the bar to draw, and a job the frontend reads as evictable.

A baseline that covers the whole expected total is no longer subtracted, which
is the same protection the complete_on_disk clause already gave, extended to
the case where completion cannot be confirmed.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Studio: clear both scope spellings on delete, and never infer a completion

Two holes in the scope fan-out and the progress reconciler, both found by
mutation-testing the branch rather than by reading it.

The legacy scope probe fires on the read path and nowhere else. It
recovers state filed under the pre-resolve digest only when it is handed
an unresolved path, and every production caller that deletes or indexes
resolves first: models/deletion.py and datasets/cache_inventory.py both
pass resolve_delete_target_root, whose every branch calls resolve, and
build_variant_state_index's inventory callers pass a directory derived
from huggingface_hub.scan_cache_dir, which resolves too. So reads probed
two scopes while deletes cleared one. A purged variant survived under the
legacy digest for the next read to bring straight back, and the cached
model views disagreed with the progress endpoint about state that was
sitting right there. That is the asymmetry the fan-out was added to
prevent, reintroduced one call frame further out.

_scope_spellings also probes the configured cache's own spelling, but
only when it names the same directory as the root it was handed. That
guard is the important half: borrowing it unconditionally would make a
delete aimed at an inactive cache sweep the active cache's state for the
same repo, which is a much worse outcome than the resurrection it fixes.
Three tests, each verified to fail without the change.

On the frontend, holding the last reading through an unmeasured poll made
a new pairing reachable: last poll's completed_bytes beside this poll's
complete_on_disk. Neither reading showed a completion; together they
satisfied hasObservedExpectedBytes, retiring the card and dropping a
download that had not finished. The backend does hold the invariant that
rules that out, but nothing on this side checks it and a false completion
is the one direction that loses work, so the flag is now only honoured on
a poll that measured the counter it is a claim about. Also coerce
non-finite readings to "not measured": the payload is cast off raw JSON
with no validation, and a NaN reached the bar as a width of "NaN%".

Four mutants that survived the original suite now fail: honouring
complete_on_disk unconditionally, collapsing the expected branch to the
reported value, removing the fraction cap, and dropping the finite guard.
The expected branch is the "of 33 GB" half of the reported card and had
no coverage at all.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Quote only the forward reference, so Sequence reads as used

The import-hoist safety check parses the source: with the whole annotation inside one
string literal, the Sequence import has no AST-visible use and it flagged it as an
un-normalized alias. Only download_manifest.ExpectedFile needs deferring.

* Stop reading a zero as evidence of something it is not

Three findings, one shape: a zero byte count standing in for a fact it does not establish.

Adopting a job whose persisted generation differs from the one the backend reports seeded the
previous run's counters while setting serverGeneration to the new value, so the first poll saw
no generation change and the new run's legitimate zero read as "could not measure". The card
stayed pinned to the old run's bytes until a positive count arrived. The seed is now dropped
when the generation moved, and kept when either side is unknown, which is the adopt-after-
reload path where the counters are all there is.

Hydration classified an idle job as gone on a zero reading. A transient measurement failure
comes back as a perfectly successful all-zero response, so that dropped jobs whose partial
cache was still on disk. cache_path is the discriminator: the backend answers null only when
no cache dir for the repo exists at all, and a dir it scanned and measured at zero still names
itself. An older backend omitting the field is treated as unknown, not absent.

The variant manifest fallback returned the first cache's manifest while snapshot_progress picks
its reading by bytes across every preferred cache dir, so an older revision's hashes could
filter out every blob of a later cache holding the complete variant and report it as 0 or
partial. Manifests that disagree now yield none, degrading to the name-based fallback, which
stays attributable per entry. Agreement is still answered.

The two frontend decisions moved into a leaf module so they can be tested without the poll
loop's import graph. Backend test confirmed against a mutation.

* Distinguish a failed scan from an absent cache, and stop the active manifest bypassing the check

Three follow-ups on last round's fixes.

The cache_path discriminator was defeated by the layer above it: snapshot_progress_response
catches a scan failure and returns _empty_progress, whose cache_path was null, so a transient
failure still read as "the cache is gone" and hydration retired a job whose partial cache was
still on disk. The error fallback now OMITS the key instead. Null means absent, absent means
unknown, and unknown is also what an older backend that never sent the field looks like, so the
frontend rule covers both without a new field.

The manifest disagreement check only compared the remembered caches; the active cache still
early-returned. Its repo dir can be gone while its scoped state holds an old manifest, and idle
progress goes on scanning the remembered ones, so a stale revision's hashes filtered out every
blob of a remembered cache that had the complete variant. It is a candidate like any other now.

And the variant state enumerator used cache_scope_names where the delete and index paths use
_scope_spellings. cache_scope_names recovers the legacy digest only from an unresolved path,
and a variant request carrying local_path is resolved before it gets here, so on a cache reached
through a symlink or junction the offline listing lost the partial download and its resume
control while the progress endpoint could still see it.

Three tests, each confirmed against a mutation of the line it guards.

* Refuse a manifest when a scanned cache has none of its own

* Let a cache root that cannot be listed read as unknown, not as an absent cache

The exception fallback already knew the difference between "no cache dir for this repo"
and "the scan itself failed", but the enumeration never raised: iter_repo_cache_dirs and
iter_active_repo_cache_dirs swallow OSError per root and continue. An EACCES or EIO
therefore arrived as an empty dir list, the reading came back all-zero with
cache_path: null, and hydration retired a persisted job whose partial cache was sitting
on the disk behind that error.

The two iterators now take an optional scan_errors list, preferred_repo_cache_dirs
threads it through, and a reading with nothing measured AND a skipped root omits
cache_path -- the unknown answer the frontend rule already handles. Callers that pass
nothing keep the old suppression.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Judge a variant by its own files, not by the directory its siblings keep alive

Sibling quants share one repo cache dir, so deleting a variant's files leaves the dir
standing and the reading came back "zero bytes, and cache_path names a directory" -- which
hydration reads as resumable. It adopted a phantom card, and that card blocked a fresh
download of the same variant until the idle-evict grace expired sixty seconds later.

The reading now carries target_present: false only on positive evidence of absence (a
named variant, a file set that resolved, no manifest of its own, nothing counted), true
when something is there, and null wherever it cannot be established -- including a
whole-repo job, which owns its directory and is already answered at the repo level. The
verdict retires a job on false and otherwise leaves the cache_path rule in charge, so an
older backend that never sends the field behaves exactly as before.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Declare target_present on the poll-loop's own progress shape too

The hydration path reads it through ProgressLike, not through the api.ts type, so
`tsc -b` failed the Tauri frontend build even though the field was declared.

* Carry the two new answers through the response model, and count a failed stat as a scan

The reading was right and the wire was not. Both fields are serialized through
DownloadProgressResponse, which declared neither: FastAPI dropped target_present entirely,
so the deleted-variant case arrived as undefined and hydration went on adopting the
phantom. And cache_path defaults to None there, so omitting the key -- the way a failed
scan was signalled -- came out as an explicit "no cache dir exists", which is the one
thing that retires a job. The scan-failed state travels as its own cache_measured flag
now, and both fields are declared, with tests that read the model's annotations rather
than a stub's serializer.

The failure can also happen a step before the listing: hf_cache_root goes through
_safe_is_dir, which swallows the OSError from probing a restricted configured cache and
answers None, so an inaccessible active root still produced a measured "absent". Statting
the root is part of the scan, and reports like one.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Fan the delete out over both cache scopes, and never claim absence from a partial scan

The single-variant delete reaches purge_state with a root the delete resolver has
already resolved, so the pre-resolve scope digest was never probed and a legacy
scoped manifest or cancel marker survived the delete for the next read to
resurrect the variant from. It now uses the same _scope_spellings fan-out the
repo-wide purge does, which stays guarded to the same directory.

A progress reading taken while one cache root could not be listed is a lower
bound, not an absence: the active root raising EACCES while a remembered cache
holds only a sibling quant produced target_present false, and hydration retires
the job on that. Any zero-or-absent verdict built with scan errors outstanding is
now unknown, and cache_measured carries that through on every response shape.

Also repairs test stubs that predated the scan_errors parameter.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Retire a hydrated job when a measured scan finds no cache path

The GGUF progress route sets response_model_exclude_none, so its measured-empty
answer omits cache_path instead of sending null. The adopt rule read that
omission as an older backend's unknown and returned active, so a job whose cache
directory had been deleted was re-adopted and blocked a fresh download of the
same variant. A measured scan now settles it either way; an older backend, which
sends no cache_measured at all, keeps the null-only rule.

* Repair the remaining test stubs that predate scan_errors

hf_cache_roots and hf_cache_root grew a keyword the fakes did not accept, so
every caller that threads it through raised TypeError inside the route and came
back as a 500.

* A shared companion is not evidence the quant is here, and an unresolvable root is not an empty one

mmproj and the MTP drafter are downloaded with every quant in a repo, so the
fallback reading counted them for a variant whose own shard had been deleted --
and hydration reads any positive reading as active, re-adopting the stale job
and blocking a fresh download of that quant. The scan now looks for the quant's
own main shard first and counts companions only behind it.

A cache root that stats but will not resolve, an intermittent network mount or a
Windows reparse point, was dropped without a word, so the scan answered
'measured, no cache' and hydration retired a download whose files may be intact.
It is a scan error now.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Answer target presence from every cache scanned, and scope the manifest to the live one

Three ways the download card could report the wrong state.

An unresolvable hash set left target_present null even after the snapshot
scan had already looked: a repo dir kept alive by a sibling quant read as
zero bytes with a real cache_path, which the idle probe adopts as an active
job and which then blocks a fresh download of the deleted quant until the
grace expires. The snapshot dir is named per file, so absence is answerable
there, and it now answers.

Presence was also taken from whichever cache held the most bytes. Two
zero-byte readings let root order pick a sibling-only false over a cache
whose manifest proves the variant is there. A positive reading anywhere
wins now.

The manifest resolver still consulted remembered caches while the progress
scan was pinned to the active root, so a superseded revision's manifest
either supplied the hashes for the live root or disagreed with the active
one and refused both. It takes the same force_active/active_root scoping the
scan uses.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Absence needs positive evidence from every cache that was read

Three ways the reading claimed the target was gone without knowing it.

An unreadable blobs/ dir was converted to an empty one, so an EACCES or a
transient network-filesystem error produced a measured zero and idle
hydration retired a job whose cache had never been read. It goes into
scan_errors like any other scan failure.

The state-dir manifest counted as evidence of presence. It describes what
the target should contain and survives a deletion made outside the app, so
with a sibling quant keeping the repo dir alive a variant with nothing left
on disk read as present. Only bytes, or the variant's own file found by
name, count now.

And the cross-cache aggregate reported false whenever no reading said true.
One unknown cache -- no readable snapshot to identify the variant from, and
a shared blobs dir that may hold an unattributable partial -- now keeps the
verdict unknown.

* A file that cannot be stated is not a file that is not there

rglob succeeding does not mean every entry can be read. One is_file() raising
-- a transient network-filesystem error, a Windows ACL denial -- was skipped
silently and the snapshot scan then reported the variant absent, though the
skipped entry may have been its main shard and idle hydration retires a
persisted download on exactly that verdict. A positive match elsewhere still
settles it; otherwise the reading is unknown.

* An explicit absence outranks a companion byte count

Four more readings that claimed more than they knew.

A blob the loop could not stat was skipped silently, so a transient
network-filesystem or ACL failure produced a measured absence on the path
where the hashes DID resolve. It joins scan_errors like the directory listing
already does.

Presence was decided from the newest snapshot by mtime alone. A cache can
retain several revisions, and the requested quant living in an older one
while the newest holds a sibling read as absent; it is now established across
every snapshot the repo dir keeps.

The byte-ordered pick between caches ignored verified completion, so two
readings that clamped to the same total let root order carry the unverified
one and the response stayed capped below 100% with Retry offered.

And idleProbeVerdict believed the byte count ahead of the explicit verdict.
The two genuinely disagree on the unknown-hash path, where the byte reading
falls back to a retained manifest and counts a shared companion that outlived
the main shard while the by-name scan reports the quant gone.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Read every retained snapshot, and stop is_dir hiding a failed stat

Path.is_dir() swallows ELOOP and its neighbours and answers False, so a
failure on the blobs directory itself read as "no blobs here" -- a measured
absence, which idle hydration retires the job on. It is an explicit os.stat
now, recorded in scan_errors; the same try also contains the EACCES case,
which is_dir() re-raises and which previously escaped the whole reading.

The byte fallback and the manifest verification still looked only at the
newest snapshot. The presence check already spans all of them, so a variant
complete in an older revision was known to be there and still reported 0
bytes and 99%, adoptable forever. Both now take the best across every
retained snapshot.

And a generation change resets the fraction, not only the byte counters. The
GGUF high-water mark carried the previous run's fraction across, pinning a
retry that starts at 0 B to 99% for its whole life.

* Presence from the materialized file, completion from the resolved revision

The resolved-hash path read target presence off the shared blobs/ tally.
Deleting a variant's snapshot entry leaves its finalized blob behind, and a
companion blob shared with a sibling keeps that tally positive on its own, so
a quant that is gone read as present and idle hydration re-adopted the
phantom. It uses the by-name scan the unknown-hash branch already uses;
bytes stand in only when there is nothing readable to look at.

And the any-snapshot completion check accepted a retained older revision that
merely carried the same filenames at the same sizes -- verify_against_disk
does not read sha256. With a blob finalized but not yet linked, that let a
stale snapshot settle the job on files the app would not load. Where the
resolved hashes are known, the snapshot's entries must resolve to them; a
copy-layout cache has no link target to read and is unaffected.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Keep a refused manifest refused, and hold the card through an unmeasured scan

_variant_manifest_in_any_cache returns None both when no cache on disk has a
manifest and when it found manifests but ruled that none of them may be applied
across the caches snapshot_progress scans. The resolver could not tell those
apart, so a refusal fell through to gguf_variant_blob_hashes, which reads the
default cache's manifest with none of that scoping and reinstated the very
hashes just rejected. It now reports a verdict, and a refusal resolves to an
empty hash set so the reading degrades to the per-entry name-based fallback.

resolveProgressUpdate treated cache_measured false as an idle poll. The adopt
probe already refuses to retire on that shape, but once the job was adopted the
poll loop's idle grace finalized it as gone even though the cache was only
unreadable.

* Do not let a suppressed scan error read as a measured absence

Three fixes from the review round, all the same shape: an unreadable thing was
being reported as an absent thing, and idle hydration retires a persisted
download on exactly that verdict.

Path.is_dir() answers False for every OSError as of 3.14 and suppresses several
of them before that, so the handler wrapped around it in _safe_is_dir could
never see a permission or network-mount failure on a cache root. It stats now,
and a genuinely missing directory is still a plain False.

Path.rglob() suppresses every OSError raised while scanning, documented since
3.13, so an unreadable subtree came back as a short list indistinguishable from
an empty one and the by-name presence scan answered a confident absent. Replaced
with an os.scandir walk that reports whether the traversal was complete.

With the variant hashes unresolved, an .incomplete blob that cannot be
attributed to any target was ignored entirely, and the by-name scan cannot see
it because a partial is not linked into a snapshot yet. Presence stays unknown
while one is there.

---------

Co-authored-by: Daniel Han <moonshotaisubstack@gmail.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
2026-08-09 00:13:46 -07:00

298 lines
10 KiB
Python

# SPDX-License-Identifier: AGPL-3.0-only
# Copyright 2026-present the Unsloth AI Inc. team. All rights reserved. See /studio/LICENSE.AGPL-3.0
import os
import time
from pathlib import Path
import pytest
from hub.utils import hf_cache_state
latest_snapshot_from_cache_path = hf_cache_state.latest_snapshot_from_cache_path
@pytest.fixture(autouse = True)
def _known_cache_root(monkeypatch, tmp_path):
monkeypatch.setattr(hf_cache_state, "hf_cache_roots", lambda **kw: [tmp_path])
def _model_repo(root: Path, repo_id: str) -> Path:
repo_root = root / f"models--{repo_id.replace('/', '--')}"
(repo_root / "snapshots").mkdir(parents = True)
return repo_root
def _snapshot(
repo_root: Path,
name: str,
files: tuple[str, ...] = (),
) -> Path:
snap = repo_root / "snapshots" / name
snap.mkdir()
for filename in files:
(snap / filename).write_text("{}")
return snap
def test_returns_newest_snapshot_with_metadata(tmp_path):
repo_root = _model_repo(tmp_path, "Org/Model")
old = _snapshot(repo_root, "old", ("config.json",))
new = _snapshot(repo_root, "new", ("config.json",))
past = time.time() - 3600
os.utime(old, (past, past))
resolved = latest_snapshot_from_cache_path(
str(repo_root), "model", "Org/Model", ("config.json",)
)
assert resolved == str(new.resolve())
def test_requires_metadata_filenames_when_given(tmp_path):
repo_root = _model_repo(tmp_path, "Org/Model")
_snapshot(repo_root, "rev")
assert (
latest_snapshot_from_cache_path(str(repo_root), "model", "Org/Model", ("config.json",))
is None
)
def test_accepts_adapter_metadata(tmp_path):
repo_root = _model_repo(tmp_path, "Org/Model")
snap = _snapshot(repo_root, "rev", ("adapter_config.json",))
resolved = latest_snapshot_from_cache_path(
str(repo_root), "model", "Org/Model", ("config.json", "adapter_config.json")
)
assert resolved == str(snap.resolve())
def test_rejects_paths_outside_the_repo_cache_dir(tmp_path):
foreign = tmp_path / "somewhere-else"
foreign.mkdir()
(foreign / "config.json").write_text("{}")
assert (
latest_snapshot_from_cache_path(str(foreign), "model", "Org/Model", ("config.json",))
is None
)
def test_rejects_mismatched_repo_id(tmp_path):
repo_root = _model_repo(tmp_path, "Org/Model")
_snapshot(repo_root, "rev", ("config.json",))
assert (
latest_snapshot_from_cache_path(str(repo_root), "model", "Other/Repo", ("config.json",))
is None
)
def test_accepts_snapshot_dir_directly(tmp_path):
repo_root = _model_repo(tmp_path, "Org/Model")
snap = _snapshot(repo_root, "rev", ("config.json",))
resolved = latest_snapshot_from_cache_path(str(snap), "model", "Org/Model", ("config.json",))
assert resolved == str(snap.resolve())
def test_none_inputs_return_none(tmp_path):
assert latest_snapshot_from_cache_path(None, "model", "Org/Model") is None
assert latest_snapshot_from_cache_path(str(tmp_path), "model", "") is None
def test_refs_main_preferred_over_newer_mtime(tmp_path):
repo_root = _model_repo(tmp_path, "Org/Model")
old = _snapshot(repo_root, "commit-old", ("config.json",))
new = _snapshot(repo_root, "commit-new", ("config.json",))
past = time.time() - 3600
os.utime(old, (past, past))
refs = repo_root / "refs"
refs.mkdir()
(refs / "main").write_text("commit-old")
resolved = latest_snapshot_from_cache_path(
str(repo_root), "model", "Org/Model", ("config.json",)
)
assert resolved == str(old.resolve())
assert resolved != str(new.resolve())
def test_refs_main_skipped_without_metadata_or_missing_target(tmp_path):
repo_root = _model_repo(tmp_path, "Org/Model")
_snapshot(repo_root, "commit-pinned")
fallback = _snapshot(repo_root, "commit-fallback", ("config.json",))
refs = repo_root / "refs"
refs.mkdir()
(refs / "main").write_text("commit-pinned")
resolved = latest_snapshot_from_cache_path(
str(repo_root), "model", "Org/Model", ("config.json",)
)
assert resolved == str(fallback.resolve())
def test_rejects_lookalike_repo_outside_known_cache(monkeypatch, tmp_path):
allowed = tmp_path / "allowed"
allowed.mkdir()
repo_root = _model_repo(tmp_path / "outside", "Org/Model")
_snapshot(repo_root, "rev", ("config.json",))
monkeypatch.setattr(hf_cache_state, "hf_cache_roots", lambda **kw: [allowed])
assert (
latest_snapshot_from_cache_path(str(repo_root), "model", "Org/Model", ("config.json",))
is None
)
def test_refs_main_cannot_escape_snapshots(tmp_path):
repo_root = _model_repo(tmp_path, "Org/Model")
fallback = _snapshot(repo_root, "rev", ("config.json",))
escaped = tmp_path / "escaped"
escaped.mkdir()
(escaped / "config.json").write_text("{}")
refs = repo_root / "refs"
refs.mkdir()
(refs / "main").write_text("../../escaped")
resolved = latest_snapshot_from_cache_path(
str(repo_root), "model", "Org/Model", ("config.json",)
)
assert resolved == str(fallback.resolve())
(refs / "main").write_text("commit-missing")
resolved = latest_snapshot_from_cache_path(
str(repo_root), "model", "Org/Model", ("config.json",)
)
assert resolved == str(fallback.resolve())
def test_snapshot_symlink_cannot_escape_cache(tmp_path):
repo_root = _model_repo(tmp_path, "Org/Model")
fallback = _snapshot(repo_root, "rev", ("config.json",))
escaped = tmp_path / "escaped-snapshot"
escaped.mkdir()
(escaped / "config.json").write_text("{}")
link = repo_root / "snapshots" / "linked"
link.symlink_to(escaped, target_is_directory = True)
future = time.time() + 3600
os.utime(escaped, (future, future))
resolved = latest_snapshot_from_cache_path(
str(repo_root), "model", "Org/Model", ("config.json",)
)
assert resolved == str(fallback.resolve())
def test_snapshots_directory_symlink_cannot_escape_cache(tmp_path):
repo_root = _model_repo(tmp_path, "Org/Model")
external = tmp_path / "external-snapshots"
snapshot = external / "rev"
snapshot.mkdir(parents = True)
(snapshot / "config.json").write_text("{}")
(repo_root / "snapshots").rmdir()
(repo_root / "snapshots").symlink_to(external, target_is_directory = True)
resolved = latest_snapshot_from_cache_path(
str(repo_root), "model", "Org/Model", ("config.json",)
)
assert resolved is None
def test_ref_file_symlink_cannot_escape_cache(tmp_path):
repo_root = _model_repo(tmp_path, "Org/Model")
fallback = _snapshot(repo_root, "rev", ("config.json",))
external_ref = tmp_path / "external-ref"
external_ref.write_text("rev")
refs = repo_root / "refs"
refs.mkdir()
(refs / "main").symlink_to(external_ref)
resolved = latest_snapshot_from_cache_path(
str(repo_root), "model", "Org/Model", ("config.json",)
)
assert resolved == str(fallback.resolve())
def test_refs_directory_symlink_cannot_escape_cache(tmp_path):
repo_root = _model_repo(tmp_path, "Org/Model")
fallback = _snapshot(repo_root, "rev", ("config.json",))
external_refs = tmp_path / "external-refs"
external_refs.mkdir()
(external_refs / "main").write_text("rev")
(repo_root / "refs").symlink_to(external_refs, target_is_directory = True)
resolved = latest_snapshot_from_cache_path(
str(repo_root), "model", "Org/Model", ("config.json",)
)
assert resolved == str(fallback.resolve())
def test_training_pin_prefers_a_snapshot_that_has_weights(tmp_path):
# refs/main can point at a metadata-only revision while a complete snapshot sits beside it.
from core.training.training import _resolve_model_snapshot
repo_root = _model_repo(tmp_path, "Org/Model")
metadata_only = _snapshot(repo_root, "commit-metadata", ("config.json",))
complete = _snapshot(repo_root, "commit-complete", ("config.json", "model.safetensors"))
refs = repo_root / "refs"
refs.mkdir()
(refs / "main").write_text("commit-metadata")
resolved = _resolve_model_snapshot("Org/Model", str(repo_root))
assert resolved == str(complete.resolve())
assert resolved != str(metadata_only.resolve())
def test_training_pin_still_falls_back_to_metadata_only_snapshots(tmp_path):
# With no weights anywhere the pin is unchanged, so the worker's Hub retry still runs.
from core.training.training import _resolve_model_snapshot
repo_root = _model_repo(tmp_path, "Org/Model")
metadata_only = _snapshot(repo_root, "commit-metadata", ("config.json",))
assert _resolve_model_snapshot("Org/Model", str(repo_root)) == str(metadata_only.resolve())
def test_training_pin_skips_a_weights_only_snapshot_without_metadata(tmp_path):
# A newer weights-only fetch (interrupted download, or an allow_patterns pull that never took
# config.json) must not displace an older complete sibling: the start route rejects a snapshot
# with no loader metadata, so picking it 400s a run that used to work.
from core.training.training import _resolve_model_snapshot
repo_root = _model_repo(tmp_path, "Org/Model")
complete = _snapshot(repo_root, "commit-complete", ("config.json", "model.safetensors"))
weights_only = _snapshot(repo_root, "commit-weights", ("model.safetensors",))
past = time.time() - 3600
os.utime(complete, (past, past))
resolved = _resolve_model_snapshot("Org/Model", str(repo_root))
assert resolved == str(complete.resolve())
assert resolved != str(weights_only.resolve())
def test_training_pin_ignores_weight_names_the_start_route_rejects(tmp_path):
# consolidated.safetensors has no transformers loader path and is not in _MODEL_WEIGHT_CANDIDATES,
# so treating it as "has weights" selects a snapshot the start route then rejects.
from core.training.training import _resolve_model_snapshot
repo_root = _model_repo(tmp_path, "Org/Model")
loadable = _snapshot(repo_root, "commit-loadable", ("config.json", "model.safetensors"))
consolidated = _snapshot(
repo_root, "commit-consolidated", ("config.json", "consolidated.safetensors")
)
past = time.time() - 3600
os.utime(loadable, (past, past))
resolved = _resolve_model_snapshot("Org/Model", str(repo_root))
assert resolved == str(loadable.resolve())
assert resolved != str(consolidated.resolve())