fix(kb): single-sentence chunks skip subclass metadata (#14467)

[mrveiss]

Closes #14467

Thinking Path

SemanticChunkerBase.chunk_text() has two return paths that produce chunks successfully: the primary boundary-detection path, which calls _enrich_chunks_with_metadata() (→ _build_chunk_metadata() → _extra_chunk_metadata()), and the zero/one-sentence fallback via _create_single_sentence_chunk(), which returned directly without ever calling enrichment. Identical input differing only in sentence count therefore produced chunks with different metadata shapes — the short-input chunk silently missing chunk_index, total_chunks, embedding_model, chunking_method, and any subclass-contributed key (optimization_version on GPUSemanticChunker).

Before touching the fix, I mapped the blast radius:

• Subclasses of SemanticChunkerBase: AutoBotSemanticChunker (utils/semantic_chunker.py, CPU) and GPUSemanticChunker (utils/semantic_chunker_gpu.py). Both override only _extra_chunk_metadata(), _initialize_model(), _compute_embeddings() — neither overrides chunk_text() in a way that bypasses the shared enrichment call (GPUSemanticChunker.chunk_text() wraps super().chunk_text() for perf logging only).
  • AutoBotSemanticChunker._extra_chunk_metadata() adds chunking_method="semantic_percentile", percentile_threshold.
  • GPUSemanticChunker._extra_chunk_metadata() adds chunking_method="gpu_optimized_semantic", gpu_batch_size, optimization_version="rtx4070_gpu".
• Consumers of chunk_text()/chunk.metadata: knowledge_sync_incremental.py (_process_file_with_gpu_chunking → _store_chunks_in_knowledge_base) is the only production caller that both invokes chunk_text() and persists chunk data — but it builds the stored metadata dict itself from base_metadata plus a handful of fields it computes directly (semantic_score, sentence_count, character_count, chunk_index, total_chunks, gpu_optimized); it never reads chunk.metadata, so optimization_version/chunking_method were never persisted to the knowledge base by this caller either way. advanced_rag_optimizer.py constructs a GPUSemanticChunker but never calls chunk_text() on it (used only as an availability flag). structured_ops.py reads only chunk.content. The public chunk_document() LlamaIndex-style entry point (which does surface chunk.metadata) has no production caller — only a test does.
  • Conclusion: no already-stored data is affected. Nothing in the production path ever persisted the subclass metadata this bug drops, on either the buggy or the fixed path. No migration or backfill is needed or included.
  • Why the search came back nearly empty: kb: two independent semantic chunkers — the ingest pipeline uses neither the embedding-based one nor its metadata #14485. The document-ingest pipeline (knowledge/pipeline/extractors/semantic_chunker.py, the ProcessedChunk-based ChromaDBLoader path) uses a completely separate chunker that does not subclass SemanticChunkerBase at all. This class's metadata — buggy or fixed — never reaches ingested documents through that pipeline. kb: two independent semantic chunkers — the ingest pipeline uses neither the embedding-based one nor its metadata #14485 tracks that as its own finding; cross-linked here so the next reader doesn't re-derive it.
• While tracing all three return paths out of chunk_text() (primary, single/zero-sentence fallback, and the exception-driven _fallback_chunking()), I found the exception path has the identical gap. Kept out of scope for this PR and filed as bug(kb): exception-driven _fallback_chunking also bypasses subclass metadata enrichment #14480 to avoid mixing an unreported symptom into this fix.

Review follow-up: the first version of this fix made the single/zero-sentence branch call _enrich_chunks_with_metadata() itself, leaving chunk_text() with two separate call sites that happened to agree — the same "two paths, each independently responsible for remembering the same step" shape that produced #14467. Collapsed both branches to only assign chunks, converging on one trailing _enrich_chunks_with_metadata() call before the coherence-logging block, so a future branch added to this function cannot reintroduce the defect by omission.

Scope note: after this change, chunk_text() is single-sited for two of its three return paths (the primary path and the single/zero-sentence fallback). The third — the exception-driven _fallback_chunking() path reached via the except block — still builds its own metadata dict in _create_fallback_chunk() and bypasses _build_chunk_metadata() entirely. That gap is unchanged by this PR and is tracked separately as #14480.

What Changed

• autobot-backend/utils/semantic_chunker_base.py:
  • _create_single_sentence_chunk() no longer accepts/returns the caller-supplied metadata as the chunk's final metadata. It now only sets the chunk-shape markers (single_sentence, plus empty_input when the sentence list is empty).
  • chunk_text()'s two success branches (single/zero-sentence and boundary-detection) now only assign chunks; a single trailing self._enrich_chunks_with_metadata(chunks, metadata) call runs before the existing coherence-logging block, for both. The exception-driven _fallback_chunking() call in the except block is untouched (tracked as bug(kb): exception-driven _fallback_chunking also bypasses subclass metadata enrichment #14480).
• autobot-backend/utils/semantic_chunker_gpu_test.py: added TestShortInputChunksCarryTheSameMetadataAsLongInput, driving the real GPUSemanticChunker.chunk_text() (not _create_single_sentence_chunk()/_build_chunk_metadata() directly) with multi-sentence, single-sentence, and empty (filtered-to-zero-sentence) input, asserting the same subclass/base metadata key set (chunk_index, total_chunks, source_metadata, embedding_model, chunking_method, gpu_batch_size, optimization_version) is present on all three.

Verification

• python3 -m py_compile on both changed files — syntax clean.
• Extracted the enrichment-relevant logic into a standalone script outside the repo and ran the same key-set assertion against three shapes:
  • Pre-fix shape: fails — AssertionError: [single] missing subclass metadata keys: {'chunking_method', 'optimization_version', 'embedding_model', 'source_metadata', 'chunk_index', 'total_chunks'} (got ['single_sentence']).
  • First-fix shape (two enrichment call sites): passes.
  • Collapsed shape (single trailing enrichment call, matching this PR's current state): passes — PASS: single-sentence and empty-input chunks carry the same subclass metadata keys as multi-sentence chunks.
  • This confirms the new test discriminates the underlying bug (fails pre-fix, passes post-fix) and that collapsing to one call site is behavior-preserving, not just structural in name.
• The existing test assertions were not edited by the collapse — autobot-backend/utils/semantic_chunker_gpu_test.py has zero diff between the two fix commits; only semantic_chunker_base.py changed.
• CI will run the actual pytest suite (autobot-backend/utils/semantic_chunker_gpu_test.py) against the real repo modules, per project policy of not executing repo code locally.

Model Used

Claude Sonnet 5 (model ID: claude-sonnet-5)

[mrveiss]

Review — APPROVE with one structural item

The fix is correct and the test discriminates. Verified independently rather than on report:

• The signature change is safe. _create_single_sentence_chunk has exactly one call site (chunk_text itself), and neither subclass overrides it or reaches it independently.
• The test starts where the data originates. It drives the real GPUSemanticChunker.chunk_text(), never the private method. Hand-traced _split_into_sentences: "Hi." is filtered to zero sentences by the len(s.split()) >= 3 rule, and the single-sentence fixture yields exactly one qualifying sentence — so the fixtures really do exercise the paths their names claim. Both key-set assertions fail against the pre-fix source and pass against the fixed one.
• No stored data is affected — confirmed by tracing, not by accepting the claim. 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:502-529 reads only content/semantic_score/sentences and builds its own metadata dict, and advanced_rag_optimizer.py:306 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.

That last point independently corroborates #14485: the ingest pipeline uses a different chunker entirely.

The one item — enrichment is two-sited, and that is the shape of the bug

semantic_chunker_base.py:359-362 and :373-374 are two separate self._enrich_chunks_with_metadata(chunks, metadata) calls inside chunk_text(). They agree today. But "two paths, each independently responsible for remembering the same step" is precisely what produced #14467 in the first place — the fix currently makes the forgotten path remember, rather than removing the need to remember.

Collapse the two branches into one assignment converging on a single trailing enrichment call before the if chunks: logging block. Then the next path added to this function cannot reintroduce the defect by omission.

Noted, not counted against this PR: the exception-driven return await self._fallback_chunking(...) at :386-388 is a third return that is still un-enriched — _create_fallback_chunk builds its own dict and bypasses _build_chunk_metadata entirely. Same class of defect, honestly disclosed in the PR body and tracked as #14480, whose description names the same two functions and the same missing-key set. Worth being explicit that after this PR the function is single-sited for two of three paths, not all three.

Standards clean: no print(), logging through the canonical wrapper, no file I/O added, no hardcoded config, no variant-prefix naming.

[github-actions]

✅ SSOT Configuration Compliance: Passing

🎉 No hardcoded values detected that have SSOT config equivalents!

[mrveiss]

The red here is inherited from the base — do not "fix" it in this PR

code-quality (and python_file_size_ratchet_test.py::test_recorded_ceilings_match_the_files_today in the Python suite) are failing on:

autobot-backend/chat_workflow/tool_handler.py: 4081 lines, over its recorded ceiling of 4063.

This PR did not cause it. Dev_new_gui itself is over the ceiling — from my merge of #14470 (0bd3326cb, closing #14420) — and Actions checks out the pull-request merge commit, so every open PR fails on the base's content regardless of what it changes.

Do not raise KNOWN_LARGE to make this green. The ratchet exists to refuse exactly that, and its message says so: "the exemption freezes the size it was granted for, it does not license more."

Fix in flight: PR #14497 (#14495) extracts the six _enforce_* dispatch guards into chat_workflow/tool_dispatch_guards.py, taking the file 4081 → 3863 and lowering the ceiling to match. Once that lands, rebase or push an empty commit and this goes green on its own.

Related: #14498 records a gap in the ratchet itself — its header says "raising a ceiling must fail" while the test only blocks raises above a frozen baseline, so a shrink can be spent back one compliant-looking PR at a time.