fix(vector): tighten get_chunk_bbox_and_page_from_qdrant doc_id to str
🔴 Blocking finding from PR #773 latest review: `get_chunk_bbox_and_page_from_qdrant` (`search/context.py:199`) still declared `doc_id: int | str` and passed the raw value into `MatchValue(value=doc_id)` at lines 239 and 256 without `str()` coercion. After this branch's startup backfill normalises every Qdrant `doc_id` payload to a string, an `int` filter would silently match zero points — the function would return `(None, None)` instead of the chunk bbox / page, and PDF highlight overlays would fail in production. Take option 2 from the reviewer's two suggestions (annotation tightening over inline coercion): the producer side of this PR has already narrowed every other `doc_id` annotation to `str`, so this function is the last hold-out. Pushing the contract into the type system means `ty` will catch any future regression at the call site. Production callers in `api/visualization.py` and `auth/viz_routes.py` already pass `doc_id` (str) verbatim after the recent merge with master's chunk_index-first refactor, so no caller-side changes needed. Update the 9 calls in `tests/unit/test_chunk_bbox_helper.py` to use string literals (`"42"` / `"99"` / `"1"`) instead of integers. The mock doesn't validate `MatchValue` value types, so the tests passed with stale int doc_ids today — but they were exercising a path production no longer takes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.7
parent
d83c32a9dd
commit
a27f738dbf
@@ -198,7 +198,7 @@ async def _get_deck_metadata_from_qdrant(
|
||||
|
||||
async def get_chunk_bbox_and_page_from_qdrant(
|
||||
user_id: str,
|
||||
doc_id: int | str,
|
||||
doc_id: str,
|
||||
chunk_index: int | None,
|
||||
chunk_start: int,
|
||||
chunk_end: int,
|
||||
@@ -214,7 +214,11 @@ async def get_chunk_bbox_and_page_from_qdrant(
|
||||
|
||||
Args:
|
||||
user_id: User ID who owns the document
|
||||
doc_id: Document ID (int for file/note, str for some doc types)
|
||||
doc_id: Document ID — always a string. Producers stringify their
|
||||
native ID before writing to Qdrant so the keyword payload
|
||||
index on ``doc_id`` matches every point regardless of source
|
||||
doc_type. An ``int`` filter against the str-indexed payload
|
||||
would silently match zero points.
|
||||
chunk_index: Zero-based chunk index, or None to use offset fallback
|
||||
chunk_start: Character offset where chunk starts (used when
|
||||
chunk_index is None)
|
||||
|
||||
@@ -58,7 +58,7 @@ class TestIndexedPath:
|
||||
with ctx:
|
||||
result = await get_chunk_bbox_and_page_from_qdrant(
|
||||
user_id="alice",
|
||||
doc_id=42,
|
||||
doc_id="42",
|
||||
chunk_index=3,
|
||||
chunk_start=0,
|
||||
chunk_end=100,
|
||||
@@ -84,7 +84,7 @@ class TestOffsetFallbackPath:
|
||||
with ctx:
|
||||
result = await get_chunk_bbox_and_page_from_qdrant(
|
||||
user_id="bob",
|
||||
doc_id=99,
|
||||
doc_id="99",
|
||||
chunk_index=None,
|
||||
chunk_start=500,
|
||||
chunk_end=600,
|
||||
@@ -105,7 +105,7 @@ class TestOffsetFallbackPath:
|
||||
with ctx, caplog.at_level("WARNING"):
|
||||
result = await get_chunk_bbox_and_page_from_qdrant(
|
||||
user_id="bob",
|
||||
doc_id=99,
|
||||
doc_id="99",
|
||||
chunk_index=None,
|
||||
chunk_start=0,
|
||||
chunk_end=100,
|
||||
@@ -124,7 +124,7 @@ class TestPayloadShape:
|
||||
with ctx:
|
||||
result = await get_chunk_bbox_and_page_from_qdrant(
|
||||
user_id="alice",
|
||||
doc_id=1,
|
||||
doc_id="1",
|
||||
chunk_index=0,
|
||||
chunk_start=0,
|
||||
chunk_end=10,
|
||||
@@ -143,7 +143,7 @@ class TestPayloadShape:
|
||||
with ctx:
|
||||
result = await get_chunk_bbox_and_page_from_qdrant(
|
||||
user_id="alice",
|
||||
doc_id=42,
|
||||
doc_id="42",
|
||||
chunk_index=3,
|
||||
chunk_start=0,
|
||||
chunk_end=100,
|
||||
@@ -157,7 +157,7 @@ class TestPayloadShape:
|
||||
with ctx:
|
||||
result = await get_chunk_bbox_and_page_from_qdrant(
|
||||
user_id="alice",
|
||||
doc_id=42,
|
||||
doc_id="42",
|
||||
chunk_index=3,
|
||||
chunk_start=0,
|
||||
chunk_end=100,
|
||||
@@ -171,7 +171,7 @@ class TestPayloadShape:
|
||||
with ctx:
|
||||
result = await get_chunk_bbox_and_page_from_qdrant(
|
||||
user_id="alice",
|
||||
doc_id=42,
|
||||
doc_id="42",
|
||||
chunk_index=3,
|
||||
chunk_start=0,
|
||||
chunk_end=100,
|
||||
@@ -188,7 +188,7 @@ class TestPayloadShape:
|
||||
with ctx:
|
||||
result = await get_chunk_bbox_and_page_from_qdrant(
|
||||
user_id="alice",
|
||||
doc_id=42,
|
||||
doc_id="42",
|
||||
chunk_index=3,
|
||||
chunk_start=0,
|
||||
chunk_end=100,
|
||||
@@ -205,7 +205,7 @@ class TestExceptionHandling:
|
||||
with ctx, caplog.at_level("WARNING"):
|
||||
result = await get_chunk_bbox_and_page_from_qdrant(
|
||||
user_id="alice",
|
||||
doc_id=42,
|
||||
doc_id="42",
|
||||
chunk_index=3,
|
||||
chunk_start=0,
|
||||
chunk_end=100,
|
||||
|
||||
Reference in New Issue
Block a user