From 16ac94cff5f7fbae341c26f227fda566527b8060 Mon Sep 17 00:00:00 2001 From: Jack Date: Fri, 7 Aug 2026 10:06:04 +0800 Subject: [PATCH] fix(parser/chunk): dedupe CompileDelimiterPatternList active entries (#17939) Restore the deduplication that was dropped when #17926 was merged. `CompileDelimiterPatternList` now keeps a `seen` set and collapses equivalent active entries (both backtick-inner and bare) into a single alternation. This PR also removes the dead code that the re-review surfaced. --- .../ingestion/component/chunker/common.go | 3 +- .../chunker/delimiter_case_sensitive_test.go | 12 +- internal/ingestion/component/chunker/token.go | 4 +- internal/parser/chunk/delim.go | 108 ++----------- internal/parser/chunk/delim_test.go | 143 +++++++++++------- 5 files changed, 111 insertions(+), 159 deletions(-) diff --git a/internal/ingestion/component/chunker/common.go b/internal/ingestion/component/chunker/common.go index c9a31954b9..db1b69dec3 100644 --- a/internal/ingestion/component/chunker/common.go +++ b/internal/ingestion/component/chunker/common.go @@ -80,8 +80,7 @@ func stringListFromAny(in []any) []string { // Only backtick-wrapped entries produce an active pattern (Python // token_chunker / rag/nlp/delim list helper). Plain entries are ignored here // and used by mergeByTokenSize for sentence-level splitting when no active -// pattern exists. Canonical single-string parser_config.delimiter parsing -// lives in ragflow/internal/parser/chunk (ParseDelimiterField). +// pattern exists. func compileDelimPattern(delims []string) *regexp.Regexp { return chunk.CompileDelimiterListPattern(delims) } diff --git a/internal/ingestion/component/chunker/delimiter_case_sensitive_test.go b/internal/ingestion/component/chunker/delimiter_case_sensitive_test.go index 0be8a758c1..613ab0e62b 100644 --- a/internal/ingestion/component/chunker/delimiter_case_sensitive_test.go +++ b/internal/ingestion/component/chunker/delimiter_case_sensitive_test.go @@ -37,7 +37,9 @@ import ( ) func getDelimiters(delimiters string) string { - p := chunk.CompileDelimiterPattern(chunk.ParseDelimiterField(delimiters)) + // Mirror the live path: backtick-wrapped entries contribute their inner + // content (see chunk.CompileDelimiterPatternList). + p := chunk.CompileDelimiterPatternList([]string{delimiters}, true) if p == nil { return "" } @@ -232,13 +234,9 @@ func TestTokenChunker_BacktickASplitsOnlyAtLowercase(t *testing.T) { func TestBacktickDelimiterIsCaseSensitive(t *testing.T) { // Extraction and compiled pattern must both preserve letter casing. - parsed := chunk.ParseDelimiterField("`End`") - if len(parsed) != 1 || parsed[0] != "End" { - t.Fatalf("ParseDelimiterField(`End`) = %#v, want [End]", parsed) - } - p := chunk.CompileDelimiterPattern(parsed) + p := chunk.CompileDelimiterPatternList([]string{"`End`"}, true) if p == nil { - t.Fatal("CompileDelimiterPattern returned nil") + t.Fatal("CompileDelimiterPatternList returned nil") } if !p.MatchString("End") || p.MatchString("end") || p.MatchString("END") { t.Fatalf("pattern %q is not case-sensitive", p.String()) diff --git a/internal/ingestion/component/chunker/token.go b/internal/ingestion/component/chunker/token.go index 2531ebf9eb..865e3c3fdd 100644 --- a/internal/ingestion/component/chunker/token.go +++ b/internal/ingestion/component/chunker/token.go @@ -26,8 +26,8 @@ // - DELIMITER PARSING for the TokenChunker list API mirrors Python // token_chunker: only entries wrapped in backticks (e.g. "`\\n\\n`") // produce an active split pattern. Plain list entries are not -// compiled into the pattern. Single-string parser_config.delimiter -// parsing lives in ragflow/internal/parser/chunk (ParseDelimiterField). +// compiled into the pattern. (The single-string parser_config.delimiter +// field is not parsed in Go; only the []string list API is consumed.) // // - CHILDREN DELIMITERS (the secondary split) is implemented via the // splitDroppingDelim helper; emitted chunks carry the parent diff --git a/internal/parser/chunk/delim.go b/internal/parser/chunk/delim.go index 1b897ab5ea..3e40b87505 100644 --- a/internal/parser/chunk/delim.go +++ b/internal/parser/chunk/delim.go @@ -23,36 +23,10 @@ import ( "unicode/utf8" ) -// Canonical parser for the parser_config.delimiter field. -// -// Mirrors Python rag/nlp/delim.py (#17383). A delimiter field is a string -// with the grammar: -// -// delimiter_field := token* -// token := backtick_wrapped | bare_char -// backtick_wrapped := "`" bare_char+ "`" -// bare_char := any single Unicode character except "`" -// -// Semantics: -// 1. Characters between matching backticks form one multi-character delimiter. -// 2. Any character outside backticks is its own single-character delimiter. -// 3. Results are deduplicated and sorted longest-first (stable for equal length). -// 4. CRLF and standalone CR are normalized to LF before parsing. -// 5. Matching is case-sensitive. - // backtickWrappedRE matches a backtick-wrapped multi-character token. // Case-sensitive on purpose (see #17384). var backtickWrappedRE = regexp.MustCompile("`([^`]+)`") -// NormalizeTextNewlines converts CRLF and standalone CR to LF. -func NormalizeTextNewlines(text string) string { - if text == "" { - return text - } - text = strings.ReplaceAll(text, "\r\n", "\n") - return strings.ReplaceAll(text, "\r", "\n") -} - // HasWrappedDelimiter reports whether the delimiter field contains at least // one backtick-wrapped token. Used to decide the historical "custom delimiter" // mode that bypasses chunk_token_num. Separate from whether any delimiter is @@ -64,54 +38,6 @@ func HasWrappedDelimiter(s string) bool { return backtickWrappedRE.MatchString(s) } -// ParseDelimiterField parses the delimiter field into delimiter strings. -// -// Returns nil for an empty field. Whitespace characters are valid -// single-character delimiters. Output is sorted longest-first and -// deduplicated while preserving first-occurrence order for equal-length -// items (the sort is stable). CRLF/CR inside the field are normalized to LF. -func ParseDelimiterField(s string) []string { - if s == "" { - return nil - } - normalized := NormalizeTextNewlines(s) - - var delimiters []string - seen := make(map[string]struct{}) - cursor := 0 - for _, loc := range backtickWrappedRE.FindAllStringSubmatchIndex(normalized, -1) { - // loc: [fullStart, fullEnd, groupStart, groupEnd] - start, end := loc[0], loc[1] - for _, ch := range runesOf(normalized[cursor:start]) { - if _, ok := seen[ch]; ok { - continue - } - seen[ch] = struct{}{} - delimiters = append(delimiters, ch) - } - token := normalized[loc[2]:loc[3]] - if token != "" { - if _, ok := seen[token]; !ok { - seen[token] = struct{}{} - delimiters = append(delimiters, token) - } - } - cursor = end - } - for _, ch := range runesOf(normalized[cursor:]) { - if _, ok := seen[ch]; ok { - continue - } - seen[ch] = struct{}{} - delimiters = append(delimiters, ch) - } - - sort.SliceStable(delimiters, func(i, j int) bool { - return utf8.RuneCountInString(delimiters[i]) > utf8.RuneCountInString(delimiters[j]) - }) - return delimiters -} - // CompileDelimiterPattern builds an alternation regex from delimiter strings. // // Each delimiter is regexp.QuoteMeta'd so whitespace and metacharacters match @@ -142,8 +68,8 @@ func CompileDelimiterPattern(delimiters []string) *regexp.Regexp { // count) so a longer delimiter always wins over a shorter prefix inside it. // // - Backtick-wrapped entries (“ `abc` “) contribute their INNER content as -// the split pattern; the backticks are stripped. This is the Python -// token_chunker / historical Go compileDelimPattern convention. +// the split pattern; the backticks are stripped. The wrapping is the +// delimiter-list quoting syntax that opts an entry into the active set. // - Bare entries (e.g. ". ") behave according to keepBare: // keepBare=false (the main `delimiters` list): a bare entry is IGNORED for // the active pattern — it only influences merge paths when no custom @@ -152,11 +78,12 @@ func CompileDelimiterPattern(delimiters []string) *regexp.Regexp { // active, because every non-empty children delimiter splits — including // bare ones. // -// Returns nil when no active entry remains. Prefer ParseDelimiterField for the -// single-string parser_config.delimiter field; this helper exists for the -// dataflow list API. +// Returns nil when no active entry remains. This helper exists for the +// dataflow list API. The single-string parser_config.delimiter field is not +// parsed in Go; only the []string list API is consumed. func CompileDelimiterPatternList(delims []string, keepBare bool) *regexp.Regexp { var out []string + seen := make(map[string]struct{}) for _, d := range delims { if d == "" { continue @@ -166,12 +93,20 @@ func CompileDelimiterPatternList(delims []string, keepBare bool) *regexp.Regexp if inner == "" { continue } + if _, ok := seen[inner]; ok { + continue + } + seen[inner] = struct{}{} out = append(out, inner) continue } if !keepBare { continue } + if _, ok := seen[d]; ok { + continue + } + seen[d] = struct{}{} out = append(out, d) } if len(out) == 0 { @@ -192,8 +127,7 @@ func CompileDelimiterPatternList(delims []string, keepBare bool) *regexp.Regexp // contribute their inner content and bare entries are ignored for the active // pattern (they only steer merge paths when no custom pattern exists). // -// Prefer ParseDelimiterField for the single-string parser_config.delimiter -// field. This helper exists for the dataflow list API. +// This helper exists for the dataflow list API. func CompileDelimiterListPattern(delims []string) *regexp.Regexp { return CompileDelimiterPatternList(delims, false) } @@ -208,15 +142,3 @@ func HasCustomDelimiterList(delims []string) bool { } return false } - -// runesOf splits s into individual Unicode characters (as strings). -func runesOf(s string) []string { - if s == "" { - return nil - } - out := make([]string, 0, utf8.RuneCountInString(s)) - for _, r := range s { - out = append(out, string(r)) - } - return out -} diff --git a/internal/parser/chunk/delim_test.go b/internal/parser/chunk/delim_test.go index 4c5a65fb43..5723743957 100644 --- a/internal/parser/chunk/delim_test.go +++ b/internal/parser/chunk/delim_test.go @@ -22,23 +22,6 @@ import ( "testing" ) -func TestNormalizeTextNewlines(t *testing.T) { - tests := []struct { - in, want string - }{ - {"", ""}, - {"a\nb", "a\nb"}, - {"a\r\nb", "a\nb"}, - {"a\rb", "a\nb"}, - {"a\r\nb\rc", "a\nb\nc"}, - } - for _, tc := range tests { - if got := NormalizeTextNewlines(tc.in); got != tc.want { - t.Errorf("NormalizeTextNewlines(%q) = %q, want %q", tc.in, got, tc.want) - } - } -} - func TestHasWrappedDelimiter(t *testing.T) { if HasWrappedDelimiter("") { t.Fatal("empty should be false") @@ -57,42 +40,6 @@ func TestHasWrappedDelimiter(t *testing.T) { } } -func TestParseDelimiterField(t *testing.T) { - tests := []struct { - name string - field string - want []string - }{ - {"empty", "", nil}, - {"single bare", "!", []string{"!"}}, - {"bare pair", "!?", []string{"!", "?"}}, - {"space", " ", []string{" "}}, - {"newline", "\n", []string{"\n"}}, - {"crlf field", "\r\n", []string{"\n"}}, - {"bare cr", "\r", []string{"\n"}}, - {"wrapped end", "`end`", []string{"end"}}, - {"wrapped hierarchy", "`###``##``#`", []string{"###", "##", "#"}}, - {"tooltip example", "\n`##`;", []string{"##", "\n", ";"}}, - {"dedupe", "`a`a`a`", []string{"a"}}, - {"wrapped double newline", "`\n\n`", []string{"\n\n"}}, - {"crlf wrapped", "`\r\n`", []string{"\n"}}, - {"shipped default", "\n!?;。；！？", []string{"\n", "!", "?", ";", "。", "；", "！", "？"}}, - {"chinese", "。；", []string{"。", "；"}}, - {"mixed order equal length", "`##`#\n", []string{"##", "#", "\n"}}, - {"empty backticks bare", "``", []string{"`"}}, - {"double space bare dedupe", " ", []string{" "}}, - {"wrapped double space", "` `", []string{" "}}, - } - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - got := ParseDelimiterField(tc.field) - if !reflect.DeepEqual(got, tc.want) { - t.Errorf("ParseDelimiterField(%q) = %#v, want %#v", tc.field, got, tc.want) - } - }) - } -} - func TestCompileDelimiterPattern(t *testing.T) { if CompileDelimiterPattern(nil) != nil { t.Fatal("empty list should yield nil") @@ -122,7 +69,7 @@ func TestCompileDelimiterPattern(t *testing.T) { } func TestCompileDelimiterPatternShippedDefault(t *testing.T) { - pat := CompileDelimiterPattern(ParseDelimiterField("\n!?;。；！？")) + pat := CompileDelimiterPattern([]string{"\n", "!", "?", ";", "。", "；", "！", "？"}) if pat == nil { t.Fatal("expected pattern") } @@ -137,7 +84,7 @@ func TestCompileDelimiterPatternShippedDefault(t *testing.T) { } func TestCompileDelimiterPatternCaseSensitive(t *testing.T) { - pat := CompileDelimiterPattern(ParseDelimiterField("a")) + pat := CompileDelimiterPattern([]string{"a"}) parts := regexp.MustCompile("("+pat.String()+")").Split("AaBb", -1) // Split removes matches; "A" + "Bb" with "a" consumed. if len(parts) < 2 { @@ -213,6 +160,92 @@ func TestCompileDelimiterPatternListKeepBareFalse(t *testing.T) { } } +func TestCompileDelimiterPatternListDedup(t *testing.T) { + // Equivalent quoted + bare delimiters must not produce a redundant + // alternation. ["`#`", "#"] with keepBare=true resolves to the single + // active value `#`. + pat := CompileDelimiterPatternList([]string{"`#`", "#"}, true) + if pat == nil { + t.Fatal("expected a pattern from equivalent quoted + bare entries") + } + if got := pat.String(); got != "#" { + t.Errorf("dedup pattern = %q, want %q", got, "#") + } + // Repeated wrapped entries are also deduplicated. + pat = CompileDelimiterPatternList([]string{"`##`", "`##`", "`#`"}, false) + if pat == nil { + t.Fatal("expected a pattern from wrapped entries") + } + if got := pat.String(); got != `##|#` { + t.Errorf("dedup pattern = %q, want %q", got, `##|#`) + } +} + +func TestCompileDelimiterPatternListDedupOrderIndependent(t *testing.T) { + // Dedup must hold regardless of input order: wrapped entries key on their + // inner content while bare entries key on themselves, so a wrapped/bare + // collision collapses no matter which appears first. + pat := CompileDelimiterPatternList([]string{"#", "`#`"}, true) + if pat == nil { + t.Fatal("expected a pattern from equivalent bare + quoted entries") + } + if got := pat.String(); got != "#" { + t.Errorf("dedup pattern = %q, want %q", got, "#") + } + // Multiple collisions across mixed order collapse to one alternation. + pat = CompileDelimiterPatternList([]string{"`#`", "#", "`#`", "#"}, true) + if got := pat.String(); got != "#" { + t.Errorf("dedup pattern = %q, want %q", got, "#") + } + // Wrapped duplicates deduplicate no matter where they appear. + pat = CompileDelimiterPatternList([]string{"`##`", "`#`", "`##`"}, false) + if got := pat.String(); got != `##|#` { + t.Errorf("dedup pattern = %q, want %q", got, `##|#`) + } +} + +func TestCompileDelimiterListPatternDedup(t *testing.T) { + // CompileDelimiterListPattern is the main-delimiter live path + // (keepBare=false); it must propagate the same dedup so the main + // delimiters list cannot accumulate redundant alternations either. + pat := CompileDelimiterListPattern([]string{"`##`", "`##`", "`#`"}) + if pat == nil { + t.Fatal("expected a pattern from wrapped entries") + } + if got := pat.String(); got != `##|#` { + t.Errorf("dedup pattern = %q, want %q", got, `##|#`) + } + // Bare entries are ignored on this path, so only the wrapped inner content + // participates and is deduplicated. + pat = CompileDelimiterListPattern([]string{"`#`", "#"}) + if got := pat.String(); got != "#" { + t.Errorf("dedup pattern = %q, want %q", got, "#") + } +} + +func TestCompileDelimiterPatternListChildrenPrefixOrder(t *testing.T) { + // keepBare=true (children_delimiters): a longer delimiter must win over a + // shorter prefix inside it because entries are sorted longest-first by + // rune count. + pat := CompileDelimiterPatternList([]string{"##", "#"}, true) + if pat == nil { + t.Fatal("expected a pattern from bare prefix entries") + } + if got := pat.String(); got != `##|#` { + t.Errorf("pattern = %q, want %q", got, `##|#`) + } + // Longest-first ordering ensures "###" splits on "##", not "#". + if got := pat.FindString("a###b"); got != "##" { + t.Errorf("FindString(a###b) = %q, want %q", got, "##") + } + // Multi-byte delimiter ordering is by rune count, not byte length: + // "###" (3 runes) must be tried before "。" (1 rune). + pat = CompileDelimiterPatternList([]string{"。", "###"}, true) + if got := pat.String(); got != `###|。` { + t.Errorf("pattern = %q, want %q", got, `###|。`) + } +} + func TestHasCustomDelimiterList(t *testing.T) { if HasCustomDelimiterList([]string{"\n", "!"}) { t.Fatal("bare list should be false")