chore(wren-ai-service): fix qdrant indexing #1323
Conversation
WalkthroughThe change updates the document indexing strategy in the Qdrant document store. The field used in the Changes
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
cyyeh
changed the title
cyyeh
added
module/ai-service
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
wren-ai-service/src/providers/document_store/qdrant.py (1)
161-163:❓ Verification inconclusive
Verify document schema compatibility with the new index field.
The change from "id" to "project_id" in the payload index might cause issues as:
- The document conversion logic in
convert_haystack_documents_to_qdrant_pointsuses "id" field- Existing queries might rely on the "id" index for filtering
Run this script to check if documents contain the "project_id" field:
Consider:
- Documenting the rationale for using "project_id" over "id"
- Adding a migration plan if existing data needs to be updated
- Updating the document conversion logic if "project_id" should be included
🏁 Script executed:
#!/bin/bash # Description: Check if documents in the collection have the project_id field # and verify no queries depend on the id index # Check for project_id field usage in the codebase echo "Checking for project_id field usage:" rg -l 'project_id' # Check for potential id-based filtering echo "\nChecking for id-based filtering:" rg -A 5 'filters.*["\']id["\']'Length of output: 4399
Critical: Verify Document Schema Alignment and Query Compatibility
The change in
create_payload_indexfrom"id"to"project_id"may lead to inconsistencies:
- The conversion logic (e.g., in
convert_haystack_documents_to_qdrant_points) appears to still use the"id"field.- Although usage of
"project_id"is widespread across the codebase, our automated id-filtering check encountered a quoting error; please manually confirm that no queries still rely on the old"id"index.- Ensure that documents conform to the expected schema and update the necessary conversion logic or perform a migration if existing data uses
"id".Please update the documentation to clarify the rationale and the impact of switching the index field name.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
wren-ai-service/src/providers/document_store/qdrant.py(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: pytest
- GitHub Check: pytest
Summary by CodeRabbit