From 0227b2684e9e2975d12ac231d5962d5c61124807 Mon Sep 17 00:00:00 2001 From: Jack Date: Wed, 5 Aug 2026 17:26:36 +0800 Subject: [PATCH] fix(chunker): drop delimiter from chunk text on primary and children splits (#17868) Go's `TokenChunker` kept the captured delimiter glued to the preceding segment on **both** the primary (`chunkFromItem`) and secondary (`children_delimiters`) split paths, while Python's reference `token_chunker` drops it via `_split_text_by_pattern` (`token_chunker.py:79-93`, used by both `_build_json_chunks` and `_split_chunk_docs_by_children`). The divergence leaked the delimiter into every emitted chunk's `text`. --- .../ingestion/component/chunker/common.go | 44 ++--------- .../component/chunker/custom_delim_test.go | 33 ++++---- internal/ingestion/component/chunker/token.go | 12 +-- .../chunker/token_children_delim_test.go | 78 +++++++++++++++++++ 4 files changed, 106 insertions(+), 61 deletions(-) create mode 100644 internal/ingestion/component/chunker/token_children_delim_test.go diff --git a/internal/ingestion/component/chunker/common.go b/internal/ingestion/component/chunker/common.go index 1dfeadf237..c9a31954b9 100644 --- a/internal/ingestion/component/chunker/common.go +++ b/internal/ingestion/component/chunker/common.go @@ -86,45 +86,13 @@ func compileDelimPattern(delims []string) *regexp.Regexp { return chunk.CompileDelimiterListPattern(delims) } -// splitKeepingDelim mirrors Python token_chunker._split_text_by_pattern -// (token_chunker.py:79-94): re.split with a captured delimiter group yields -// [text, delim, text, delim, ...]; each delimiter is glued to the END of the -// preceding text segment, so it never surfaces as a standalone chunk. A -// delimiter with no preceding text (a leading delimiter or one adjacent to -// another) is dropped together with the empty segment, matching Python's -// `if not chunk: continue`. -func splitKeepingDelim(text string, pattern *regexp.Regexp) []string { - if pattern == nil { - return []string{text} - } - idxs := pattern.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] - if start == cursor { - cursor = end - continue - } - out = append(out, text[cursor:end]) - cursor = end - } - if cursor < len(text) { - out = append(out, text[cursor:]) - } - return out -} - // splitDroppingDelim mirrors Python's _split_text_by_pattern -// (token_chunker.py:79-90). Unlike splitKeepingDelim, the captured delimiter -// is DISCARDED rather than glued to the preceding segment: re.split with a -// captured group keeps delimiters at odd indices, and only the even-index -// (text) parts are kept. This is the behaviour the text/markdown/html path -// must reproduce so a split chunk reads "first sentence here" without the -// trailing delimiter. +// (token_chunker.py:79-90). The captured delimiter is DISCARDED rather than +// glued to a segment: re.split with a captured group keeps delimiters at odd +// indices, and only the even-index (text) parts are kept. This is the +// behaviour every delimiter path (primary and children, text/markdown/html +// and json) must reproduce so a split chunk reads "first sentence here" +// without the trailing delimiter. func splitDroppingDelim(text string, pattern *regexp.Regexp) []string { if pattern == nil { return []string{text} diff --git a/internal/ingestion/component/chunker/custom_delim_test.go b/internal/ingestion/component/chunker/custom_delim_test.go index 58c18d6066..9ed9701b9a 100644 --- a/internal/ingestion/component/chunker/custom_delim_test.go +++ b/internal/ingestion/component/chunker/custom_delim_test.go @@ -8,20 +8,17 @@ import ( // custom_delim_test pins the backtick-wrapped newline delimiter behaviour of // TokenChunker against Python's rag/flow/chunker/token_chunker.py. // -// There are two distinct, path-specific divergences that this file locks down: +// All delimiter paths (primary and children, text/markdown/html and json) +// must DROP the captured delimiter from each chunk's text, matching Python's +// _split_text_by_pattern (token_chunker.py:79-90, used by both _build_json_chunks +// and _split_chunk_docs_by_children). Go's splitDroppingDelim reproduces this. // -// 1. text/markdown/html path: Python's _split_text_by_pattern (token_chunker.py:79-90) -// uses re.split with a captured group and keeps only the even-index -// (text) parts, so the delimiter is DROPPED. Go's splitKeepingDelim glues -// the delimiter to the end of the preceding segment, so a chunk reads -// "first sentence here\n" instead of "first sentence here". The fix makes -// the text path drop the delimiter like Python. -// -// 2. json path: Python's _build_json_chunks + _finalize_json_chunks never -// strip the chunk text, so the delimiter's trailing newline survives -// ("first segment line one\n"). Go's invokeJSONPayload ran every chunk -// through strings.TrimSpace, which deleted that newline. The fix stops -// trimming, so the newline is kept like Python. +// The json primary path is the one most prone to regress: Python's +// _build_json_chunks (token_chunker.py:121) splits each item through +// _split_text_by_pattern, which keeps only the even-index (text) parts and +// DISCARDS the captured delimiter. So a "first segment line one\n" item yields +// "first segment line one" with the newline dropped. Go's chunkFromItem does +// the same via splitDroppingDelim, matching the Python reference. // // doc_type_kwd is intentionally asserted to remain present on every chunk. // It is a load-bearing Go field (index column + media dispatch) and is NOT @@ -81,8 +78,10 @@ func TestCustomDelimTextDropsDelimiter(t *testing.T) { } } -// TestCustomDelimJSONKeepsNewline reproduces token__json_backtick. -func TestCustomDelimJSONKeepsNewline(t *testing.T) { +// TestCustomDelimJSONDropsDelimiter reproduces token__json_backtick: the +// primary (custom backtick) delimiter is dropped from every chunk text, +// matching Python's _split_text_by_pattern. +func TestCustomDelimJSONDropsDelimiter(t *testing.T) { params := map[string]any{"chunk_token_size": float64(128), "delimiters": []string{backtickNewline}} input := map[string]any{ "name": "t", "output_format": "json", @@ -94,8 +93,8 @@ func TestCustomDelimJSONKeepsNewline(t *testing.T) { chunks := invokeTokenChunks(t, params, input) want := []string{ - "first segment line one\n", "first segment line two", - "second segment line one\n", "second segment line two", + "first segment line one", "first segment line two", + "second segment line one", "second segment line two", } if len(chunks) != len(want) { t.Fatalf("chunk count: want %d got %d (%v)", len(want), len(chunks), chunkTexts(chunks)) diff --git a/internal/ingestion/component/chunker/token.go b/internal/ingestion/component/chunker/token.go index f6b1c83db3..5d4167fac3 100644 --- a/internal/ingestion/component/chunker/token.go +++ b/internal/ingestion/component/chunker/token.go @@ -30,8 +30,8 @@ // 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 -// ("mom") and the split child ("text") keys. +// splitDroppingDelim helper; emitted chunks carry the parent +// ("mom") and the split child ("text") keys, with the delimiter dropped. // // - MODE "delimiter" uses the regex-aware delimiter pattern to split // text into segments; unlike token_size, these segments are NOT @@ -768,7 +768,7 @@ func chunkFromItem(it schema.ChunkDoc, delimPattern *regexp.Regexp) []schema.Chu if !hasActiveDelimiter(delimPattern) { return []schema.ChunkDoc{buildChunkDoc(it, "text", txt, "", "")} } - parts := splitKeepingDelim(txt, delimPattern) + parts := splitDroppingDelim(txt, delimPattern) if !delimPattern.MatchString(txt) { return []schema.ChunkDoc{buildChunkDoc(it, "text", txt, "", "")} } @@ -1172,7 +1172,7 @@ func splitByChildren(chunks []schema.ChunkDoc, pattern *regexp.Regexp) []schema. continue } mom := ck.Text - parts := splitKeepingDelim(mom, pattern) + parts := splitDroppingDelim(mom, pattern) for _, p := range parts { if strings.TrimSpace(p) == "" { continue @@ -1223,7 +1223,7 @@ func applyChildrenDelim(segs []string, pattern *regexp.Regexp) []schema.ChunkDoc if strings.TrimSpace(seg) == "" { continue } - for _, child := range splitKeepingDelim(seg, pattern) { + for _, child := range splitDroppingDelim(seg, pattern) { if strings.TrimSpace(child) == "" { continue } @@ -1243,7 +1243,7 @@ func applyChildrenDelimText(docs []schema.ChunkDoc, pattern *regexp.Regexp) []sc if strings.TrimSpace(t) == "" { continue } - for _, child := range splitKeepingDelim(t, pattern) { + for _, child := range splitDroppingDelim(t, pattern) { if strings.TrimSpace(child) == "" { continue } diff --git a/internal/ingestion/component/chunker/token_children_delim_test.go b/internal/ingestion/component/chunker/token_children_delim_test.go new file mode 100644 index 0000000000..8337b1b0e9 --- /dev/null +++ b/internal/ingestion/component/chunker/token_children_delim_test.go @@ -0,0 +1,78 @@ +package chunker + +import ( + "context" + "testing" +) + +// TestTokenChunker_ChildrenDelimiterDroppedJSON asserts that the JSON-path +// secondary children_delimiters split DROPS the delimiter from each child's +// text (matching Python's _split_chunk_docs_by_children / +// _split_text_by_pattern), while keeping the full source text in "mom". +func TestTokenChunker_ChildrenDelimiterDroppedJSON(t *testing.T) { + c, err := NewTokenChunker(map[string]any{ + "children_delimiters": []string{"。"}, + }) + if err != nil { + t.Fatalf("NewTokenChunker: %v", err) + } + out, err := c.Invoke(context.Background(), nil, map[string]any{ + "name": "doc.json", + "output_format": "json", + "json": []map[string]any{ + {"text": "第一句内容。第二句内容。第三句内容。", "doc_type_kwd": "text"}, + }, + }) + if err != nil { + t.Fatalf("Invoke: %v", err) + } + chunks, _ := out["chunks"].([]map[string]any) + if len(chunks) != 3 { + t.Fatalf("chunk count: want 3 got %d (%v)", len(chunks), chunkTexts(chunks)) + } + want := []string{"第一句内容", "第二句内容", "第三句内容"} + const mom = "第一句内容。第二句内容。第三句内容。" + for i, w := range want { + got := chunks[i]["text"].(string) + if got != w { + t.Errorf("chunk[%d] text: want %q got %q", i, w, got) + } + if m, _ := chunks[i]["mom"].(string); m != mom { + t.Errorf("chunk[%d] mom: want %q got %q", i, mom, m) + } + } +} + +// TestTokenChunker_ChildrenDelimiterDroppedText asserts the text/markdown/html +// children_delimiters split also DROPS the delimiter (applyChildrenDelim / +// applyChildrenDelimText mirror _split_text_by_pattern), keeping the parent +// segment in "mom". +func TestTokenChunker_ChildrenDelimiterDroppedText(t *testing.T) { + c, err := NewTokenChunker(map[string]any{ + "delimiter_mode": "delimiter", + "delimiters": []string{"\n"}, + "children_delimiters": []string{". "}, + }) + 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": "alpha one. alpha two. alpha three.", + }) + if err != nil { + t.Fatalf("Invoke: %v", err) + } + chunks, _ := out["chunks"].([]map[string]any) + want := []string{"alpha one", "alpha two", "alpha three."} + if len(chunks) != len(want) { + t.Fatalf("chunk count: want %d got %d (%v)", len(want), len(chunks), chunkTexts(chunks)) + } + for i, w := range want { + got := chunks[i]["text"].(string) + if got != w { + t.Errorf("chunk[%d] text: want %q got %q", i, w, got) + } + } +}