You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
SemanticChunkerBase.chunk_text() enriches chunks with _extra_chunk_metadata() on the normal path, but the zero/one-sentence fallback returns via _create_single_sentence_chunk (autobot-backend/utils/semantic_chunker_base.py:259-269, reached from chunk_text() at :351), which bypasses _enrich_chunks_with_metadata/_build_chunk_metadata entirely.
So for short input, subclass metadata — optimization_version, and anything else a subclass contributes — is absent, while the same call on longer input carries it.
How it surfaced
Found while reviewing PR #14456 (#14215), which removed OptimizedSemanticChunker after proving GPUSemanticChunker already stamps optimization_version via _extra_chunk_metadata().
That equivalence holds for the multi-sentence path and not for this one. The deleted chunk_text_optimized() looped over the returned chunks after the fact, so it stamped unconditionally regardless of which path produced them.
The removal is still correct — no production consumer ever read that metadata, verified by grepping the value string across the whole tree. But the covering claim is narrower than "every chunk from chunk_text()", and that distinction should live somewhere other than a closed PR's review.
Why it is worth fixing rather than just documenting
The inconsistency is invisible in the shape that matters: a caller reading chunk.metadata["optimization_version"] gets a value for most inputs and a KeyError or None for short ones. Nothing fails; the field is simply missing sometimes. That is the kind of conditional-shape defect that surfaces as a confusing bug report much later, and it would be a genuine one the moment any consumer starts keying on subclass metadata.
It is also a subclass-extension seam: _extra_chunk_metadata() is the documented way for a subclass to contribute metadata, and it silently does not apply on one of the two return paths. Any future subclass inherits the same surprise.
What is needed
Have _create_single_sentence_chunk apply _extra_chunk_metadata() too — or route it through _build_chunk_metadata so there is one place that decides a chunk's metadata rather than two.
Check the empty-input path as well while there; the fallback covers zero and one sentence, and they may not behave identically.
Acceptance criteria
A chunk produced from single-sentence input carries the same subclass metadata keys as one from multi-sentence input
Asserted by a test that calls the realchunk_text() with one sentence and with several, comparing the metadata key sets rather than checking one literal
The empty-input path is covered by the same assertion or explicitly excluded with a reason
Metadata assembly happens in one place, so a future subclass cannot inherit a path that skips it
Closed by PR #14482, squash-merged as 812d6fd5afbb and verified in origin/Dev_new_gui.
chunk_text() now converges both success branches on a single enrichment call site, so zero- and one-sentence input carries the same subclass metadata as longer input.
That structural point was the review item, and it matters more than the original bug: the first fix added an enrichment call to the path that was missing one, leaving two sites that had to agree. "Two paths, each independently responsible for remembering the same step" is exactly what produced #14467. Now the branches only assign chunks and one trailing call enriches — a future path added to that function cannot reintroduce the defect by omission. The existing tests were untouched by that collapse (verified: zero diff), confirming it was structural rather than behavioural.
The test drives the real GPUSemanticChunker.chunk_text() with multi-, single- and empty-sentence input rather than calling the private method, and it discriminates: against the pre-fix shape it fails with missing subclass metadata keys: {'chunking_method', 'optimization_version', 'embedding_model', 'source_metadata', 'chunk_index', 'total_chunks'}.
No stored data is affected, and that was traced rather than assumed.knowledge/pipeline/loaders/chromadb_loader.py:91's **chunk.metadata only ever receives ProcessedChunk objects from the pipeline's own extractor, which imports none of these classes. Of the two production callers of SemanticChunkerBase.chunk_text(), knowledge_sync_incremental.py reads only content/semantic_score/sentences and builds its own metadata dict, and advanced_rag_optimizer.py never calls chunk_text() at all — it reads the chunker as a truthy flag. chunk_document(), the one method surfacing chunk.metadata verbatim, has zero production callers. So these fields were never persisted on either the buggy or the fixed path, and no migration is owed.
Two follow-ups filed rather than folded in:
bug(kb): exception-driven _fallback_chunking also bypasses subclass metadata enrichment #14480 — the exception-driven _fallback_chunking path is a third return from chunk_text() and is still un-enriched; _create_fallback_chunk builds its own dict and bypasses _build_chunk_metadata. Same class of defect. After this PR the function is single-sited for two of three paths, and the PR body says so explicitly rather than letting "one enrichment call site" read as covering everything.
kb: two independent semantic chunkers — the ingest pipeline uses neither the embedding-based one nor its metadata #14485 — the reason the blast-radius search kept coming back nearly empty: there are two independent semantic chunkers, and the ingest pipeline uses the token-budget one (knowledge/pipeline/extractors/semantic_chunker.py), not the embedding-similarity one this issue is about. So this class's metadata never reaches ingested documents at all. The emptiness was the finding.
Activity
refactor(kb): converge chunk_text on a single enrichment call site (#…
fix(kb): single-sentence chunks skip subclass metadata (#14467)
mrveiss commentedon Aug 18, 2026
Closed by PR #14482, squash-merged as
812d6fd5afbband verified inorigin/Dev_new_gui.chunk_text()now converges both success branches on a single enrichment call site, so zero- and one-sentence input carries the same subclass metadata as longer input.That structural point was the review item, and it matters more than the original bug: the first fix added an enrichment call to the path that was missing one, leaving two sites that had to agree. "Two paths, each independently responsible for remembering the same step" is exactly what produced #14467. Now the branches only assign
chunksand one trailing call enriches — a future path added to that function cannot reintroduce the defect by omission. The existing tests were untouched by that collapse (verified: zero diff), confirming it was structural rather than behavioural.The test drives the real
GPUSemanticChunker.chunk_text()with multi-, single- and empty-sentence input rather than calling the private method, and it discriminates: against the pre-fix shape it fails withmissing subclass metadata keys: {'chunking_method', 'optimization_version', 'embedding_model', 'source_metadata', 'chunk_index', 'total_chunks'}.No stored data is affected, and that was traced rather than assumed.
knowledge/pipeline/loaders/chromadb_loader.py:91's**chunk.metadataonly ever receivesProcessedChunkobjects from the pipeline's own extractor, which imports none of these classes. Of the two production callers ofSemanticChunkerBase.chunk_text(),knowledge_sync_incremental.pyreads onlycontent/semantic_score/sentencesand builds its own metadata dict, andadvanced_rag_optimizer.pynever callschunk_text()at all — it reads the chunker as a truthy flag.chunk_document(), the one method surfacingchunk.metadataverbatim, has zero production callers. So these fields were never persisted on either the buggy or the fixed path, and no migration is owed.Two follow-ups filed rather than folded in:
_fallback_chunkingpath is a third return fromchunk_text()and is still un-enriched;_create_fallback_chunkbuilds its own dict and bypasses_build_chunk_metadata. Same class of defect. After this PR the function is single-sited for two of three paths, and the PR body says so explicitly rather than letting "one enrichment call site" read as covering everything.knowledge/pipeline/extractors/semantic_chunker.py), not the embedding-similarity one this issue is about. So this class's metadata never reaches ingested documents at all. The emptiness was the finding.