Skip to content

fix: scope offline cache checks to the selected revision - #798

Open
hulkbig wants to merge 1 commit into
qdrant:mainfrom
hulkbig:fix/scope-offline-cache-metadata-to-revision-20261009
Open

hulkbig wants to merge 1 commit into
qdrant:mainfrom
hulkbig:fix/scope-offline-cache-metadata-to-revision-20261009

Conversation

@hulkbig

@hulkbig hulkbig commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

files_metadata.json is stored at the repository-cache level and can describe a different snapshot from the one the caller is loading. The offline check currently validates every recorded model file before asking the hub which snapshot to use. A truncated file in an old snapshot can therefore prevent an intact current snapshot from loading, including with local_files_only=True.

Resolve the cached snapshot first and ignore metadata explicitly scoped to other revisions. Keep checking the selected snapshot and legacy unscoped entries. If snapshot resolution fails, preserve the existing corruption error so download_model can still request a forced re-download. Standalone local_dir results retain their existing behavior.

This complements #780, which limits metadata collection after an online download. The regression here occurs in the offline verification path.

Tests use tiny local cache fixtures and real Hugging Face cache resolution, with no model downloads. They cover main, named refs, pinned revisions, old-only and mixed metadata, selected-snapshot corruption, missing refs, forced re-downloads, legacy repository casing, and standalone directories.

Validation:

  • Focused cache tests: 18 passed; the same tests on the unpatched base produce 8 failures
  • Broader offline selection: 45 passed
  • Ruff, mypy, Pyright, and compileall passed

Tested on Python 3.12 / Linux with huggingface_hub 1.33.0. The full model-download/inference suite and platform matrix were not run locally.

All Submissions

  • Followed the contributing guidelines.
  • Checked current open pull requests for the same change.

@hulkbig
hulkbig marked this pull request as ready for review October 9, 2026 17:19
@hulkbig
hulkbig requested a review from joein as a code owner October 9, 2026 17:19
@coderabbitai

coderabbitai Bot commented Oct 9, 2026

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 6be1d268-d0e1-48fc-859e-7c1845fffe9c

📥 Commits

Reviewing files that changed from the base of the PR and between d076f08 and 905f019.


📒 Files selected for processing (2)
  • fastembed/common/model_management.py
  • tests/test_model_cache_revision.py

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.



📝 Walkthrough

Walkthrough

Local-only cache lookup now resolves the requested cached snapshot before validating metadata. When the snapshot belongs to the selected source’s cache, validation checks metadata for that revision. The function returns the resolved path after successful checks or raises the saved resolution error. Tests cover revision selection, selected and unselected revision corruption, unresolved revisions, legacy-cased cache fallback, online redownload, and local_dir validation.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~12 minutes

Change: Bug fix


Merge Risk: ⚪ Minimal · up to 905f0

No actionable issue is established for this change; it is ready to merge after normal checks.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 10.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: limiting offline cache validation to the selected revision.
Description check Passed The description is directly related to the changeset and explains the offline cache resolution issue, intended behavior, tests, and validation results.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.


  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR


  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

This branch has not been deployed

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

1 participant