Skip to content

fix(download): adopt unmarked caches for pinned revisions - #977

Merged
Alex-Wengg merged 3 commits into
FluidInference:mainfrom
JulianPscheid:fix/adopt-unmarked-pinned-cache
Oct 5, 2026
Merged

Alex-Wengg merged 3 commits into
FluidInference:mainfrom
JulianPscheid:fix/adopt-unmarked-pinned-cache

Conversation

@JulianPscheid

Copy link
Copy Markdown
Contributor

Fixes #976.

Caches written before revision markers existed are now reused when their files match the pinned listing, instead of being deleted and downloaded again.

When a pinned repo's directory has no .fluidaudio-revision marker, ModelHub lists every bundle and root file already on disk (not only the ones the current variant asked for) and compares each file's size with the size in the tree listing. A bundle with any missing or mismatched file is removed as a whole, so allModelsExist stays false and the next loadModels still downloads it, even if the process dies before the refetch. Files that match are kept, stale .partial and .etag files next to them are removed, and the marker is written. The normal download loop then fetches only what is missing.

The full-tree check matters for speaker-diarization, where the streaming and offline models share one directory and one marker. Without it, a streaming load would mark the directory current and the offline bundles would never be compared.

Unchanged: main repos, caches whose marker matches, caches whose marker names a different revision (still wiped), offline mode, and the purge and force-redownload paths. No public API changes, and FileDownloader is untouched. Comparison is by byte size, not hash. A removed bundle loses its partial downloads, since it is being discarded anyway.

The adoption loop treats a file that disappears between the existence check and the remove (another load adopting the same directory) as already removed, so the race no longer surfaces as an error that triggers loadModels' purge.

Tests: ModelCacheLegacyAdoptionTests covers both download entry points, a stale bundle from a sibling variant, a bundle with a missing or wrong inner file (checked at the allModelsExist gate that loadModelsOnce uses), sidecar cleanup, main and existing markers, and concurrent adopters. TreeStubURLProtocol now serves pinned revisions as well as main. The ModelCache, ModelHub, subdirectory, progress and downloader suites pass (90 tests), along with swift build and swift format lint --strict.

I validate every cached compiled bundle and root file against the pinned
listing before adopting a legacy cache. I remove incomplete bundles and
stale sidecars beside kept files, and tolerate missing-file races.

I preserve existing main and marked-cache behavior and cover both download
entry points with regression tests.
@Alex-Wengg

Copy link
Copy Markdown
Member

Thanks! Two things to fix before merge: the repo-root fallback in filesForLegacyAdoption (ModelHub.swift:708) also runs for subdirectory downloads, so <sub>/ files get checked against unrelated root files and root files get downloaded into repoDirectory. Separately, the repo download path (ModelHub.swift:505) never filters the merged cache list back to the requested files, so incomplete bundles from other variants are deleted and then fully re-downloaded. Smaller: adoption compares sizes only (same-size retrained weights get stamped with the pinned marker; comparing the LFS sha256 would fix that), files outside top-level .mlmodelc folders (e.g. voices/) are adopted unchecked, and size > 0 rejects legitimate 0-byte and unknown-size (-1) files.

I verify legacy files against Hugging Face LFS SHA-256 or Git blob SHA-1
identities using bounded reads, including empty and unknown-size files.
I check nested plain files individually and remove incomplete compiled
bundles at any depth before writing the revision marker.

I limit repo-root fallback to flattened repo downloads and filter the
validated list back to the requested paths so other variants are fetched
only when their own next load needs them.

Validation: RED had 19 assertion failures across 25 tests on the parent.
GREEN passes 38 focused tests, 74 selected cache/download/progress tests,
and 25 downloader tests. Full swift build and strict format lint pass.
I also confirmed lfs.oid and oid in the live pinned Hugging Face tree.
I remove partial and etag sidecars for rejected or missing legacy files
before writing the pinned revision marker. This prevents finished stale
partials from restoring bytes that failed content verification.

I cover rejected and missing destinations, and a repo download that
refetches its requested JSON while leaving another bad variant absent.

Validation: RED compiled and produced 9 assertion failures across 30
tests on c6bdeb3. GREEN passes 40 focused, 76 selected, and 25 downloader
tests. Full swift build and strict format lint pass.
@JulianPscheid

Copy link
Copy Markdown
Contributor Author

Thanks for the careful review. I pushed two commits on top (c6bdeb3 and 617eaa1) that cover all five points:

  • Subdirectory downloads no longer use the repo-root fallback. It only runs for a repo download with a subPath, so <sub>/ files are never compared against root files and nothing from the root lands in repoDirectory.
  • The repo download path still validates every cached variant before writing the marker, but the download loop is filtered back to the requested files. A bad bundle from another variant is removed and then fetched by that variant's own next load.
  • Adoption now compares content. RemoteFile carries lfs.oid for LFS files and the git blob oid for regular files, and local files are hashed in 1 MiB chunks. The LFS pointer oid is ignored, and a listed file with no id counts as a mismatch. I checked the pinned diarizer tree: all 15 weight files have lfs.oid and all 19 JSON files have oid.
  • Every local file the listing covers is checked, including nested plain folders like voices/. .mlmodelc bundles at any depth are still removed as a whole when any file in them fails.
  • Size is only a pre-check now. 0-byte files and entries with size -1 are kept when their hash matches.

617eaa1 also removes the .partial and .partial.etag of any file that fails the check before the marker is written. Without that, FileDownloader could promote a finished partial of the old bytes, since it only checks the size.

New tests cover each case. ModelCache|ModelHub|SubdirectoryDownloadTests|ProgressSequenceTests (76 tests) and the downloader suites (25) pass, along with swift build and swift format lint --strict.

@Alex-Wengg
Alex-Wengg merged commit 04e363c into FluidInference:main Oct 5, 2026
21 checks passed
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.

Upgrading from v0.15.7 re-downloads an unchanged speaker-diarization cache

2 participants