fix: skip the bm25 mock model_file in the offline cache probe - #676
fix: skip the bm25 mock model_file in the offline cache probe#676codechrl wants to merge 3 commits into
Conversation
Qdrant/bm25 uses model_file="mock.file", a placeholder that never exists on disk, so download_model's offline short-circuit always failed and a fully cached bm25 fell through to a live Hub fetch. Filter the mock file out of the required-files check; real models still require their weight file.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe change adds Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This change lets fully cached BM25 models resolve locally without an unnecessary Hugging Face fetch, improving offline use and load behavior; no actionable merge-blocking risk remains beyond normal checks and review. Possibly related issues
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
Resolves the tests/test_common.py conflict introduced by the qwen test additions (qdrant#678, qdrant#680). Both sides are kept: the last-token-pooling tests from main and the bm25 offline-cache-probe test from this branch. The fix itself (fastembed/common/model_management.py) was untouched by the merge.
1 participant
Summary
For the Qdrant/bm25 sparse model, a fully-cached install still triggers a live Hugging Face Hub fetch instead of resolving offline. This makes Bm25 unusable with HF_HUB_OFFLINE=1 / local_files_only=True and adds an avoidable network round-trip on every load even when all files are already on disk.
Fixes #675.
The bug
Qdrant/bm25 is a mock model with no real ONNX weight file; it declares a placeholder model_file of "mock.file", and its real required assets are the per-language stop-word lists in additional_files. The offline cache probe in
ModelManagement.download_modelrequired the model_file to exist on disk, but "mock.file" is never downloaded, so the check always failed and a fully-cached bm25 fell through to the live network fetch.The fix
Add a
MOCK_MODEL_FILEconstant onModelManagementand filter it out of the required-files set before the existence check. For any real model, model_file is a genuine weight file that is not the mock placeholder, so it stays required and behavior is unchanged.Tests
A regression test seeds the cache with only the real additional_files, monkeypatches the Hub download, and asserts the resolved path is the cached snapshot and the network branch was never reached. Placed in tests/test_common.py to avoid colliding with open PR #642, which adds a new tests/test_model_management.py.
Caveat
Verified against the real download_model control flow with a faked Hub call, not a live end-to-end download.