diff --git a/.claude/skills/pre-push-review/SKILL.md b/.claude/skills/pre-push-review/SKILL.md index 26883d49..efc906b0 100644 --- a/.claude/skills/pre-push-review/SKILL.md +++ b/.claude/skills/pre-push-review/SKILL.md @@ -7,6 +7,7 @@ description: | in this repo's automated PR reviews. Use when the user is about to push, says "ready to push", "review my work", "check before PR", or invokes /pre-push-review. Report-only — does not modify code. +model: sonnet allowed-tools: - Bash - Read diff --git a/nextcloud_mcp_server/search/pdf_highlighter.py b/nextcloud_mcp_server/search/pdf_highlighter.py index fac8b09d..18ebafcd 100644 --- a/nextcloud_mcp_server/search/pdf_highlighter.py +++ b/nextcloud_mcp_server/search/pdf_highlighter.py @@ -739,17 +739,26 @@ class PDFHighlighter: start_offset, end_offset, _, - chunk_text, + _, ) in chunks: chunk_page_info = PDFHighlighter.find_chunk_page( start_offset, end_offset, page_boundaries ) if not chunk_page_info: - logger.debug(f"Chunk {chunk_index}: not found on any page") + logger.debug("Chunk %s: not found on any page", chunk_index) continue page_num = chunk_page_info["page_num"] - page_boundary = page_boundaries[page_num - 1] + page_boundary = next( + (b for b in page_boundaries if b["page"] == page_num), None + ) + if page_boundary is None: + logger.debug( + "Chunk %s: page %s not found in boundaries", + chunk_index, + page_num, + ) + continue page_text_length = ( page_boundary["end_offset"] - page_boundary["start_offset"] ) diff --git a/nextcloud_mcp_server/vector/processor.py b/nextcloud_mcp_server/vector/processor.py index 7f943508..f3acc8ff 100644 --- a/nextcloud_mcp_server/vector/processor.py +++ b/nextcloud_mcp_server/vector/processor.py @@ -582,7 +582,7 @@ async def _index_document( assert content_bytes is not None with trace_operation( - "vector_sync.generate_highlights", + "vector_sync.compute_chunk_bboxes", attributes={ "vector_sync.chunk_count": len(chunks), "vector_sync.pdf_size": len(content_bytes), diff --git a/tests/unit/search/test_pdf_highlighter_bbox.py b/tests/unit/search/test_pdf_highlighter_bbox.py index 0c1c51bd..60985916 100644 --- a/tests/unit/search/test_pdf_highlighter_bbox.py +++ b/tests/unit/search/test_pdf_highlighter_bbox.py @@ -185,3 +185,48 @@ def test_compute_chunk_bboxes_assigns_correct_page(page_index: int): assert results, "expected a bbox for the chunk" _, page_num = results[0] assert page_num == page_index + 1 + + +@pytest.mark.unit +def test_compute_chunk_bboxes_handles_unordered_page_boundaries(): + """Page lookup must match by ``page`` key, not by list position. + + Regression guard: an earlier implementation indexed + ``page_boundaries[page_num - 1]``, which silently produces a wrong + bbox if boundaries are passed out of order. Reverse the boundaries + and assert the result is identical to the in-order case. + """ + pages = [ + "Page one talks about apples and oranges in detail.", + "Page two discusses bananas and grapes thoroughly.", + ] + pdf_bytes = _make_pdf(pages) + boundaries, full_text = _page_boundaries(pages) + + chunks = [ + (0, 0, len(pages[0]), 1, "apples and oranges"), + ( + 1, + len(pages[0]), + len(pages[0]) + len(pages[1]), + 2, + "bananas and grapes", + ), + ] + + in_order = PDFHighlighter.compute_chunk_bboxes_batch( + pdf_bytes=pdf_bytes, + chunks=chunks, + page_boundaries=boundaries, + full_text=full_text, + ) + reversed_order = PDFHighlighter.compute_chunk_bboxes_batch( + pdf_bytes=pdf_bytes, + chunks=chunks, + page_boundaries=list(reversed(boundaries)), + full_text=full_text, + ) + + assert in_order == reversed_order + assert reversed_order[0][1] == 1 + assert reversed_order[1][1] == 2