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) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
4977216b62
commit
8d20339b3a
@@ -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",
|
||||
|
||||
@@ -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."]
|
||||
|
||||
Reference in New Issue
Block a user