From addc5acdc0b528c8a72456c9864f0890a892f00f Mon Sep 17 00:00:00 2001 From: Jack Date: Thu, 6 Aug 2026 15:52:50 +0800 Subject: [PATCH] fix(tokenizer): align important_kwd split to English comma (DSL parity, A2) (#17928) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Port the DSL tokenizer's `important_kwd` splitting into the Go `Tokenizer` component so the indexed keyword array is byte-compatible with the Python DSL pipeline and with the keyword-extraction prompt contract. - **Problem:** The Go component split `keywords` on the full ASCII+CJK delimiter set (`utility.SplitKeywords`, regex `[,，;；、\r\n]+`), while the DSL baseline `rag/flow/tokenizer/tokenizer.py:153` uses `keywords.split(",")`, and `rag/prompts/keyword_prompt.md` instructs the LLM to delimit keywords by **ENGLISH COMMA**. For a dataflow canvas that includes the Tokenizer component, this divergence made Go's indexed `important_kwd` differ from the Python-DSL-built index (CJK commas/semicolons were split in Go but kept whole in Python). - **Fix:** Use `strings.Split(kw, ",")` at `tokenizer.go:701`, preserving empty middle elements to match Python's `"a,,b".split(",") == ["a","","b"]`. The indexing fallback layer (`internal/ingestion/task/indexdoc/process.go`) already mirrors the Python multi-delimiter fallback (`dataflow_service.py:322`), so only the component layer diverged and only it is changed. ## Test plan - `TestTokenizerComponent_ImportantKwd_CommaOnly` (no build tag, default `go test ./...`): switches the tokenizer to the identity engine (no CGo pool needed) and asserts `"kw1,kw2;kw3，kw4"` → `["kw1","kw2;kw3，kw4"]`; also asserts `important_tks` still tokenizes the full keyword string. - `TestTokenizerComponent_Invoke_KeywordSplitCommaOnly` (`integration` tag, real CGo analyzer): covers comma-split, CJK/semicolon-not-split, and empty-middle preservation. - Both tiers pass (unit `ok`, integration `ok`). ## Regression notes - Intentional behavior change for canvases that include the Tokenizer component: keywords containing `;`/`、`/newlines now stay as one keyword (matching Python DSL) instead of being split. Re-indexing existing Go-built data will change the `important_kwd` set — expected parity cost, documented in code comments and commit message. - Canvases without a Tokenizer component are unaffected (they hit the unchanged multi-delimiter fallback). - Other fields (`important_tks`, `questions`, `summary`, `text`) are untouched; the `utility` import was removed cleanly. --- internal/ingestion/component/tokenizer.go | 10 ++- .../ingestion/component/tokenizer_test.go | 65 ++++++++++++------- .../component/tokenizer_unit_test.go | 53 +++++++++++++++ 3 files changed, 104 insertions(+), 24 deletions(-) diff --git a/internal/ingestion/component/tokenizer.go b/internal/ingestion/component/tokenizer.go index db9c030eb6..f8001736f6 100644 --- a/internal/ingestion/component/tokenizer.go +++ b/internal/ingestion/component/tokenizer.go @@ -96,7 +96,6 @@ import ( "ragflow/internal/ingestion/component/globals" "ragflow/internal/ingestion/component/schema" "ragflow/internal/tokenizer" - "ragflow/internal/utility" ) const ComponentNameTokenizer = "Tokenizer" @@ -698,7 +697,14 @@ func tokenizeChunks(chunks []schema.ChunkDoc, titleStem string, language string) } } if kw := ck.Keywords; kw != "" { - if err = ck.SetExtraValue("important_kwd", utility.SplitKeywords(kw)); err != nil { + // A2: split on the ENGLISH COMMA only, matching the DSL tokenizer + // (rag/flow/tokenizer/tokenizer.py:153 `keywords.split(",")`) and + // the keyword_prompt contract ("delimited by ENGLISH COMMA"). CJK + // commas/semicolons and newlines stay part of the keyword so the + // Go index is byte-compatible with the Python-DSL-built index. + // strings.Split preserves empty elements, matching Python's + // "a,,b".split(",") == ["a","","b"]. + if err = ck.SetExtraValue("important_kwd", strings.Split(kw, ",")); err != nil { return fmt.Errorf("tokenizer: keyword list marshal: %w", err) } it, err := tok.Tokenize(kw) diff --git a/internal/ingestion/component/tokenizer_test.go b/internal/ingestion/component/tokenizer_test.go index 5ab725a6a1..1dbe579168 100644 --- a/internal/ingestion/component/tokenizer_test.go +++ b/internal/ingestion/component/tokenizer_test.go @@ -262,37 +262,58 @@ func TestTokenizerComponent_Invoke_FullTextOnly(t *testing.T) { } } -// TestTokenizerComponent_Invoke_KeywordSplitCJK verifies important_kwd is -// split by the full ASCII+CJK delimiter set, not just ASCII comma. A Chinese -// LLM commonly emits CJK commas/semicolons even when asked for -// "comma-separated"; ASCII-only splitting would leave keywords glued together. -func TestTokenizerComponent_Invoke_KeywordSplitCJK(t *testing.T) { +// TestTokenizerComponent_Invoke_KeywordSplitCommaOnly verifies important_kwd +// is split on the ENGLISH COMMA ONLY, matching the DSL tokenizer +// (rag/flow/tokenizer/tokenizer.py:153 `keywords.split(",")`) and the +// keyword_prompt contract ("delimited by ENGLISH COMMA"). CJK commas, +// semicolons, and newlines are NOT delimiters — they stay part of the +// keyword. This keeps the Go index byte-compatible with the +// Python-DSL-built index (A2 alignment). +func TestTokenizerComponent_Invoke_KeywordSplitCommaOnly(t *testing.T) { requireTokenizerPool(t) _, stub := withStubEmbedder(t, 4) c, _ := NewTokenizerComponent(map[string]any{ "search_method": []any{"full_text"}, }) - out, err := c.Invoke(context.Background(), nil, map[string]any{ - "output_format": "chunks", - "chunks": []map[string]any{{"text": "alpha", "keywords": "kw1，kw2；kw3"}}, - }) - if err != nil { - t.Fatalf("Invoke: %v", err) + + check := func(keywords string, want []string) { + t.Helper() + out, err := c.Invoke(context.Background(), nil, map[string]any{ + "output_format": "chunks", + "chunks": []map[string]any{{"text": "alpha", "keywords": keywords}}, + }) + if err != nil { + t.Fatalf("Invoke(%q): %v", keywords, err) + } + got, _ := out["chunks"].([]map[string]any) + if len(got) != 1 { + t.Fatalf("chunks len = %d, want 1", len(got)) + } + kwd, ok := got[0]["important_kwd"].([]string) + if !ok { + t.Fatalf("important_kwd should be []string, got %T", got[0]["important_kwd"]) + } + if len(kwd) != len(want) { + t.Errorf("important_kwd(%q) = %v, want %v", keywords, kwd, want) + return + } + for i := range want { + if kwd[i] != want[i] { + t.Errorf("important_kwd(%q) = %v, want %v", keywords, kwd, want) + return + } + } } + if stub.calls.Load() != 0 { t.Errorf("embedder should not be called in full_text-only mode, got %d", stub.calls.Load()) } - got, _ := out["chunks"].([]map[string]any) - if len(got) != 1 { - t.Fatalf("chunks len = %d, want 1", len(got)) - } - kwd, ok := got[0]["important_kwd"].([]string) - if !ok { - t.Fatalf("important_kwd should be []string, got %T", got[0]["important_kwd"]) - } - if len(kwd) != 3 { - t.Errorf("important_kwd must split CJK delimiters into 3 elements, got %d: %v", len(kwd), kwd) - } + // Only the English comma splits. + check("kw1,kw2,kw3", []string{"kw1", "kw2", "kw3"}) + // CJK commas and semicolons are NOT delimiters. + check("kwA，kwB；kwC", []string{"kwA，kwB；kwC"}) + // Empty middle elements are preserved, matching Python "a,,b".split(","). + check("a,,b", []string{"a", "", "b"}) } func TestTokenizerComponent_Invoke_FullTextAndEmbedding(t *testing.T) { diff --git a/internal/ingestion/component/tokenizer_unit_test.go b/internal/ingestion/component/tokenizer_unit_test.go index 2bbd48c199..f82fc434f2 100644 --- a/internal/ingestion/component/tokenizer_unit_test.go +++ b/internal/ingestion/component/tokenizer_unit_test.go @@ -31,6 +31,7 @@ import ( "ragflow/internal/agent/runtime" "ragflow/internal/ingestion/component/schema" + "ragflow/internal/tokenizer" ) // stubEmbedder records every call and returns canned vectors. @@ -535,3 +536,55 @@ func TestChunksFromTokenizerUpstream_FiltersPhantomChunks(t *testing.T) { t.Errorf("chunk 1 text = %q, want %q", chunks[1]["text"], "another valid") } } + +// TestTokenizerComponent_ImportantKwd_CommaOnly is the no-tag parity test for +// A2: important_kwd must be split on the ENGLISH COMMA ONLY, matching the DSL +// tokenizer (rag/flow/tokenizer/tokenizer.py:153 `keywords.split(",")`). It +// runs without the C++ analyzer pool by switching the tokenizer engine to +// "infinity" (identity: Tokenize returns its input unchanged), so it executes +// in the default `go test ./...` CI tier and gives real regression protection. +func TestTokenizerComponent_ImportantKwd_CommaOnly(t *testing.T) { + // Switch to identity tokenizer so tokenizeChunks needs no CGo pool, then + // restore the default engine type afterwards. + tokenizer.SetEngineType("infinity") + defer tokenizer.SetEngineType("") + + c, err := NewTokenizerComponent(map[string]any{ + "search_method": []any{"full_text"}, + }) + if err != nil { + t.Fatalf("NewTokenizerComponent: %v", err) + } + out, err := c.Invoke(context.Background(), nil, map[string]any{ + "output_format": "chunks", + "chunks": []map[string]any{ + {"text": "doc body", "keywords": "kw1,kw2;kw3，kw4"}, + }, + }) + if err != nil { + t.Fatalf("Invoke: %v", err) + } + got, ok := out["chunks"].([]map[string]any) + if !ok || len(got) != 1 { + t.Fatalf("chunks = %v, want 1 chunk", out["chunks"]) + } + kwd, ok := got[0]["important_kwd"].([]string) + if !ok { + t.Fatalf("important_kwd should be []string, got %T", got[0]["important_kwd"]) + } + // Only the English comma splits; CJK comma and semicolon stay attached. + want := []string{"kw1", "kw2;kw3，kw4"} + if len(kwd) != len(want) { + t.Fatalf("important_kwd = %v, want %v", kwd, want) + } + for i := range want { + if kwd[i] != want[i] { + t.Errorf("important_kwd = %v, want %v (only comma splits)", kwd, want) + } + } + // important_tks still tokenizes the full keyword string (identity mode + // returns it unchanged). + if tks, ok := got[0]["important_tks"].(string); !ok || tks != "kw1,kw2;kw3，kw4" { + t.Errorf("important_tks = %v, want full keyword string", got[0]["important_tks"]) + } +}