refactor(vector): address PR #775 review round 2 — drop dead page field, add omission tests
- chunk_bboxes is now dict[int, list[tuple[...]]] holding the bbox list
directly, not {"bbox": ..., "page": ...}. The page from text-search was
stored but never read; page_number from offset-based assignment is
authoritative for the Qdrant payload.
- Add two unit tests for the documented omission contract: chunks whose
offsets fall outside every page boundary, and chunks whose text cannot
be located on the rendered page, are silently dropped from the result.
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
80b27b1bc6
commit
8bc87ed37d
@@ -537,9 +537,11 @@ async def _index_document(
|
|||||||
# Initialize results containers
|
# Initialize results containers
|
||||||
dense_embeddings: list = []
|
dense_embeddings: list = []
|
||||||
sparse_embeddings: list = []
|
sparse_embeddings: list = []
|
||||||
# chunk_index -> {"bbox": list[(x0,y0,x1,y1)], "page": int}
|
# chunk_index -> list[(x0, y0, x1, y1)] of normalized rectangles
|
||||||
# Bboxes are normalized to [0, 1] relative to page width/height.
|
# in [0, 1] relative to page width/height. The page is taken from
|
||||||
chunk_bboxes: dict[int, dict] = {}
|
# `chunk.page_number` (offset-based) and stored as `page_number`
|
||||||
|
# in the Qdrant payload, so we don't carry an `actual_page_num` here.
|
||||||
|
chunk_bboxes: dict[int, list[tuple[float, float, float, float]]] = {}
|
||||||
|
|
||||||
# Determine if we need PDF highlighting
|
# Determine if we need PDF highlighting
|
||||||
is_pdf = doc_task.doc_type == "file" and content_type == "application/pdf"
|
is_pdf = doc_task.doc_type == "file" and content_type == "application/pdf"
|
||||||
@@ -612,11 +614,8 @@ async def _index_document(
|
|||||||
)
|
)
|
||||||
)
|
)
|
||||||
|
|
||||||
for chunk_index, (bboxes, actual_page_num) in batch_results.items():
|
for chunk_index, (bboxes, _) in batch_results.items():
|
||||||
chunk_bboxes[chunk_index] = {
|
chunk_bboxes[chunk_index] = bboxes
|
||||||
"bbox": bboxes,
|
|
||||||
"page": actual_page_num,
|
|
||||||
}
|
|
||||||
|
|
||||||
logger.info(f"Computed bboxes for {len(chunk_bboxes)}/{len(chunks)} chunks")
|
logger.info(f"Computed bboxes for {len(chunk_bboxes)}/{len(chunks)} chunks")
|
||||||
|
|
||||||
@@ -738,11 +737,7 @@ async def _index_document(
|
|||||||
# relative to page width/height. Replaces the legacy
|
# relative to page width/height. Replaces the legacy
|
||||||
# `highlighted_page_image` (Deck #76). The page number
|
# `highlighted_page_image` (Deck #76). The page number
|
||||||
# comes from `page_number` (set above for PDF chunks).
|
# comes from `page_number` (set above for PDF chunks).
|
||||||
**(
|
**({"chunk_bbox": chunk_bboxes[i]} if i in chunk_bboxes else {}),
|
||||||
{"chunk_bbox": chunk_bboxes[i]["bbox"]}
|
|
||||||
if i in chunk_bboxes
|
|
||||||
else {}
|
|
||||||
),
|
|
||||||
},
|
},
|
||||||
)
|
)
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -103,6 +103,58 @@ def test_compute_chunk_bboxes_empty_input():
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.unit
|
||||||
|
def test_compute_chunk_bboxes_omits_when_offsets_out_of_range():
|
||||||
|
"""Chunks whose offsets fall outside every page boundary are omitted.
|
||||||
|
|
||||||
|
Verifies the docstring contract: *"Chunks whose bbox cannot be located
|
||||||
|
are omitted from the result."* (path: ``find_chunk_page`` returns None).
|
||||||
|
"""
|
||||||
|
pages = ["Page one body text content here for the test."]
|
||||||
|
pdf_bytes = _make_pdf(pages)
|
||||||
|
boundaries, full_text = _page_boundaries(pages)
|
||||||
|
|
||||||
|
# Offsets way beyond the document end — no page boundary matches.
|
||||||
|
out_of_range_start = len(full_text) + 1000
|
||||||
|
out_of_range_end = out_of_range_start + 50
|
||||||
|
chunks = [(0, out_of_range_start, out_of_range_end, 1, "irrelevant")]
|
||||||
|
|
||||||
|
results = PDFHighlighter.compute_chunk_bboxes_batch(
|
||||||
|
pdf_bytes=pdf_bytes,
|
||||||
|
chunks=chunks,
|
||||||
|
page_boundaries=boundaries,
|
||||||
|
full_text=full_text,
|
||||||
|
)
|
||||||
|
|
||||||
|
assert results == {}
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.unit
|
||||||
|
def test_compute_chunk_bboxes_omits_when_text_not_in_pdf():
|
||||||
|
"""Chunks whose page-relative text isn't on the page are omitted.
|
||||||
|
|
||||||
|
Verifies the second omission path: ``_find_chunk_bbox`` returns None
|
||||||
|
when the supplied text cannot be located on the rendered page.
|
||||||
|
"""
|
||||||
|
pages = ["Hello world."]
|
||||||
|
pdf_bytes = _make_pdf(pages)
|
||||||
|
# Build boundaries from the real text but pass a *different* full_text
|
||||||
|
# so the page-relative slice is content that does not exist in the PDF.
|
||||||
|
boundaries, _ = _page_boundaries(pages)
|
||||||
|
bogus_full_text = "Z" * len(pages[0])
|
||||||
|
|
||||||
|
chunks = [(0, 0, len(pages[0]), 1, "ignored")]
|
||||||
|
|
||||||
|
results = PDFHighlighter.compute_chunk_bboxes_batch(
|
||||||
|
pdf_bytes=pdf_bytes,
|
||||||
|
chunks=chunks,
|
||||||
|
page_boundaries=boundaries,
|
||||||
|
full_text=bogus_full_text,
|
||||||
|
)
|
||||||
|
|
||||||
|
assert results == {}
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.unit
|
@pytest.mark.unit
|
||||||
@pytest.mark.parametrize("page_index", [0, 1])
|
@pytest.mark.parametrize("page_index", [0, 1])
|
||||||
def test_compute_chunk_bboxes_assigns_correct_page(page_index: int):
|
def test_compute_chunk_bboxes_assigns_correct_page(page_index: int):
|
||||||
|
|||||||
Reference in New Issue
Block a user