fix: fix local inference on list of prefetches #965
Conversation
✅ Deploy Preview for poetic-froyo-8baba7 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
📝 WalkthroughWalkthroughThe changes in this pull request update the logic for determining when inference (embedding) is required in the Possibly related issues
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
qdrant_client/async_qdrant_client.py(2 hunks)qdrant_client/embed/type_inspector.py(1 hunks)qdrant_client/qdrant_client.py(2 hunks)tests/embed_tests/test_inspectors.py(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
qdrant_client/async_qdrant_client.py (2)
qdrant_client/embed/type_inspector.py (1)
inspect(24-50)qdrant_client/embed/embed_inspector.py (1)
inspect(23-47)
tests/embed_tests/test_inspectors.py (4)
qdrant_client/http/models/models.py (1)
Prefetch(1769-1793)qdrant_client/embed/type_inspector.py (1)
inspect(24-50)qdrant_client/embed/embed_inspector.py (1)
inspect(23-47)qdrant_client/embed/utils.py (1)
as_str_list(10-28)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- 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.13.x on ubuntu-latest test
- GitHub Check: Python 3.12.x on ubuntu-latest test
- GitHub Check: Python 3.11.x on ubuntu-latest test
- GitHub Check: Python 3.10.x on ubuntu-latest test
- GitHub Check: Python 3.9.x on ubuntu-latest test
🔇 Additional comments (6)
qdrant_client/embed/type_inspector.py (1)
48-49: Improved inspection control flow for better inference detection.This change alters the behavior of the
inspectmethod to immediately returnFalsewhen a non-BaseModelelement is encountered in an iterable, rather than continuing to check subsequent elements. This ensures that inference requirements are properly detected in complex nested structures.qdrant_client/qdrant_client.py (1)
730-732: Fixed inference detection for prefetch lists in query_points_groups.Similar to the fix in
query_points, this change implements the same sequential inspection approach for query and prefetch in thequery_points_groupsmethod, ensuring consistent behavior across both methods.qdrant_client/async_qdrant_client.py (2)
530-532: Improved inference detection logicThe sequential inspection approach is better than the previous combined approach. By checking
queryfirst and only checkingprefetchif necessary, the code properly handles cases where only one of them requires inference, particularly in nested structures.
696-698: Consistent implementation of the improved inspection patternThis change correctly applies the same sequential inference detection pattern to the
query_points_groupsmethod, maintaining consistency with the implementation inquery_points.tests/embed_tests/test_inspectors.py (2)
249-255: Well-designed test case for nested prefetch inferenceThis new test case properly validates the inspector's behavior with a complex nested structure where a document requiring inference is deeply nested within a list of
Prefetchobjects. The test ensures the inspection correctly identifies this pattern and returns the expected path.
286-287: Fixed test to use proper Prefetch objectsGood improvement to the test by using
none_prefetchinstead of a rawNone. This ensures the inspector is tested with validPrefetchobjects throughout, which better matches real-world usage and validates the inspector's behavior with proper object types.
| requires_inference = self._inference_inspector.inspect(query) | ||
| if not requires_inference: | ||
| requires_inference = self._inference_inspector.inspect(prefetch) |
There was a problem hiding this comment.
Fixed inference detection for prefetch lists.
The previous implementation checked query and prefetch together, which could miss inference requirements in prefetch if query already required it. This change implements a sequential inspection approach that properly detects when inference is needed in either input.
This fix ensures that embedding is correctly applied to prefetch data even when query doesn't require it, resolving the issue described in the PR title.
No description provided.