fix(windows): HF cache disk-fallback for WinError 448 (plan-01, closes #117 #118) - #137
Conversation
) plan-01 fix-sequence step 2. On Windows, huggingface_hub's scan_cache_dir() raises WinError 448 "untrusted mount point"; the three call sites in setup/models.py swallowed it and reported "not cached", so the app re-downloaded models it already had — looping 5× and giving up (#117/#118). - _is_cached_on_disk / _scan_cache_on_disk: walk the canonical HF layout <cache>/models--<org>--<name>/snapshots/<rev>/ directly (honours HF_HUB_CACHE/HF_HOME, so a relocated models dir works too). - is_cached / list_models / recommendations now fall back to the disk scan when scan_cache_dir() raises. An empty snapshot dir is not counted. Symlink-disable env + local_dir_use_symlinks=False were already shipped (main.py, setup/download.py); this closes the remaining failure path. Tests (TDD, fail-before/pass-after): tests/test_hf_cache_fallback.py (4). No regression on the non-Windows path (fallback only triggers on raise). Closes #117, #118. Addresses #128 (#64 configurable-dir is the follow-up). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a filesystem-based fallback for Hugging Face cache detection when scan_cache_dir() fails (WinError 448): on-disk cache scanning helpers, wired fallback paths in is_cached(), /models, and /setup/recommendations, plus spec/docs and regression tests. ChangesWindows HF Cache Fallback
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/api/routers/setup/models.py`:
- Around line 132-141: The fallback in _is_cached_on_disk currently builds snaps
under hf_cache_dir() directly, which misses the hub subdir when only HF_HOME is
set; change the snaps root to use the hub cache root (i.e., derive the hub root
as hf_cache_dir() plus the "hub" subdirectory or reuse huggingface_hub's
HF_HUB_CACHE constant) and then join _repo_dir_name(repo_id) and "snapshots" (so
use a hub_root = os.path.join(hf_cache_dir(), "hub") or equivalent before
os.path.join(hub_root, _repo_dir_name(repo_id), "snapshots") in
_is_cached_on_disk).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: c7802c10-2b29-409c-9299-0e2350d0f982
📒 Files selected for processing (5)
backend/api/routers/setup/models.pyspecs/002-windows-model-storage/plan.mdspecs/002-windows-model-storage/spec.mdspecs/002-windows-model-storage/tasks.mdtests/test_hf_cache_fallback.py
|
| Filename | Overview |
|---|---|
| backend/api/routers/setup/models.py | Adds filesystem fallback helpers and wires them into all three scan_cache_dir call sites; previously-flagged off-by-one directory issue resolved by _hub_cache_roots(). |
| tests/test_hf_cache_fallback.py | Five regression tests covering cached/uncached detection, HF_HOME-only path, empty snapshot dir, and disk scan size/file reporting. |
| specs/002-windows-model-storage/tasks.md | Task tracking doc; T005 full-backend-suite gate still open (unchecked). |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["is_cached / list_models / recommendations called"] --> B["scan_cache_dir()"]
B -->|success| C["Use HF CacheInfo"]
B -->|raises WinError 448| D["_hub_cache_roots() → base + base/hub"]
D --> E["_is_cached_on_disk() or _scan_cache_on_disk()"]
E -->|files found| F["Report cached"]
E -->|no files| G["Report not cached"]
Reviews (2): Last reviewed commit: "fix(models): probe HF /hub subdir + clos..." | Re-trigger Greptile
Addresses #137 review: - CodeRabbit (critical): hf_cache_dir() returns HF_HOME when HF_HUB_CACHE is unset, but repos live under $HF_HOME/hub/models--…. Added _hub_cache_roots() so the WinError-448 fallback probes both <dir> (HF_HUB_CACHE-set case) and <dir>/hub (HF_HOME-only case); previously it could miss the cache and re-download. Regression test added (HF_HOME-only layout). - Greptile: wrap os.scandir() in `with` so the dir handle closes even when any() short-circuits (avoids handle leaks on repeated /models polls). - CodeQL: drop unused `os` import in the test. 5 tests pass, incl. -W error::ResourceWarning. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
plan-01 fix-sequence step 2 — Closes #117, #118. Addresses #128.
On Windows,
huggingface_hub.scan_cache_dir()raisesWinError 448 "untrusted mount point". The three call sites insetup/models.pyswallowed it and reported "not cached", so the app re-downloaded models it already had — looping 5× and giving up, leaving the app unusable (#117/#118).Fix
A direct-filesystem fallback over the canonical HF layout
<cache>/models--<org>--<name>/snapshots/<rev>/:_is_cached_on_disk()/_scan_cache_on_disk()— honourHF_HUB_CACHE/HF_HOME, so a relocated models dir works toois_cached()/list_models()/recommendations()fall back to it whenscan_cache_dir()raisesThe symlink-disable env (
HF_HUB_DISABLE_SYMLINKS=1) andlocal_dir_use_symlinks=False(steps 1) already shipped inmain.py/setup/download.py; this closes the remaining failure path.Cross-platform parity
The fallback only triggers when
scan_cache_dir()raises — macOS/Linux behaviour is unchanged.Tests (TDD, fail-before/pass-after — Constitution V)
tests/test_hf_cache_fallback.py(4): cached repo detected when scan raises, uncached not, empty snapshot not counted, disk scan reports size/files. Targeted regression set (model/cache/setup): 23 passed, 0 failed.Remaining cluster item
#64 (configurable models directory) builds on the existing
OMNIVOICE_CACHE_DIR/HF_HOMEplumbing — a Settings field + startup read — tracked as the follow-up within the v0.3.0 line.Spec/plan/tasks in
specs/002-windows-model-storage/.🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests
Documentation