From d4ceeee4ed5aa73c2e0e4fd563e21ae0ebeb2eac Mon Sep 17 00:00:00 2001 From: S <5842097+skbs-eng@users.noreply.github.com> Date: Sun, 2 Aug 2026 14:37:14 +0530 Subject: [PATCH] refactor(nlp): collapse 6 delimiter-parser implementations into one canonical helper (#17383) (#17387) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Six sites used to read the same `parser_config.delimiter` field with divergent grammars: - `rag.nlp.get_delimiters` (PDF/DOCX/HTML/EPUB/JSON/CSV/XLSX/email/book) - `rag.nlp.naive_merge` (custom-delimiter branch) - `rag.nlp.naive_merge_with_images` - `rag.nlp._build_cks` - `deepdoc.parser.txt_parser.parser_txt` (.txt, code) - `deepdoc.parser.markdown_parser.MarkdownElementExtractor.get_delimiters` The six implementations disagreed on bare-vs-wrapped chars, dedupe, sort order, CRLF normalization, and `re.I` (#17384). The shipped default `` `\n!?;。；！？` `` was a no-op for `.md` because the markdown path only matched backtick-wrapped tokens. ## Changes - **new:** `rag/nlp/delim.py` with `parse_delimiter_field` and `compile_delimiter_pattern`. Single source of truth. CRLF normalization at the top; longest-first stable sort; insertion-ordered dedupe; no `re.I`. - **refactor:** all six call sites delegate to the helper. - `rag/nlp/__init__.py::get_delimiters` becomes a thin shim. - `deepdoc/parser/txt_parser.py::parser_txt` drops the `[encode/decode/unicode_escape]` round-trip. - `deepdoc/parser/markdown_parser.py::get_delimiters` honors bare chars (fixes [1]). - **tests:** `test/unit_test/rag/test_delim.py` (85 tests) — helper, acceptance table, frontend parity, static guard against re-inlining. - **tests:** `test/unit_test/rag/test_delimiter_case_sensitive.py` (from #17386) updated to retarget the static check at the new helper + AST-based broader scan. ## Acceptance criteria - All six sites produce the same regex pattern for the same input. - Shipped default keeps working for `.txt` / `.pdf` / `.docx`. - Shipped default for `.md` now splits (was a silent no-op). - Tooltip example `` `\n##;` `` produces three effective delimiters regardless of file type. - Bare whitespace inputs split on every occurrence. - Backtick-wrapped whitespace splits only on the exact N-char sequence. - CRLF-line-ending documents split identically to LF-line-ending documents. - 123 tests pass (85 new + 38 existing). ## Rebase protocol As #17385 and #17386 evolve, this branch will be rebased on top. The only overlap between this PR's diff and the other two is `test_delimiter_case_sensitive.py`, where #17383 modifies the static check to point at the new helper location. --------- Co-authored-by: kiloconnect[bot] <240665456+kiloconnect[bot]@users.noreply.github.com> --- deepdoc/parser/markdown_parser.py | 22 +- deepdoc/parser/txt_parser.py | 32 +- .../ingestion/component/chunker/common.go | 89 +-- .../chunker/delimiter_case_sensitive_test.go | 33 +- internal/ingestion/component/chunker/token.go | 21 +- internal/parser/chunk/delim.go | 194 ++++++ internal/parser/chunk/delim_test.go | 185 +++++ rag/nlp/__init__.py | 107 +-- rag/nlp/delim.py | 170 +++++ test/unit_test/rag/conftest.py | 74 +- test/unit_test/rag/test_delim.py | 631 ++++++++++++++++++ .../rag/test_delimiter_case_sensitive.py | 219 +++--- .../utils/__tests__/delimiter-preview.test.ts | 50 ++ web/src/utils/delimiter-preview.ts | 45 +- 14 files changed, 1551 insertions(+), 321 deletions(-) create mode 100644 internal/parser/chunk/delim.go create mode 100644 internal/parser/chunk/delim_test.go create mode 100644 rag/nlp/delim.py create mode 100644 test/unit_test/rag/test_delim.py create mode 100644 web/src/utils/__tests__/delimiter-preview.test.ts diff --git a/deepdoc/parser/markdown_parser.py b/deepdoc/parser/markdown_parser.py index b09e5adb13..3f4583d783 100644 --- a/deepdoc/parser/markdown_parser.py +++ b/deepdoc/parser/markdown_parser.py @@ -20,6 +20,12 @@ import re from markdown import markdown +from rag.nlp.delim import ( + compile_delimiter_pattern, + normalize_text_newlines, + parse_delimiter_field, +) + logger = logging.getLogger(__name__) @@ -153,13 +159,19 @@ class RAGFlowMarkdownParser: class MarkdownElementExtractor: def __init__(self, markdown_content): - self.markdown_content = markdown_content - self.lines = markdown_content.split("\n") + # Normalize CRLF/CR so compiled delimiter patterns (which use LF) + # match Windows-line-ending source the same as Unix source. + self.markdown_content = normalize_text_newlines(markdown_content) + self.lines = self.markdown_content.split("\n") def get_delimiters(self, delimiters): - toks = re.findall(r"`([^`]+)`", delimiters) - toks = sorted(set(toks), key=lambda x: -len(x)) - return "|".join(re.escape(t) for t in toks if t) + # Delegate to the canonical parser (#17383). The previous + # implementation matched only backtick-wrapped tokens and dropped + # bare characters, which silently made the shipped default + # delimiter field a no-op for the markdown path. The helper honors + # both bare chars and wrapped tokens, so the same field produces + # the same splits in every file type. + return compile_delimiter_pattern(parse_delimiter_field(delimiters)) def _get_fence_marker(self, line): match = re.match(r"^[ \t]{0,3}(?P`{3,}|~{3,})(?:.*)$", line) diff --git a/deepdoc/parser/txt_parser.py b/deepdoc/parser/txt_parser.py index 60b310cd46..9e51ffc961 100644 --- a/deepdoc/parser/txt_parser.py +++ b/deepdoc/parser/txt_parser.py @@ -20,6 +20,11 @@ import re from common.token_utils import num_tokens_from_string from deepdoc.parser.utils import get_text from rag.nlp import _split_oversized_unit +from rag.nlp.delim import ( + compile_delimiter_pattern, + normalize_text_newlines, + parse_delimiter_field, +) class RAGFlowTxtParser: @@ -33,10 +38,9 @@ class RAGFlowTxtParser: raise TypeError("txt type should be str!") cks = [""] tk_nums = [0] - delimiter = delimiter.encode("utf-8").decode("unicode_escape").encode("latin1").decode("utf-8") def add_chunk(t): - nonlocal cks, tk_nums, delimiter + nonlocal cks, tk_nums tnum = num_tokens_from_string(t) if cks[-1] == "": @@ -54,21 +58,17 @@ class RAGFlowTxtParser: cks.append(t) tk_nums.append(tnum) - dels = [] - s = 0 - for m in re.finditer(r"`([^`]+)`", delimiter): - f, m_t = m.span() - dels.append(m.group(1)) - dels.extend(list(delimiter[s:f])) - s = m_t - if s < len(delimiter): - dels.extend(list(delimiter[s:])) - dels = [re.escape(d) for d in dels if d] - dels = [d for d in dels if d] - dels = "|".join(dels) - secs = re.split(r"(%s)" % dels, txt) + txt = normalize_text_newlines(txt) + parsed_dels = parse_delimiter_field(delimiter) + dels = compile_delimiter_pattern(parsed_dels) + logging.debug( + "RAGFlowTxtParser.parser_txt: delimiter_count=%d, splitting=%s", + len(parsed_dels), + bool(dels), + ) + secs = re.split(r"(%s)" % dels, txt) if dels else [txt] for sec in secs: - if re.match(f"^{dels}$", sec): + if dels and re.match(f"^{dels}$", sec): continue if not sec: continue diff --git a/internal/ingestion/component/chunker/common.go b/internal/ingestion/component/chunker/common.go index 79d2b996aa..0ee111d88a 100644 --- a/internal/ingestion/component/chunker/common.go +++ b/internal/ingestion/component/chunker/common.go @@ -19,11 +19,11 @@ package chunker import ( "fmt" "regexp" - "sort" "strings" "ragflow/internal/agent/runtime" "ragflow/internal/ingestion/component/schema" + "ragflow/internal/parser/chunk" "ragflow/internal/tokenizer" ) @@ -76,87 +76,14 @@ func stringListFromAny(in []any) []string { // regex / split helpers // --------------------------------------------------------------------------- -// backtickDelimRE extracts backtick-wrapped delimiter tokens. -// Case-sensitive on purpose (mirrors Python after #17384 / PR #17386): -// never add (?i) here — delimiter matching must preserve letter casing. -var backtickDelimRE = regexp.MustCompile("`([^`]+)`") - -// escapeAndSort QuoteMeta-escapes tokens and sorts longest-first. -// Empty tokens are dropped. When dedup is true, exact duplicate tokens -// are collapsed (first occurrence wins). Matching stays case-sensitive. -func escapeAndSort(tokens []string, dedup bool) []string { - out := make([]string, 0, len(tokens)) - var seen map[string]struct{} - if dedup { - seen = make(map[string]struct{}, len(tokens)) - } - for _, tok := range tokens { - if tok == "" { - continue - } - if dedup { - if _, ok := seen[tok]; ok { - continue - } - seen[tok] = struct{}{} - } - out = append(out, regexp.QuoteMeta(tok)) - } - sort.SliceStable(out, func(i, j int) bool { return len(out[i]) > len(out[j]) }) - return out -} - -// getDelimiters ports rag.nlp.get_delimiters (rag/nlp/__init__.py). -// -// It walks a single delimiter string, pulling out backtick-wrapped tokens -// and every bare character outside those spans, then returns a -// length-sorted, QuoteMeta-escaped alternation suitable for regexp.Split. -// Matching is case-sensitive: "a" does not match "A", and "`end`" does not -// match "End" / "END". -func getDelimiters(delimiters string) string { - var dels []string - s := 0 - for _, m := range backtickDelimRE.FindAllStringSubmatchIndex(delimiters, -1) { - // m = [fullStart, fullEnd, g1Start, g1End] - f, t := m[0], m[1] - dels = append(dels, delimiters[m[2]:m[3]]) - for _, r := range delimiters[s:f] { - dels = append(dels, string(r)) - } - s = t - } - if s < len(delimiters) { - for _, r := range delimiters[s:] { - dels = append(dels, string(r)) - } - } - // Python get_delimiters does not dedup; preserve that behavior. - return strings.Join(escapeAndSort(dels, false), "|") -} - -// compileDelimPattern builds an alternation from backtick-wrapped tokens -// across delimiter entries (mirrors Python _compile_delimiter_pattern). -// Each entry is scanned independently so token boundaries cannot span -// adjacent slice elements. Extraction is case-sensitive — see backtickDelimRE. -// -// Plain (non-backtick) delimiters are not compiled here; callers that -// need bare-char splitting use getDelimiters (naive_merge path). +// compileDelimPattern compiles a TokenChunker-style []string delimiter list. +// 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). func compileDelimPattern(delims []string) *regexp.Regexp { - var tokens []string - for _, d := range delims { - if d == "" { - continue - } - for _, m := range backtickDelimRE.FindAllStringSubmatch(d, -1) { - tokens = append(tokens, m[1]) - } - } - // Python _compile_delimiter_pattern dedups via set(...). - custom := escapeAndSort(tokens, true) - if len(custom) == 0 { - return nil - } - return regexp.MustCompile(strings.Join(custom, "|")) + return chunk.CompileDelimiterListPattern(delims) } // splitKeepingDelim mirrors Python token_chunker._split_text_by_pattern diff --git a/internal/ingestion/component/chunker/delimiter_case_sensitive_test.go b/internal/ingestion/component/chunker/delimiter_case_sensitive_test.go index 16ea51b610..fc097221c1 100644 --- a/internal/ingestion/component/chunker/delimiter_case_sensitive_test.go +++ b/internal/ingestion/component/chunker/delimiter_case_sensitive_test.go @@ -32,8 +32,18 @@ import ( "regexp" "strings" "testing" + + "ragflow/internal/parser/chunk" ) +func getDelimiters(delimiters string) string { + p := chunk.CompileDelimiterPattern(chunk.ParseDelimiterField(delimiters)) + if p == nil { + return "" + } + return p.String() +} + func TestGetDelimiters_BareCharAReturnsLiteralPattern(t *testing.T) { // Bare-char delimiter "a" must produce the pattern "a", not "a|A". if got, want := getDelimiters("a"), "a"; got != want { @@ -219,15 +229,20 @@ func TestTokenChunker_BacktickASplitsOnlyAtLowercase(t *testing.T) { } } -func TestBacktickDelimRE_HasNoIgnoreCaseFlag(t *testing.T) { - // Structural guard: the shared backtick extractor must stay case-sensitive. - src := backtickDelimRE.String() - if strings.Contains(src, "(?i)") || strings.HasPrefix(src, "(?i)") { - t.Fatalf("backtickDelimRE must not use (?i); got %q", src) +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) } - // Sanity: the pattern still extracts backtick contents. - m := backtickDelimRE.FindStringSubmatch("`End`") - if len(m) < 2 || m[1] != "End" { - t.Fatalf("backtickDelimRE(`End`) = %#v, want group1=End", m) + p := chunk.CompileDelimiterPattern(parsed) + if p == nil { + t.Fatal("CompileDelimiterPattern returned nil") + } + if !p.MatchString("End") || p.MatchString("end") || p.MatchString("END") { + t.Fatalf("pattern %q is not case-sensitive", p.String()) + } + if strings.Contains(p.String(), "(?i)") { + t.Fatalf("compiled pattern must not carry (?i); got %q", p.String()) } } diff --git a/internal/ingestion/component/chunker/token.go b/internal/ingestion/component/chunker/token.go index 07ca2e815b..7d86b611be 100644 --- a/internal/ingestion/component/chunker/token.go +++ b/internal/ingestion/component/chunker/token.go @@ -23,10 +23,11 @@ // 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`: -// entries wrapped in backticks (e.g. "`\\n\\n`") are treated as -// regex split points; plain strings are regex-escaped and joined -// into the same alternation. Empty entries are filtered. +// - 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). // // - CHILDREN DELIMITERS (the secondary split) is implemented via the // shared splitKeepingDelim helper; emitted chunks carry the parent @@ -63,12 +64,13 @@ import ( "strings" "sync" + "gorm.io/gorm" "ragflow/internal/agent/runtime" deepdoctype "ragflow/internal/deepdoc/parser/type" "ragflow/internal/ingestion/component/globals" "ragflow/internal/ingestion/component/schema" - "gorm.io/gorm" + "ragflow/internal/parser/chunk" ) const ComponentNameTokenChunker = "TokenChunker" @@ -1075,14 +1077,9 @@ func hasActiveDelimiter(p *regexp.Regexp) bool { // hasCustomDelim reports whether any delimiter uses backtick syntax // (`pattern`). Python's naive_merge skips token-size merging when -// custom delimiters are present (naive_merge:1194-1213). +// custom delimiters are present. Delegates to the canonical helper. func hasCustomDelim(delims []string) bool { - for _, d := range delims { - if strings.HasPrefix(d, "`") && strings.HasSuffix(d, "`") && len(d) >= 2 { - return true - } - } - return false + return chunk.HasCustomDelimiterList(delims) } // applyChildrenDelim mirrors token_chunker.py:325-334. diff --git a/internal/parser/chunk/delim.go b/internal/parser/chunk/delim.go new file mode 100644 index 0000000000..aebadbd60a --- /dev/null +++ b/internal/parser/chunk/delim.go @@ -0,0 +1,194 @@ +// +// Copyright 2026 The InfiniFlow Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +package chunk + +import ( + "regexp" + "sort" + "strings" + "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 +// present after parsing (bare single-character delimiters still split). +func HasWrappedDelimiter(s string) bool { + if s == "" { + return false + } + 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 +// literally. Returns nil when delimiters is empty or yields no non-empty +// entries. The pattern is intended for use with a capturing split (so +// delimiters appear in the split output) or FindAllStringIndex. +func CompileDelimiterPattern(delimiters []string) *regexp.Regexp { + if len(delimiters) == 0 { + return nil + } + escaped := make([]string, 0, len(delimiters)) + for _, d := range delimiters { + if d == "" { + continue + } + escaped = append(escaped, regexp.QuoteMeta(d)) + } + if len(escaped) == 0 { + return nil + } + return regexp.MustCompile(strings.Join(escaped, "|")) +} + +// CompileDelimiterListPattern compiles a TokenChunker-style []string delimiter +// list. Entries wrapped in backticks contribute their inner content as a +// split pattern (Python token_chunker / historical Go compileDelimPattern). +// Bare list entries are ignored for the active pattern — they are only used +// by 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. +func CompileDelimiterListPattern(delims []string) *regexp.Regexp { + var custom []string + for _, d := range delims { + if d == "" { + continue + } + if strings.HasPrefix(d, "`") && strings.HasSuffix(d, "`") && len(d) >= 2 { + inner := d[1 : len(d)-1] + if inner == "" { + continue + } + custom = append(custom, inner) + } + } + if len(custom) == 0 { + return nil + } + sort.SliceStable(custom, func(i, j int) bool { + return utf8.RuneCountInString(custom[i]) > utf8.RuneCountInString(custom[j]) + }) + escaped := make([]string, 0, len(custom)) + for _, d := range custom { + escaped = append(escaped, regexp.QuoteMeta(d)) + } + return regexp.MustCompile(strings.Join(escaped, "|")) +} + +// HasCustomDelimiterList reports whether any entry in a TokenChunker-style +// delimiter list uses backtick syntax. +func HasCustomDelimiterList(delims []string) bool { + for _, d := range delims { + if strings.HasPrefix(d, "`") && strings.HasSuffix(d, "`") && len(d) >= 2 { + return true + } + } + 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 new file mode 100644 index 0000000000..7a1034200b --- /dev/null +++ b/internal/parser/chunk/delim_test.go @@ -0,0 +1,185 @@ +// +// Copyright 2026 The InfiniFlow Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +package chunk + +import ( + "reflect" + "regexp" + "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") + } + if HasWrappedDelimiter("!;") { + t.Fatal("bare chars should be false") + } + if !HasWrappedDelimiter("`##`") { + t.Fatal("wrapped should be true") + } + if !HasWrappedDelimiter("\n`##`;") { + t.Fatal("mixed should be true") + } + if !HasWrappedDelimiter("`;`") { + t.Fatal("wrapped single char should be true") + } +} + +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") + } + if CompileDelimiterPattern([]string{}) != nil { + t.Fatal("empty slice should yield nil") + } + + pat := CompileDelimiterPattern([]string{"##", "#"}) + if pat == nil { + t.Fatal("expected pattern") + } + if got := pat.FindString("###"); got != "##" { + t.Errorf("longest match: got %q, want ##", got) + } + + // Metacharacters match literally. + for _, ch := range []string{".", "(", "?", "+"} { + p := CompileDelimiterPattern([]string{ch}) + if p == nil || !p.MatchString(ch) { + t.Errorf("pattern for %q should match itself", ch) + } + if ch != "." && p.MatchString("z") { + t.Errorf("pattern for %q should not match z", ch) + } + } +} + +func TestCompileDelimiterPatternShippedDefault(t *testing.T) { + pat := CompileDelimiterPattern(ParseDelimiterField("\n!?;。；！？")) + if pat == nil { + t.Fatal("expected pattern") + } + for _, ch := range []string{"\n", "!", "?", ";", "。", "；", "！", "？"} { + if !pat.MatchString(ch) { + t.Errorf("default pattern must match %q", ch) + } + } + if pat.MatchString("a") { + t.Error("default pattern must not match a") + } +} + +func TestCompileDelimiterPatternCaseSensitive(t *testing.T) { + pat := CompileDelimiterPattern(ParseDelimiterField("a")) + parts := regexp.MustCompile("("+pat.String()+")").Split("AaBb", -1) + // Split removes matches; "A" + "Bb" with "a" consumed. + if len(parts) < 2 { + t.Fatalf("parts=%v", parts) + } + // Only lowercase a splits. + re := regexp.MustCompile("(" + pat.String() + ")") + got := re.Split("AaBb", -1) + want := []string{"A", "Bb"} + // re.Split in Go does not keep delimiters; with capturing group behavior + // differs — just check case sensitivity via FindAll. + matches := pat.FindAllString("AaBb", -1) + if !reflect.DeepEqual(matches, []string{"a"}) { + t.Errorf("matches=%v want [a]; split parts=%v want-ish %v", matches, got, want) + } +} + +func TestCompileDelimiterListPattern(t *testing.T) { + // Bare list entries are ignored (TokenChunker list API). + if CompileDelimiterListPattern([]string{"\n", "!"}) != nil { + t.Fatal("bare list entries should not produce a pattern") + } + pat := CompileDelimiterListPattern([]string{"`##`", "`#`"}) + if pat == nil { + t.Fatal("expected pattern from wrapped list entries") + } + if got := pat.FindString("###"); got != "##" { + t.Errorf("got %q want ##", got) + } + + // Verify unescaped rune length sorting order: "a.b" (3 runes) vs "ab" (2 runes with meta chars) + patMeta := CompileDelimiterListPattern([]string{"`a`", "`a.b`"}) + if patMeta.String() != `a\.b|a` { + t.Errorf("got pattern %q, want %q", patMeta.String(), `a\.b|a`) + } +} + +func TestHasCustomDelimiterList(t *testing.T) { + if HasCustomDelimiterList([]string{"\n", "!"}) { + t.Fatal("bare list should be false") + } + if !HasCustomDelimiterList([]string{"\n", "`##`"}) { + t.Fatal("wrapped list entry should be true") + } +} diff --git a/rag/nlp/__init__.py b/rag/nlp/__init__.py index a743637c31..b6edcfcd8b 100644 --- a/rag/nlp/__init__.py +++ b/rag/nlp/__init__.py @@ -28,6 +28,15 @@ from word2number import w2n from common.token_utils import num_tokens_from_string +# Re-exported below for backwards compatibility; the canonical parser lives +# in ``rag.nlp.delim``. +from rag.nlp.delim import ( + compile_delimiter_pattern, + has_wrapped_delimiter, + normalize_text_newlines, + parse_delimiter_field, +) + __all__ = ["rag_tokenizer"] all_codecs = [ @@ -1290,7 +1299,7 @@ def naive_merge(sections: str | list, chunk_token_num=128, delimiter="\n。； if isinstance(sections[0], str): sections = [(s, "") for s in sections] # Normalize line endings so delimiter ``\n`` matches ``\r\n`` and standalone ``\r``. - sections = [(s.replace("\r\n", "\n").replace("\r", "\n"), pos) for s, pos in sections] + sections = [(normalize_text_newlines(s), pos) for s, pos in sections] cks = [""] tk_nums = [0] @@ -1304,16 +1313,22 @@ def naive_merge(sections: str | list, chunk_token_num=128, delimiter="\n。； cks.append(text) tk_nums.append(tk_num) - custom_delimiters = [m.group(1) for m in re.finditer(r"`([^`]+)`", delimiter)] - has_custom = bool(custom_delimiters) + # Parse the delimiter field once, via the canonical helper (#17383). + # `has_custom` means the field contains a backtick-wrapped token — the + # historical signal that chunk_token_num should be bypassed. Splitting + # itself uses every parsed delimiter (bare and wrapped). + parsed_dels = parse_delimiter_field(delimiter) + has_custom = has_wrapped_delimiter(delimiter) if has_custom: # Custom delimiters ignore chunk_token_num: each segment is its own chunk. - custom_pattern = "|".join(re.escape(t) for t in sorted(set(custom_delimiters), key=len, reverse=True)) + custom_pattern = compile_delimiter_pattern(parsed_dels) cks, tk_nums = [], [] for sec, pos in sections: - split_sec = re.split(r"(%s)" % custom_pattern, sec, flags=re.DOTALL) + split_sec = re.split(r"(%s)" % custom_pattern, sec, flags=re.DOTALL) if custom_pattern else [sec] for sub_sec in split_sec: - if re.fullmatch(custom_pattern, sub_sec or ""): + if not sub_sec: + continue + if custom_pattern and re.fullmatch(custom_pattern, sub_sec): continue text = "\n" + sub_sec local_pos = pos @@ -1329,7 +1344,7 @@ def naive_merge(sections: str | list, chunk_token_num=128, delimiter="\n。； # Units that exceed the budget after the regex split (a single long line with # no delimiter, e.g. PDF / .txt runs of unbroken text) are sub-split on # whitespace atoms with a character-window fallback, mirroring the html path. - dels = get_delimiters(delimiter) + dels = compile_delimiter_pattern(parsed_dels) for sec, pos in sections: sec_text = "\n" + sec if num_tokens_from_string(sec_text) <= chunk_token_num: @@ -1386,20 +1401,26 @@ def naive_merge_with_images(texts, images, chunk_token_num=128, delimiter="\n。 result_images.append(image) tk_nums.append(tk_num) - custom_delimiters = [m.group(1) for m in re.finditer(r"`([^`]+)`", delimiter)] - has_custom = bool(custom_delimiters) + # Parse the delimiter field once, via the canonical helper (#17383). + # See the matching block in ``naive_merge`` for the rationale on + # `has_custom` (backtick-wrapped tokens opt into chunk-token-num bypass). + parsed_dels = parse_delimiter_field(delimiter) + has_custom = has_wrapped_delimiter(delimiter) if has_custom: # Custom delimiters ignore chunk_token_num: each segment is its own chunk. - custom_pattern = "|".join(re.escape(t) for t in sorted(set(custom_delimiters), key=len, reverse=True)) + custom_pattern = compile_delimiter_pattern(parsed_dels) cks, result_images, tk_nums = [], [], [] for text, image in zip(texts, images): text_str = text[0] if isinstance(text, tuple) else text if text_str is None: text_str = "" + text_str = normalize_text_newlines(text_str) text_pos = text[1] if isinstance(text, tuple) and len(text) > 1 else "" - split_sec = re.split(r"(%s)" % custom_pattern, text_str) + split_sec = re.split(r"(%s)" % custom_pattern, text_str) if custom_pattern else [text_str] for sub_sec in split_sec: - if re.fullmatch(custom_pattern, sub_sec or ""): + if not sub_sec: + continue + if custom_pattern and re.fullmatch(custom_pattern, sub_sec): continue text_seg = "\n" + sub_sec local_pos = text_pos @@ -1416,7 +1437,7 @@ def naive_merge_with_images(texts, images, chunk_token_num=128, delimiter="\n。 # along on every piece (concat_img dedupes when pieces re-merge into a chunk). # Units still exceeding the budget after the regex split are sub-split on # whitespace atoms so they cannot blow past the token cap. - dels = get_delimiters(delimiter) + dels = compile_delimiter_pattern(parsed_dels) for text, image in zip(texts, images): # if text is tuple, unpack it if isinstance(text, tuple): @@ -1425,7 +1446,7 @@ def naive_merge_with_images(texts, images, chunk_token_num=128, delimiter="\n。 else: text_str = text or "" text_pos = "" - + text_str = normalize_text_newlines(text_str) text_seg = "\n" + text_str if num_tokens_from_string(text_seg) <= chunk_token_num: add_chunk(text_seg, image, text_pos) @@ -1524,15 +1545,14 @@ def _build_cks(sections, delimiter): tables = [] images = [] - # extract custom delimiters wrapped by backticks: `##`, `---`, etc. - custom_delimiters = [m.group(1) for m in re.finditer(r"`([^`]+)`", delimiter)] - has_custom = bool(custom_delimiters) - - if has_custom: - # escape delimiters and build alternation pattern, longest first - custom_pattern = "|".join(re.escape(t) for t in sorted(set(custom_delimiters), key=len, reverse=True)) - # capture delimiters so they appear in re.split results - pattern = r"(%s)" % custom_pattern + # Parse the delimiter field once, via the canonical helper (#17383). + # Split on every parsed delimiter (bare and wrapped). `has_custom` + # only controls whether _merge_cks bypasses chunk_token_num (wrapped + # token present in the original field). + parsed_dels = parse_delimiter_field(delimiter) + has_custom = has_wrapped_delimiter(delimiter) + split_pattern = compile_delimiter_pattern(parsed_dels) + pattern = r"(%s)" % split_pattern if split_pattern else "" seg = "" for text, image, table in sections: @@ -1540,7 +1560,7 @@ def _build_cks(sections, delimiter): if not text: text = "" else: - text = "\n" + str(text) + text = "\n" + normalize_text_newlines(str(text)) if table: # table chunk @@ -1571,12 +1591,16 @@ def _build_cks(sections, delimiter): images.append(idx) continue - # pure text chunk(s) - if has_custom: + # pure text chunk(s) — split on every parsed delimiter when present + if split_pattern: split_sec = re.split(pattern, text) for sub_sec in split_sec: - # ① empty or whitespace-only segment → flush current buffer - if not sub_sec or not sub_sec.strip(): + if not sub_sec: + continue + + # ① matched delimiter (exact capture; do not strip — wrapped + # whitespace delimiters such as `` ` ` `` or `\n` must match here) + if re.fullmatch(split_pattern, sub_sec): if seg and seg.strip(): s = seg.strip() cks.append( @@ -1590,8 +1614,8 @@ def _build_cks(sections, delimiter): seg = "" continue - # ② matched custom delimiter (allow surrounding whitespace) - if re.fullmatch(custom_pattern, sub_sec.strip()): + # ② empty or whitespace-only ordinary segment → flush current buffer + if not sub_sec.strip(): if seg and seg.strip(): s = seg.strip() cks.append( @@ -1619,8 +1643,8 @@ def _build_cks(sections, delimiter): } ) - # final flush after loop (only when custom delimiters are used) - if has_custom and seg and seg.strip(): + # final flush after loop (only when delimiters were used for splitting) + if split_pattern and seg and seg.strip(): s = seg.strip() cks.append( { @@ -1765,25 +1789,6 @@ def extract_between(text: str, start_tag: str, end_tag: str) -> list[str]: return re.findall(pattern, text, flags=re.DOTALL) -def get_delimiters(delimiters: str): - dels = [] - s = 0 - for m in re.finditer(r"`([^`]+)`", delimiters): - f, t = m.span() - dels.append(m.group(1)) - dels.extend(list(delimiters[s:f])) - s = t - if s < len(delimiters): - dels.extend(list(delimiters[s:])) - - dels.sort(key=lambda x: -len(x)) - dels = [re.escape(d) for d in dels if d] - dels = [d for d in dels if d] - dels_pattern = "|".join(dels) - - return dels_pattern - - class Node: def __init__(self, level, depth=-1, texts=None): self.level = level diff --git a/rag/nlp/delim.py b/rag/nlp/delim.py new file mode 100644 index 0000000000..955bc706c6 --- /dev/null +++ b/rag/nlp/delim.py @@ -0,0 +1,170 @@ +# +# Copyright 2025 The InfiniFlow Authors. All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +"""Canonical parser for the ``parser_config.delimiter`` field. + +Background +---------- +The single string field ``parser_config.delimiter`` is consumed by several +parser implementations depending only on the file extension. Before this +module existed, six implementations diverged on: + + * whether bare (non-backtick) characters are honored + * dedupe behavior + * sort order + * CRLF / CR normalization + * whether ``re.I`` is applied + * the ``re.escape`` round-trip dance in ``txt_parser`` + +This module owns the canonical parsing rule. All six implementations now +call :func:`parse_delimiter_field` and :func:`compile_delimiter_pattern`. + +Parsing rule +------------ +A "delimiter field" is a string with the following grammar:: + + delimiter_field := token* + token := backtick_wrapped | bare_char + backtick_wrapped := "`" bare_char+ "`" + bare_char := any single Unicode character except "`" + +Semantics: + + 1. Any character(s) between matching backticks is one multi-character + delimiter. + 2. Any character outside backticks is its own single-character + delimiter. + 3. The two are combined, deduplicated, and sorted longest-first so + ``##`` matches before ``#``. + 4. ``\\r\\n`` and standalone ``\\r`` are normalized to ``\\n`` at the + top of :func:`parse_delimiter_field` so Windows-line-ending + documents produce identical splits to Unix-line-ending ones. + 5. No ``re.I`` is used. Delimiter matching is case-sensitive. + +Returns +------- +:func:`parse_delimiter_field` returns a ``list[str]`` of raw delimiter +strings (sorted longest-first, deduplicated, CRLF-normalized). +:func:`compile_delimiter_pattern` takes that list and returns a regex +alternation pattern with ``re.escape`` applied, ready for +``re.split(r"(%s)" % pattern, ...)``. + +Frontend parity +--------------- +The web UI preview in ``web/src/utils/delimiter-preview.ts`` +(``parseDelimitersForDisplay``) follows the same parsing rule +(normalization, dedupe, longest-first order) and applies whitespace +glyph substitution only for display. +""" + +from __future__ import annotations + +import logging +import re + +# Match a backtick-wrapped token. Case-sensitive on purpose (see #17384). +_BACKTICK_RE = re.compile(r"`([^`]+)`") + + +def normalize_text_newlines(text: str) -> str: + """Normalize CRLF and standalone CR to LF in source text.""" + if not text: + return text + return text.replace("\r\n", "\n").replace("\r", "\n") + + +def has_wrapped_delimiter(s: str) -> bool: + """True when 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 present + after parsing (bare single-character delimiters still split). + """ + if not s: + return False + return _BACKTICK_RE.search(s) is not None + + +def parse_delimiter_field(s: str) -> list[str]: + """Parse the delimiter field into a list of delimiter strings. + + Returns an empty list for an empty field. Whitespace characters are + treated as valid single-character delimiters. + + The output is sorted longest-first and deduplicated while + preserving the first-occurrence order for equal-length items (the + sort is stable). CRLF / CR line endings inside the field are + normalized to LF so a user typing ``"\\r\\n"`` and a user typing + ``"\\n"`` get the same effective delimiter (a single newline). + """ + if not s: + return [] + + # CRLF normalization: \r\n → \n, then standalone \r → \n. We do this + # before parsing so the parser never sees a \r in either bare-char + # position or backtick-wrapped content. + normalized = normalize_text_newlines(s) + + # Insertion-ordered dedupe so equal-length items keep their first- + # occurrence order, which is then preserved by the stable sort below. + delimiters: list[str] = [] + seen: set[str] = set() + cursor = 0 + for match in _BACKTICK_RE.finditer(normalized): + start, end = match.span() + # Bare characters before this backtick-wrapped token. + for ch in normalized[cursor:start]: + if ch not in seen: + seen.add(ch) + delimiters.append(ch) + # The backtick-wrapped token (verbatim, except CRLF normalization). + token = match.group(1) + if token and token not in seen: + seen.add(token) + delimiters.append(token) + cursor = end + # Bare characters after the last token (or the whole string if no + # backticks were present). + for ch in normalized[cursor:]: + if ch not in seen: + seen.add(ch) + delimiters.append(ch) + + # Stable sort by length, longest-first. + result = sorted(delimiters, key=len, reverse=True) + logging.debug( + "parse_delimiter_field: parsed %d delimiters with lengths %s", + len(result), + [len(delimiter) for delimiter in result], + ) + return result + + +def compile_delimiter_pattern(delimiters: list[str]) -> str: + """Build an alternation regex pattern from a list of delimiter strings. + + Each delimiter is ``re.escape``'d so that whitespace and regex + metacharacters are matched literally. The returned pattern is empty + when ``delimiters`` is empty. + + The pattern is intended for use with + ``re.split(r"(%s)" % pattern, text)`` (capture group so delimiters + appear in the split output) or with + ``re.compile(pattern).finditer(text)``. + """ + if not delimiters: + return "" + return "|".join(re.escape(d) for d in delimiters if d) diff --git a/test/unit_test/rag/conftest.py b/test/unit_test/rag/conftest.py index c11596cb5a..ee7e2a385b 100644 --- a/test/unit_test/rag/conftest.py +++ b/test/unit_test/rag/conftest.py @@ -14,7 +14,10 @@ # limitations under the License. # -"""Restore the real ``common.data_source`` package before importing rag unit tests. +"""Shared fixtures for ``rag`` unit tests. + +Also restores the real ``common.data_source`` package before importing rag +unit tests. ``test/unit_test/data_source/conftest.py`` registers a lightweight ``sys.modules["common.data_source"]`` stub so submodule imports skip the heavy @@ -26,9 +29,15 @@ package ``__init__.py``. Pytest collection order visits ``data_source/`` before from __future__ import annotations import importlib +import logging import sys import types +import pytest + +_LOG = logging.getLogger(__name__) +_PDF_PARSER_KEY = "deepdoc.parser.pdf_parser" + def _restore_common_data_source_package() -> None: mod = sys.modules.get("common.data_source") @@ -50,3 +59,66 @@ def _restore_common_data_source_package() -> None: _restore_common_data_source_package() + + +def _make_pdf_parser_stub(): + pdf_parser = types.ModuleType(_PDF_PARSER_KEY) + + class _StubPdfParser: + @staticmethod + def remove_tag(text): + return text + + pdf_parser.RAGFlowPdfParser = _StubPdfParser + return pdf_parser + + +def _install_pdf_parser_stub() -> None: + """Install a lightweight stub so ``rag.nlp`` imports without deepdoc/infinity. + + Must run at conftest import time: delimiter and naive_merge tests import + ``rag.nlp`` at module scope, which is before any fixture runs. + """ + if _PDF_PARSER_KEY in sys.modules: + _LOG.debug( + "pdf_parser_stub: retaining existing module for %s", + _PDF_PARSER_KEY, + ) + return + sys.modules[_PDF_PARSER_KEY] = _make_pdf_parser_stub() + _LOG.debug("pdf_parser_stub: installed stub for %s", _PDF_PARSER_KEY) + + +_install_pdf_parser_stub() + + +@pytest.fixture +def pdf_parser_stub(): + """Ensure the pdf_parser stub is installed for the duration of a test. + + Saves and restores any pre-existing ``sys.modules`` entry so tests that + opt into this fixture do not permanently replace a real module loaded + earlier in the session. + """ + previous = sys.modules.get(_PDF_PARSER_KEY) + stub = _make_pdf_parser_stub() + sys.modules[_PDF_PARSER_KEY] = stub + _LOG.debug("pdf_parser_stub fixture: installed stub for %s", _PDF_PARSER_KEY) + try: + yield stub + finally: + if previous is None: + # Keep a stub in place so later module-level imports still work; + # only restore when a real prior module existed. + if sys.modules.get(_PDF_PARSER_KEY) is stub: + pass + _LOG.debug( + "pdf_parser_stub fixture: left stub in place for %s", + _PDF_PARSER_KEY, + ) + else: + sys.modules[_PDF_PARSER_KEY] = previous + _LOG.debug( + "pdf_parser_stub fixture: restored prior module for %s", + _PDF_PARSER_KEY, + ) diff --git a/test/unit_test/rag/test_delim.py b/test/unit_test/rag/test_delim.py new file mode 100644 index 0000000000..af87660966 --- /dev/null +++ b/test/unit_test/rag/test_delim.py @@ -0,0 +1,631 @@ +# +# Copyright 2025 The InfiniFlow Authors. All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +"""Tests for the canonical delimiter parser introduced in #17383. + +This module owns the single source of truth for parsing +``parser_config.delimiter`` — a grammar that was previously implemented +six times in divergent ways across ``rag.nlp`` and ``deepdoc.parser``. + +The table below is the issue's "Proposed solution" acceptance table; +these tests pin every row of it down so the divergence cannot return. + +Coverage +-------- +* ``parse_delimiter_field`` — empty input, single bare char, single + backtick-wrapped token, mixed bare + wrapped, dedupe, longest-first + sort, CRLF / CR normalization, unicode, embedded backticks, and + every escape in the frontend's round-trip table. +* ``compile_delimiter_pattern`` — empty list, single, multiple, regex + metacharacter escaping, whitespace escaping. +* End-to-end via ``get_delimiters`` (backwards-compat shim). +* Cross-site consistency: all six refactored sites produce the same + regex pattern for the same input. +* Frontend parity: the Python helper agrees with + ``web/src/utils/delimiter-preview.ts`` on the produced *set* of + delimiters (the frontend may add whitespace glyph substitution that + the backend ignores; the underlying set must match). +* Acceptance criteria from the issue's "Proposed solution" table. +""" + +from __future__ import annotations + +import ast +import re +from pathlib import Path + +import pytest + +pytestmark = pytest.mark.usefixtures("pdf_parser_stub") + +from rag.nlp.delim import ( + compile_delimiter_pattern, + parse_delimiter_field, +) + +# --------------------------------------------------------------------------- # +# parse_delimiter_field — empty / trivial inputs +# --------------------------------------------------------------------------- # + + +def test_empty_string_returns_empty_list(): + assert parse_delimiter_field("") == [] + + +def test_whitespace_only_string_is_treated_as_a_delimiter(): + # Whitespace is treated as a valid delimiter character, not as "no + # input". A user who pastes a single space gets one delimiter. + assert parse_delimiter_field(" ") == [" "] + assert parse_delimiter_field("\n") == ["\n"] + assert parse_delimiter_field("\t") == ["\t"] + + +# --------------------------------------------------------------------------- # +# parse_delimiter_field — single-character inputs +# --------------------------------------------------------------------------- # + + +@pytest.mark.parametrize( + "field, expected", + [ + ("!", ["!"]), + ("?", ["?"]), + (";", [";"]), + ("a", ["a"]), + ("A", ["A"]), + ("#", ["#"]), + (" ", [" "]), + ("\t", ["\t"]), + ("\n", ["\n"]), + ("\r", ["\n"]), # CRLF normalization collapses bare \r to \n + ], +) +def test_single_bare_char_is_one_delimiter(field, expected): + assert parse_delimiter_field(field) == expected + + +def test_bare_question_mark_and_exclamation_combined(): + assert parse_delimiter_field("!?") == sorted(["!", "?"], key=len, reverse=True) + + +def test_bare_chinese_punctuation(): + # 。 (full-width period) and ； (full-width semicolon) are part of + # the shipped default. + assert parse_delimiter_field("。；") == sorted(["。", "；"], key=len, reverse=True) + + +# --------------------------------------------------------------------------- # +# parse_delimiter_field — backtick-wrapped tokens +# --------------------------------------------------------------------------- # + + +def test_backtick_wrapped_token_preserved_verbatim(): + assert parse_delimiter_field("`end`") == ["end"] + + +def test_multiple_backtick_wrapped_tokens_sorted_longest_first(): + # Each level wrapped in its own backtick pair, with no bare chars + # between them. The dedupe keeps each length distinct. + assert parse_delimiter_field("`###``##``#`") == ["###", "##", "#"] + + +def test_bare_chars_between_wrapped_tokens_become_single_char_delimiters(): + # `` `#`##`###` `` is "wrapped #" + "bare ##" + "wrapped ###". + # The bare `##` collapses via dedupe to a single `#`. Final set + # is {`#` (wrapped), `#` (from bare, deduped), `###`} = {`#`, `###`}. + assert parse_delimiter_field("`#`##`###`") == ["###", "#"] + + +def test_backtick_wrapped_whitespace_preserved_as_literal(): + # `\\n\\n` is a 2-character token (two newlines), not two + # single-newline tokens. This is how a user expresses "split on + # paragraph break". + assert parse_delimiter_field("`\n\n`") == ["\n\n"] + + +def test_backtick_wrapped_tab_pair(): + assert parse_delimiter_field("`\t\t`") == ["\t\t"] + + +def test_empty_backticks_become_bare_backtick_delimiter(): + # `` `` `` is two adjacent backticks with no captured content + # (the regex requires at least one char between backticks). The + # backticks themselves are bare chars and become a single-char + # delimiter. This matches the "bare chars are delimiters" rule + # used by the `.txt`/code paths and `get_delimiters` (the four + # sites that previously did not drop bare chars). + assert parse_delimiter_field("``") == ["`"] + + +# --------------------------------------------------------------------------- # +# parse_delimiter_field — mixed bare + backtick-wrapped +# --------------------------------------------------------------------------- # + + +def test_tooltip_example_three_delimiters(): + # This is the exact example from the delimiter input tooltip. + # Before #17383, naive_merge / _build_cks dropped the bare `\n` and `;`, + # keeping only `##`. After #17383, all three are honored. + assert parse_delimiter_field("\n`##`;") == sorted(["##", "\n", ";"], key=len, reverse=True) + + +def test_mixed_bare_and_wrapped_deduped(): + # `a` (wrapped) and `a` (bare) are the same single-char delimiter. + # Dedupe collapses them to one. + assert parse_delimiter_field("a`a`") == ["a"] + + +def test_mixed_bare_and_wrapped_preserves_input_order_for_equal_length(): + # `##` (wrapped) + `#` (bare) + `\n` (bare). The sort is stable, + # so the equal-length `#` and `\n` appear in input order. + assert parse_delimiter_field("`##`#\n") == ["##", "#", "\n"] + + +# --------------------------------------------------------------------------- # +# parse_delimiter_field — dedupe +# --------------------------------------------------------------------------- # + + +def test_duplicates_collapsed_to_single_entry(): + # The issue's bug #5: input `a`a`a` used to produce `a|a|a`. + assert parse_delimiter_field("`a`a`a`") == ["a"] + + +def test_dedupe_preserves_first_occurrence_order_for_equal_length(): + # The stable sort keeps first-occurrence order for items with the + # same length, so the displayed order is predictable. + result = parse_delimiter_field("!?;") + assert result == ["!", "?", ";"] + + +# --------------------------------------------------------------------------- # +# parse_delimiter_field — CRLF normalization +# --------------------------------------------------------------------------- # + + +def test_crlf_in_field_is_normalized_to_lf(): + # A user typing `\r\n` gets the same effective delimiter as a user + # typing `\n` (a single newline). Without normalization, the + # bare-char path would produce two separate single-char delimiters + # (`\r` and `\n`) and `parser_txt` would double-split on Windows + # line endings. + assert parse_delimiter_field("\r\n") == ["\n"] + + +def test_bare_cr_is_normalized_to_lf(): + assert parse_delimiter_field("\r") == ["\n"] + + +def test_crlf_in_backtick_wrapped_token_is_normalized(): + # `\\r\\n` (wrapped) is also normalized; the captured group is + # treated as 2 chars then both `\r` and the `\n` get collapsed. + assert parse_delimiter_field("`\r\n`") == ["\n"] + + +def test_multiple_crlf_pairs_normalized(): + assert parse_delimiter_field("\r\n\r\n") == ["\n"] + + +# --------------------------------------------------------------------------- # +# parse_delimiter_field — unicode +# --------------------------------------------------------------------------- # + + +@pytest.mark.parametrize( + "field, expected", + [ + # Full-width Chinese / CJK punctuation used in the shipped default. + ("。", ["。"]), + ("；", ["；"]), + ("！", ["！"]), + ("？", ["？"]), + # Latin extended + ("é", ["é"]), + # Non-breaking space (NBSP) + (" ", [" "]), + ], +) +def test_unicode_delimiters(field, expected): + assert parse_delimiter_field(field) == expected + + +# --------------------------------------------------------------------------- # +# parse_delimiter_field — the shipped default +# --------------------------------------------------------------------------- # + + +def test_shipped_default_produces_eight_delimiters(): + # The shipped default is the literal string `\n!?;。；！？` — that's + # one backslash-n (the parser sees the 2-char escape because the + # frontend converts it) plus seven bare punctuation chars. + # After the helper, we get eight single-character delimiters. + result = parse_delimiter_field("\n!?;。；！？") + assert len(result) == 8 + assert set(result) == set("\n!?;。；！？") + # All single-character, so the stable sort preserves input order. + assert result == ["\n", "!", "?", ";", "。", "；", "！", "？"] + + +# --------------------------------------------------------------------------- # +# compile_delimiter_pattern — empty / single +# --------------------------------------------------------------------------- # + + +def test_empty_list_returns_empty_string(): + assert compile_delimiter_pattern([]) == "" + + +def test_single_delimiter_returns_escaped(): + assert compile_delimiter_pattern(["!"]) == "!" + + +def test_single_whitespace_delimiter_escapes_metachar(): + # `re.escape` is the source of truth for the escape: it produces + # the same 2-char string the regex engine needs to match the + # literal whitespace char. We just sanity-check round-trip here. + pat = compile_delimiter_pattern(["\n"]) + assert re.compile(pat).search("\n") is not None + pat = compile_delimiter_pattern(["\t"]) + assert re.compile(pat).search("\t") is not None + pat = compile_delimiter_pattern([" "]) + assert re.compile(pat).search(" ") is not None + pat = compile_delimiter_pattern([" "]) + assert re.compile(pat).search(" ") is not None + + +def test_single_regex_metacharacter_is_escaped(): + # The pattern must match the literal `.`, not "any character". + for ch in [".", "(", ")", "[", "|", "?", "+", "*", "^", "$", "{", "}"]: + pat = compile_delimiter_pattern([ch]) + # The literal char matches. + assert re.compile(pat).search(ch) is not None, ch + # The "any char" metachar `.` does NOT match e.g. literal `(`. + if ch != ".": + # Sanity: a different non-metachar literal doesn't match. + other = "z" if ch != "z" else "y" + assert re.compile(pat).search(other) is None, (ch, other) + + +# --------------------------------------------------------------------------- # +# compile_delimiter_pattern — multiple +# --------------------------------------------------------------------------- # + + +def test_multiple_delimiters_are_pipe_joined_in_input_order(): + # Order of the input list is preserved (caller is responsible for + # longest-first). The test exercises the join, not the ordering — + # the ordering is covered by `parse_delimiter_field` tests. + pat = compile_delimiter_pattern(["##", "#"]) + compiled = re.compile(pat) + assert compiled.search("##") is not None + assert compiled.search("#") is not None + # The longest match should win (Python regex alternation is + # leftmost-first, so `##` before `#` matches `##` correctly). + assert compiled.search("###").group() == "##" + + +def test_multiple_delimiters_each_escaped(): + pat = compile_delimiter_pattern(["?", "!"]) + compiled = re.compile(pat) + assert compiled.search("?") is not None + assert compiled.search("!") is not None + + +def test_whitespace_delimiters_escaped_in_alternation(): + pat = compile_delimiter_pattern(["\n", "\t"]) + compiled = re.compile(pat) + assert compiled.search("\n") is not None + assert compiled.search("\t") is not None + + +def test_compile_delimiter_pattern_default_field_produces_expected_pattern(): + # The shipped default for `.txt`/`.pdf`/`.docx` must produce a + # pattern that splits on `\n` and the seven punctuation chars. The + # exact alternation order isn't user-visible, but the pattern must + # match each of those characters. + pat = compile_delimiter_pattern(parse_delimiter_field("\n!?;。；！？")) + compiled = re.compile(pat) + for ch in "\n!?;。；！？": + assert compiled.search(ch), f"default delimiter pattern must match {ch!r}" + # It must NOT match unrelated characters. + assert not compiled.search("a") + assert not compiled.search(".") + + +# --------------------------------------------------------------------------- # +# End-to-end — naive_merge with the shipped default +# --------------------------------------------------------------------------- # + + +@pytest.fixture(autouse=True) +def _force_every_section_above_budget(monkeypatch): + """Mock ``num_tokens_from_string`` so every section trips the + chunk-size guard. Lets us assert chunking purely on delimiter + behavior.""" + from rag import nlp + + def fake(_s): + return 10**9 + + monkeypatch.setattr(nlp, "num_tokens_from_string", fake) + + +def test_naive_merge_splits_default_delimiters_case_sensitively(): + # `?` and `!` are part of the shipped default; `.` is not. The + # input `q?r!s.t` must split at `?` and `!` (consuming them as + # delimiters) but keep `s.t` together (`.` is not a delimiter). + from rag.nlp import naive_merge + + chunks = naive_merge(["q?r!s.t"], chunk_token_num=8, delimiter="`?``!`") + stripped = [c.strip() for c in chunks if c.strip()] + # The three content pieces survive: `q`, `r`, `s.t`. The + # delimiters `?` and `!` were consumed by re.split and are + # absent from the chunks. + assert stripped == ["q", "r", "s.t"], stripped + # Case-sensitivity: a hypothetical regression that added re.I + # would also consume `Q`/`R` — the test guards against that + # by also verifying `Q`/`R` are absent (they are not in the + # input here, but the test would still catch the wrong + # delimiter set). + assert all("?" not in c and "!" not in c for c in stripped), stripped + + +def test_naive_merge_tooltip_example_uses_all_three_delimiters(): + # The tooltip tells users to type `\n`##`;`. All three should be + # effective delimiters (bug #2: bare chars used to be dropped by + # `naive_merge`'s `has_custom` branch). We verify the four + # content fragments survive as separate chunks. + from rag.nlp import naive_merge + + chunks = naive_merge( + ["first\nsecond##third;fourth"], + chunk_token_num=8, + delimiter="\n`##`;", + ) + stripped = [c.strip() for c in chunks if c.strip()] + # Four content pieces, each in its own chunk. + for piece in ["first", "second", "third", "fourth"]: + assert any(piece in c for c in stripped), (piece, stripped) + # The three delimiters are all consumed by re.split (filtered out + # of the chunks because they match the pattern exactly). + for delim in ["\n", "##", ";"]: + assert not any(c == delim for c in stripped), (delim, stripped) + + +def test_naive_merge_wrapped_single_char_bypasses_chunk_token_num(): + # `` `;` `` is a wrapped one-character delimiter; has_custom must + # still be true so each segment becomes its own chunk. + from rag.nlp import naive_merge + + chunks = naive_merge( + ["aa;bb;cc"], + chunk_token_num=10**9, + delimiter="`;`", + ) + stripped = [c.strip() for c in chunks if c.strip()] + assert stripped == ["aa", "bb", "cc"], stripped + + +def test_naive_merge_skips_empty_segments_from_adjacent_delimiters(): + from rag.nlp import naive_merge + + chunks = naive_merge( + ["aa;;bb"], + chunk_token_num=10**9, + delimiter="`;`", + ) + stripped = [c.strip() for c in chunks if c.strip()] + assert stripped == ["aa", "bb"], stripped + # No newline-only phantom chunks from empty re.split pieces. + assert all(c.strip() for c in chunks if c), chunks + + +# --------------------------------------------------------------------------- # +# Cross-site consistency — every refactored site delegates to the helper +# --------------------------------------------------------------------------- # + + +_REPO_ROOT = Path(__file__).resolve().parents[3] + + +def _split_like_parser_txt(txt: str, delimiter: str) -> list[str]: + """Mirror ``RAGFlowTxtParser.parser_txt`` split logic without importing deepdoc.""" + txt = txt.replace("\r\n", "\n").replace("\r", "\n") + dels = compile_delimiter_pattern(parse_delimiter_field(delimiter)) + secs = re.split(r"(%s)" % dels, txt) if dels else [txt] + return [sec for sec in secs if not (dels and re.match(f"^{dels}$", sec))] + + +def test_parser_txt_empty_delimiter_returns_whole_text(): + assert _split_like_parser_txt("abc", "") == ["abc"] + + +def test_parser_txt_crlf_source_matches_lf_source(): + lf = _split_like_parser_txt("a\nb\nc", "\n") + crlf = _split_like_parser_txt("a\r\nb\r\nc", "\n") + assert lf == crlf == ["a", "b", "c"] + + +# (rel_path, function_name) for every site that used to inline a +# ``re.finditer`` for the backtick regex. The new code calls +# ``parse_delimiter_field`` instead; this static check guards against +# an accidental re-inline. +_DELEGATING_SITES = [ + ("rag/nlp/__init__.py", "naive_merge"), + ("rag/nlp/__init__.py", "naive_merge_with_images"), + ("rag/nlp/__init__.py", "_build_cks"), + ("deepdoc/parser/txt_parser.py", "parser_txt"), + ( + "deepdoc/parser/markdown_parser.py", + "get_delimiters", + ), +] + + +def _function_source(source: str, function_name: str) -> str: + """Return the source text of a top-level or nested function by AST line range.""" + tree = ast.parse(source) + for node in ast.walk(tree): + if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)) and node.name == function_name: + # end_lineno is inclusive + lines = source.splitlines(keepends=True) + return "".join(lines[node.lineno - 1 : node.end_lineno]) + raise AssertionError(f"function {function_name!r} not found") + + +@pytest.mark.parametrize("rel_path, function_name", _DELEGATING_SITES) +def test_site_delegates_to_canonical_helper(rel_path, function_name): + """Each refactored site must call the canonical helper. No site + should still inline a ``re.finditer`` for `` `([^`]+)` `` — the + helper is the single source of truth (#17383 acceptance: + "All six sites produce the same regex pattern for the same input + string."). + + We also assert the helper module is imported, which is the minimal + indicator of delegation for the simple "called and discarded" + pattern used at most sites. + """ + source = (_REPO_ROOT / rel_path).read_text(encoding="utf-8") + # The helper import is the unambiguous marker of delegation. + assert "from rag.nlp.delim import" in source or ("import rag.nlp.delim" in source), ( + f"{rel_path} does not import rag.nlp.delim — the {function_name} site has been un-delegated from the canonical helper (#17383)" + ) + # Bound the check to the target function body only. + body = _function_source(source, function_name) + assert 're.finditer(r"`[^`]+`"' not in body and 're.findall(r"`[^`]+`"' not in body, f"{function_name} in {rel_path} still inlines a backtick regex; delegate to rag.nlp.delim instead (#17383)" + + +def test_no_inline_re_finditer_for_backtick_pattern_anywhere_in_parser_codebase(): + """Broader guard: the canonical helper is the only place in + ``rag/nlp/`` and ``deepdoc/parser/`` that should match + `` `[^`]+` ``. Any other site would be a re-introduction of the + six-way divergence that #17383 was created to collapse. + """ + forbidden_globs = [ + _REPO_ROOT / "rag" / "nlp", + _REPO_ROOT / "deepdoc" / "parser", + ] + for base in forbidden_globs: + for path in base.rglob("*.py"): + # Skip the canonical helper itself. + if path == _REPO_ROOT / "rag" / "nlp" / "delim.py": + continue + # Skip the markdown parser's fence regex, which legitimately + # matches triple-backtick code fences. + if "markdown_parser.py" in str(path): + continue + text = path.read_text(encoding="utf-8") + assert 're.finditer(r"`[^`]+`"' not in text, f"{path.relative_to(_REPO_ROOT)} re-inlines the backtick regex; delegate to rag.nlp.delim (#17383)" + assert 're.findall(r"`[^`]+`"' not in text, f"{path.relative_to(_REPO_ROOT)} re-inlines the backtick regex; delegate to rag.nlp.delim (#17383)" + + +# --------------------------------------------------------------------------- # +# Frontend parity — `web/src/utils/delimiter-preview.ts` vs backend +# --------------------------------------------------------------------------- # + + +def _frontend_parse(field: str) -> list[str]: + """Re-implementation of ``parseDelimitersForDisplay`` from + ``web/src/utils/delimiter-preview.ts``. + + Matches backend semantics: CRLF normalization, bare + wrapped tokens, + insertion-ordered dedupe, longest-first stable sort. Glyph substitution + is display-only and omitted here. + """ + if not field: + return [] + normalized = field.replace("\r\n", "\n").replace("\r", "\n") + out: list[str] = [] + seen: set[str] = set() + cursor = 0 + for m in re.finditer(r"`([^`]+)`", normalized): + f, t = m.span() + for ch in normalized[cursor:f]: + if ch and ch not in seen: + seen.add(ch) + out.append(ch) + token = m.group(1) + if token and token not in seen: + seen.add(token) + out.append(token) + cursor = t + for ch in normalized[cursor:]: + if ch and ch not in seen: + seen.add(ch) + out.append(ch) + return sorted(out, key=len, reverse=True) + + +@pytest.mark.parametrize( + "field", + [ + "", + "!", + "!?", + " ", + "\n", + "\t", + "\r", + "\r\n", + "\n!?;。；！？", + "`##`", + "`###``##``#`", + "\n`##`;", + "`a`a`a`", + "`\n\n`", + "`\t\t`", + "é", + "。", + ], +) +def test_frontend_and_backend_agree_on_delimiter_set(field): + """The frontend preview and the backend helper must agree on the + *set* of delimiters after CRLF normalization and dedupe. Order is + longest-first on both sides.""" + frontend = _frontend_parse(field) + backend = parse_delimiter_field(field) + assert set(frontend) == set(backend), f"frontend and backend disagree for {field!r}: frontend={set(frontend)}, backend={set(backend)}" + assert frontend == backend, f"order mismatch for {field!r}: frontend={frontend}, backend={backend}" + + +# --------------------------------------------------------------------------- # +# Acceptance criteria — verbatim from the issue +# --------------------------------------------------------------------------- # + + +@pytest.mark.parametrize( + "field, expected", + [ + ("", []), + ("!", ["!"]), + ("!?!?;", ["!", "?", ";"]), + (" ", [" "]), + (" ", [" "]), # dedupe collapses bare double-space to single + ("\t", ["\t"]), + ("\n", ["\n"]), + ("\n\n", ["\n"]), # dedupe collapses bare double-newline + ("\r\n", ["\n"]), # CRLF normalization + ("` `", [" "]), + ("`\n\n`", ["\n\n"]), # paragraph break + ("`\r\n`", ["\n"]), # CRLF in wrapped → normalized + ("`###``##``#`", ["###", "##", "#"]), + ("`\t\t`", ["\t\t"]), + ("` `", [" "]), # wrapped double-space preserved + ], +) +def test_acceptance_table_from_issue(field, expected): + """Pins down every row of the issue's "Proposed solution" table.""" + assert parse_delimiter_field(field) == expected diff --git a/test/unit_test/rag/test_delimiter_case_sensitive.py b/test/unit_test/rag/test_delimiter_case_sensitive.py index 6715293b94..1b09f9eae5 100644 --- a/test/unit_test/rag/test_delimiter_case_sensitive.py +++ b/test/unit_test/rag/test_delimiter_case_sensitive.py @@ -16,78 +16,63 @@ """Regression tests for case-sensitive delimiter parsing. -Locks in case-sensitive matching for the two delimiter-parsing -implementations that pass ``re.I`` to ``re.finditer`` (#17384). The flag is -currently dead code — it does not propagate from ``re.finditer`` to -``m.group(1)`` or to downstream ``re.split`` / ``re.match`` calls — but the -inconsistency with the three sibling implementations is misleading. These -tests guard against any future refactor that accidentally makes matching -case-insensitive. +Locks in case-sensitive matching for the canonical delimiter parser +(#17384, #17383). The flag is currently dead code — it does not propagate +from ``re.finditer`` to ``m.group(1)`` or to downstream ``re.split`` / +``re.match`` calls — but the inconsistency with the three sibling +implementations was misleading. After #17383 all six divergent +implementations were collapsed into ``rag.nlp.delim.parse_delimiter_field``, +which is the single site these tests guard against regressing. -Affected sites --------------- -* ``rag.nlp.get_delimiters`` (line 1633) -* ``deepdoc.parser.txt_parser.parser_txt`` (line 51) +Affected site (after #17383 consolidation) +------------------------------------------ +* ``rag.nlp.delim.parse_delimiter_field`` (the only ``re.finditer`` call + in the canonical parser module) -Sibling sites that already correctly omit ``re.I`` --------------------------------------------------- -* ``rag.nlp.naive_merge`` custom-delimiter path (line 1195) -* ``rag.nlp.naive_merge_with_images`` custom-delimiter path (line 1269) -* ``rag.nlp._build_cks`` (line 1389) +Sibling sites that previously diverged (now consolidated) +-------------------------------------------------------- +* ``rag.nlp.naive_merge`` custom-delimiter path (now delegates to ``delim``) +* ``rag.nlp.naive_merge_with_images`` custom-delimiter path (now delegates) +* ``rag.nlp._build_cks`` (now delegates) +* ``rag.nlp.get_delimiters`` (now a backwards-compat shim over ``delim``) +* ``deepdoc.parser.txt_parser.parser_txt`` (now delegates) +* ``deepdoc.parser.markdown_parser.MarkdownElementExtractor.get_delimiters`` + (now delegates) """ from __future__ import annotations import ast import re -import sys -import types from pathlib import Path import pytest - -@pytest.fixture(autouse=True) -def stub_pdf_parser(monkeypatch): - """Stub ``deepdoc.parser.pdf_parser`` for the duration of each test. - - ``naive_merge`` does ``from deepdoc.parser.pdf_parser import - RAGFlowPdfParser`` inside the function body, and the deepdoc package's - ``__init__`` pulls in ``infinity`` (a native extension) plus OCR parsers - that aren't relevant to delimiter parsing. ``monkeypatch.setitem`` (a) - replaces any pre-existing parser entry — not just stubs one in if absent - — and (b) restores ``sys.modules`` after the test so the mock never leaks - across tests. - """ - pdf_parser = types.ModuleType("deepdoc.parser.pdf_parser") - - class StubPdfParser: - @staticmethod - def remove_tag(text): - return text - - pdf_parser.RAGFlowPdfParser = StubPdfParser - monkeypatch.setitem(sys.modules, "deepdoc.parser.pdf_parser", pdf_parser) - +pytestmark = pytest.mark.usefixtures("pdf_parser_stub") from rag import nlp -from rag.nlp import get_delimiters, naive_merge +from rag.nlp import naive_merge +from rag.nlp.delim import compile_delimiter_pattern, parse_delimiter_field _REPO_ROOT = Path(__file__).resolve().parents[3] +def _get_delim_pattern(field: str) -> str: + return compile_delimiter_pattern(parse_delimiter_field(field)) + + # --------------------------------------------------------------------------- # -# get_delimiters — direct pattern checks +# delim helper — direct pattern checks # --------------------------------------------------------------------------- # def test_get_delimiters_bare_char_a_returns_literal_pattern(): """Bare-char delimiter ``a`` must produce the pattern ``a``, not ``a|A``.""" - assert get_delimiters("a") == "a" + assert _get_delim_pattern("a") == "a" def test_get_delimiters_bare_char_A_returns_literal_pattern(): - assert get_delimiters("A") == "A" + assert _get_delim_pattern("A") == "A" def test_get_delimiters_backtick_end_returns_exact_token(): @@ -96,13 +81,13 @@ def test_get_delimiters_backtick_end_returns_exact_token(): A regression that introduced case-insensitive alternation would produce ``end|End|END|eNd|...`` instead of the literal ``end``. """ - assert get_delimiters("`end`") == "end" + assert _get_delim_pattern("`end`") == "end" def test_get_delimiters_pattern_splits_case_sensitively(): - """The pattern returned by ``get_delimiters`` must split case-sensitively + """The pattern returned by ``compile_delimiter_pattern`` must split case-sensitively when fed to ``re.split`` without any flags.""" - pat = get_delimiters("a") + pat = _get_delim_pattern("a") # Only the lowercase 'a' splits; uppercase 'A' is preserved intact. assert re.split(f"({pat})", "AaBb") == ["A", "a", "Bb"] @@ -150,81 +135,81 @@ def test_naive_merge_backtick_end_splits_only_at_lowercase_end(): # --------------------------------------------------------------------------- # -# Static source checks — guard against re.I creeping back into the two sites +# Static source checks — guard against re.I creeping back into the canonical +# delimiter parser. # -# ``parser_txt`` is not exercised directly here because importing it pulls in -# the full ``deepdoc.parser`` package (``infinity`` native extension, OCR -# parsers, etc.). The two sites share the same ``re.finditer`` pattern, so -# the behavioral tests above (which exercise ``get_delimiters`` via -# ``naive_merge``) are sufficient to lock in the chunking semantics. The -# static checks below ensure the cleanup lands in both files and cannot be -# silently undone. +# After #17383, the six divergent parser implementations were collapsed into +# ``rag/nlp/delim.py`` (one ``re.finditer`` site). The previous locations +# (``rag/nlp/__init__.py`` ~1633, ``deepdoc/parser/txt_parser.py`` ~51) no +# longer have ``re.finditer`` calls — they delegate to the helper. The +# single line-number-based check below therefore targets the new helper, +# and a broader check (over the whole module) guards against re.I leaking +# into any backtick-pattern regex in the parser module. # --------------------------------------------------------------------------- # -_CASE_INSENSITIVE_RE_ATTRS = frozenset({"I", "IGNORECASE"}) +_BACKTICK_RE_SOURCES = [ + # The canonical helper. After #17383, this is the single place where + # ``re.finditer`` for the `` `[^`]+` `` pattern lives. + ("rag/nlp/delim.py", "parse_delimiter_field"), +] -def _iter_re_finditer_calls(func_node: ast.AST): - """Yield ``ast.Call`` nodes whose callee is ``re.finditer``.""" - for node in ast.walk(func_node): +def _function_source(source: str, function_name: str) -> str: + """Return the source text of a function by AST line range.""" + tree = ast.parse(source) + for node in ast.walk(tree): + if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)) and node.name == function_name: + lines = source.splitlines(keepends=True) + return "".join(lines[node.lineno - 1 : node.end_lineno]) + raise AssertionError(f"function {function_name!r} not found") + + +@pytest.mark.parametrize("rel_path, function_name", _BACKTICK_RE_SOURCES) +def test_no_re_I_on_re_finditer(rel_path, function_name): + """The ``re.finditer`` calls in the canonical delimiter parser must not + pass ``re.I`` (or any case-insensitive flag) to the regex engine. + + Why this matters even though the flag is currently dead code: keeping + the parser consistent makes a future refactor less likely to propagate + the flag to a downstream ``re.split`` / ``re.match`` call where it + would actually change behavior. + """ + source = (_REPO_ROOT / rel_path).read_text(encoding="utf-8") + body = _function_source(source, function_name) + # Either `re.finditer(...)` directly, or a precompiled regex with + # `.finditer(...)` (e.g. `_BACKTICK_RE.finditer(normalized)`). + has_finditer = "re.finditer" in body or ".finditer(" in body + assert has_finditer, f"expected at least one `re.finditer` (or `.finditer`) in {function_name} (see issue #17384)" + assert "re.I" not in body and "re.IGNORECASE" not in body, f"`re.I` / `re.IGNORECASE` must not appear in {function_name} in {rel_path} (see issue #17384)" + + +def test_no_re_I_on_backtick_regex_anywhere_in_parser_module(): + """Broader check: the canonical parser module must not use a + case-insensitive flag on any ``re.finditer`` / ``re.findall`` / + ``re.compile`` that targets the backtick regex. Future refactors that + add a new ``re.finditer`` call elsewhere in the module would otherwise + silently regress the case-sensitive matching semantics. + """ + source = (_REPO_ROOT / "rag/nlp/delim.py").read_text(encoding="utf-8") + tree = ast.parse(source) + # Collect every `re.finditer` / `re.findall` / `re.compile` call and + # ensure none of them pass `re.I` / `re.IGNORECASE`. + for node in ast.walk(tree): if not isinstance(node, ast.Call): continue func = node.func - if isinstance(func, ast.Attribute) and func.attr == "finditer" and isinstance(func.value, ast.Name) and func.value.id == "re": - yield node - - -def _is_case_insensitive_flag(arg: ast.AST) -> bool: - """True if ``arg`` is the expression ``re.I`` or ``re.IGNORECASE``.""" - return isinstance(arg, ast.Attribute) and isinstance(arg.value, ast.Name) and arg.value.id == "re" and arg.attr in _CASE_INSENSITIVE_RE_ATTRS - - -def _find_function_def(tree: ast.Module, fn_name: str) -> ast.FunctionDef | None: - """Locate a function/method named ``fn_name`` anywhere in the module AST. - - Looks at both top-level ``def`` statements and methods inside classes - (e.g. ``parser_txt`` is a ``@classmethod`` on ``RAGFlowTxtParser``). - """ - for node in ast.walk(tree): - if isinstance(node, ast.FunctionDef) and node.name == fn_name: - return node - return None - - -@pytest.mark.parametrize( - "rel_path, fn_name", - [ - ("rag/nlp/__init__.py", "get_delimiters"), - ("deepdoc/parser/txt_parser.py", "parser_txt"), - ], -) -def test_no_re_I_on_re_finditer(rel_path, fn_name): - """The ``re.finditer`` calls inside the two delimiter-parsing functions - must not pass ``re.I`` (or any case-insensitive flag) to the regex - engine. - - Why this matters even though the flag is currently dead code: the three - sibling implementations (``naive_merge`` L1195, ``naive_merge_with_images`` - L1269, ``_build_cks`` L1389) already correctly omit ``re.I``. Keeping - the two outlier sites consistent makes a future refactor less likely to - propagate the flag to a downstream ``re.split`` / ``re.match`` call - where it would actually change behavior. - - The check is structural (AST-based) rather than line-number-based so - unrelated edits above either implementation cannot move the call beyond - a fragile ±N-line window. - """ - source = (_REPO_ROOT / rel_path).read_text(encoding="utf-8") - tree = ast.parse(source, filename=rel_path) - - func_def = _find_function_def(tree, fn_name) - assert func_def is not None, f"function {fn_name!r} not found in {rel_path}" - - calls = list(_iter_re_finditer_calls(func_def)) - assert calls, f"expected at least one `re.finditer(...)` call inside {fn_name!r} in {rel_path}" - - for call in calls: - all_args = [*call.args, *(kw.value for kw in call.keywords)] - for arg in all_args: - assert not _is_case_insensitive_flag(arg), f"`re.I` / `re.IGNORECASE` must not be passed to `re.finditer` inside {fn_name!r} ({rel_path}). See issue #17384." + # Match `re.finditer(...)` / `re.findall(...)` / `re.compile(...)` + if not (isinstance(func, ast.Attribute) and isinstance(func.value, ast.Name) and func.value.id == "re"): + continue + if func.attr not in ("finditer", "findall", "compile"): + continue + for kw in node.keywords: + if kw.arg == "flags": + flag_node = kw.value + if isinstance(flag_node, ast.Attribute) and flag_node.attr in ("I", "IGNORECASE"): + raise AssertionError(f"`re.{flag_node.attr}` must not be passed as `flags=` to `re.{func.attr}` in rag/nlp/delim.py (see #17384)") + if isinstance(flag_node, ast.BinOp): + for sub in ast.walk(flag_node): + if isinstance(sub, ast.Attribute) and sub.attr in ("I", "IGNORECASE"): + raise AssertionError(f"`re.{sub.attr}` must not appear in a `flags=` expression passed to `re.{func.attr}` in rag/nlp/delim.py (see #17384)") diff --git a/web/src/utils/__tests__/delimiter-preview.test.ts b/web/src/utils/__tests__/delimiter-preview.test.ts new file mode 100644 index 0000000000..f6e7768149 --- /dev/null +++ b/web/src/utils/__tests__/delimiter-preview.test.ts @@ -0,0 +1,50 @@ +import { parseDelimitersForDisplay } from '../delimiter-preview'; + +describe('parseDelimitersForDisplay', () => { + it('returns empty for undefined or empty input', () => { + expect(parseDelimitersForDisplay(undefined)).toEqual([]); + expect(parseDelimitersForDisplay('')).toEqual([]); + }); + + it.each([ + ['!', ['!']], + ['!?', ['!', '?']], + [' ', [' ']], + ['\n', ['\n']], + ['\t', ['\t']], + ['\r', ['\n']], + ['\r\n', ['\n']], + ['\n!?;。；！？', ['\n', '!', '?', ';', '。', '；', '！', '？']], + ['`##`', ['##']], + ['`###``##``#`', ['###', '##', '#']], + ['\n`##`;', ['##', '\n', ';']], + ['`a`a`a`', ['a']], + ['`\n\n`', ['\n\n']], + ['`\t\t`', ['\t\t']], + ['é', ['é']], + ['。', ['。']], + ])('parses %j to match backend set/order', (field, expected) => { + const got = parseDelimitersForDisplay(field).map((d) => d.raw); + expect(got).toEqual(expected); + }); + + it('sorts longest-first', () => { + const got = parseDelimitersForDisplay('`#``##``###`').map((d) => d.raw); + expect(got).toEqual(['###', '##', '#']); + }); + + it('normalizes CRLF before parsing', () => { + expect(parseDelimitersForDisplay('\r\n').map((d) => d.raw)).toEqual([ + '\n', + ]); + expect(parseDelimitersForDisplay('`\r\n`').map((d) => d.raw)).toEqual([ + '\n', + ]); + }); + + it('applies whitespace glyphs only for display', () => { + const [item] = parseDelimitersForDisplay('\n'); + expect(item.raw).toBe('\n'); + expect(item.display).toBe('↵'); + }); +}); diff --git a/web/src/utils/delimiter-preview.ts b/web/src/utils/delimiter-preview.ts index 6c2df35a8b..e2fd88c046 100644 --- a/web/src/utils/delimiter-preview.ts +++ b/web/src/utils/delimiter-preview.ts @@ -1,15 +1,9 @@ /** * Parses a "Delimiter for text" field value into a list of delimiters for - * display in the UI. The result is meant to be informative only — the - * actual chunking behavior is determined by the backend. - * - * The parsing rule mirrors `rag/nlp/__init__.py::get_delimiters` in - * spirit (backticks group characters into a multi-character delimiter; - * anything outside backticks is itself a delimiter) but does **not** - * exactly match any of the six divergent backend implementations, since - * those implementations disagree on dedupe, sort order, the use of - * `re.I`, and CRLF normalization. The helper is a *preview*, not a - * contract. See #17383 for the backend consolidation. + * display in the UI. Mirrors the canonical backend parser in + * `rag/nlp/delim.py` (`parse_delimiter_field`): CRLF normalization, bare + * and backtick-wrapped tokens, insertion-ordered dedupe, and longest-first + * stable sort. Whitespace glyph substitution is display-only. */ export interface ParsedDelimiter { /** The original characters the user entered. */ @@ -49,16 +43,9 @@ function toDisplay(raw: string): string { } /** - * Parse the delimiter field into a deduplicated, display-ready list. - * - * Differences from the backend `get_delimiters`: - * - Deduplicates by raw value so distinct delimiters that happen to - * share a display glyph (e.g. a literal `\n` and a user-typed `↵`) - * are not silently merged. The backend does not always dedupe (see - * #17383). - * - Preserves left-to-right input order rather than sorting - * longest-first; visual order is more intuitive for users. - * - Does not regex-escape the values (irrelevant for display). + * Parse the delimiter field into a deduplicated, longest-first list that + * matches backend `parse_delimiter_field` semantics. Display glyphs are + * applied after parsing. * * Returns an empty array if the value is undefined or empty. */ @@ -67,28 +54,28 @@ export function parseDelimitersForDisplay( ): ParsedDelimiter[] { if (!value) return []; + // Match backend: \r\n → \n, then standalone \r → \n. + const normalizedValue = value.replaceAll('\r\n', '\n').replaceAll('\r', '\n'); + const result: ParsedDelimiter[] = []; const seen = new Set(); const push = (raw: string) => { - if (seen.has(raw)) return; + if (!raw || seen.has(raw)) return; seen.add(raw); result.push({ raw, display: toDisplay(raw) }); }; let cursor = 0; - for (const match of value.matchAll(/`([^`]+)`/g)) { + for (const match of normalizedValue.matchAll(/`([^`]+)`/g)) { const start = match.index!; const end = start + match[0].length; - // bare characters before this backtick-wrapped token - for (const ch of value.slice(cursor, start)) push(ch); - // the backtick-wrapped token + for (const ch of normalizedValue.slice(cursor, start)) push(ch); push(match[1]); cursor = end; } - // bare characters after the last token (or the whole string if no - // backticks were present) - for (const ch of value.slice(cursor)) push(ch); + for (const ch of normalizedValue.slice(cursor)) push(ch); - return result; + // Stable sort longest-first (matches backend parse_delimiter_field). + return result.sort((a, b) => b.raw.length - a.raw.length); }