Reviewing files that changed from the base of the PR and between d1a369c and a37f293.

• tests/embed_tests/test_local_inference.py (23 hunks)
• tests/test_fastembed.py (0 hunks)

• tests/test_fastembed.py

• GitHub Check: Redirect rules - poetic-froyo-8baba7
• GitHub Check: Header rules - poetic-froyo-8baba7
• GitHub Check: Pages changed - poetic-froyo-8baba7
• GitHub Check: Python 3.12.x on ubuntu-latest test
• GitHub Check: Python 3.10.x on ubuntu-latest test
• GitHub Check: Python 3.13.x on ubuntu-latest test
• GitHub Check: Python 3.9.x on ubuntu-latest test
• GitHub Check: Python 3.11.x on ubuntu-latest test

tests/embed_tests/test_local_inference.py (6)

6-8: LGTM: Clean temporary directory scaffolding.

The temporary directory setup and cleanup fixture are properly implemented. The module-scoped cleanup ensures the temp directory is removed after all tests in the module complete.

Also applies to: 36-47

---

81-81: Fixture scope change aligns with new caching strategy.

The scope change from session to module is appropriate given the introduction of module-level SESSION_TMP_DIR and per-model cache directories. This ensures proper isolation between test modules.

---

352-360: Verify the necessity of mutating document options after creation.

The code mutates the options dictionary of previously created documents (lines 352-360), setting local_files_only based on cache existence. While this may ensure the cache is available for subsequent operations, mutating shared document state can be fragile and make tests order-dependent.

Consider whether these mutations are necessary, or if the documents should be recreated with the correct options for each test phase. If the mutation is intentional to reflect that models are now cached after the first upload, consider adding a comment explaining this behavior.

# After first upload, models are cached locally
for text_doc in (dense_doc_1, dense_doc_2, dense_doc_3):
    text_doc.options["local_files_only"] = Path(text_doc.options["cache_dir"]).exists()

---

426-444: LGTM: Consistent cache options propagation.

The cache directory and options handling in these test functions follows the same correct pattern: compute cache directory, prepare options dict with conditional local_files_only, then create documents with those options.

Also applies to: 609-622, 685-703, 799-817

---

1520-1520: Verify the reason for skipping these tests.

Three tests are marked with @pytest.mark.skip but don't include explanations for why they're disabled:

• test_upsert_batch_with_different_options (line 1520)
• test_batch_size_propagation (line 1584)
• test_embed_multimodal (line 1675)

These tests were modified to incorporate the new caching infrastructure but then skipped. This suggests they may be temporarily disabled during the transition.

Please clarify:

1. Why are these tests skipped?
2. Is there a plan to re-enable them?
3. Should the skip markers include a reason string?

Consider using @pytest.mark.skip(reason="...") to document why tests are disabled, or create tracking issues for re-enabling them.

Also applies to: 1584-1584, 1675-1675

---

874-881: LGTM: Comprehensive cache options integration across test suite.

The remaining test functions have been systematically updated to use the new caching infrastructure. The pattern is consistent and correct throughout:

1. Compute cache directory using _get_cache_dir_name
2. Create options dict with cache_dir and conditional local_files_only
3. Pass options when creating Document and Image objects

The repeated local_files_only checks (e.g., lines 1312-1313, 1449-1450) appropriately handle progressive cache population during test execution.

Also applies to: 995-1014, 1270-1398, 1414-1517