From 2f2a7f9659429534c454a9e3e1d7f3088886b03f Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Sat, 6 Jun 2026 13:42:25 +0200 Subject: [PATCH 1/7] feat(vector): page-aware PDF chunking for predictable per-page retrieval Add PageAwareChunker, which splits paginated documents (PDFs) on page boundaries first and only character-splits pages larger than chunk_size. No chunk spans a page boundary, so page_number is always exact and stored excerpts never lead with a neighbouring page's text. When chunk_size is at least the largest page, this yields exactly one chunk per page: a predictable vector count (== page count), a flat per-page embedding cost, and zero cross-page overlap duplication. Gated by DOCUMENT_CHUNK_PAGE_AWARE (default true). When false, the legacy char-based DocumentChunker + post-hoc assign_page_numbers path runs unchanged. Only doc_type="file" with page_boundaries (PDFs) takes the page-aware path; notes/deck/news are unaffected. Measured on a 15-page record (query "leadership award louis", target = top-half of page 15): char-based degraded the target to dense-rank 10 at cs=2048 (OCR) and mislabeled its page; page-aware restored rank 1 across every fusion/modality and chunk size, with correct page labels and clean snippets. BREAKING CHANGE: PDFs are re-chunked page-aware by default. Existing deployments will re-index PDF content on the next vector sync (different chunk counts and page_number labels). Set DOCUMENT_CHUNK_PAGE_AWARE=false to retain the previous char-based behaviour. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/configuration.md | 1 + env.sample | 4 + nextcloud_mcp_server/config.py | 12 ++ .../vector/document_chunker.py | 148 ++++++++++++++++++ nextcloud_mcp_server/vector/processor.py | 42 +++-- tests/unit/test_config.py | 14 ++ tests/unit/test_document_chunker.py | 136 ++++++++++++++++ 7 files changed, 347 insertions(+), 10 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index ce535de7..cb4f6052 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -743,6 +743,7 @@ equivalent.** Operators who need a runtime toggle should open an issue. | `SIMPLE_EMBEDDING_DIMENSION` | ⚠️ Optional | `384` | Dimension for the fallback Simple provider | | `DOCUMENT_CHUNK_SIZE` | ⚠️ Optional | `512` | Words per chunk for document embedding | | `DOCUMENT_CHUNK_OVERLAP` | ⚠️ Optional | `50` | Overlapping words between chunks (must be < chunk size) | +| `DOCUMENT_CHUNK_PAGE_AWARE` | ⚠️ Optional | `true` | Split PDFs on page boundaries first (one chunk per page; oversized pages split within the page). Exact page numbers, clean snippets, and a predictable ~1 chunk/page when chunk size ≥ the largest page. Set `false` for the legacy char-based path. | **Deprecated variables (still functional):** - `VECTOR_SYNC_ENABLED` - Use `ENABLE_SEMANTIC_SEARCH` instead (will be removed in v1.0.0) diff --git a/env.sample b/env.sample index 600c2ffc..30799fdc 100644 --- a/env.sample +++ b/env.sample @@ -206,6 +206,10 @@ NEXTCLOUD_PASSWORD= # Configure how documents are split before embedding #DOCUMENT_CHUNK_SIZE=512 #DOCUMENT_CHUNK_OVERLAP=50 +# Page-aware chunking for PDFs: split on page boundaries first so no chunk spans +# a page (exact page numbers, clean snippets, ~1 chunk/page when chunk size >= +# the largest page). Set false to use the legacy char-based path. Default: true +#DOCUMENT_CHUNK_PAGE_AWARE=true # ===== SEMANTIC SEARCH TUNING ===== # Advanced parameters for vector sync background operations diff --git a/nextcloud_mcp_server/config.py b/nextcloud_mcp_server/config.py index 226f192c..8f298cba 100644 --- a/nextcloud_mcp_server/config.py +++ b/nextcloud_mcp_server/config.py @@ -132,6 +132,10 @@ _DEFAULTS: dict[str, Any] = { # Document chunking "document_chunk_size": 2048, "document_chunk_overlap": 200, + # Page-aware chunking for paginated docs (PDFs): split on page boundaries + # first so no chunk spans a page (exact page_number, clean snippets, and + # predictable ~1 chunk/page when chunk_size >= the largest page). + "document_chunk_page_aware": True, # PDF parse isolation (OOM guard) "document_pdf_graphics_limit": 1000, "document_parse_timeout_seconds": 120.0, @@ -736,6 +740,13 @@ class Settings: # Document chunking settings (for vector embeddings) document_chunk_size: int = 2048 # Characters per chunk document_chunk_overlap: int = 200 # Overlapping characters between chunks + # Page-aware chunking for paginated docs (PDFs). When True (default), PDF + # text is split on page boundaries first (one chunk per page; oversized + # pages are character-split within the page), giving exact page numbers, + # snippets that never lead with a neighbouring page, and a predictable + # ~1 chunk/page when document_chunk_size >= the largest page. When False, + # the legacy char-based path runs with post-hoc assign_page_numbers. + document_chunk_page_aware: bool = True # PDF parse isolation (OOM guard). The parse runs in a subprocess so one # pathological file fails that doc, not the pod. @@ -1389,6 +1400,7 @@ def get_settings() -> Settings: # Document chunking settings "document_chunk_size": "DOCUMENT_CHUNK_SIZE", "document_chunk_overlap": "DOCUMENT_CHUNK_OVERLAP", + "document_chunk_page_aware": "DOCUMENT_CHUNK_PAGE_AWARE", "document_pdf_graphics_limit": "DOCUMENT_PDF_GRAPHICS_LIMIT", "document_parse_timeout_seconds": "DOCUMENT_PARSE_TIMEOUT_SECONDS", "document_parse_mem_limit_mb": "DOCUMENT_PARSE_MEM_LIMIT_MB", diff --git a/nextcloud_mcp_server/vector/document_chunker.py b/nextcloud_mcp_server/vector/document_chunker.py index 6c4fb168..a71d4b5f 100644 --- a/nextcloud_mcp_server/vector/document_chunker.py +++ b/nextcloud_mcp_server/vector/document_chunker.py @@ -2,6 +2,7 @@ import logging from dataclasses import dataclass +from typing import Any import anyio from langchain_text_splitters import RecursiveCharacterTextSplitter @@ -97,3 +98,150 @@ class DocumentChunker: self.overlap, ) return chunks + + +class PageAwareChunker: + """Page-first chunker for paginated documents (PDFs). + + Unlike :class:`DocumentChunker`, which splits the concatenated document text + on character boundaries and is therefore page-agnostic, this chunker splits + on the page boundaries FIRST and only falls back to character splitting for + pages larger than ``chunk_size``. As a result: + + * No chunk ever spans a page boundary, so ``page_number`` is always exact + and the stored excerpt never leads with a neighbouring page's text (the + char-based path can bury a short page's content in the tail of a chunk + whose majority — and thus :func:`assign_page_numbers` label — is the + previous page). + * Chunks-per-page is ``ceil(page_chars / chunk_size)``. When ``chunk_size`` + is at least the largest page, that is exactly one chunk per page, giving a + predictable vector count (== page count), a flat per-page embedding cost, + and zero cross-page overlap duplication. + + Page numbers are assigned inline, so callers must NOT additionally run + ``assign_page_numbers`` on the result. + """ + + def __init__(self, chunk_size: int = 2048, overlap: int = 200): + """ + Initialize page-aware chunker. + + Args: + chunk_size: Number of characters per chunk (default: 2048). Pages at + or below this size become a single chunk; larger pages are + character-split (with overlap) within the page only. + overlap: Overlapping characters between sub-chunks of an oversized + page (default: 200). Pages that fit in one chunk carry no + overlap. + """ + self.chunk_size = chunk_size + self.overlap = overlap + + # Only used for pages that exceed chunk_size. Same hierarchical splitter + # as DocumentChunker so oversized pages keep semantic-boundary splitting. + self.splitter = RecursiveCharacterTextSplitter( + chunk_size=chunk_size, + chunk_overlap=overlap, + add_start_index=True, + strip_whitespace=True, + ) + + async def chunk_text( + self, content: str, page_boundaries: list[dict[str, Any]] + ) -> list[ChunkWithPosition]: + """ + Split ``content`` into per-page chunks using ``page_boundaries``. + + Args: + content: Full document text. Offsets in ``page_boundaries`` must + index into this string (the extractor contract — see + ``document_processors``). + page_boundaries: Ordered list of ``{"page", "start_offset", + "end_offset"}`` dicts. When empty, falls back to plain + character chunking (no page numbers), matching + :class:`DocumentChunker` for non-paginated input. + + Returns: + List of chunks with character positions and ``page_number`` set. + """ + if not content: + return [ChunkWithPosition(text="", start_offset=0, end_offset=0)] + + # No page info (e.g. a non-PDF that reached this path) — degrade to the + # char-based behaviour so the caller still gets sensible chunks. + if not page_boundaries: + docs = await anyio.to_thread.run_sync( # type: ignore[attr-defined] + self.splitter.create_documents, + [content], + ) + return [ + ChunkWithPosition( + text=doc.page_content, + start_offset=doc.metadata.get("start_index", 0), + end_offset=doc.metadata.get("start_index", 0) + + len(doc.page_content), + ) + for doc in docs + ] + + chunks = await anyio.to_thread.run_sync( # type: ignore[attr-defined] + self._chunk_by_page, + content, + page_boundaries, + ) + + logger.debug( + "Page-aware chunked document into %s chunks across %s pages " + "(chunk_size=%s, overlap=%s)", + len(chunks), + len(page_boundaries), + self.chunk_size, + self.overlap, + ) + return chunks + + def _chunk_by_page( + self, content: str, page_boundaries: list[dict[str, Any]] + ) -> list[ChunkWithPosition]: + """CPU-bound per-page splitting (runs in a worker thread).""" + chunks: list[ChunkWithPosition] = [] + for boundary in page_boundaries: + page = boundary["page"] + start = boundary["start_offset"] + end = boundary["end_offset"] + page_text = content[start:end] + + # Skip blank pages: embedding an empty/whitespace-only string wastes + # a provider call and a vector slot. + if not page_text.strip(): + continue + + if len(page_text) <= self.chunk_size: + stripped = page_text.strip() + # Tighten offsets to the stripped text so they stay meaningful + # even though the whole page is one chunk. + lead = len(page_text) - len(page_text.lstrip()) + chunk_start = start + lead + chunks.append( + ChunkWithPosition( + text=stripped, + start_offset=chunk_start, + end_offset=chunk_start + len(stripped), + page_number=page, + ) + ) + continue + + # Oversized page: split within the page only, keeping offsets + # absolute and the page number fixed. + for doc in self.splitter.create_documents([page_text]): + sub_start = start + doc.metadata.get("start_index", 0) + chunks.append( + ChunkWithPosition( + text=doc.page_content, + start_offset=sub_start, + end_offset=sub_start + len(doc.page_content), + page_number=page, + ) + ) + return chunks diff --git a/nextcloud_mcp_server/vector/processor.py b/nextcloud_mcp_server/vector/processor.py index 490f129d..ba969d7b 100644 --- a/nextcloud_mcp_server/vector/processor.py +++ b/nextcloud_mcp_server/vector/processor.py @@ -30,7 +30,10 @@ from nextcloud_mcp_server.observability.metrics import ( from nextcloud_mcp_server.observability.tracing import trace_operation from nextcloud_mcp_server.search.pdf_highlighter import PDFHighlighter from nextcloud_mcp_server.vector import payload_keys -from nextcloud_mcp_server.vector.document_chunker import DocumentChunker +from nextcloud_mcp_server.vector.document_chunker import ( + DocumentChunker, + PageAwareChunker, +) from nextcloud_mcp_server.vector.html_processor import html_to_markdown from nextcloud_mcp_server.vector.placeholder import ( delete_placeholder_point, @@ -682,27 +685,46 @@ async def _index_document( logger.error("Failed to process file %s: %s", file_path, e) raise - # Tokenize and chunk (using configured chunk size and overlap) + # Tokenize and chunk (using configured chunk size and overlap). Paginated + # files (PDFs with page_boundaries) use the page-aware chunker when enabled, + # which assigns page numbers inline; everything else uses the char-based + # chunker followed by post-hoc page assignment. + page_boundaries = file_metadata.get("page_boundaries") + use_page_aware = ( + settings.document_chunk_page_aware + and doc_task.doc_type == "file" + and page_boundaries is not None + ) with trace_operation( "vector_sync.chunk_text", attributes={ "vector_sync.input_chars": len(content), "vector_sync.chunk_size": settings.document_chunk_size, "vector_sync.overlap": settings.document_chunk_overlap, + "vector_sync.page_aware": use_page_aware, }, ) as chunk_span: - chunker = DocumentChunker( - chunk_size=settings.document_chunk_size, - overlap=settings.document_chunk_overlap, - ) - chunks = await chunker.chunk_text(content) + if use_page_aware: + page_boundaries_list = cast(list[dict[str, Any]], page_boundaries) + chunks = await PageAwareChunker( + chunk_size=settings.document_chunk_size, + overlap=settings.document_chunk_overlap, + ).chunk_text(content, page_boundaries_list) + else: + chunks = await DocumentChunker( + chunk_size=settings.document_chunk_size, + overlap=settings.document_chunk_overlap, + ).chunk_text(content) record_document_chunks(doc_task.doc_type, len(chunks)) if chunk_span is not None: chunk_span.set_attribute(_ATTR_CHUNK_COUNT, len(chunks)) - # Assign page numbers to chunks if page boundaries are available (PDFs) - page_boundaries = file_metadata.get("page_boundaries") - if doc_task.doc_type == "file" and page_boundaries is not None: + # Assign page numbers for the char-based path (page-aware already sets them). + if ( + not use_page_aware + and doc_task.doc_type == "file" + and page_boundaries is not None + ): # Type narrowing: page_boundaries is guaranteed to be list[dict] here page_boundaries_list = cast(list[dict[str, Any]], page_boundaries) with trace_operation( diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py index aa65155b..42cc35b9 100644 --- a/tests/unit/test_config.py +++ b/tests/unit/test_config.py @@ -167,6 +167,20 @@ class TestChunkConfigValidation: assert settings.document_chunk_size == 2048 assert settings.document_chunk_overlap == 200 + def test_page_aware_enabled_by_default(self): + """Page-aware chunking is on by default.""" + assert Settings().document_chunk_page_aware is True + + @patch.dict( + os.environ, + {"DOCUMENT_CHUNK_PAGE_AWARE": "false"}, + clear=True, + ) + def test_page_aware_disabled_via_env(self): + """DOCUMENT_CHUNK_PAGE_AWARE=false disables page-aware chunking.""" + _reload_config() + assert get_settings().document_chunk_page_aware is False + def test_valid_chunk_settings(self): """Test valid chunk size and overlap configuration.""" settings = Settings( diff --git a/tests/unit/test_document_chunker.py b/tests/unit/test_document_chunker.py index 66102a7e..94c6fe19 100644 --- a/tests/unit/test_document_chunker.py +++ b/tests/unit/test_document_chunker.py @@ -3,9 +3,28 @@ from nextcloud_mcp_server.vector.document_chunker import ( ChunkWithPosition, DocumentChunker, + PageAwareChunker, ) +def _make_doc(pages: list[str]) -> tuple[str, list[dict]]: + """Build (full_text, page_boundaries) the way the PDF extractors do. + + Page texts are concatenated with no separator and boundaries index exactly + into the result (the pypdfium2_fast contract). + """ + content = "" + boundaries: list[dict] = [] + offset = 0 + for i, text in enumerate(pages, start=1): + boundaries.append( + {"page": i, "start_offset": offset, "end_offset": offset + len(text)} + ) + content += text + offset += len(text) + return content, boundaries + + class TestDocumentChunkerPositions: """Test suite for DocumentChunker position tracking functionality.""" @@ -286,3 +305,120 @@ Fourth paragraph here.""" overlap_text = content[overlap_start:overlap_end] assert overlap_text in chunks[i].text assert overlap_text in chunks[i + 1].text + + +class TestPageAwareChunker: + """Test suite for the page-aware chunker.""" + + async def test_one_chunk_per_page_when_chunk_size_exceeds_pages(self): + """chunk_size >= largest page => exactly one chunk per page.""" + pages = [ + "Page one content.", + "Page two has rather more text than the first page does.", + "Third.", + ] + content, boundaries = _make_doc(pages) + + chunks = await PageAwareChunker(chunk_size=2048, overlap=200).chunk_text( + content, boundaries + ) + + # Predictable vector count == page count. + assert len(chunks) == len(pages) + for i, chunk in enumerate(chunks): + assert chunk.page_number == i + 1 + # No leading/trailing whitespace in these pages -> text == page text. + assert chunk.text == pages[i] + # Offsets remain exact against the original document. + assert content[chunk.start_offset : chunk.end_offset] == chunk.text + + async def test_no_chunk_spans_a_page_boundary(self): + """Every chunk's character range stays within one page (the invariant).""" + pages = [ + "Short.", + "word " * 200, # oversized page -> will be split within the page + "Another short page of text.", + " ", # blank page + "Final page content here.", + ] + content, boundaries = _make_doc(pages) + + chunks = await PageAwareChunker(chunk_size=200, overlap=20).chunk_text( + content, boundaries + ) + + for chunk in chunks: + assert chunk.page_number is not None + pb = boundaries[chunk.page_number - 1] + assert pb["start_offset"] <= chunk.start_offset + assert chunk.end_offset <= pb["end_offset"] + assert content[chunk.start_offset : chunk.end_offset] == chunk.text + + async def test_oversized_page_splits_others_stay_single(self): + """Only the oversized page yields multiple chunks; page numbers fixed.""" + pages = ["small page one", "word " * 200, "small page three"] + content, boundaries = _make_doc(pages) + + chunks = await PageAwareChunker(chunk_size=200, overlap=20).chunk_text( + content, boundaries + ) + + per_page = {1: 0, 2: 0, 3: 0} + for chunk in chunks: + per_page[chunk.page_number] += 1 + assert per_page[1] == 1 + assert per_page[2] > 1 + assert per_page[3] == 1 + + async def test_blank_pages_skipped(self): + """Whitespace-only pages produce no chunks (no wasted embeddings).""" + pages = ["Real content here.", " \n ", "More real content."] + content, boundaries = _make_doc(pages) + + chunks = await PageAwareChunker(chunk_size=2048, overlap=200).chunk_text( + content, boundaries + ) + + assert len(chunks) == 2 + assert {c.page_number for c in chunks} == {1, 3} + + async def test_offsets_tightened_around_page_whitespace(self): + """Leading/trailing page whitespace is stripped and offsets adjusted.""" + pages = [" Leading and trailing. ", "Normal page."] + content, boundaries = _make_doc(pages) + + chunks = await PageAwareChunker(chunk_size=2048, overlap=200).chunk_text( + content, boundaries + ) + + first = chunks[0] + assert first.text == "Leading and trailing." + assert content[first.start_offset : first.end_offset] == first.text + + async def test_no_page_boundaries_falls_back_to_char_chunking(self): + """Without page boundaries, behaves like the char-based chunker.""" + content = "This is sentence one. " * 40 + + pa_chunks = await PageAwareChunker(chunk_size=100, overlap=20).chunk_text( + content, [] + ) + char_chunks = await DocumentChunker(chunk_size=100, overlap=20).chunk_text( + content + ) + + assert len(pa_chunks) > 1 + # Same chunk boundaries as the char-based path, and no page numbers. + assert [(c.text, c.start_offset) for c in pa_chunks] == [ + (c.text, c.start_offset) for c in char_chunks + ] + assert all(c.page_number is None for c in pa_chunks) + + async def test_empty_content_returns_single_empty_chunk(self): + """Empty content returns one empty chunk regardless of boundaries.""" + chunks = await PageAwareChunker().chunk_text( + "", [{"page": 1, "start_offset": 0, "end_offset": 0}] + ) + assert len(chunks) == 1 + assert chunks[0].text == "" + assert chunks[0].start_offset == 0 + assert chunks[0].end_offset == 0 From 4977216b62bf806e99352fbb16dc4f5d4cff1e04 Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Sat, 6 Jun 2026 13:46:32 +0200 Subject: [PATCH 2/7] docs: correct chunk-size units (characters, default 2048) in configuration DOCUMENT_CHUNK_SIZE/OVERLAP were documented as "words" with a 512/50 default; the implementation measures characters and defaults to 2048/200 (config.py, DocumentChunker). Update docs/configuration.md (config block, tuning guidance, examples, env-var table) and env.sample accordingly, and cross-reference DOCUMENT_CHUNK_PAGE_AWARE for the PDF path. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/configuration.md | 34 ++++++++++++++++++---------------- env.sample | 6 +++--- 2 files changed, 21 insertions(+), 19 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index cb4f6052..064c9995 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -512,8 +512,8 @@ VECTOR_SYNC_PROCESSOR_WORKERS=3 # Concurrent indexing workers (default: 3) VECTOR_SYNC_QUEUE_MAX_SIZE=10000 # Max queued documents (default: 10000) # Document chunking settings (for vector embeddings) -DOCUMENT_CHUNK_SIZE=512 # Words per chunk (default: 512) -DOCUMENT_CHUNK_OVERLAP=50 # Overlapping words between chunks (default: 50) +DOCUMENT_CHUNK_SIZE=2048 # Characters per chunk (default: 2048) +DOCUMENT_CHUNK_OVERLAP=200 # Overlapping characters between chunks (default: 200) ``` > **Note:** The `VECTOR_SYNC_*` tuning parameters keep their names as they're implementation details. Only the user-facing feature flag was renamed to `ENABLE_SEMANTIC_SEARCH`. @@ -592,44 +592,46 @@ The server chunks documents before embedding to handle documents larger than the #### Choosing Chunk Size -**Smaller chunks (256-384 words)**: +**Smaller chunks (1024-1536 characters)**: - More precise matching - Less context per chunk - Better for finding specific information - Higher storage requirements (more vectors) -**Larger chunks (768-1024 words)**: +**Larger chunks (3072-4096 characters)**: - More context per chunk - Less precise matching - Better for understanding broader topics - Lower storage requirements (fewer vectors) -**Default (512 words)**: +**Default (2048 characters)**: - Balanced approach suitable for most use cases - Works well with typical note lengths - Good compromise between precision and context +> For PDFs, `DOCUMENT_CHUNK_PAGE_AWARE` (default `true`) overrides this trade-off by chunking one page at a time — see the entry below. + #### Choosing Overlap Overlap preserves context across chunk boundaries. Recommended settings: -- **10-20% of chunk size** (e.g., 50-100 words for 512-word chunks) +- **10-20% of chunk size** (e.g., 200-400 characters for 2048-character chunks) - **Too small** (<10%): May lose context at boundaries - **Too large** (>20%): Redundant storage, diminishing returns **Examples**: ```dotenv # Precise matching for short notes -DOCUMENT_CHUNK_SIZE=256 -DOCUMENT_CHUNK_OVERLAP=25 - -# Default balanced configuration -DOCUMENT_CHUNK_SIZE=512 -DOCUMENT_CHUNK_OVERLAP=50 - -# More context for long documents DOCUMENT_CHUNK_SIZE=1024 DOCUMENT_CHUNK_OVERLAP=100 + +# Default balanced configuration +DOCUMENT_CHUNK_SIZE=2048 +DOCUMENT_CHUNK_OVERLAP=200 + +# More context for long documents +DOCUMENT_CHUNK_SIZE=4096 +DOCUMENT_CHUNK_OVERLAP=400 ``` **Important**: Changing chunk size requires re-embedding all documents. The collection naming strategy (see "Qdrant Collection Naming" above) helps manage this by creating separate collections for different configurations. @@ -741,8 +743,8 @@ equivalent.** Operators who need a runtime toggle should open an issue. | `BEDROCK_EMBEDDING_MODEL` | ⚠️ Optional | - | Bedrock embedding model ID | | `BEDROCK_GENERATION_MODEL` | ⚠️ Optional | - | Bedrock generation model ID | | `SIMPLE_EMBEDDING_DIMENSION` | ⚠️ Optional | `384` | Dimension for the fallback Simple provider | -| `DOCUMENT_CHUNK_SIZE` | ⚠️ Optional | `512` | Words per chunk for document embedding | -| `DOCUMENT_CHUNK_OVERLAP` | ⚠️ Optional | `50` | Overlapping words between chunks (must be < chunk size) | +| `DOCUMENT_CHUNK_SIZE` | ⚠️ Optional | `2048` | Characters per chunk for document embedding | +| `DOCUMENT_CHUNK_OVERLAP` | ⚠️ Optional | `200` | Overlapping characters between chunks (must be < chunk size) | | `DOCUMENT_CHUNK_PAGE_AWARE` | ⚠️ Optional | `true` | Split PDFs on page boundaries first (one chunk per page; oversized pages split within the page). Exact page numbers, clean snippets, and a predictable ~1 chunk/page when chunk size ≥ the largest page. Set `false` for the legacy char-based path. | **Deprecated variables (still functional):** diff --git a/env.sample b/env.sample index 30799fdc..e845e94d 100644 --- a/env.sample +++ b/env.sample @@ -203,9 +203,9 @@ NEXTCLOUD_PASSWORD= # Uses basic in-memory embeddings if no provider configured # # Document Chunking: -# Configure how documents are split before embedding -#DOCUMENT_CHUNK_SIZE=512 -#DOCUMENT_CHUNK_OVERLAP=50 +# Configure how documents are split before embedding (units are characters) +#DOCUMENT_CHUNK_SIZE=2048 +#DOCUMENT_CHUNK_OVERLAP=200 # Page-aware chunking for PDFs: split on page boundaries first so no chunk spans # a page (exact page numbers, clean snippets, ~1 chunk/page when chunk size >= # the largest page). Set false to use the legacy char-based path. Default: true From 8d20339b3a99fd308da6b61c54c3b1a51c274dd8 Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Sat, 6 Jun 2026 13:56:30 +0200 Subject: [PATCH 3/7] 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."] From bb4ef809c2c83c1cd0a595c659157c24b2f3ce86 Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Sat, 6 Jun 2026 14:02:05 +0200 Subject: [PATCH 4/7] test(vector): unit-test page-aware routing; clarify fallback comment Address claude-review round 2 on PR #868: - Extract the use_page_aware branching into a pure `should_use_page_aware` helper and cover the (doc_type, page_boundaries, page_aware_setting) matrix in tests/unit/test_processor_routing.py (file+boundaries+enabled, empty list, None, non-file doc types, disabled setting). - Clarify the PageAwareChunker.chunk_text no-boundaries comment: the processor pre-filters via should_use_page_aware, so that branch is a direct-call safety net, not a production indexing path. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../vector/document_chunker.py | 7 ++- nextcloud_mcp_server/vector/processor.py | 30 ++++++--- tests/unit/test_processor_routing.py | 62 +++++++++++++++++++ 3 files changed, 90 insertions(+), 9 deletions(-) create mode 100644 tests/unit/test_processor_routing.py diff --git a/nextcloud_mcp_server/vector/document_chunker.py b/nextcloud_mcp_server/vector/document_chunker.py index a71d4b5f..0f45f892 100644 --- a/nextcloud_mcp_server/vector/document_chunker.py +++ b/nextcloud_mcp_server/vector/document_chunker.py @@ -167,8 +167,11 @@ class PageAwareChunker: if not content: return [ChunkWithPosition(text="", start_offset=0, end_offset=0)] - # No page info (e.g. a non-PDF that reached this path) — degrade to the - # char-based behaviour so the caller still gets sensible chunks. + # No page info — degrade to char-based behaviour so the class is safe to + # call directly. The vector-sync processor pre-filters this case + # (``should_use_page_aware`` requires a truthy boundary list), so in + # production this branch is only reached by direct callers/tests, not + # the indexing path. if not page_boundaries: docs = await anyio.to_thread.run_sync( # type: ignore[attr-defined] self.splitter.create_documents, diff --git a/nextcloud_mcp_server/vector/processor.py b/nextcloud_mcp_server/vector/processor.py index 55e998a4..a879e340 100644 --- a/nextcloud_mcp_server/vector/processor.py +++ b/nextcloud_mcp_server/vector/processor.py @@ -92,6 +92,25 @@ def assign_page_numbers(chunks, page_boundaries): chunk.page_number = assigned_page +def should_use_page_aware( + *, page_aware_enabled: bool, doc_type: str, page_boundaries: Any +) -> bool: + """Decide whether the page-aware chunker applies to this document. + + Page-aware chunking applies only to paginated files (PDFs) that actually + carry page boundaries. ``page_boundaries`` is tested for truthiness, not + just ``is not None``: an empty list carries no pages, so it routes through + the char-based path rather than the page-aware chunker's no-boundaries + fallback. + + Args: + page_aware_enabled: ``settings.document_chunk_page_aware``. + doc_type: The document type (only ``"file"`` is paginated). + page_boundaries: The extractor's page-boundary list (or ``None``). + """ + return page_aware_enabled and doc_type == "file" and bool(page_boundaries) + + async def processor_task( worker_id: int, receive_stream: MemoryObjectReceiveStream[DocumentTask], @@ -690,13 +709,10 @@ async def _index_document( # which assigns page numbers inline; everything else uses the char-based # chunker followed by post-hoc page assignment. page_boundaries = file_metadata.get("page_boundaries") - use_page_aware = ( - settings.document_chunk_page_aware - and doc_task.doc_type == "file" - # 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) + use_page_aware = should_use_page_aware( + page_aware_enabled=settings.document_chunk_page_aware, + doc_type=doc_task.doc_type, + page_boundaries=page_boundaries, ) with trace_operation( "vector_sync.chunk_text", diff --git a/tests/unit/test_processor_routing.py b/tests/unit/test_processor_routing.py new file mode 100644 index 00000000..5e581625 --- /dev/null +++ b/tests/unit/test_processor_routing.py @@ -0,0 +1,62 @@ +"""Unit tests for the page-aware chunker routing decision (processor.py).""" + +import pytest + +from nextcloud_mcp_server.vector.processor import should_use_page_aware + +pytestmark = pytest.mark.unit + +_BOUNDARIES = [{"page": 1, "start_offset": 0, "end_offset": 10}] + + +class TestShouldUsePageAware: + """Cover the (doc_type, page_boundaries, page_aware_setting) matrix.""" + + def test_pdf_with_boundaries_and_enabled_uses_page_aware(self): + assert ( + should_use_page_aware( + page_aware_enabled=True, + doc_type="file", + page_boundaries=_BOUNDARIES, + ) + is True + ) + + def test_empty_boundaries_falls_back_to_char_based(self): + """Empty list carries no pages -> char-based path.""" + assert ( + should_use_page_aware( + page_aware_enabled=True, doc_type="file", page_boundaries=[] + ) + is False + ) + + def test_none_boundaries_falls_back_to_char_based(self): + assert ( + should_use_page_aware( + page_aware_enabled=True, doc_type="file", page_boundaries=None + ) + is False + ) + + @pytest.mark.parametrize("doc_type", ["note", "deck_card", "news_item"]) + def test_non_file_doc_types_never_page_aware(self, doc_type): + """Only paginated files are page-aware, even with boundaries present.""" + assert ( + should_use_page_aware( + page_aware_enabled=True, + doc_type=doc_type, + page_boundaries=_BOUNDARIES, + ) + is False + ) + + def test_disabled_setting_forces_char_based(self): + assert ( + should_use_page_aware( + page_aware_enabled=False, + doc_type="file", + page_boundaries=_BOUNDARIES, + ) + is False + ) From 20b4bc6ab96e007444fea5576ed5fd478f13b4e1 Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Sat, 6 Jun 2026 14:07:01 +0200 Subject: [PATCH 5/7] test(vector): mark test_document_chunker as unit Address claude-review round 3 on PR #868: add module-level `pytestmark = pytest.mark.unit` so TestPageAwareChunker and TestDocumentChunkerPositions are collected under `-m unit`, matching test_processor_routing.py. Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/unit/test_document_chunker.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/unit/test_document_chunker.py b/tests/unit/test_document_chunker.py index 9927c1c2..5ee184b2 100644 --- a/tests/unit/test_document_chunker.py +++ b/tests/unit/test_document_chunker.py @@ -1,11 +1,15 @@ """Unit tests for DocumentChunker with LangChain text splitters.""" +import pytest + from nextcloud_mcp_server.vector.document_chunker import ( ChunkWithPosition, DocumentChunker, PageAwareChunker, ) +pytestmark = pytest.mark.unit + def _make_doc(pages: list[str]) -> tuple[str, list[dict]]: """Build (full_text, page_boundaries) the way the PDF extractors do. From 446320983a99b4c98a6933657e2dc0a3acd0c0dc Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Sat, 6 Jun 2026 14:13:07 +0200 Subject: [PATCH 6/7] fix(vector): skip page-assignment span/warning on empty boundaries Address claude-review round 4 on PR #868: tighten the assign_page_numbers guard from `page_boundaries is not None` to a truthy check, so a PDF with an empty boundary list no longer enters the trace span and fires the alarming "NO page numbers assigned" warning for a harmless no-op. Co-Authored-By: Claude Opus 4.8 (1M context) --- nextcloud_mcp_server/vector/processor.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/nextcloud_mcp_server/vector/processor.py b/nextcloud_mcp_server/vector/processor.py index a879e340..4e99cb6e 100644 --- a/nextcloud_mcp_server/vector/processor.py +++ b/nextcloud_mcp_server/vector/processor.py @@ -739,11 +739,9 @@ async def _index_document( chunk_span.set_attribute(_ATTR_CHUNK_COUNT, len(chunks)) # Assign page numbers for the char-based path (page-aware already sets them). - if ( - not use_page_aware - and doc_task.doc_type == "file" - and page_boundaries is not None - ): + # Truthy guard (not "is not None"): an empty boundary list has nothing to + # assign, so skip the span and the "NO page numbers assigned" warning. + if not use_page_aware and doc_task.doc_type == "file" and page_boundaries: # Type narrowing: page_boundaries is guaranteed to be list[dict] here page_boundaries_list = cast(list[dict[str, Any]], page_boundaries) with trace_operation( From 0fada20d35f3834ae5ca0cd0ef3fd1eec7a1723d Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Sat, 6 Jun 2026 14:20:03 +0200 Subject: [PATCH 7/7] test(vector): pin empty-chunk-list parity for all-blank pages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address claude-review round 5 on PR #868: add test_all_blank_pages_returns_empty_list documenting that PageAwareChunker returns [] when every page is blank — and asserting parity with DocumentChunker, which already returns [] for whitespace-only non-empty content. The empty-chunk-list case is therefore pre-existing pipeline behavior, not new to this PR. Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/unit/test_document_chunker.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/unit/test_document_chunker.py b/tests/unit/test_document_chunker.py index 5ee184b2..fbdeaf50 100644 --- a/tests/unit/test_document_chunker.py +++ b/tests/unit/test_document_chunker.py @@ -439,6 +439,22 @@ class TestPageAwareChunker: ] assert all(c.page_number is None for c in pa_chunks) + async def test_all_blank_pages_returns_empty_list(self): + """Non-empty content whose every page is blank yields no chunks. + + Matches DocumentChunker, which also returns [] for whitespace-only + non-empty content — the downstream pipeline handles an empty chunk + list identically for both chunkers. + """ + pages = [" ", "\n\n", "\t"] + content, boundaries = _make_doc(pages) + + chunks = await PageAwareChunker().chunk_text(content, boundaries) + + assert chunks == [] + # Parity: the char-based chunker behaves the same for blank content. + assert await DocumentChunker().chunk_text(content) == [] + async def test_empty_content_returns_single_empty_chunk(self): """Empty content returns one empty chunk regardless of boundaries.""" chunks = await PageAwareChunker().chunk_text(