From 8d20339b3a99fd308da6b61c54c3b1a51c274dd8 Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Sat, 6 Jun 2026 13:56:30 +0200 Subject: [PATCH] fix(vector): route empty page_boundaries to char-based path; test ws offsets Address claude-review round 1 on PR #868: - use_page_aware now gates on `bool(page_boundaries)` instead of `is not None`, so a PDF that yields an empty boundary list takes the char-based path explicitly (assign_page_numbers no-ops on []) rather than the page-aware chunker's no-boundaries fallback. Same result, clearer intent. - add test_oversized_page_with_leading_whitespace_offsets, exercising the start+start_index offset path for an oversized page whose sub-chunks have leading whitespace. Co-Authored-By: Claude Opus 4.8 (1M context) --- nextcloud_mcp_server/vector/processor.py | 5 ++++- tests/unit/test_document_chunker.py | 22 ++++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/nextcloud_mcp_server/vector/processor.py b/nextcloud_mcp_server/vector/processor.py index ba969d7b..55e998a4 100644 --- a/nextcloud_mcp_server/vector/processor.py +++ b/nextcloud_mcp_server/vector/processor.py @@ -693,7 +693,10 @@ async def _index_document( use_page_aware = ( settings.document_chunk_page_aware and doc_task.doc_type == "file" - and page_boundaries is not None + # Truthy (not just "is not None"): an empty list carries no pages, so + # route it through the char-based path rather than the page-aware + # chunker's no-boundaries fallback. + and bool(page_boundaries) ) with trace_operation( "vector_sync.chunk_text", diff --git a/tests/unit/test_document_chunker.py b/tests/unit/test_document_chunker.py index 94c6fe19..9927c1c2 100644 --- a/tests/unit/test_document_chunker.py +++ b/tests/unit/test_document_chunker.py @@ -370,6 +370,28 @@ class TestPageAwareChunker: assert per_page[2] > 1 assert per_page[3] == 1 + async def test_oversized_page_with_leading_whitespace_offsets(self): + """Offset invariant holds for oversized-page sub-chunks with leading ws. + + Guards the ``start + start_index`` path: LangChain's start_index points + at the first non-whitespace char, so offsets must still extract exactly. + """ + pages = [" \n " + "word " * 200, "Tail page."] + content, boundaries = _make_doc(pages) + + chunks = await PageAwareChunker(chunk_size=200, overlap=20).chunk_text( + content, boundaries + ) + + page_one = [c for c in chunks if c.page_number == 1] + assert len(page_one) > 1 # oversized page really did split + for chunk in chunks: + assert chunk.page_number is not None + assert content[chunk.start_offset : chunk.end_offset] == chunk.text + pb = boundaries[chunk.page_number - 1] + assert pb["start_offset"] <= chunk.start_offset + assert chunk.end_offset <= pb["end_offset"] + async def test_blank_pages_skipped(self): """Whitespace-only pages produce no chunks (no wasted embeddings).""" pages = ["Real content here.", " \n ", "More real content."]