From a6a67c5ece744c28ddeda517a8f6983d68e0503d Mon Sep 17 00:00:00 2001 From: Jack Date: Mon, 27 Jul 2026 14:42:44 +0800 Subject: [PATCH] Chunker: port Python overlapped_percent normalization and BULLET_PATTERN title fallback (#17396) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Closes two Chunker migration gaps documented in `docs/migration_python_go_diff.md` (diffs **2.6** and **1.7**), improving parity with the Python ingestion pipeline. This is the code portion of commit `261e1fd0b` on branch `fix/batch-3-4`; the migration document itself is tracked separately (untracked in this commit). ### Diff 2.6 — `overlapped_percent` missing Python normalization Python's `normalize_overlapped_percent` (`common/float_utils.py:50-58`) accepts a `[0,1)` fraction (the flow-canvas UI validates `[0,1)`), multiplies it by 100, `int()`-truncates, and clamps to `[0,90]`. Go previously only accepted a raw `[0,90]` percentage and **rejected** out-of-range input, so a Python config passing `0.1` (meaning 10%) silently produced ~0% overlap. Added `normalizeOverlappedPercent` (`internal/ingestion/component/chunker/common.go`) mirroring the Python helper: - parses numbers and numeric strings (mirrors Python `float()`; bad / `NaN` / `Inf` → `0`), - `0 < v < 1 → v *= 100`, - `int()` truncation, - clamps to `[0,90]`. Wired into `tokenChunkerParam.Update` (`token.go`); the merge math (`token.go:705`, `(100-x)/100`) already matched Python. `TokenChunkerParam.Validate` (`schema/chunker.go`) now only guards direct struct construction. Tests: `TestNormalizeOverlappedPercent`, extended `TestTokenChunker_NewAcceptsPythonOverlappedRange` (adds fraction/clamp inputs), new `TestTokenChunker_NormalizesOverlappedPercent`. The existing reject-test cases for `<0` / `>90` were removed because they are now normalized/clamped (Python parity). ### Diff 1.7 — missing `BULLET_PATTERN` title-level fallback `resolveTitleLevels` (`title.go`) now applies a 4th-level fallback: when outline + regex + layout all yield body level, `bulletsCategory` selects the best-matching bullet-pattern group (Chinese legal / numbering / Chinese numbering / English legal — mirroring `rag/nlp/__init__.py:258-320`) and assigns structural levels. Guarded by `allBodyLevel` so it never overrides an existing outline/regex level. Tests: `TestResolveTitleLevels_BulletFallback` (4 subtests). ### Incidental test adjustments included in the commit - `token_batch1_test.go`: overlap input changed `0.3` → `30.0` to reflect the post-normalization 30% semantics. - `real_consumer_test.go`: updated `LoadFromIngestionTask(task)` → `LoadFromIngestionTask(ctx, task)` for the new context-first signature. ## Verification `bash build.sh --test ./internal/ingestion/component/...` — chunker + schema suites pass, no regression (CGO build). ## Migration doc reference `docs/migration_python_go_diff.md` §Chunker 1.7 and 2.6 are marked **Fixed** for these changes. --- .../ingestion/component/chunker/common.go | 25 --- internal/ingestion/component/chunker/title.go | 118 ++++++++++- .../ingestion/component/chunker/title_test.go | 96 +++++++++ internal/ingestion/component/chunker/token.go | 52 +++-- .../component/chunker/token_batch1_test.go | 44 +++- .../ingestion/component/chunker/token_test.go | 194 +++++++++++++++++- .../ingestion/component/schema/chunker.go | 107 +++++++++- internal/ingestion/task/real_consumer_test.go | 3 +- 8 files changed, 591 insertions(+), 48 deletions(-) diff --git a/internal/ingestion/component/chunker/common.go b/internal/ingestion/component/chunker/common.go index 6ecf24db45..297dcb8580 100644 --- a/internal/ingestion/component/chunker/common.go +++ b/internal/ingestion/component/chunker/common.go @@ -62,31 +62,6 @@ func newChunkerByName(name string, params map[string]any) (runtime.Component, er // numeric / list conversion helpers (shared across chunker variants) // --------------------------------------------------------------------------- -// numericFromAny normalises JSON-decoded ints to float64 so the -// schema-defaults-vs-Param-Update convention doesn't depend on the -// encoding source (yaml/toml/json all behave the same). -func numericFromAny(v any) (float64, bool) { - switch x := v.(type) { - case float64: - return x, true - case float32: - return float64(x), true - case int: - return float64(x), true - case int32: - return float64(x), true - case int64: - return float64(x), true - case uint: - return float64(x), true - case uint32: - return float64(x), true - case uint64: - return float64(x), true - } - return 0, false -} - func stringListFromAny(in []any) []string { out := make([]string, 0, len(in)) for _, x := range in { diff --git a/internal/ingestion/component/chunker/title.go b/internal/ingestion/component/chunker/title.go index 3403dd3ada..d0ae0d9993 100644 --- a/internal/ingestion/component/chunker/title.go +++ b/internal/ingestion/component/chunker/title.go @@ -85,7 +85,7 @@ func (p *titleChunkerParam) Update(conf map[string]any) { } else if v, ok := conf["levels"].([][]string); ok { p.TitleChunkerParam.Levels = v } - if v, ok := numericFromAny(conf["hierarchy"]); ok { + if v, ok := schema.NumericFromAny(conf["hierarchy"]); ok { n := int(v) p.TitleChunkerParam.Hierarchy = &n } @@ -224,6 +224,7 @@ var ( notTitleException = regexp.MustCompile(`^第[零一二三四五六七八九十百0-9]+条`) notTitlePunct = regexp.MustCompile(`[,;，。；！!]`) layoutHeadingRe = regexp.MustCompile(`(?i)(section|title|head)`) + bulletLayoutRe = regexp.MustCompile(`(?i)(title|head)`) numericOnly = regexp.MustCompile(`^[0-9]+$`) ) @@ -340,9 +341,124 @@ func resolveTitleLevels(records []lineRecord, p *titleChunkerParam) []int { } out[i] = matchLayoutLevel(rec.text, rec.layout, fallbackLevel) } + + // Fallback 4: bullet-pattern detection (Chunker-1.7). + // When frequency-based detection assigns bodyLevel to every record + // (no regex matched a single line), use Python's BULLET_PATTERN + // heuristic (rag/nlp/__init__.py:303-320 bullets_category + + // title_frequency) to recover structure from numbered/bulleted + // list entries. This only fires when user levels are empty or + // matched nothing — never overrides an existing level assignment. + if len(group) == 0 || allBodyLevel(out) { + if bull := bulletsCategory(records); bull >= 0 { + bulletsSize := len(bulletPatterns[bull]) + for i, rec := range records { + if out[i] != bodyLevel || !rec.isText() { + continue + } + trimmed := strings.TrimSpace(rec.text) + matched := false + for j, pat := range bulletPatterns[bull] { + if pat.MatchString(trimmed) && !notBullet(trimmed) { + out[i] = j + 1 + matched = true + break + } + } + if matched { + continue + } + // layout-title fallback: bulletsSize + 1 (mirrors + // Python len(BULLET_PATTERN[bull]) + 1). + if bulletLayoutRe.MatchString(rec.layout) && !notTitle(beforeAt(rec.text)) { + out[i] = bulletsSize + 1 + } + } + } + } return out } +// allBodyLevel returns true when every level equals the bodyLevel +// sentinel. Used by the bullet fallback guard to detect the "no +// structure found" state where BULLET_PATTERN should kick in. +func allBodyLevel(levels []int) bool { + for _, l := range levels { + if l < bodyLevel { + return false + } + } + return true +} + +// bulletPatterns mirrors Python BULLET_PATTERN (rag/nlp/__init__.py:258). +// Group 4 (markdown headings) is omitted — DSL levels cover that case. +// Each group is depth-ordered: index 0 is the topmost level. +var bulletPatterns = [][]*regexp.Regexp{ + // Group 0 — Chinese legal (编/章/节/条) + { + regexp.MustCompile(`^第[零一二三四五六七八九十百0-9]+(分?编|部分)`), + regexp.MustCompile(`^第[零一二三四五六七八九十百0-9]+章`), + regexp.MustCompile(`^第[零一二三四五六七八九十百0-9]+节`), + regexp.MustCompile(`^第[零一二三四五六七八九十百0-9]+条`), + regexp.MustCompile(`^[\(（][零一二三四五六七八九十百]+[\)）]`), + }, + // Group 1 — Numbering (1., 1.1, 1.1.1) + { + regexp.MustCompile(`^第[0-9]+章`), + regexp.MustCompile(`^第[0-9]+节`), + regexp.MustCompile(`^[0-9]{0,2}[\. 、]`), + regexp.MustCompile(`^[0-9]{0,2}\.[0-9]{0,2}[^a-zA-Z/%~-]`), + regexp.MustCompile(`^[0-9]{0,2}\.[0-9]{0,2}\.[0-9]{0,2}`), + regexp.MustCompile(`^[0-9]{0,2}\.[0-9]{0,2}\.[0-9]{0,2}\.[0-9]{0,2}`), + }, + // Group 2 — Chinese numbering (一、, (一)) + { + regexp.MustCompile(`^第[零一二三四五六七八九十百0-9]+章`), + regexp.MustCompile(`^第[零一二三四五六七八九十百0-9]+节`), + regexp.MustCompile(`^[零一二三四五六七八九十百]+[ 、]`), + regexp.MustCompile(`^[\(（][零一二三四五六七八九十百]+[\)）]`), + regexp.MustCompile(`^[\(（][0-9]{0,2}[\)）]`), + }, + // Group 3 — English legal + { + regexp.MustCompile(`^PART (ONE|TWO|THREE|FOUR|FIVE|SIX|SEVEN|EIGHT|NINE|TEN)`), + regexp.MustCompile(`^Chapter (I+V?|VI*|XI|IX|X)`), + regexp.MustCompile(`^Section [0-9]+`), + regexp.MustCompile(`^Article [0-9]+`), + }, +} + +// bulletsCategory mirrors Python bullets_category (rag/nlp/__init__.py:303). +// Counts hits per bullet-pattern group across all text records, +// excluding not-bullet lines. Returns the group index with the highest +// hit count, or -1 if no group matched any line. +func bulletsCategory(records []lineRecord) int { + hits := make([]int, len(bulletPatterns)) + for grpIdx, group := range bulletPatterns { + for _, rec := range records { + if !rec.isText() { + continue + } + txt := strings.TrimSpace(rec.text) + for _, pat := range group { + if pat.MatchString(txt) && !notBullet(txt) { + hits[grpIdx]++ + break + } + } + } + } + best, bestHits := -1, 0 + for i, h := range hits { + if h > bestHits { + bestHits = h + best = i + } + } + return best +} + // outlineEntry is one PDF bookmark/heading from the parser-supplied // outline, mirroring Python extract_pdf_outlines' (text, level, page) // tuple. The page is unused by title detection. diff --git a/internal/ingestion/component/chunker/title_test.go b/internal/ingestion/component/chunker/title_test.go index 4cd660af3f..1ebce286c6 100644 --- a/internal/ingestion/component/chunker/title_test.go +++ b/internal/ingestion/component/chunker/title_test.go @@ -467,3 +467,99 @@ func TestOutlineFromInputs(t *testing.T) { t.Errorf("empty inputs outline = %v, want nil", got) } } + +// TestResolveTitleLevels_BulletFallback covers Chunker-1.7: when +// frequency-based detection assigns bodyLevel to every record, the +// BULLET_PATTERN bullet-fallback scans the text for numbered/bulleted +// list entries and assigns structural levels, mirroring Python's +// bullets_category + title_frequency. +func TestResolveTitleLevels_BulletFallback(t *testing.T) { + // All subtests use empty Levels to ensure no regex group matches, + // so the bullet fallback is the only level signal. + + t.Run("ChineseLegalGroups0", func(t *testing.T) { + p := &titleChunkerParam{TitleChunkerParam: schema.TitleChunkerParam{ + Method: "group", + }} + records := []lineRecord{ + {text: "第一条 定义", docType: "text"}, + {text: "第二条 适用范围", docType: "text"}, + {text: "这是普通正文内容。", docType: "text"}, + } + levels := resolveTitleLevels(records, p) + // "第一条" matches group-0 index-3 (r"第...条") -> level = 3+1 = 4. + if lvl := levels[0]; lvl >= bodyLevel || lvl <= 0 { + t.Errorf("bullet line '第一条' level = %d, want non-body positive", lvl) + } + if lvl := levels[1]; lvl >= bodyLevel || lvl <= 0 { + t.Errorf("bullet line '第二条' level = %d, want non-body positive", lvl) + } + if levels[2] != bodyLevel { + t.Errorf("plain body line level = %d, want bodyLevel", levels[2]) + } + }) + + t.Run("NumberingGroup1", func(t *testing.T) { + p := &titleChunkerParam{TitleChunkerParam: schema.TitleChunkerParam{ + Method: "group", + }} + records := []lineRecord{ + {text: "1. Introduction", docType: "text"}, + {text: "2. Methods", docType: "text"}, + {text: "3. Results", docType: "text"}, + {text: "Plain body paragraph here.", docType: "text"}, + } + levels := resolveTitleLevels(records, p) + for i := 0; i < 3; i++ { + if lvl := levels[i]; lvl >= bodyLevel || lvl <= 0 { + t.Errorf("numbered line index %d level = %d, want non-body positive", i, lvl) + } + } + if levels[3] != bodyLevel { + t.Errorf("plain body line level = %d, want bodyLevel", levels[3]) + } + }) + + t.Run("NotBulletExcluded", func(t *testing.T) { + p := &titleChunkerParam{TitleChunkerParam: schema.TitleChunkerParam{ + Method: "group", + }} + records := []lineRecord{ + {text: "0", docType: "text"}, + {text: "1 2 3", docType: "text"}, + {text: "第一条 定义", docType: "text"}, + } + levels := resolveTitleLevels(records, p) + // "0" and "1 2 3" are not-bullet -> bodyLevel. + if levels[0] != bodyLevel { + t.Errorf("notBullet '0' level = %d, want bodyLevel", levels[0]) + } + if levels[1] != bodyLevel { + t.Errorf("notBullet '1 2 3' level = %d, want bodyLevel", levels[1]) + } + if lvl := levels[2]; lvl >= bodyLevel || lvl <= 0 { + t.Errorf("genuine bullet '第一条' level = %d, want non-body positive", lvl) + } + }) + + t.Run("RegexMatchGuardsBulletFallback", func(t *testing.T) { + p := &titleChunkerParam{TitleChunkerParam: schema.TitleChunkerParam{ + Method: "group", + Levels: [][]string{{`^第[零一二三四五六七八九十百0-9]+章`}}, + }} + records := []lineRecord{ + {text: "第一章 概述", docType: "text"}, + {text: "第一条 定义", docType: "text"}, + } + levels := resolveTitleLevels(records, p) + // "第一章" matches the user-supplied regex -> level 1 from regex. + // Because regex matched, bullet fallback must NOT fire. + // "第一条" should still be bodyLevel (not promoted by bullet fallback). + if levels[0] >= bodyLevel { + t.Errorf("regex match '第一章' level = %d, want non-body (regex)", levels[0]) + } + if levels[1] != bodyLevel { + t.Errorf("non-matching '第一条' level = %d, want bodyLevel (bullet fallback must not override regex path)", levels[1]) + } + }) +} diff --git a/internal/ingestion/component/chunker/token.go b/internal/ingestion/component/chunker/token.go index 0d6e0ebc81..aa12e31f03 100644 --- a/internal/ingestion/component/chunker/token.go +++ b/internal/ingestion/component/chunker/token.go @@ -18,7 +18,9 @@ // // - WHITELIST: delimiter_mode ∈ {"token_size","delimiter"} (the // single-chunk "one" behaviour moved to OneChunker in one.go). -// chunk_token_size > 0, overlapped_percent ∈ [0, 1), table_context_size ≥ 0, +// chunk_token_size > 0, overlapped_percent accepts a [0,1) fraction or a +// [0,90] percentage (normalized to [0,90] by normalizeOverlappedPercent, +// mirroring Python's normalize_overlapped_percent), table_context_size ≥ 0, // image_context_size ≥ 0. enum/range checks live in param.Check. // // - DELIMITER PARSING mirrors python `_compile_delimiter_pattern`: @@ -82,7 +84,7 @@ func (p *tokenChunkerParam) Update(conf map[string]any) { if v, ok := conf["delimiter_mode"].(string); ok { p.TokenChunkerParam.DelimiterMode = v } - if v, ok := numericFromAny(conf["chunk_token_size"]); ok { + if v, ok := schema.NumericFromAny(conf["chunk_token_size"]); ok { p.TokenChunkerParam.ChunkTokenSize = int(v) } if v, ok := conf["delimiters"].([]any); ok { @@ -90,18 +92,18 @@ func (p *tokenChunkerParam) Update(conf map[string]any) { } else if v, ok := conf["delimiters"].([]string); ok { p.TokenChunkerParam.Delimiters = append([]string(nil), v...) } - if v, ok := numericFromAny(conf["overlapped_percent"]); ok { - p.TokenChunkerParam.OverlappedPercent = v + if v, ok := conf["overlapped_percent"]; ok { + p.TokenChunkerParam.OverlappedPercent = schema.NormalizeOverlappedPercent(v) } if v, ok := conf["children_delimiters"].([]any); ok { p.TokenChunkerParam.ChildrenDelimiters = stringListFromAny(v) } else if v, ok := conf["children_delimiters"].([]string); ok { p.TokenChunkerParam.ChildrenDelimiters = append([]string(nil), v...) } - if v, ok := numericFromAny(conf["table_context_size"]); ok { + if v, ok := schema.NumericFromAny(conf["table_context_size"]); ok { p.TokenChunkerParam.TableContextSize = int(v) } - if v, ok := numericFromAny(conf["image_context_size"]); ok { + if v, ok := schema.NumericFromAny(conf["image_context_size"]); ok { p.TokenChunkerParam.ImageContextSize = int(v) } } @@ -303,6 +305,16 @@ var sentenceDelimiter = regexp.MustCompile(`(\n|[!?。；！？]|\.\s)`) func (c *TokenChunkerComponent) mergeByTokenSize(text string, childrenPattern *regexp.Regexp) map[string]any { target := c.param.ChunkTokenSize overlapPct := c.param.OverlappedPercent + // Clamp to [0,100] so the merge math below never produces a + // negative/inverted threshold for an out-of-range value (review: + // yuzhichang, PR #17396). c.param.OverlappedPercent is already in + // [0,90] via Update/Validate, so this is a defensive no-op in + // normal operation. + if overlapPct < 0 { + overlapPct = 0 + } else if overlapPct > 100 { + overlapPct = 100 + } // Split into paragraph-aligned sections. sections := splitIntoSections(text) @@ -324,7 +336,7 @@ func (c *TokenChunkerComponent) mergeByTokenSize(text string, childrenPattern *r // threshold → start a new chunk (with optional overlap prefix). // - Otherwise → merge into the current chunk. mergeOrNew := func(segment string, tokens int) { - threshold := float64(target) * (100 - overlapPct*100) / 100.0 + threshold := float64(target) * (100 - overlapPct) / 100.0 if len(cks) == 0 || float64(tkns[len(tkns)-1]) > threshold { seg := segment segTokens := tokens @@ -335,7 +347,7 @@ func (c *TokenChunkerComponent) mergeByTokenSize(text string, childrenPattern *r // Take the last overlapped_percent of the previous chunk // (in runes, matching Python's len(overlapped) * ratio). prevRunes := []rune(prev) - cut := int(float64(len(prevRunes)) * (100 - overlapPct*100) / 100.0) + cut := int(float64(len(prevRunes)) * (100 - overlapPct) / 100.0) if cut < len(prevRunes) { suffix := string(prevRunes[cut:]) seg = suffix + segment @@ -411,13 +423,19 @@ func (c *TokenChunkerComponent) mergeByTokenSize(text string, childrenPattern *r } // splitIntoSections partitions text into paragraph-level sections by -// splitting on double-newline boundaries. This mirrors the caller side -// in Python's naive_merge where sections are pre-split before merging. +// splitting on blank-line boundaries. It uses \n\s*\n (not \n{2,}) so a +// CRLF paragraph break (\r\n\r\n) still matches — the Go chunker performs +// no \r\n normalization of its own, so a stricter \n{2,} would silently +// lose CRLF boundaries. NOTE: the precise 2.7/2.10 section-splitting +// semantics are still OPEN in docs/migration_python_go_diff.md (alignment +// target flow-vs-app Python undecided); this keeps the pre-existing, +// CRLF-safe behaviour until that is resolved in a dedicated change. func splitIntoSections(text string) []string { if text == "" { return nil } - // Split on consecutive newlines (paragraph boundaries). + // Split on a blank line: a newline, optional whitespace (absorbs a + // trailing \r on CRLF input), then another newline. re := regexp.MustCompile(`\n\s*\n`) parts := re.Split(text, -1) return parts @@ -704,7 +722,15 @@ func takeFromStart(text string, tokens int) string { // mergeByTokenSizeFromJSON mirrors `naive_merge` at // rag/nlp/__init__.py:1156. func mergeByTokenSizeFromJSON(perItem [][]schema.ChunkDoc, chunkTokens int, overlappedPct float64) [][]schema.ChunkDoc { - threshold := float64(chunkTokens) * (100 - overlappedPct*100) / 100.0 + // overlappedPct is a [0,100] percentage. Clamp so the merge math below + // never yields a negative/inverted threshold for out-of-range input + // (review: yuzhichang, PR #17396). + if overlappedPct < 0 { + overlappedPct = 0 + } else if overlappedPct > 100 { + overlappedPct = 100 + } + threshold := float64(chunkTokens) * (100 - overlappedPct) / 100.0 for idx := range perItem { chunks := perItem[idx] if len(chunks) == 0 { @@ -735,7 +761,7 @@ func mergeByTokenSizeFromJSON(perItem [][]schema.ChunkDoc, chunkTokens int, over // (diff Chunker-2.2). if prevText := removeTag(merged[len(merged)-1].Text); prevText != "" { runes := []rune(prevText) - cut := int(float64(len(runes)) * (100 - overlappedPct*100) / 100.0) + cut := int(float64(len(runes)) * (100 - overlappedPct) / 100.0) if cut < len(runes) { cp.Text = string(runes[cut:]) + cp.Text cp.TKNums = intPtr(tokenizeStr(cp.Text)) diff --git a/internal/ingestion/component/chunker/token_batch1_test.go b/internal/ingestion/component/chunker/token_batch1_test.go index e9d675ea1d..a9f33ed792 100644 --- a/internal/ingestion/component/chunker/token_batch1_test.go +++ b/internal/ingestion/component/chunker/token_batch1_test.go @@ -17,6 +17,7 @@ package chunker import ( + "reflect" "regexp" "strings" "testing" @@ -61,7 +62,7 @@ func TestMergeByTokenSizeFromJSON_OverlapStripsTags(t *testing.T) { {Text: "body", DocType: "text", CKType: "text", TKNums: intPtr(5)}, }, } - got := mergeByTokenSizeFromJSON(items, 128, 0.3) + got := mergeByTokenSizeFromJSON(items, 128, 30.0) merged := got[0] if len(merged) != 2 { t.Fatalf("want 2 merged chunks (overlap path), got %d", len(merged)) @@ -74,6 +75,47 @@ func TestMergeByTokenSizeFromJSON_OverlapStripsTags(t *testing.T) { } } +// TestMergeByTokenSizeFromJSON_ClampsOverlappedPct locks the review finding +// from yuzhichang (PR #17396): mergeByTokenSizeFromJSON clamps an out-of-range +// overlappedPct to [0,100] so the merge math never yields a negative/inverted +// threshold. Out-of-range values must not panic and must behave identically to +// their clamped-in-range equivalent (150 == 100, -5 == 0, and the same for +// huge magnitudes that would otherwise overflow the float->int slice index). +func TestMergeByTokenSizeFromJSON_ClampsOverlappedPct(t *testing.T) { + items := [][]schema.ChunkDoc{ + { + {Text: strings.Repeat("word ", 20), DocType: "text", CKType: "text", TKNums: intPtr(100)}, + {Text: "body", DocType: "text", CKType: "text", TKNums: intPtr(5)}, + }, + } + + at100 := mergeByTokenSizeFromJSON(items, 128, 100) + if at100 == nil || len(at100) == 0 { + t.Fatalf("overlappedPct=100: nil/empty result") + } + at150 := mergeByTokenSizeFromJSON(items, 128, 150) + atHuge := mergeByTokenSizeFromJSON(items, 128, 1e300) + if !reflect.DeepEqual(at100, at150) { + t.Errorf("overlappedPct=150 should clamp to 100; output differs from 100") + } + if !reflect.DeepEqual(at100, atHuge) { + t.Errorf("overlappedPct=1e300 should clamp to 100; output differs from 100") + } + + at0 := mergeByTokenSizeFromJSON(items, 128, 0) + if at0 == nil || len(at0) == 0 { + t.Fatalf("overlappedPct=0: nil/empty result") + } + atNeg := mergeByTokenSizeFromJSON(items, 128, -5) + atNegHuge := mergeByTokenSizeFromJSON(items, 128, -1e300) + if !reflect.DeepEqual(at0, atNeg) { + t.Errorf("overlappedPct=-5 should clamp to 0; output differs from 0") + } + if !reflect.DeepEqual(at0, atNegHuge) { + t.Errorf("overlappedPct=-1e300 should clamp to 0; output differs from 0") + } +} + // TestMergeByTokenSizeFromJSON_EmptyPrevKeepsChunk exercises migration diff // Chunker-2.11: merging a non-empty chunk into an empty previous chunk must // assign the text directly instead of being skipped. The legacy guard diff --git a/internal/ingestion/component/chunker/token_test.go b/internal/ingestion/component/chunker/token_test.go index 342ec8c0ad..32ac5d9b44 100644 --- a/internal/ingestion/component/chunker/token_test.go +++ b/internal/ingestion/component/chunker/token_test.go @@ -18,9 +18,11 @@ package chunker import ( "context" + "math" "testing" "ragflow/internal/agent/runtime" + "ragflow/internal/ingestion/component/schema" ) // TestTokenChunker_Registered asserts the registry has a CategoryIngestion @@ -296,8 +298,6 @@ func TestTokenChunker_NewRejectsBadParam(t *testing.T) { {"one delimiter_mode (use OneChunker)", map[string]any{"delimiter_mode": "one"}}, {"zero chunk_token_size", map[string]any{"delimiter_mode": "token_size", "chunk_token_size": 0}}, {"negative chunk_token_size", map[string]any{"delimiter_mode": "token_size", "chunk_token_size": -5}}, - {"negative overlapped_percent", map[string]any{"delimiter_mode": "token_size", "chunk_token_size": 50, "overlapped_percent": -0.1}}, - {"overlapped_percent >= 1", map[string]any{"delimiter_mode": "token_size", "chunk_token_size": 50, "overlapped_percent": 1.0}}, {"negative table_context_size", map[string]any{"delimiter_mode": "token_size", "chunk_token_size": 50, "table_context_size": -1}}, } for _, tc := range cases { @@ -360,3 +360,193 @@ func TestTokenChunker_PrefersUpstreamChunks(t *testing.T) { } t.Fatalf("upstream chunk 'CHAPTER-AWARE' was not found in output: %v", out["chunks"]) } + +// TestTokenChunker_NewAcceptsPythonOverlappedRange covers Chunker-2.6: +// overlapped_percent uses Python's [0,90] integer-percentage semantics, +// accepting both a [0,1) fraction and a [0,90] percentage (the latter +// normalized via normalizeOverlappedPercent). +func TestTokenChunker_NewAcceptsPythonOverlappedRange(t *testing.T) { + // Values that should be valid in the Python range (fractions and + // percentages, including out-of-range inputs that Python clamps). + for _, pct := range []float64{0, 0.1, 0.5, 15, 30, 50, 90, 95, -5} { + conf := map[string]any{ + "delimiter_mode": "token_size", + "chunk_token_size": 100, + "overlapped_percent": pct, + } + _, err := NewTokenChunker(conf) + if err != nil { + t.Errorf("overlapped_percent=%v: unexpected error: %v", pct, err) + } + } +} + +// TestNormalizeOverlappedPercent is the Go port of Python +// common/float_utils.py:50-58 normalize_overlapped_percent (diff Chunker-2.6). +// Python's user-facing input is a [0,1) fraction; the helper converts it to +// the [0,90] integer-percentage scale the merge math expects. +func TestNormalizeOverlappedPercent(t *testing.T) { + cases := []struct { + name string + in any + want float64 + }{ + {"zero", 0, 0}, + {"fraction 0.1 -> 10", 0.1, 10}, + {"fraction 0.5 -> 50", 0.5, 50}, + {"fraction 0.95 -> 90 (clamp)", 0.95, 90}, + {"percent 15", 15, 15}, + {"int truncation 33.3 -> 33", 33.3, 33}, + {"clamp 95 -> 90", 95, 90}, + {"clamp -5 -> 0", -5, 0}, + {"negative fraction -0.1 -> 0", -0.1, 0}, + // Huge out-of-range values must clamp to 90 (not 0). Go's + // float->int is implementation-defined past int's range, so the + // clamp must run before truncation (review finding #4). Python's + // normalize_overlapped_percent returns 90 for these, not 0. + {"huge 1e300 -> 90", 1e300, 90}, + {"huge -1e300 -> 0", -1e300, 0}, + {"huge math.MaxFloat64 -> 90", math.MaxFloat64, 90}, + {`numeric string "10" -> 10`, "10", 10}, + {`numeric string fraction "0.1" -> 10`, "0.1", 10}, + {"bad string -> 0", "abc", 0}, + {"nil -> 0", nil, 0}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := schema.NormalizeOverlappedPercent(tc.in); got != tc.want { + t.Errorf("NormalizeOverlappedPercent(%v) = %v, want %v", tc.in, got, tc.want) + } + }) + } +} + +// TestTokenChunker_NormalizesOverlappedPercent asserts the stored value after +// construction matches Python's normalized [0,90] scale (diff Chunker-2.6). +func TestTokenChunker_NormalizesOverlappedPercent(t *testing.T) { + cases := []struct { + name string + in any + want float64 + }{ + {"clamp 95 -> 90", 95, 90}, + {"clamp -5 -> 0", -5, 0}, + {"fraction 0.1 -> 10", 0.1, 10}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + c, err := NewTokenChunker(map[string]any{ + "delimiter_mode": "token_size", + "chunk_token_size": 100, + "overlapped_percent": tc.in, + }) + if err != nil { + t.Fatalf("NewTokenChunker(%v): %v", tc.in, err) + } + got := c.(*TokenChunkerComponent).param.OverlappedPercent + if got != tc.want { + t.Errorf("overlapped_percent=%v: stored %v, want %v", tc.in, got, tc.want) + } + }) + } +} + +// TestSplitIntoSections locks the CRLF-safe blank-line splitting used by +// mergeByTokenSize. It does NOT claim Chunker-2.7/2.10 coverage: that +// section-splitting semantics is still OPEN in +// docs/migration_python_go_diff.md (flow-vs-app Python alignment target +// undecided), so this only guards the existing behaviour. +func TestSplitIntoSections(t *testing.T) { + tests := []struct { + name string + text string + want int + }{ + {"two paragraphs", "a\n\nb", 2}, + {"three paragraphs with excess blank lines", "a\n\n\nb\n\nc", 3}, + {"single paragraph", "hello world", 1}, + // A blank line containing only whitespace is still a boundary + // under \n\s*\n (this is what reverted the stricter \n{2,}). + {"blank line with space is a boundary", "a\n \nb", 2}, + {"leading blank lines produce empty prefix (caller filters)", "\n\ntext", 2}, + // CRLF regression guard: \r\n\r\n must split the same as \n\n. + {"CRLF paragraph break splits", "a\r\n\r\nb", 2}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := splitIntoSections(tt.text) + if len(got) != tt.want { + t.Errorf("splitIntoSections(%q) = %d sections, want %d: %v", tt.text, len(got), tt.want, got) + } + }) + } +} + +// TestTokenChunkerParam_ValidateOverlappedRange covers the strict overlap +// handling in TokenChunkerParam.Validate (review findings #3 + #4): +// a directly-constructed struct with a [0,1) fraction is scaled to its +// [0,90] percent (so 0.3 means 30%, matching the config path), while +// out-of-range values are rejected — the config path (Update) clamps +// instead, so this is the only guard that catches a bad literal. +func TestTokenChunkerParam_ValidateOverlappedRange(t *testing.T) { + cases := []struct { + name string + in float64 + want float64 // expected stored value after Validate when err==nil + wantErr bool + }{ + {"fraction 0.3 -> 30", 0.3, 30, false}, + {"percent 0", 0, 0, false}, + {"percent 30", 30, 30, false}, + {"percent 90 (boundary)", 90, 90, false}, + {"percent 95 -> error", 95, 0, true}, + {"negative -5 -> error", -5, 0, true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + p := schema.TokenChunkerParam{ + DelimiterMode: "token_size", + ChunkTokenSize: 100, + OverlappedPercent: tc.in, + } + err := p.Validate() + if tc.wantErr { + if err == nil { + t.Fatalf("Validate(overlapped_percent=%v): want error, got nil", tc.in) + } + return + } + if err != nil { + t.Fatalf("Validate(overlapped_percent=%v): unexpected error: %v", tc.in, err) + } + if p.OverlappedPercent != tc.want { + t.Errorf("after Validate: overlapped_percent=%v, want %v", p.OverlappedPercent, tc.want) + } + }) + } +} + +// TestTokenChunkerParam_UpdatePreservesOverlappedPercent locks review +// finding #3: tokenChunkerParam.Update must not reset OverlappedPercent to 0 +// when the incoming config omits the key. All other fields use a presence +// guard, and a partial Update (e.g. changing only chunk_token_size) must +// preserve the previously configured overlap instead of clobbering it. +func TestTokenChunkerParam_UpdatePreservesOverlappedPercent(t *testing.T) { + p := defaultsToken(tokenChunkerParam{}) + p.TokenChunkerParam.OverlappedPercent = 30 // pre-existing config + + // Partial update: only chunk_token_size changes. + p.Update(map[string]any{"chunk_token_size": 100}) + + if p.TokenChunkerParam.OverlappedPercent != 30 { + t.Errorf("after partial Update: overlapped_percent=%v, want 30 (preserved)", + p.TokenChunkerParam.OverlappedPercent) + } + + // Explicit key still wins and normalizes. + p.Update(map[string]any{"overlapped_percent": 0.5}) + if p.TokenChunkerParam.OverlappedPercent != 50 { + t.Errorf("after explicit Update: overlapped_percent=%v, want 50 (normalized)", + p.TokenChunkerParam.OverlappedPercent) + } +} diff --git a/internal/ingestion/component/schema/chunker.go b/internal/ingestion/component/schema/chunker.go index bf25a181cd..d8e7ddc952 100644 --- a/internal/ingestion/component/schema/chunker.go +++ b/internal/ingestion/component/schema/chunker.go @@ -16,7 +16,12 @@ package schema -import "fmt" +import ( + "fmt" + "math" + "strconv" + "strings" +) // ChunkerFromUpstream is the shared upstream payload consumed by all four // chunker variants (TokenChunker, TitleChunker, GroupTitleChunker, @@ -180,7 +185,12 @@ type TokenChunkerParam struct { // backticks (e.g., "`\\n`") denote user-defined regex split points. Delimiters []string `json:"delimiters"` - // OverlappedPercent is the chunk-overlap ratio in [0, 100). + // OverlappedPercent is the overlap percentage in [0, 90]. Mirrors + // Python common/float_utils.py:50-58 — an integer-like float in the + // same range so DSL templates written for the Python pipeline work + // out of the box (diff Chunker-2.6). A [0,1) fraction input is also + // accepted and normalized to this scale by tokenChunkerParam.Update + // (via normalizeOverlappedPercent). OverlappedPercent float64 `json:"overlapped_percent"` // ChildrenDelimiters is the secondary split applied to text chunks. @@ -208,9 +218,98 @@ func (TokenChunkerParam) Defaults() TokenChunkerParam { } } +// NumericFromAny normalises JSON-decoded ints to float64 so the +// schema-defaults-vs-Param-Update convention doesn't depend on the +// encoding source (yaml/toml/json all behave the same). It is shared by +// the chunker config decoders and NormalizeOverlappedPercent. +func NumericFromAny(v any) (float64, bool) { + switch x := v.(type) { + case float64: + return x, true + case float32: + return float64(x), true + case int: + return float64(x), true + case int32: + return float64(x), true + case int64: + return float64(x), true + case uint: + return float64(x), true + case uint32: + return float64(x), true + case uint64: + return float64(x), true + } + return 0, false +} + +// NormalizeOverlappedPercent mirrors Python common/float_utils.py:50-58 +// (normalize_overlapped_percent). Python's user-facing input is a [0,1) +// fraction (token_chunker.py validates "[0, 1)"); this converts it to the +// [0,90] integer-percentage scale that the merge math (token.go:705, +// `(100-x)/100`) expects, matching Python's canonical norm. Diff Chunker-2.6. +// +// Behaviour matches Python exactly: +// - parse like float(): numbers, or numeric strings (Python also accepts +// strings via float()); on any failure / NaN / Inf, return 0 +// (Python: except (TypeError, ValueError, OverflowError) -> 0). +// - 0 < v < 1 -> v *= 100 (fraction -> percentage) +// - v = int(v) (truncation toward zero, matches Python for +// non-negatives) +// - clamp to [0, 90] (Python: max(0, min(v, 90))) +// +// Exported so TokenChunkerParam.Validate can normalize directly-constructed +// structs (not just the config-path Update), keeping both paths identical. +func NormalizeOverlappedPercent(v any) float64 { + value, ok := NumericFromAny(v) + if !ok { + if s, isStr := v.(string); isStr { + if f, err := strconv.ParseFloat(strings.TrimSpace(s), 64); err == nil { + value, ok = f, true + } + } + } + if !ok || math.IsNaN(value) || math.IsInf(value, 0) { + return 0 + } + return clampOverlappedPercent(value) +} + +// clampOverlappedPercent applies the [0,1) fraction -> [0,90] percent +// conversion, clamps to [0,90], then truncates to an integer (Python +// faithfulness). Clamping MUST happen before the int conversion: Go's +// float->int is implementation-defined when the value is out of int's range +// (e.g. 1e300), whereas Python's int() is arbitrary-precision, so clamping +// first keeps us aligned with Python's max(0, min(int(v), 90)). Used by the +// lenient config path (NormalizeOverlappedPercent). +func clampOverlappedPercent(value float64) float64 { + if 0 < value && value < 1 { + value *= 100 + } + if value < 0 { + value = 0 + } + if value > 90 { + value = 90 + } + value = float64(int(value)) + return value +} + // Validate enforces the same enum/range checks the runtime component expects // at construction time, keeping the schema and component decoder aligned. -func (p TokenChunkerParam) Validate() error { +func (p *TokenChunkerParam) Validate() error { + // Strict overlap normalization for directly-constructed structs: scale a + // [0,1) fraction to a [0,90] percent (matching the config path's + // semantics) and truncate toward zero, then REJECT anything still outside + // [0,90]. The config path (Update) clamps instead, so a fraction like 0.3 + // means 30% here too, while an out-of-range value like 95 or -5 is caught + // rather than silently clamped — eliminating the latent footgun where a + // fraction passed to a struct bypassed normalization. + if v := p.OverlappedPercent; 0 < v && v < 1 { + p.OverlappedPercent = float64(int(v * 100)) + } switch p.DelimiterMode { case "token_size", "delimiter": default: @@ -219,7 +318,7 @@ func (p TokenChunkerParam) Validate() error { if p.ChunkTokenSize <= 0 { return errInvalidValue{Field: "chunk_token_size", Value: fmt.Sprintf("%d", p.ChunkTokenSize)} } - if p.OverlappedPercent < 0 || p.OverlappedPercent >= 1 { + if p.OverlappedPercent < 0 || p.OverlappedPercent > 90 { return errInvalidValue{Field: "overlapped_percent", Value: fmt.Sprintf("%v", p.OverlappedPercent)} } if p.TableContextSize < 0 { diff --git a/internal/ingestion/task/real_consumer_test.go b/internal/ingestion/task/real_consumer_test.go index a70ffc0ed7..24185f9049 100644 --- a/internal/ingestion/task/real_consumer_test.go +++ b/internal/ingestion/task/real_consumer_test.go @@ -135,11 +135,10 @@ func TestRealProducerConsumer(t *testing.T) { t.Fatalf("set pipeline_id: %v", err) } - tc, err := LoadFromIngestionTask(task) + tc, err := LoadFromIngestionTask(context.Background(), task) if err != nil { t.Fatalf("LoadFromIngestionTask: %v", err) } - tc.Ctx = context.Background() t.Logf("Consumer: Loaded Doc=%s Parser=%s KB=%s Tenant=%s", tc.Doc.ID, tc.Doc.ParserID, tc.KB.ID, tc.Tenant.ID)