Merge pull request #868 from cbcoutinho/feat/page-aware-chunking
feat(vector): page-aware PDF chunking for predictable per-page retrieval
This commit is contained in:
+19
-16
@@ -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)
|
VECTOR_SYNC_QUEUE_MAX_SIZE=10000 # Max queued documents (default: 10000)
|
||||||
|
|
||||||
# Document chunking settings (for vector embeddings)
|
# Document chunking settings (for vector embeddings)
|
||||||
DOCUMENT_CHUNK_SIZE=512 # Words per chunk (default: 512)
|
DOCUMENT_CHUNK_SIZE=2048 # Characters per chunk (default: 2048)
|
||||||
DOCUMENT_CHUNK_OVERLAP=50 # Overlapping words between chunks (default: 50)
|
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`.
|
> **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
|
#### Choosing Chunk Size
|
||||||
|
|
||||||
**Smaller chunks (256-384 words)**:
|
**Smaller chunks (1024-1536 characters)**:
|
||||||
- More precise matching
|
- More precise matching
|
||||||
- Less context per chunk
|
- Less context per chunk
|
||||||
- Better for finding specific information
|
- Better for finding specific information
|
||||||
- Higher storage requirements (more vectors)
|
- Higher storage requirements (more vectors)
|
||||||
|
|
||||||
**Larger chunks (768-1024 words)**:
|
**Larger chunks (3072-4096 characters)**:
|
||||||
- More context per chunk
|
- More context per chunk
|
||||||
- Less precise matching
|
- Less precise matching
|
||||||
- Better for understanding broader topics
|
- Better for understanding broader topics
|
||||||
- Lower storage requirements (fewer vectors)
|
- Lower storage requirements (fewer vectors)
|
||||||
|
|
||||||
**Default (512 words)**:
|
**Default (2048 characters)**:
|
||||||
- Balanced approach suitable for most use cases
|
- Balanced approach suitable for most use cases
|
||||||
- Works well with typical note lengths
|
- Works well with typical note lengths
|
||||||
- Good compromise between precision and context
|
- 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
|
#### Choosing Overlap
|
||||||
|
|
||||||
Overlap preserves context across chunk boundaries. Recommended settings:
|
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 small** (<10%): May lose context at boundaries
|
||||||
- **Too large** (>20%): Redundant storage, diminishing returns
|
- **Too large** (>20%): Redundant storage, diminishing returns
|
||||||
|
|
||||||
**Examples**:
|
**Examples**:
|
||||||
```dotenv
|
```dotenv
|
||||||
# Precise matching for short notes
|
# 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_SIZE=1024
|
||||||
DOCUMENT_CHUNK_OVERLAP=100
|
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.
|
**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,9 @@ equivalent.** Operators who need a runtime toggle should open an issue.
|
|||||||
| `BEDROCK_EMBEDDING_MODEL` | ⚠️ Optional | - | Bedrock embedding model ID |
|
| `BEDROCK_EMBEDDING_MODEL` | ⚠️ Optional | - | Bedrock embedding model ID |
|
||||||
| `BEDROCK_GENERATION_MODEL` | ⚠️ Optional | - | Bedrock generation model ID |
|
| `BEDROCK_GENERATION_MODEL` | ⚠️ Optional | - | Bedrock generation model ID |
|
||||||
| `SIMPLE_EMBEDDING_DIMENSION` | ⚠️ Optional | `384` | Dimension for the fallback Simple provider |
|
| `SIMPLE_EMBEDDING_DIMENSION` | ⚠️ Optional | `384` | Dimension for the fallback Simple provider |
|
||||||
| `DOCUMENT_CHUNK_SIZE` | ⚠️ Optional | `512` | Words per chunk for document embedding |
|
| `DOCUMENT_CHUNK_SIZE` | ⚠️ Optional | `2048` | Characters per chunk for document embedding |
|
||||||
| `DOCUMENT_CHUNK_OVERLAP` | ⚠️ Optional | `50` | Overlapping words between chunks (must be < chunk size) |
|
| `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):**
|
**Deprecated variables (still functional):**
|
||||||
- `VECTOR_SYNC_ENABLED` - Use `ENABLE_SEMANTIC_SEARCH` instead (will be removed in v1.0.0)
|
- `VECTOR_SYNC_ENABLED` - Use `ENABLE_SEMANTIC_SEARCH` instead (will be removed in v1.0.0)
|
||||||
|
|||||||
+7
-3
@@ -203,9 +203,13 @@ NEXTCLOUD_PASSWORD=
|
|||||||
# Uses basic in-memory embeddings if no provider configured
|
# Uses basic in-memory embeddings if no provider configured
|
||||||
#
|
#
|
||||||
# Document Chunking:
|
# Document Chunking:
|
||||||
# Configure how documents are split before embedding
|
# Configure how documents are split before embedding (units are characters)
|
||||||
#DOCUMENT_CHUNK_SIZE=512
|
#DOCUMENT_CHUNK_SIZE=2048
|
||||||
#DOCUMENT_CHUNK_OVERLAP=50
|
#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
|
||||||
|
#DOCUMENT_CHUNK_PAGE_AWARE=true
|
||||||
|
|
||||||
# ===== SEMANTIC SEARCH TUNING =====
|
# ===== SEMANTIC SEARCH TUNING =====
|
||||||
# Advanced parameters for vector sync background operations
|
# Advanced parameters for vector sync background operations
|
||||||
|
|||||||
@@ -132,6 +132,10 @@ _DEFAULTS: dict[str, Any] = {
|
|||||||
# Document chunking
|
# Document chunking
|
||||||
"document_chunk_size": 2048,
|
"document_chunk_size": 2048,
|
||||||
"document_chunk_overlap": 200,
|
"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)
|
# PDF parse isolation (OOM guard)
|
||||||
"document_pdf_graphics_limit": 1000,
|
"document_pdf_graphics_limit": 1000,
|
||||||
"document_parse_timeout_seconds": 120.0,
|
"document_parse_timeout_seconds": 120.0,
|
||||||
@@ -736,6 +740,13 @@ class Settings:
|
|||||||
# Document chunking settings (for vector embeddings)
|
# Document chunking settings (for vector embeddings)
|
||||||
document_chunk_size: int = 2048 # Characters per chunk
|
document_chunk_size: int = 2048 # Characters per chunk
|
||||||
document_chunk_overlap: int = 200 # Overlapping characters between chunks
|
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
|
# PDF parse isolation (OOM guard). The parse runs in a subprocess so one
|
||||||
# pathological file fails that doc, not the pod.
|
# pathological file fails that doc, not the pod.
|
||||||
@@ -1389,6 +1400,7 @@ def get_settings() -> Settings:
|
|||||||
# Document chunking settings
|
# Document chunking settings
|
||||||
"document_chunk_size": "DOCUMENT_CHUNK_SIZE",
|
"document_chunk_size": "DOCUMENT_CHUNK_SIZE",
|
||||||
"document_chunk_overlap": "DOCUMENT_CHUNK_OVERLAP",
|
"document_chunk_overlap": "DOCUMENT_CHUNK_OVERLAP",
|
||||||
|
"document_chunk_page_aware": "DOCUMENT_CHUNK_PAGE_AWARE",
|
||||||
"document_pdf_graphics_limit": "DOCUMENT_PDF_GRAPHICS_LIMIT",
|
"document_pdf_graphics_limit": "DOCUMENT_PDF_GRAPHICS_LIMIT",
|
||||||
"document_parse_timeout_seconds": "DOCUMENT_PARSE_TIMEOUT_SECONDS",
|
"document_parse_timeout_seconds": "DOCUMENT_PARSE_TIMEOUT_SECONDS",
|
||||||
"document_parse_mem_limit_mb": "DOCUMENT_PARSE_MEM_LIMIT_MB",
|
"document_parse_mem_limit_mb": "DOCUMENT_PARSE_MEM_LIMIT_MB",
|
||||||
|
|||||||
@@ -2,6 +2,7 @@
|
|||||||
|
|
||||||
import logging
|
import logging
|
||||||
from dataclasses import dataclass
|
from dataclasses import dataclass
|
||||||
|
from typing import Any
|
||||||
|
|
||||||
import anyio
|
import anyio
|
||||||
from langchain_text_splitters import RecursiveCharacterTextSplitter
|
from langchain_text_splitters import RecursiveCharacterTextSplitter
|
||||||
@@ -97,3 +98,153 @@ class DocumentChunker:
|
|||||||
self.overlap,
|
self.overlap,
|
||||||
)
|
)
|
||||||
return chunks
|
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 — 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,
|
||||||
|
[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
|
||||||
|
|||||||
@@ -30,7 +30,10 @@ from nextcloud_mcp_server.observability.metrics import (
|
|||||||
from nextcloud_mcp_server.observability.tracing import trace_operation
|
from nextcloud_mcp_server.observability.tracing import trace_operation
|
||||||
from nextcloud_mcp_server.search.pdf_highlighter import PDFHighlighter
|
from nextcloud_mcp_server.search.pdf_highlighter import PDFHighlighter
|
||||||
from nextcloud_mcp_server.vector import payload_keys
|
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.html_processor import html_to_markdown
|
||||||
from nextcloud_mcp_server.vector.placeholder import (
|
from nextcloud_mcp_server.vector.placeholder import (
|
||||||
delete_placeholder_point,
|
delete_placeholder_point,
|
||||||
@@ -89,6 +92,25 @@ def assign_page_numbers(chunks, page_boundaries):
|
|||||||
chunk.page_number = assigned_page
|
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(
|
async def processor_task(
|
||||||
worker_id: int,
|
worker_id: int,
|
||||||
receive_stream: MemoryObjectReceiveStream[DocumentTask],
|
receive_stream: MemoryObjectReceiveStream[DocumentTask],
|
||||||
@@ -682,27 +704,44 @@ async def _index_document(
|
|||||||
logger.error("Failed to process file %s: %s", file_path, e)
|
logger.error("Failed to process file %s: %s", file_path, e)
|
||||||
raise
|
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 = 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(
|
with trace_operation(
|
||||||
"vector_sync.chunk_text",
|
"vector_sync.chunk_text",
|
||||||
attributes={
|
attributes={
|
||||||
"vector_sync.input_chars": len(content),
|
"vector_sync.input_chars": len(content),
|
||||||
"vector_sync.chunk_size": settings.document_chunk_size,
|
"vector_sync.chunk_size": settings.document_chunk_size,
|
||||||
"vector_sync.overlap": settings.document_chunk_overlap,
|
"vector_sync.overlap": settings.document_chunk_overlap,
|
||||||
|
"vector_sync.page_aware": use_page_aware,
|
||||||
},
|
},
|
||||||
) as chunk_span:
|
) as chunk_span:
|
||||||
chunker = DocumentChunker(
|
if use_page_aware:
|
||||||
chunk_size=settings.document_chunk_size,
|
page_boundaries_list = cast(list[dict[str, Any]], page_boundaries)
|
||||||
overlap=settings.document_chunk_overlap,
|
chunks = await PageAwareChunker(
|
||||||
)
|
chunk_size=settings.document_chunk_size,
|
||||||
chunks = await chunker.chunk_text(content)
|
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))
|
record_document_chunks(doc_task.doc_type, len(chunks))
|
||||||
if chunk_span is not None:
|
if chunk_span is not None:
|
||||||
chunk_span.set_attribute(_ATTR_CHUNK_COUNT, len(chunks))
|
chunk_span.set_attribute(_ATTR_CHUNK_COUNT, len(chunks))
|
||||||
|
|
||||||
# Assign page numbers to chunks if page boundaries are available (PDFs)
|
# Assign page numbers for the char-based path (page-aware already sets them).
|
||||||
page_boundaries = file_metadata.get("page_boundaries")
|
# Truthy guard (not "is not None"): an empty boundary list has nothing to
|
||||||
if doc_task.doc_type == "file" and page_boundaries is not None:
|
# 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
|
# Type narrowing: page_boundaries is guaranteed to be list[dict] here
|
||||||
page_boundaries_list = cast(list[dict[str, Any]], page_boundaries)
|
page_boundaries_list = cast(list[dict[str, Any]], page_boundaries)
|
||||||
with trace_operation(
|
with trace_operation(
|
||||||
|
|||||||
@@ -167,6 +167,20 @@ class TestChunkConfigValidation:
|
|||||||
assert settings.document_chunk_size == 2048
|
assert settings.document_chunk_size == 2048
|
||||||
assert settings.document_chunk_overlap == 200
|
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):
|
def test_valid_chunk_settings(self):
|
||||||
"""Test valid chunk size and overlap configuration."""
|
"""Test valid chunk size and overlap configuration."""
|
||||||
settings = Settings(
|
settings = Settings(
|
||||||
|
|||||||
@@ -1,10 +1,33 @@
|
|||||||
"""Unit tests for DocumentChunker with LangChain text splitters."""
|
"""Unit tests for DocumentChunker with LangChain text splitters."""
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
from nextcloud_mcp_server.vector.document_chunker import (
|
from nextcloud_mcp_server.vector.document_chunker import (
|
||||||
ChunkWithPosition,
|
ChunkWithPosition,
|
||||||
DocumentChunker,
|
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.
|
||||||
|
|
||||||
|
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:
|
class TestDocumentChunkerPositions:
|
||||||
"""Test suite for DocumentChunker position tracking functionality."""
|
"""Test suite for DocumentChunker position tracking functionality."""
|
||||||
@@ -286,3 +309,158 @@ Fourth paragraph here."""
|
|||||||
overlap_text = content[overlap_start:overlap_end]
|
overlap_text = content[overlap_start:overlap_end]
|
||||||
assert overlap_text in chunks[i].text
|
assert overlap_text in chunks[i].text
|
||||||
assert overlap_text in chunks[i + 1].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_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."]
|
||||||
|
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_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(
|
||||||
|
"", [{"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
|
||||||
|
|||||||
@@ -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
|
||||||
|
)
|
||||||
Reference in New Issue
Block a user