-
Notifications
You must be signed in to change notification settings - Fork 30
limiting the exact limit search window to 5000 records #358
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,6 +9,14 @@ | |
| from db_repo_module.repositories.sql_alchemy_repository import SQLAlchemyRepository | ||
| from sqlalchemy.exc import SQLAlchemyError | ||
|
|
||
| # Hard ceiling on exact_match_dino's candidate count -- no config/env var can | ||
| # exceed this, so a misconfiguration can't fully disable the safety guard. | ||
| EXACT_MATCH_HARD_CEILING = 5_000 | ||
|
|
||
| # Fallback candidate cap used when the caller doesn't supply one Deliberately conservative; | ||
| # tune based on real p95 latency measurements against the target KB size. | ||
| DEFAULT_EXACT_MATCH_MAX_CANDIDATES = 1_000 | ||
|
|
||
|
|
||
| @dataclass | ||
| class ImageMatch: | ||
|
|
@@ -116,6 +124,7 @@ async def exact_match_dino( | |
| filter6: Optional[str] = None, | ||
| created_at_start=None, | ||
| created_at_end=None, | ||
| max_candidates: Optional[int] = None, | ||
| ) -> list[dict]: | ||
| """ | ||
| Exact (non-ANN) DINO similarity match, restricted to documents in | ||
|
|
@@ -133,7 +142,20 @@ async def exact_match_dino( | |
| (`QueryGenerator.get_image_embedding_dino_exact_match`) never engages the | ||
| HNSW index -- see that method's docstring -- so scores returned here | ||
| are always exact, not approximate. | ||
|
|
||
| The candidate set is capped directly in SQL via `ORDER BY d.id LIMIT | ||
| max_candidates + 1` (clamped to `EXACT_MATCH_HARD_CEILING`), | ||
| so an oversized candidate set never gets fully brute-forced. | ||
| The extra "+1" row lets us detect an overflow (raise a 422) | ||
| using just the row count of the one query already run. | ||
| The `threshold` filter is applied in Python (after the overflow check) | ||
| rather than in SQL. | ||
| """ | ||
| effective_cap = min( | ||
| max_candidates or DEFAULT_EXACT_MATCH_MAX_CANDIDATES, | ||
| EXACT_MATCH_HARD_CEILING, | ||
| ) | ||
|
Comment on lines
+154
to
+157
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Validate non-positive candidate caps before generating the query. The controller accepts negative Use the default for non-positive values in both Proposed fix- configured_cap = int(
- knowledge_base_config.get('exact_match_max_candidates')
- or DEFAULT_EXACT_MATCH_MAX_CANDIDATES
- )
+ configured_cap = int(
+ knowledge_base_config.get('exact_match_max_candidates')
+ or DEFAULT_EXACT_MATCH_MAX_CANDIDATES
+ )
+ if configured_cap <= 0:
+ configured_cap = DEFAULT_EXACT_MATCH_MAX_CANDIDATES- effective_cap = min(
- max_candidates or DEFAULT_EXACT_MATCH_MAX_CANDIDATES,
- EXACT_MATCH_HARD_CEILING,
- )
+ requested_cap = (
+ max_candidates
+ if max_candidates is not None and max_candidates > 0
+ else DEFAULT_EXACT_MATCH_MAX_CANDIDATES
+ )
+ effective_cap = min(requested_cap, EXACT_MATCH_HARD_CEILING)🤖 Prompt for AI Agents |
||
|
|
||
| data = {'image_data': image_data} | ||
| internal_api_url = f'{inference_url}/inference/v1/query/embeddings' | ||
| try: | ||
|
|
@@ -176,7 +198,7 @@ async def exact_match_dino( | |
| filter1, | ||
| document_date_start, | ||
| document_date_end, | ||
| threshold, | ||
| effective_cap, | ||
| filter2, | ||
| filter3, | ||
| filter4, | ||
|
|
@@ -186,7 +208,7 @@ async def exact_match_dino( | |
| created_at_end, | ||
| ) | ||
| ) | ||
| return await self.knowledge_base_embeddings_repository.execute_query( | ||
| raw_rows = await self.knowledge_base_embeddings_repository.execute_query( | ||
| sql_query, | ||
| query_params, | ||
| ) | ||
|
|
@@ -195,6 +217,18 @@ async def exact_match_dino( | |
| f'Failed to execute the query for exact match retrieval: {e}' | ||
| ) | ||
|
|
||
| if len(raw_rows) > effective_cap: | ||
| raise HTTPException( | ||
| status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, | ||
| detail=( | ||
| f'More than {effective_cap} documents match the given filters, ' | ||
| 'which exceeds the exact-match safety limit. Narrow your date ' | ||
| 'range or filters and try again.' | ||
| ), | ||
| ) | ||
|
|
||
| return [row for row in raw_rows if row['dino_score'] > threshold] | ||
|
|
||
| async def image_retrieve_clip( | ||
| self, | ||
| clip_embedding, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Normalize non-positive candidate caps.
If
KB_EXACT_MATCH_MAX_CANDIDATES=-1, the controller returns-1and the service retains it. Then even a zero candidate count is greater than the cap, so every exact-match request returns HTTP 422.wavefront/server/modules/knowledge_base_module/knowledge_base_module/controllers/rag_retreival_controller.py#L226-L226: replace non-positive configured values withDEFAULT_EXACT_MATCH_MAX_CANDIDATESbefore applying the hard ceiling.wavefront/server/modules/knowledge_base_module/knowledge_base_module/services/image_rag_retrieve.py#L153-L156: enforce the same positive-value invariant for direct callers.📍 Affects 2 files
wavefront/server/modules/knowledge_base_module/knowledge_base_module/controllers/rag_retreival_controller.py#L226-L226(this comment)wavefront/server/modules/knowledge_base_module/knowledge_base_module/services/image_rag_retrieve.py#L153-L156🤖 Prompt for AI Agents