diff --git a/internal/application/service/knowledge.go b/internal/application/service/knowledge.go index 1411f86b6..b7f226254 100644 --- a/internal/application/service/knowledge.go +++ b/internal/application/service/knowledge.go @@ -2021,6 +2021,7 @@ func (s *knowledgeService) processChunks(ctx context.Context, KnowledgeID: knowledge.ID, KnowledgeBaseID: knowledge.KnowledgeBaseID, Content: chunkData.Content, + ContextHeader: chunkData.ContextHeader, ChunkIndex: int(chunkData.Seq), IsEnabled: true, CreatedAt: time.Now(), @@ -2102,7 +2103,10 @@ func (s *knowledgeService) processChunks(ctx context.Context, titlePrefix = t + "\n" } for _, chunk := range textChunks { - indexContent := titlePrefix + chunk.Content + // chunk.EmbeddingContent prepends ContextHeader (heading breadcrumb) + // when the chunker populated it during Tier-1 splitting; falls back + // to plain Content otherwise. Title prefix sits outermost. + indexContent := titlePrefix + chunk.EmbeddingContent() indexInfoList = append(indexInfoList, &types.IndexInfo{ Content: indexContent, SourceID: chunk.ID, @@ -7494,11 +7498,12 @@ func (s *knowledgeService) triggerManualProcessing(ctx context.Context, parsed = make([]types.ParsedChunk, len(pcResult.Children)) for i, c := range pcResult.Children { parsed[i] = types.ParsedChunk{ - Content: c.Content, - Seq: c.Seq, - Start: c.Start, - End: c.End, - ParentIndex: c.ParentIndex, + Content: c.Content, + ContextHeader: c.ContextHeader, + Seq: c.Seq, + Start: c.Start, + End: c.End, + ParentIndex: c.ParentIndex, } } parentChunks := make([]types.ParsedParentChunk, len(pcResult.Parents)) @@ -7511,10 +7516,11 @@ func (s *knowledgeService) triggerManualProcessing(ctx context.Context, parsed = make([]types.ParsedChunk, len(splitChunks)) for i, c := range splitChunks { parsed[i] = types.ParsedChunk{ - Content: c.Content, - Seq: c.Seq, - Start: c.Start, - End: c.End, + Content: c.Content, + ContextHeader: c.ContextHeader, + Seq: c.Seq, + Start: c.Start, + End: c.End, } } } @@ -8350,11 +8356,12 @@ func (s *knowledgeService) ProcessDocument(ctx context.Context, t *asynq.Task) e chunks = make([]types.ParsedChunk, len(pcResult.Children)) for i, c := range pcResult.Children { chunks[i] = types.ParsedChunk{ - Content: c.Content, - Seq: c.Seq, - Start: c.Start, - End: c.End, - ParentIndex: c.ParentIndex, + Content: c.Content, + ContextHeader: c.ContextHeader, + Seq: c.Seq, + Start: c.Start, + End: c.End, + ParentIndex: c.ParentIndex, } } parentChunks := make([]types.ParsedParentChunk, len(pcResult.Parents)) @@ -8369,10 +8376,11 @@ func (s *knowledgeService) ProcessDocument(ctx context.Context, t *asynq.Task) e chunks = make([]types.ParsedChunk, len(splitChunks)) for i, c := range splitChunks { chunks[i] = types.ParsedChunk{ - Content: c.Content, - Seq: c.Seq, - Start: c.Start, - End: c.End, + Content: c.Content, + ContextHeader: c.ContextHeader, + Seq: c.Seq, + Start: c.Start, + End: c.End, } } logger.Infof(ctx, "Split document into %d chunks for knowledge %s", len(chunks), knowledge.ID) diff --git a/internal/infrastructure/chunker/heading_splitter.go b/internal/infrastructure/chunker/heading_splitter.go index 3789aa148..4a9e13416 100644 --- a/internal/infrastructure/chunker/heading_splitter.go +++ b/internal/infrastructure/chunker/heading_splitter.go @@ -74,35 +74,34 @@ func splitByHeadingsImpl(text string, cfg SplitterConfig) []Chunk { } bcLen := utf8.RuneCountInString(breadcrumb) - // Reserve some headroom (breadcrumb + 2 newlines) when fitting a section. + // Single-chunk section: emit as-is, breadcrumb tracked separately. + // The breadcrumb is delivered via Chunk.ContextHeader (not Content) + // to preserve End-Start == len(Content) invariants relied on by + // document reconstruction (knowledge.go:2278+). if bcLen+2+secLen <= cfg.ChunkSize { - content := prependBreadcrumb(sectionContent, breadcrumb) out = append(out, Chunk{ - Content: content, - Seq: seq, - Start: b.runeStart, - End: endRune, + Content: sectionContent, + ContextHeader: breadcrumb, + Seq: seq, + Start: b.runeStart, + End: endRune, }) seq++ continue } - // Section too large for one chunk: defer to the legacy splitter for - // inner segmentation, then prepend the breadcrumb to every sub-chunk. - // Reduce the inner budget by the breadcrumb length so the final - // chunk (incl. breadcrumb) still fits under cfg.ChunkSize. - innerCfg := cfg - if bcLen > 0 && innerCfg.ChunkSize > bcLen+10 { - innerCfg.ChunkSize -= bcLen + 2 - } - subChunks := SplitText(sectionContent, innerCfg) + // Section too large: defer to the legacy splitter for inner + // segmentation. Sub-chunks inherit the same breadcrumb via + // ContextHeader. We do NOT shrink the inner ChunkSize budget here + // because the breadcrumb no longer counts against Content size. + subChunks := SplitText(sectionContent, cfg) for _, sub := range subChunks { - content := prependBreadcrumb(sub.Content, breadcrumb) out = append(out, Chunk{ - Content: content, - Seq: seq, - Start: b.runeStart + sub.Start, - End: b.runeStart + sub.End, + Content: sub.Content, + ContextHeader: breadcrumb, + Seq: seq, + Start: b.runeStart + sub.Start, + End: b.runeStart + sub.End, }) seq++ } @@ -189,16 +188,3 @@ func observeSubHeadings(runes []rune, primaryLevel int, h *HeadingHierarchy) { } } -// prependBreadcrumb attaches the breadcrumb to content unless content -// already begins with that exact breadcrumb (avoid duplication when a -// section's first line is itself the section heading). -func prependBreadcrumb(content, breadcrumb string) string { - if breadcrumb == "" { - return content - } - trimmed := strings.TrimLeft(content, " \t\r\n") - if strings.HasPrefix(trimmed, breadcrumb) { - return content - } - return breadcrumb + "\n\n" + content -} diff --git a/internal/infrastructure/chunker/heading_splitter_test.go b/internal/infrastructure/chunker/heading_splitter_test.go index 9132eb7be..f8257b306 100644 --- a/internal/infrastructure/chunker/heading_splitter_test.go +++ b/internal/infrastructure/chunker/heading_splitter_test.go @@ -23,14 +23,17 @@ content of C.` t.Fatalf("expected ≥3 chunks (one per section), got %d", len(chunks)) } - // Each chunk should contain the H1 + section heading as breadcrumb. + // Breadcrumb is delivered via ContextHeader, not Content. for i, c := range chunks { - if !strings.Contains(c.Content, "# Top") { - t.Errorf("chunk %d missing H1 breadcrumb:\n%s", i, c.Content) + if !strings.Contains(c.ContextHeader, "# Top") { + t.Errorf("chunk %d missing H1 in ContextHeader:\n%q", i, c.ContextHeader) + } + // EmbeddingContent merges header + content for the embedder. + if !strings.Contains(c.EmbeddingContent(), "# Top") { + t.Errorf("chunk %d EmbeddingContent missing H1", i) } } - // Section B chunk should mention Section B. found := false for _, c := range chunks { if strings.Contains(c.Content, "Section B") && strings.Contains(c.Content, "content of B") { @@ -60,10 +63,10 @@ func TestSplitByHeadings_LargeSectionRecursesIntoLegacy(t *testing.T) { if len(chunks) < 2 { t.Fatalf("large section should be sub-split, got %d chunks", len(chunks)) } - // Every sub-chunk should still carry the breadcrumb. + // Every sub-chunk should carry the breadcrumb via ContextHeader. for i, c := range chunks { - if !strings.Contains(c.Content, "# Top") { - t.Errorf("sub-chunk %d missing H1 breadcrumb", i) + if !strings.Contains(c.ContextHeader, "# Top") { + t.Errorf("sub-chunk %d missing H1 in ContextHeader", i) } } } @@ -82,14 +85,13 @@ text B` if len(chunks) < 3 { t.Fatalf("expected ≥3 chunks, got %d", len(chunks)) } - // Section B chunk's breadcrumb should NOT mention Section A for _, c := range chunks { if strings.Contains(c.Content, "text B") { - if strings.Contains(c.Content, "## Section A") { - t.Errorf("Section B chunk should not include Section A in breadcrumb:\n%s", c.Content) + if strings.Contains(c.ContextHeader, "## Section A") { + t.Errorf("Section B chunk should not include Section A in breadcrumb:\n%s", c.ContextHeader) } - if !strings.Contains(c.Content, "## Section B") { - t.Errorf("Section B chunk should include its own heading:\n%s", c.Content) + if !strings.Contains(c.ContextHeader, "## Section B") { + t.Errorf("Section B chunk should include its own heading in breadcrumb:\n%s", c.ContextHeader) } } } @@ -99,15 +101,12 @@ func TestSplitByHeadings_IgnoresHeadingsInsideCodeFence(t *testing.T) { doc := "# Real\n\n```\n# Fake heading inside code\n```\n\nbody" cfg := SplitterConfig{ChunkSize: 500, ChunkOverlap: 0} chunks := splitByHeadingsImpl(doc, cfg) - // The fake heading should not create a section boundary — there's only - // one real H1, so we expect either 1 chunk or fall-through. for _, c := range chunks { - if strings.Contains(c.Content, "# Real") { - // Good — found the real one. + if strings.Contains(c.ContextHeader, "# Real") || strings.Contains(c.Content, "# Real") { return } } - t.Error("expected real H1 breadcrumb in some chunk") + t.Error("expected real H1 breadcrumb on some chunk") } func TestSplitByHeadings_PreservesPositionRelativeToOriginal(t *testing.T) { @@ -123,3 +122,64 @@ func TestSplitByHeadings_PreservesPositionRelativeToOriginal(t *testing.T) { } } } + +// TestSplitByHeadings_PositionInvariant ensures End-Start == len(Content) +// and runes[Start:End] == Content for every emitted chunk. This invariant +// is required by knowledge.go:2278+ document reconstruction logic. +func TestSplitByHeadings_PositionInvariant(t *testing.T) { + doc := `# Top +intro paragraph here. + +## Section A +content of A here, several sentences. + +## Section B +content of B here. + +## Section C +content of C here.` + cfg := SplitterConfig{ChunkSize: 200, ChunkOverlap: 20} + chunks := splitByHeadingsImpl(doc, cfg) + if len(chunks) == 0 { + t.Fatal("expected chunks") + } + docRunes := []rune(doc) + for i, c := range chunks { + contentRuneLen := len([]rune(c.Content)) + span := c.End - c.Start + if span != contentRuneLen { + t.Errorf("chunk %d: span(%d) != content_runes(%d)\nContent:\n%q", i, span, contentRuneLen, c.Content) + } + if c.Start >= 0 && c.End <= len(docRunes) { + if string(docRunes[c.Start:c.End]) != c.Content { + t.Errorf("chunk %d: runes[Start:End] != Content", i) + } + } + } +} + +// TestSplitByHeadings_NoBreadcrumbDuplication ensures the section's own +// heading line does not appear twice in the chunk content (once as part of +// the breadcrumb, once as the section's first line). +func TestSplitByHeadings_NoBreadcrumbDuplication(t *testing.T) { + doc := `# Chapter 1 +intro. + +## Section A +body A. + +## Section B +body B.` + cfg := SplitterConfig{ChunkSize: 500, ChunkOverlap: 0} + chunks := splitByHeadingsImpl(doc, cfg) + for i, c := range chunks { + // Count occurrences of "## Section A" / "## Section B" + for _, heading := range []string{"## Section A", "## Section B"} { + n := strings.Count(c.Content, heading) + if n > 1 { + t.Errorf("chunk %d contains %q %d times — duplicated by breadcrumb prepend:\n%s", + i, heading, n, c.Content) + } + } + } +} diff --git a/internal/infrastructure/chunker/heuristic_splitter.go b/internal/infrastructure/chunker/heuristic_splitter.go index 502b448b1..a9bc74c53 100644 --- a/internal/infrastructure/chunker/heuristic_splitter.go +++ b/internal/infrastructure/chunker/heuristic_splitter.go @@ -82,8 +82,9 @@ func splitByHeuristicsImpl(text string, cfg SplitterConfig) []Chunk { if accumulated > cfg.ChunkSize && curEnd-chunkStart >= minChunkSize { // Flush accumulated content as a chunk, restart at curEnd. out = appendChunk(out, runes, chunkStart, curEnd, &seq) - // Apply overlap by shifting chunkStart back a bit. - chunkStart = applyOverlap(curEnd, cfg.ChunkOverlap) + // Snap overlap start to the nearest semantic boundary or line + // break instead of slicing mid-line / mid-word. + chunkStart = applyOverlapAligned(runes, curEnd, cfg.ChunkOverlap, bounds) } curEnd = nextEnd } @@ -227,16 +228,42 @@ func appendOversizeBlock(out []Chunk, runes []rune, start, end int, cfg Splitter return out } -// applyOverlap shifts the next chunk's start back by `overlap` runes (but -// not before zero) so consecutive chunks share content. Mirrors the -// computeOverlap behavior of the legacy splitter. -func applyOverlap(curEnd, overlap int) int { +// applyOverlapAligned returns the rune offset where the next chunk should +// start. The target is `curEnd - overlap`, but we snap to the nearest +// preceding boundary (within 2x overlap) or, failing that, the previous +// newline so chunks don't begin mid-line / mid-word. Falls back to the raw +// target only if neither option is available. +func applyOverlapAligned(runes []rune, curEnd, overlap int, bounds []boundary) int { if overlap <= 0 { return curEnd } - start := curEnd - overlap - if start < 0 { - start = 0 + target := curEnd - overlap + if target < 0 { + target = 0 } - return start + // Allowed search window: [curEnd - 2*overlap, curEnd] + windowStart := curEnd - 2*overlap + if windowStart < 0 { + windowStart = 0 + } + + // Prefer a semantic boundary inside the window. + bestBound := -1 + for _, b := range bounds { + if b.runeStart >= windowStart && b.runeStart <= curEnd && b.runeStart > bestBound { + bestBound = b.runeStart + } + } + if bestBound >= 0 { + return bestBound + } + + // Fallback: scan backwards from `target` to the previous newline, but + // not past windowStart so we keep the overlap roughly the right size. + for i := target; i > windowStart && i < len(runes); i-- { + if runes[i] == '\n' { + return i + 1 + } + } + return target } diff --git a/internal/infrastructure/chunker/splitter.go b/internal/infrastructure/chunker/splitter.go index 5e8db3514..1cf51c5b0 100644 --- a/internal/infrastructure/chunker/splitter.go +++ b/internal/infrastructure/chunker/splitter.go @@ -12,11 +12,33 @@ import ( ) // Chunk represents a piece of split text with position tracking. +// +// Content holds exactly the text from the original document between Start +// and End (rune offsets), so End-Start == utf8.RuneCountInString(Content). +// This invariant is relied on by document-reconstruction code paths +// (knowledge.go:2278+ for summary generation, UI highlighting, etc.). +// +// ContextHeader is a separately-tracked context string (e.g. a Markdown +// heading breadcrumb) that should be prepended at embedding/retrieval time +// but is NOT part of Content. Keeping the two apart preserves the +// position invariant while still letting embedding pipelines see the +// section context. type Chunk struct { - Content string - Seq int - Start int - End int + Content string + ContextHeader string + Seq int + Start int + End int +} + +// EmbeddingContent returns the text that should be fed to the embedding +// model — the ContextHeader prepended (when set) plus the chunk content. +// Use this where Content alone would lose semantic context (Tier-1 chunks). +func (c Chunk) EmbeddingContent() string { + if c.ContextHeader == "" { + return c.Content + } + return c.ContextHeader + "\n\n" + c.Content } // ImageRef is an image reference found within a chunk's content. @@ -46,9 +68,20 @@ type SplitterConfig struct { // Default sizes used by all entry points (DefaultConfig, ensureDefaults, // and buildSplitterConfig in the knowledge service). +// +// MIGRATION NOTE: Prior versions had three different overlap defaults +// (Go DefaultConfig: 64, knowledge.go buildSplitterConfig: 50, Python +// docreader: 100). This file is now the single source of truth at 80 +// (≈15% of DefaultChunkSize) — a community-recommended sweet spot. +// +// Existing knowledge bases that stored ChunkOverlap=0 in the DB will pick +// up 80 on next re-index; their previously-indexed embeddings will not +// match new ones bit-for-bit. Recall stays similar but search ranking +// can shift slightly. To freeze the old behavior on a per-KB basis, +// explicitly set ChunkingConfig.ChunkOverlap to 64 before re-indexing. const ( DefaultChunkSize = 512 - DefaultChunkOverlap = 80 // ≈ 15% of DefaultChunkSize + DefaultChunkOverlap = 80 ) // DefaultConfig returns sensible defaults. @@ -122,17 +155,24 @@ type splitUnit struct { start, end int } -// splitBySeparators splits text by separators in priority order, keeping the -// separators themselves as standalone units. Walks the separator list in -// order: the first separator that actually splits the text wins, the rest -// are not applied. Mirrors the recursive priority semantics of the Python -// reference splitter (docreader/splitter/splitter.py:_split). -func splitBySeparators(text string, separators []string) []string { - if len(separators) == 0 || text == "" { +// splitBySeparators splits text by separators in priority order, recursively +// applying the next separator to any piece that is still larger than +// chunkSize. Mirrors the recursive priority semantics of the Python +// reference splitter (docreader/splitter/splitter.py:_split): if `\n\n` +// produces a piece that's still too big, `\n` (and subsequent separators) +// are applied within that piece — not to the whole text. +// +// chunkSize == 0 disables the recursion guard; callers that don't care +// about size budget (e.g. a final mergeUnits-style pass) pass 0. +func splitBySeparators(text string, separators []string, chunkSize int) []string { + if text == "" || len(separators) == 0 { + return []string{text} + } + if chunkSize > 0 && runeLen(text) <= chunkSize { return []string{text} } - for _, sep := range separators { + for i, sep := range separators { if sep == "" { continue } @@ -143,18 +183,31 @@ func splitBySeparators(text string, separators []string) []string { continue } - var result []string - for i, s := range splits { + var pieces []string + for j, s := range splits { if s != "" { - result = append(result, s) + pieces = append(pieces, s) } - if i < len(matches) && matches[i] != "" { - result = append(result, matches[i]) + if j < len(matches) && matches[j] != "" { + pieces = append(pieces, matches[j]) } } - if len(result) > 1 { - return result + if len(pieces) <= 1 { + continue } + + // Recursively split any piece that is still too large with the + // remaining (lower-priority) separators. + var out []string + remaining := separators[i+1:] + for _, p := range pieces { + if chunkSize > 0 && runeLen(p) > chunkSize && len(remaining) > 0 { + out = append(out, splitBySeparators(p, remaining, chunkSize)...) + } else { + out = append(out, p) + } + } + return out } return []string{text} } @@ -184,8 +237,10 @@ func SplitText(text string, cfg SplitterConfig) []Chunk { // Step 1: Find protected spans protected := protectedSpans(text) - // Step 2: Split non-protected regions by separators, keep protected as atomic units - units := buildUnitsWithProtection(text, protected, separators) + // Step 2: Split non-protected regions by separators, keep protected as atomic units. + // chunkSize is forwarded so splitBySeparators can recursively apply lower-priority + // separators to oversize pieces (Python-parity recursive split). + units := buildUnitsWithProtection(text, protected, separators, chunkSize) // Step 3: Merge units into chunks with overlap return mergeUnits(units, chunkSize, chunkOverlap) @@ -196,7 +251,9 @@ func SplitText(text string, cfg SplitterConfig) []Chunk { // because downstream merge logic indexes content via []rune slicing. // If a protected span exceeds maxProtectedSize, it will be forcibly split to prevent // creating chunks that are too large for downstream processing (e.g., embedding APIs). -func buildUnitsWithProtection(text string, protected []span, separators []string) []splitUnit { +// chunkSize is forwarded to splitBySeparators so recursive splitting can keep pieces +// under the budget when one separator alone leaves a piece oversize. +func buildUnitsWithProtection(text string, protected []span, separators []string, chunkSize int) []splitUnit { const maxProtectedSize = 7500 // Maximum size for a protected unit (留余量给标题等) var units []splitUnit @@ -206,7 +263,7 @@ func buildUnitsWithProtection(text string, protected []span, separators []string for _, p := range protected { if p.start > bytePos { pre := text[bytePos:p.start] - parts := splitBySeparators(pre, separators) + parts := splitBySeparators(pre, separators, chunkSize) runeOffset := runePos for _, part := range parts { partRuneLen := runeLen(part) @@ -265,7 +322,7 @@ func buildUnitsWithProtection(text string, protected []span, separators []string if bytePos < len(text) { remaining := text[bytePos:] - parts := splitBySeparators(remaining, separators) + parts := splitBySeparators(remaining, separators, chunkSize) runeOffset := runePos for _, part := range parts { partRuneLen := runeLen(part) diff --git a/internal/infrastructure/chunker/splitter_test.go b/internal/infrastructure/chunker/splitter_test.go index b0e9d4386..91b2e581a 100644 --- a/internal/infrastructure/chunker/splitter_test.go +++ b/internal/infrastructure/chunker/splitter_test.go @@ -168,6 +168,36 @@ func TestSplitText_SimulateMergeSlicing(t *testing.T) { } } +// TestSplitText_RecursiveSeparators_NoOversizeChunks exposes the regression +// where after picking the first separator that yields >1 piece, sub-pieces +// that are still larger than ChunkSize were not split further with the next +// separator. Real-world docs with one paragraph break followed by a long +// run of newline-separated lines must still be honored. +func TestSplitText_RecursiveSeparators_NoOversizeChunks(t *testing.T) { + // One paragraph break, then 50 short newline-separated lines forming + // ~1500 chars in the second paragraph. + body := strings.Repeat("This is one fairly short line of text.\n", 50) + text := "lead paragraph that is short.\n\n" + body + cfg := SplitterConfig{ + ChunkSize: 300, + ChunkOverlap: 30, + Separators: []string{"\n\n", "\n", ". "}, + } + chunks := SplitText(text, cfg) + if len(chunks) < 2 { + t.Fatalf("expected multiple chunks, got %d", len(chunks)) + } + // No chunk should exceed roughly 1.5x ChunkSize — recursive splitting + // at the next-priority separator should keep this bounded. + maxAllowed := cfg.ChunkSize * 3 / 2 + for i, c := range chunks { + l := len([]rune(c.Content)) + if l > maxAllowed { + t.Errorf("chunk %d is %d runes, > 1.5x ChunkSize (%d) — recursive split missing", i, l, maxAllowed) + } + } +} + func TestSplitText_Empty(t *testing.T) { chunks := SplitText("", DefaultConfig()) if len(chunks) != 0 { @@ -242,7 +272,7 @@ func TestSplitText_OverlapChunks_NonNegativeStart(t *testing.T) { func TestBuildUnitsWithProtection_RuneOffsets(t *testing.T) { text := "你好世界" - units := buildUnitsWithProtection(text, nil, []string{"\n"}) + units := buildUnitsWithProtection(text, nil, []string{"\n"}, 0) if len(units) != 1 { t.Fatalf("expected 1 unit, got %d", len(units)) @@ -263,7 +293,7 @@ func TestBuildUnitsWithProtection_RuneOffsets(t *testing.T) { func TestBuildUnitsWithProtection_WithProtectedSpan(t *testing.T) { text := "前面![alt](url)后面" protected := protectedSpans(text) - units := buildUnitsWithProtection(text, protected, []string{"\n"}) + units := buildUnitsWithProtection(text, protected, []string{"\n"}, 0) textRunes := []rune(text) for i, u := range units { @@ -293,7 +323,7 @@ func TestSplitBySeparators(t *testing.T) { } for _, tt := range tests { - parts := splitBySeparators(tt.text, tt.separators) + parts := splitBySeparators(tt.text, tt.separators, 0) if len(parts) != tt.wantParts { t.Errorf("splitBySeparators(%q, %v): got %d parts %v, want %d", tt.text, tt.separators, len(parts), parts, tt.wantParts) diff --git a/internal/infrastructure/chunker/strategy.go b/internal/infrastructure/chunker/strategy.go index 5162588b6..cf5e1bde0 100644 --- a/internal/infrastructure/chunker/strategy.go +++ b/internal/infrastructure/chunker/strategy.go @@ -35,21 +35,35 @@ func Split(text string, cfg SplitterConfig) []Chunk { chain := resolveChain(text, cfg) totalChars := len([]rune(text)) - for _, tier := range chain { + var lastOut []Chunk + for i, tier := range chain { out := runTier(tier, text, cfg) if v := ValidateChunks(out, totalChars, cfg.ChunkSize); v.OK { return out } else { logger.Debugf(context.Background(), "chunker: tier %s rejected: %s", tier, v.Reason) } + // Remember the legacy tier's output: we'll return it as-is below if + // every tier rejected — running SplitText again would just produce + // the same rejected result. + if tier == TierLegacy && i == len(chain)-1 { + lastOut = out + } } - // Last-ditch fallback: always return *something*. + if lastOut != nil { + return lastOut + } + // Defensive last-ditch fallback (only reached if the chain didn't end on TierLegacy). return SplitText(text, cfg) } // SplitParentChild is the strategy-aware analog of SplitTextParentChild. // It runs the tier selector for parent splitting, then re-splits each // parent into children with the small-chunk config. +// +// Children are forced to recursive splitting — they are sub-pieces of an +// already-segmented parent, so re-profiling per parent would cost N extra +// O(N) document scans without any real chance of picking a better tier. func SplitParentChild(text string, parentCfg, childCfg SplitterConfig) ParentChildResult { if text == "" { return ParentChildResult{} @@ -62,6 +76,12 @@ func SplitParentChild(text string, parentCfg, childCfg SplitterConfig) ParentChi return ParentChildResult{} } + // Pin children to the recursive tier so SplitText runs directly without + // the profiler/strategy chain re-deciding per parent. Preserves Tier-1 + // breadcrumb context (already attached to each parent) since we only + // re-split parent content, not re-detect headings. + childCfg.Strategy = StrategyRecursive + var newParents []Chunk var children []ChildChunk childSeq := 0 @@ -77,6 +97,12 @@ func SplitParentChild(text string, parentCfg, childCfg SplitterConfig) ParentChi sub.Seq = childSeq sub.Start += parent.Start sub.End += parent.Start + // Children inherit the parent's breadcrumb so embedding still + // sees the section context — but only if the child itself does + // not already carry one (sub-splits don't, but defensive). + if sub.ContextHeader == "" { + sub.ContextHeader = parent.ContextHeader + } children = append(children, ChildChunk{Chunk: sub, ParentIndex: parentIndex}) childSeq++ } @@ -146,11 +172,15 @@ func ensureDefaults(cfg SplitterConfig) SplitterConfig { charBudget := CharsForTokenLimit(cfg.TokenLimit, lang) if charBudget > 0 && (cfg.ChunkSize == 0 || charBudget < cfg.ChunkSize) { cfg.ChunkSize = charBudget - if cfg.ChunkOverlap >= cfg.ChunkSize { - cfg.ChunkOverlap = cfg.ChunkSize / 5 - } } } + // Guard against pathological overlap configurations: if Overlap exceeds + // half of ChunkSize, almost every chunk is duplicate content. Cap it at + // ChunkSize/2 so Overlap stays a useful smoothing band rather than a + // near-clone of the previous chunk. + if cfg.ChunkOverlap > cfg.ChunkSize/2 && cfg.ChunkSize > 0 { + cfg.ChunkOverlap = cfg.ChunkSize / 2 + } return cfg } diff --git a/internal/infrastructure/chunker/strategy_test.go b/internal/infrastructure/chunker/strategy_test.go index b3a282060..662a9828d 100644 --- a/internal/infrastructure/chunker/strategy_test.go +++ b/internal/infrastructure/chunker/strategy_test.go @@ -48,6 +48,49 @@ func TestSplit_AutoStrategy_PicksHeadingForMarkdownDoc(t *testing.T) { } } +// TestSplit_PreservesPositionInvariantAcrossTiers ensures every chunk's +// (Start, End, Content) triple stays consistent — End-Start must equal the +// rune length of Content, and runes[Start:End] must equal Content. This is +// the contract that knowledge.go:2278+ relies on for document reconstruction +// during summary generation. +func TestSplit_PreservesPositionInvariantAcrossTiers(t *testing.T) { + cases := map[string]string{ + "heading-tier": "# Top\nintro paragraph here.\n\n## Section A\nbody A here.\n\n## Section B\nbody B here.\n\n## Section C\nbody C.", + "heuristic-tier": strings.Repeat("Kapitel 1: Einleitung\n", 1) + strings.Repeat("Beispieltext. ", 50) + + "\n\n" + strings.Repeat("Kapitel 2: Hauptteil\n", 1) + strings.Repeat("Mehr Text. ", 50), + "recursive-tier": strings.Repeat("plain prose without structure. ", 100), + } + cfg := SplitterConfig{ChunkSize: 300, ChunkOverlap: 30, Separators: []string{"\n\n", "\n", "。", ". "}, Strategy: StrategyAuto} + + for name, doc := range cases { + t.Run(name, func(t *testing.T) { + runes := []rune(doc) + chunks := Split(doc, cfg) + if len(chunks) == 0 { + t.Fatal("expected chunks") + } + for i, c := range chunks { + contentRuneLen := len([]rune(c.Content)) + spanLen := c.End - c.Start + if spanLen != contentRuneLen { + t.Errorf("chunk %d: End(%d)-Start(%d)=%d but Content has %d runes:\n%q", + i, c.End, c.Start, spanLen, contentRuneLen, c.Content) + } + if c.Start < 0 || c.End > len(runes) { + t.Errorf("chunk %d: position out of range Start=%d End=%d totalRunes=%d", + i, c.Start, c.End, len(runes)) + } + if c.Start >= 0 && c.End <= len(runes) { + sliced := string(runes[c.Start:c.End]) + if sliced != c.Content { + t.Errorf("chunk %d: runes[Start:End] differs from Content", i) + } + } + } + }) + } +} + func TestSplitParentChild_LegacyStrategy(t *testing.T) { text := strings.Repeat("This is a sentence. Another one.\n\n", 50) parentCfg := SplitterConfig{ChunkSize: 400, ChunkOverlap: 40, Strategy: StrategyLegacy} diff --git a/internal/types/chunk.go b/internal/types/chunk.go index 2e8e2cfed..f171b0672 100644 --- a/internal/types/chunk.go +++ b/internal/types/chunk.go @@ -163,6 +163,24 @@ type Chunk struct { UpdatedAt time.Time `json:"updated_at"` // Soft delete marker, supports data recovery DeletedAt gorm.DeletedAt `json:"deleted_at" gorm:"index"` + // ContextHeader is an in-memory-only context string (e.g. a Markdown + // heading breadcrumb) that the indexing pipeline prepends to Content + // when generating embeddings. NOT persisted — populated by the chunker + // during initial splitting and discarded after indexing. + ContextHeader string `json:"-" gorm:"-"` +} + +// EmbeddingContent returns the chunk content with ContextHeader prepended +// when set. Use this where the embedding model needs section context that +// isn't part of the literal Content. +func (c *Chunk) EmbeddingContent() string { + if c == nil { + return "" + } + if c.ContextHeader == "" { + return c.Content + } + return c.ContextHeader + "\n\n" + c.Content } // AssignChunkSeqIDs assigns sequential SeqIDs to a batch of chunks that have SeqID == 0. diff --git a/internal/types/docparser.go b/internal/types/docparser.go index b2e1f1648..44a6fa6ed 100644 --- a/internal/types/docparser.go +++ b/internal/types/docparser.go @@ -69,11 +69,16 @@ type DocParserVLMConfig struct { type ParsedChunk struct { Content string - Seq int - Start int - End int - Images []ParsedImage - ChunkID string // populated by processChunks with the actual DB UUID + // ContextHeader is an optional context string (e.g. a Markdown heading + // breadcrumb) that should be prepended at embedding time but is NOT + // part of the stored Content. Lets retrieval pipelines see section + // context without breaking End-Start == len(Content) invariants. + ContextHeader string + Seq int + Start int + End int + Images []ParsedImage + ChunkID string // populated by processChunks with the actual DB UUID // ParentIndex is set when using parent-child chunking strategy. // -1 (or unset/0 for flat chunks) means this is a top-level chunk. @@ -82,6 +87,17 @@ type ParsedChunk struct { ParentIndex int } +// EmbeddingContent returns the text that should be sent to the embedding +// model: ContextHeader (if any) prepended to Content. Mirrors +// chunker.Chunk.EmbeddingContent so the choice is consistent across the +// chunker output and the indexing pipeline. +func (c ParsedChunk) EmbeddingContent() string { + if c.ContextHeader == "" { + return c.Content + } + return c.ContextHeader + "\n\n" + c.Content +} + // ParsedParentChunk represents a parent chunk in the parent-child strategy. // Parent chunks are stored in DB for context retrieval but NOT vector-indexed. type ParsedParentChunk struct {