From 8bc87ed37d3d1daab2a3b771a253fd9b545f6e0b Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Fri, 8 May 2026 22:16:50 +0200 Subject: [PATCH] =?UTF-8?q?refactor(vector):=20address=20PR=20#775=20revie?= =?UTF-8?q?w=20round=202=20=E2=80=94=20drop=20dead=20page=20field,=20add?= =?UTF-8?q?=20omission=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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) --- nextcloud_mcp_server/vector/processor.py | 21 +++----- .../unit/search/test_pdf_highlighter_bbox.py | 52 +++++++++++++++++++ 2 files changed, 60 insertions(+), 13 deletions(-) diff --git a/nextcloud_mcp_server/vector/processor.py b/nextcloud_mcp_server/vector/processor.py index 654c7a63..7f943508 100644 --- a/nextcloud_mcp_server/vector/processor.py +++ b/nextcloud_mcp_server/vector/processor.py @@ -537,9 +537,11 @@ async def _index_document( # Initialize results containers dense_embeddings: list = [] sparse_embeddings: list = [] - # chunk_index -> {"bbox": list[(x0,y0,x1,y1)], "page": int} - # Bboxes are normalized to [0, 1] relative to page width/height. - chunk_bboxes: dict[int, dict] = {} + # chunk_index -> list[(x0, y0, x1, y1)] of normalized rectangles + # in [0, 1] relative to page width/height. The page is taken from + # `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 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(): - chunk_bboxes[chunk_index] = { - "bbox": bboxes, - "page": actual_page_num, - } + for chunk_index, (bboxes, _) in batch_results.items(): + chunk_bboxes[chunk_index] = bboxes 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 # `highlighted_page_image` (Deck #76). The page number # comes from `page_number` (set above for PDF chunks). - **( - {"chunk_bbox": chunk_bboxes[i]["bbox"]} - if i in chunk_bboxes - else {} - ), + **({"chunk_bbox": chunk_bboxes[i]} if i in chunk_bboxes else {}), }, ) ) diff --git a/tests/unit/search/test_pdf_highlighter_bbox.py b/tests/unit/search/test_pdf_highlighter_bbox.py index dcf80b55..0c1c51bd 100644 --- a/tests/unit/search/test_pdf_highlighter_bbox.py +++ b/tests/unit/search/test_pdf_highlighter_bbox.py @@ -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.parametrize("page_index", [0, 1]) def test_compute_chunk_bboxes_assigns_correct_page(page_index: int):