diff --git a/deepdoc/parser/txt_parser.py b/deepdoc/parser/txt_parser.py index 2a5e999f7d..a89e67846e 100644 --- a/deepdoc/parser/txt_parser.py +++ b/deepdoc/parser/txt_parser.py @@ -48,7 +48,7 @@ class RAGFlowTxtParser: dels = [] s = 0 - for m in re.finditer(r"`([^`]+)`", delimiter, re.I): + for m in re.finditer(r"`([^`]+)`", delimiter): f, t = m.span() dels.append(m.group(1)) dels.extend(list(delimiter[s:f])) diff --git a/internal/ingestion/component/chunker/common.go b/internal/ingestion/component/chunker/common.go index 297dcb8580..79d2b996aa 100644 --- a/internal/ingestion/component/chunker/common.go +++ b/internal/ingestion/component/chunker/common.go @@ -76,28 +76,86 @@ func stringListFromAny(in []any) []string { // regex / split helpers // --------------------------------------------------------------------------- -// compileDelimPattern joins all delimiter entries into a single -// alternation. Entries wrapped in backticks are treated as regex -// literals and regex-escaped; plain strings are simply regex-escaped. -// Longer patterns win (matches python `sorted(set, key=len, reverse=True)`). -// Mirrors Python _compile_delimiter_pattern: only backtick-wrapped delimiters -// produce an active pattern. Plain delimiters are not compiled — they are only -// used by naive_merge / mergeByTokenSize for sentence-level splitting when no -// active pattern exists. +// 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). func compileDelimPattern(delims []string) *regexp.Regexp { - var custom []string + var tokens []string for _, d := range delims { if d == "" { continue } - if strings.HasPrefix(d, "`") && strings.HasSuffix(d, "`") && len(d) >= 2 { - custom = append(custom, regexp.QuoteMeta(d[1:len(d)-1])) + 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 } - sort.SliceStable(custom, func(i, j int) bool { return len(custom[i]) > len(custom[j]) }) return regexp.MustCompile(strings.Join(custom, "|")) } diff --git a/internal/ingestion/component/chunker/delimiter_case_sensitive_test.go b/internal/ingestion/component/chunker/delimiter_case_sensitive_test.go new file mode 100644 index 0000000000..16ea51b610 --- /dev/null +++ b/internal/ingestion/component/chunker/delimiter_case_sensitive_test.go @@ -0,0 +1,233 @@ +// +// 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. +// + +// Regression tests for case-sensitive delimiter parsing (#17384 / PR #17386). +// +// Locks Go parity with the Python fix that dropped re.I from get_delimiters +// and parser_txt. Delimiter matching must preserve letter casing: +// +// - bare "a" matches only "a", not "A" +// - backtick-wrapped "`end`" matches only "end", not "End"/"END"/etc. +// +// Go's regexp package is case-sensitive by default; these tests guard against +// a future (?i) flag or case-folding change creeping into the delimiter path. + +package chunker + +import ( + "context" + "regexp" + "strings" + "testing" +) + +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 { + t.Fatalf("getDelimiters(%q) = %q, want %q", "a", got, want) + } +} + +func TestGetDelimiters_BareCharAUpperReturnsLiteralPattern(t *testing.T) { + if got, want := getDelimiters("A"), "A"; got != want { + t.Fatalf("getDelimiters(%q) = %q, want %q", "A", got, want) + } +} + +func TestGetDelimiters_BacktickEndReturnsExactToken(t *testing.T) { + // Backtick-wrapped delimiter must preserve the captured group verbatim. + if got, want := getDelimiters("`end`"), "end"; got != want { + t.Fatalf("getDelimiters(%q) = %q, want %q", "`end`", got, want) + } +} + +func TestGetDelimiters_PatternSplitsCaseSensitively(t *testing.T) { + // The pattern returned by getDelimiters must split case-sensitively. + pat := getDelimiters("a") + re := regexp.MustCompile("(" + pat + ")") + // Only the lowercase 'a' splits; uppercase 'A' is preserved intact. + got := splitKeepingCapture("AaBb", re) + want := []string{"A", "a", "Bb"} + if len(got) != len(want) { + t.Fatalf("split = %#v, want %#v", got, want) + } + for i := range want { + if got[i] != want[i] { + t.Fatalf("split[%d] = %q, want %q (full=%#v)", i, got[i], want[i], got) + } + } +} + +// splitKeepingCapture mirrors Python re.split("("+pat+")", text): +// returns [text, delim, text, delim, ...] including empty leading/trailing. +func splitKeepingCapture(text string, re *regexp.Regexp) []string { + idxs := re.FindAllStringIndex(text, -1) + if len(idxs) == 0 { + return []string{text} + } + var out []string + cursor := 0 + for _, idx := range idxs { + start, end := idx[0], idx[1] + out = append(out, text[cursor:start]) + out = append(out, text[start:end]) + cursor = end + } + out = append(out, text[cursor:]) + return out +} + +func TestCompileDelimPattern_BacktickEndIsCaseSensitive(t *testing.T) { + p := compileDelimPattern([]string{"`end`"}) + if p == nil { + t.Fatal("compileDelimPattern(`end`) returned nil") + } + if p.MatchString("End") || p.MatchString("END") || p.MatchString("eNd") { + t.Fatalf("pattern %q must not match case variants of end", p.String()) + } + if !p.MatchString("end") { + t.Fatalf("pattern %q must match exact lowercase end", p.String()) + } + // Pattern string itself must not carry a case-insensitive flag. + if strings.HasPrefix(p.String(), "(?i)") || strings.Contains(p.String(), "(?i)") { + t.Fatalf("compileDelimPattern must not emit (?i); got %q", p.String()) + } +} + +func TestCompileDelimPattern_BareCharIsNotActivePattern(t *testing.T) { + // Plain delimiters are not compiled — only backtick-wrapped tokens are. + if p := compileDelimPattern([]string{"a"}); p != nil { + t.Fatalf("compileDelimPattern([a]) = %v, want nil", p) + } +} + +func TestCompileDelimPattern_ExtractsPerEntryIndependently(t *testing.T) { + // Adjacent entries must not form a cross-boundary backtick pair. + // Concatenating "`aa" + "`bb`" would invent an "aa`bb" token; per-entry + // extraction only sees the complete "`bb`" pair in the second entry. + p := compileDelimPattern([]string{"`aa", "`bb`"}) + if p == nil { + t.Fatal("expected pattern from second entry `bb`") + } + if got := p.String(); got != "bb" { + t.Fatalf("pattern = %q, want %q (no cross-entry token)", got, "bb") + } + if p.MatchString("aa") { + t.Fatalf("must not match incomplete first-entry token; pattern=%q", p.String()) + } + if !p.MatchString("bb") { + t.Fatalf("must match token from second entry; pattern=%q", p.String()) + } + + // Multiple well-formed entries still combine. + p2 := compileDelimPattern([]string{"`end`", "`foo`"}) + if p2 == nil { + t.Fatal("expected combined pattern") + } + if !p2.MatchString("end") || !p2.MatchString("foo") { + t.Fatalf("combined pattern %q must match both tokens", p2.String()) + } +} + +func TestTokenChunker_BacktickEndSplitsOnlyAtLowercase(t *testing.T) { + // End-to-end: delimiter-mode with "`end`" must split only at lowercase + // "end", leaving "End" / "END" intact inside the following segment. + c, err := NewTokenChunker(map[string]any{ + "delimiter_mode": "delimiter", + "delimiters": []string{"`end`"}, + }) + if err != nil { + t.Fatalf("NewTokenChunker: %v", err) + } + out, err := c.Invoke(context.Background(), nil, map[string]any{ + "name": "doc.txt", + "output_format": "text", + "text": "the end and End and END come", + }) + if err != nil { + t.Fatalf("Invoke: %v", err) + } + chunks, _ := out["chunks"].([]map[string]any) + got := make([]string, 0, len(chunks)) + for _, ck := range chunks { + text, _ := ck["text"].(string) + text = strings.TrimSpace(text) + if text != "" { + got = append(got, text) + } + } + // splitKeepingDelim glues the matched delimiter onto the preceding + // segment: "the end" | " and End and END come" + want := []string{"the end", "and End and END come"} + if len(got) != len(want) { + t.Fatalf("chunks = %#v, want %#v", got, want) + } + for i := range want { + if got[i] != want[i] { + t.Fatalf("chunk[%d] = %q, want %q (full=%#v)", i, got[i], want[i], got) + } + } +} + +func TestTokenChunker_BacktickASplitsOnlyAtLowercase(t *testing.T) { + c, err := NewTokenChunker(map[string]any{ + "delimiter_mode": "delimiter", + "delimiters": []string{"`a`"}, + }) + if err != nil { + t.Fatalf("NewTokenChunker: %v", err) + } + out, err := c.Invoke(context.Background(), nil, map[string]any{ + "name": "doc.txt", + "output_format": "text", + "text": "BaAb", + }) + if err != nil { + t.Fatalf("Invoke: %v", err) + } + chunks, _ := out["chunks"].([]map[string]any) + got := make([]string, 0, len(chunks)) + for _, ck := range chunks { + text, _ := ck["text"].(string) + text = strings.TrimSpace(text) + if text != "" { + got = append(got, text) + } + } + // "B" + "a" glued → "Ba"; remainder "Ab" + want := []string{"Ba", "Ab"} + if len(got) != len(want) { + t.Fatalf("chunks = %#v, want %#v", got, want) + } + for i := range want { + if got[i] != want[i] { + t.Fatalf("chunk[%d] = %q, want %q (full=%#v)", i, got[i], want[i], got) + } + } +} + +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) + } + // 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) + } +} diff --git a/rag/nlp/__init__.py b/rag/nlp/__init__.py index 597b6b462b..793b972d13 100644 --- a/rag/nlp/__init__.py +++ b/rag/nlp/__init__.py @@ -1630,7 +1630,7 @@ def extract_between(text: str, start_tag: str, end_tag: str) -> list[str]: def get_delimiters(delimiters: str): dels = [] s = 0 - for m in re.finditer(r"`([^`]+)`", delimiters, re.I): + for m in re.finditer(r"`([^`]+)`", delimiters): f, t = m.span() dels.append(m.group(1)) dels.extend(list(delimiters[s:f])) diff --git a/test/unit_test/rag/test_delimiter_case_sensitive.py b/test/unit_test/rag/test_delimiter_case_sensitive.py new file mode 100644 index 0000000000..66c48553d7 --- /dev/null +++ b/test/unit_test/rag/test_delimiter_case_sensitive.py @@ -0,0 +1,230 @@ +# +# 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. +# + +"""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. + +Affected sites +-------------- +* ``rag.nlp.get_delimiters`` (line 1633) +* ``deepdoc.parser.txt_parser.parser_txt`` (line 51) + +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) +""" + +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) + + +from rag import nlp +from rag.nlp import get_delimiters, naive_merge + +_REPO_ROOT = Path(__file__).resolve().parents[3] + + +# --------------------------------------------------------------------------- # +# get_delimiters — 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" + + +def test_get_delimiters_bare_char_A_returns_literal_pattern(): + assert get_delimiters("A") == "A" + + +def test_get_delimiters_backtick_end_returns_exact_token(): + """Backtick-wrapped delimiter must preserve the captured group verbatim. + + A regression that introduced case-insensitive alternation would produce + ``end|End|END|eNd|...`` instead of the literal ``end``. + """ + assert get_delimiters("`end`") == "end" + + +def test_get_delimiters_pattern_splits_case_sensitively(): + """The pattern returned by ``get_delimiters`` must split case-sensitively + when fed to ``re.split`` without any flags.""" + pat = get_delimiters("a") + # Only the lowercase 'a' splits; uppercase 'A' is preserved intact. + assert re.split(f"({pat})", "AaBb") == ["A", "a", "Bb"] + + +# --------------------------------------------------------------------------- # +# naive_merge — end-to-end (exercises get_delimiters + re.split) +# --------------------------------------------------------------------------- # + + +@pytest.fixture(autouse=True) +def force_every_section_above_budget(monkeypatch): + """Mock ``num_tokens_from_string`` so every section trips the chunk-size + guard and starts a fresh chunk. This isolates delimiter behavior from + chunk-size heuristics.""" + + def fake(_s): + return 10**9 + + monkeypatch.setattr(nlp, "num_tokens_from_string", fake) + + +def test_naive_merge_bare_char_a_splits_only_at_lowercase_a(): + """Bare-char ``a`` must split only at lowercase ``a``, not at ``A``.""" + chunks = naive_merge(["BaAb"], chunk_token_num=8, delimiter="a") + assert [c.strip() for c in chunks if c.strip()] == ["B", "Ab"] + + +def test_naive_merge_bare_char_A_splits_only_at_uppercase_A(): + chunks = naive_merge(["BaAb"], chunk_token_num=8, delimiter="A") + assert [c.strip() for c in chunks if c.strip()] == ["Ba", "b"] + + +def test_naive_merge_backtick_end_splits_only_at_lowercase_end(): + """Backtick-wrapped ``end`` must split only at the exact lowercase + ``end``, not at ``End`` / ``END`` / ``eNd`` / etc.""" + chunks = naive_merge( + ["the end and End and END come"], + chunk_token_num=8, + delimiter="`end`", + ) + assert [c.strip() for c in chunks if c.strip()] == [ + "the", + "and End and END come", + ] + + +# --------------------------------------------------------------------------- # +# Static source checks — guard against re.I creeping back into the two sites +# +# ``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. +# --------------------------------------------------------------------------- # + + +_CASE_INSENSITIVE_RE_ATTRS = frozenset({"I", "IGNORECASE"}) + + +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): + 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."