fix: implement score_threshold filtering for OceanBase vector search #28536
Conversation
dosubot
bot
added
the
size:S
Summary of ChangesHello @connermo, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical issue in the OceanBase vector store where the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly implements the score_threshold filtering for OceanBase vector search, which was a missing feature. The logic for filtering results based on the score is sound. I've found a couple of areas for improvement: one is a potential unhandled exception when processing the score_threshold parameter, and the other is a performance optimization to avoid unnecessary work. Addressing these points will make the implementation more robust and efficient. Overall, a good fix that improves consistency with other vector stores.
OceanBase vector search was ignoring the score_threshold parameter, returning all top_k results regardless of their similarity scores. This caused annotation queries with high thresholds (e.g., score_threshold=1) to return low-scoring matches that should have been filtered out. Added score_threshold parameter handling to filter results based on similarity scores, bringing OceanBase behavior in line with other vector stores (Chroma, PGVector, etc.).
connermo
force-pushed
the
fix/oceanbase-score-threshold
branch
from
4d7fc8d to
aadc298
Compare
…tion - Move score_threshold conversion inside try-except block to handle ValueError - Optimize performance by checking score before parsing JSON metadata - Only parse metadata for documents that meet the threshold criteria Addresses Gemini Code Assist review comments.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request correctly implements score_threshold filtering for OceanBase vector searches, addressing a key issue where the parameter was ignored. The changes are logical and follow the intended behavior described. My review includes two main points. First, a medium-severity suggestion to improve the robustness of parsing the score_threshold parameter to avoid generic exceptions on invalid input. Second, a high-severity issue regarding unhandled exceptions during JSON metadata parsing, for which I've provided a code suggestion to make the process more resilient, similar to how it's handled elsewhere in the codebase. Overall, a good fix that brings OceanBase vector search in line with other implementations.
| metadata = json.loads(metadata) | ||
| metadata["score"] = score | ||
| docs.append( | ||
| Document( | ||
| page_content=_text, | ||
| metadata=metadata, | ||
| ) | ||
| ) |
There was a problem hiding this comment.
The call to json.loads(metadata) could raise a json.JSONDecodeError if the metadata from the database is not a valid JSON string. This would cause the entire search operation to fail. It would be more robust to handle this potential error gracefully by logging a warning and using empty metadata for the document, which is consistent with the error handling in the search_by_full_text method in this file.
| metadata = json.loads(metadata) | |
| metadata["score"] = score | |
| docs.append( | |
| Document( | |
| page_content=_text, | |
| metadata=metadata, | |
| ) | |
| ) | |
| try: | |
| metadata = json.loads(metadata) | |
| except json.JSONDecodeError: | |
| logger.warning(f"Failed to decode metadata for document with text: '{_text[:100]}...'. Using empty metadata.") | |
| metadata = {} | |
| metadata["score"] = score | |
| docs.append( | |
| Document( | |
| page_content=_text, | |
| metadata=metadata, | |
| ) | |
| ) |
| self._hnsw_ef_search = ef_search | ||
| topk = kwargs.get("top_k", 10) | ||
| try: | ||
| score_threshold = float(kwargs.get("score_threshold") or 0.0) |
There was a problem hiding this comment.
This line could raise a ValueError if kwargs.get("score_threshold") returns a string that cannot be converted to a float (e.g., 'abc'). While this would be caught by the outer try...except block, it would be re-raised as a generic Exception, which hides the root cause from the user. It would be better to handle this potential error more explicitly to provide a clearer error message.
Consider moving the parsing and validation of score_threshold outside of this try block to fail fast with a specific error.
2 participants
Problem
OceanBase vector store was completely ignoring the
score_thresholdparameter during similarity search, causing it to return all top_k results regardless of their similarity scores. This was particularly problematic for the annotation feature where users set strict thresholds (e.g.,score_threshold=1) but still received irrelevant low-scoring matches.Root Cause
The
search_by_vector()method inOceanBaseVectorclass:score_thresholdin**kwargsbut never used itann_search()without filtering by scoreSolution
Modified
api/core/rag/datasource/vdb/oceanbase/oceanbase_vector.pyto:score_thresholdparameter:score_threshold = float(kwargs.get("score_threshold") or 0.0)if score >= score_threshold:This brings OceanBase behavior in line with other vector store implementations.
Changes
File:
api/core/rag/datasource/vdb/oceanbase/oceanbase_vector.pyscore_thresholdparameter extractionTesting
make lint- All checks passedmake type-check- 0 errors, 0 warningsImpact
Related Issues
This fixes the issue where annotation feature with embedding model threshold set to 1 doesn't filter results properly when using OceanBase vector store.