Fix(tokenizer): pin alignment guards (#18011)

internal/ingestion/component/tokenizer.go

@@ -413,6 +413,13 @@ func (c *TokenizerComponent) embedChunks(ctx context.Context, tenantID, kbID, em
 		tokenCount  int
 		hasTitleVec bool
 	)
+	// go_intentional (A3): when the upstream name is empty we skip title
+	// weighting entirely (hasTitleVec stays false, so the merged vector is the
+	// content vector alone). From the end-user perspective an empty title must
+	// not contribute the filename embedding weight; the Python DSL instead
+	// computes 0.1*emb(""), injecting an undefined bias into every chunk. Go's
+	// skip is the correct behavior (go_intentional). Do NOT "align" this to
+	// the DSL.
 	if trimmedName == "" {
 		log.Printf("Tokenizer: empty name provided from upstream, embedding will skip title weighting")
 	} else {
@@ -680,13 +687,11 @@ func tokenizeChunks(chunks []schema.ChunkDoc, titleStem string, language string)
 			}
 		}
 		if kw := ck.Keywords; kw != "" {
-			// 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"].
+			// Split keywords on the ENGLISH COMMA ONLY. The keyword_prompt
+			// contract specifies "delimited by ENGLISH COMMA", so CJK commas,
+			// semicolons and newlines stay part of the keyword rather than
+			// acting as separators. strings.Split also 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)
 			}
@@ -772,6 +777,13 @@ func concatFields(ck schema.ChunkDoc, fields []string) string {
 
 // shouldHaveEmbedding reports whether the tokenizer must attach embedding
 // vectors: the search method requests embedding AND a KB is present.
+//
+// go_intentional (A4): the kbID != "" guard is deliberate. Each dataset
+// configures its own embedding model, so an empty kb_id (e.g. a canvas-debug
+// dry run) must NOT fall back to the tenant's default embedding model — doing
+// so would produce vectors a dataset cannot actually use at retrieval time.
+// This is a deliberate, go_intentional divergence. Do NOT "align" this to a
+// path that injects a default embedding.
 func shouldHaveEmbedding(searchMethods []string, kbID string) bool {
 	return contains(searchMethods, "embedding") && kbID != ""
 }

internal/ingestion/component/tokenizer_unit_test.go

@@ -702,3 +702,115 @@ func TestTokenizerComponent_ImportantKwd_CommaOnly(t *testing.T) {
 		t.Errorf("important_tks = %v, want full keyword string", got[0]["important_tks"])
 	}
 }
+
+// TestTokenizerComponent_ImportantKwd_PreservesEmptyElements locks F3: the
+// component path splits on the ENGLISH COMMA ONLY via strings.Split and
+// PRESERVES empty elements, matching Python's "a,,b".split(",") ==
+// ["a","","b"]. This is the contract asserted by the comment at
+// tokenizer.go:688-689, and it is the intentional counterpart to the executor
+// fallback (cleanupConsumedChunkFields -> utility.SplitKeywords) which DROPS
+// empty parts. Runs without the C++ analyzer pool via the identity engine.
+func TestTokenizerComponent_ImportantKwd_PreservesEmptyElements(t *testing.T) {
+	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)
+	}
+
+	// Middle empty element must be preserved (["a","","b"]), not dropped.
+	out, err := c.Invoke(context.Background(), nil, map[string]any{
+		"output_format": "chunks",
+		"chunks": []map[string]any{
+			{"text": "doc body", "keywords": "a,,b"},
+		},
+	})
+	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"])
+	}
+	want := []string{"a", "", "b"}
+	if len(kwd) != len(want) {
+		t.Fatalf("important_kwd = %v, want %v (empty elements preserved)", kwd, want)
+	}
+	for i := range want {
+		if kwd[i] != want[i] {
+			t.Errorf("important_kwd[%d] = %q, want %q (empty elements preserved)", i, kwd[i], want[i])
+		}
+	}
+
+	// Empty keyword string must not error. The component guards on a
+	// non-empty keywords field (tokenizer.go:682), so important_kwd is left
+	// unset (nil) rather than materialized as an empty array. Downstream
+	// indexing treats a missing important_kwd as "no keywords", which is safe.
+	outEmpty, err := c.Invoke(context.Background(), nil, map[string]any{
+		"output_format": "chunks",
+		"chunks": []map[string]any{
+			{"text": "doc body", "keywords": ""},
+		},
+	})
+	if err != nil {
+		t.Fatalf("Invoke(empty keywords): %v", err)
+	}
+	gotEmpty, ok := outEmpty["chunks"].([]map[string]any)
+	if !ok || len(gotEmpty) != 1 {
+		t.Fatalf("empty chunks = %v, want 1 chunk", outEmpty["chunks"])
+	}
+	if kwd, exists := gotEmpty[0]["important_kwd"]; exists && kwd != nil {
+		t.Errorf("empty keywords should leave important_kwd unset, got %v", kwd)
+	}
+}
+
+// TestTextPayloadToChunks_B1B2 pins the text-payload adapter used for
+// markdown/text/html payloads (tokenizer.go:578). This is the Go-correct
+// counterpart to a Python-DSL bug where turning full_text off dropped the
+// entire document (B1), and where an empty payload failed to emit the chunks
+// key at all (B2). The Go helper always returns a slice: non-empty payload ->
+// one chunk carrying the raw text; nil/empty/whitespace payload -> a non-nil
+// EMPTY slice so the downstream "chunks" key is present as [].
+//
+// No-tag unit test: textPayloadToChunks is a pure function, no CGo pool needed.
+func TestTextPayloadToChunks_B1B2(t *testing.T) {
+	// B1: a real payload yields exactly one chunk with the text preserved.
+	payload := "plain payload body"
+	got := textPayloadToChunks(&payload)
+	if len(got) != 1 {
+		t.Fatalf("non-empty payload: len(chunks) = %d, want 1", len(got))
+	}
+	if got[0].Text != payload {
+		t.Errorf("chunk text = %q, want %q", got[0].Text, payload)
+	}
+
+	// B2: empty/whitespace/nil payloads must still return a non-nil EMPTY
+	// slice (so the chunks key is present as [] downstream), never nil.
+	for _, tc := range []struct {
+		name string
+		in   *string
+	}{
+		{"empty string", ptr("")},
+		{"whitespace only", ptr("   \n\t  ")},
+		{"nil pointer", nil},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			out := textPayloadToChunks(tc.in)
+			if out == nil {
+				t.Fatalf("textPayloadToChunks(%s) = nil, want non-nil empty slice", tc.name)
+			}
+			if len(out) != 0 {
+				t.Errorf("textPayloadToChunks(%s) len = %d, want 0", tc.name, len(out))
+			}
+		})
+	}
+}
+
+func ptr(s string) *string { return &s }

internal/ingestion/task/indexdoc/process_test.go

@@ -345,3 +345,67 @@ func TestProcessChunkPositions_NoPositions(t *testing.T) {
 		t.Error("_pdf_positions must be pruned even when positions is missing")
 	}
 }
+
+// TestCleanupConsumedChunkFields_ImportantKwdMultiDelimiter pins the executor
+// fallback's important_kwd materialization. When the Tokenizer component did
+// NOT pre-produce important_kwd, the executor falls back to
+// utility.SplitKeywords, which splits on the full delimiter set
+// (ASCII + CJK comma/semicolon/ideographic-comma/newline) and DROPS empty
+// parts. This is intentionally different from the Tokenizer component path
+// (internal/ingestion/component/tokenizer.go:690), which splits on the ENGLISH
+// COMMA ONLY and PRESERVES empty elements to match the DSL
+// (rag/flow/tokenizer/tokenizer.py:153 `keywords.split(",")`).
+//
+// The two layers deliberately diverge: the component aligns to the DSL keyword
+// contract ("delimited by ENGLISH COMMA"); the executor fallback mirrors
+// Python task_executor.run_dataflow:879 and tolerates mixed delimiters from
+// older upstream producers. Neither side should be "unified" to the other —
+// changing one without the other silently breaks the documented parity
+// boundary. The component-side half of this contract is locked by
+// TestTokenizerComponent_ImportantKwd_CommaOnly in the component package.
+func TestCleanupConsumedChunkFields_ImportantKwdMultiDelimiter(t *testing.T) {
+	ck := map[string]any{"text": "hello", "keywords": "kw1,kw2;kw3，kw4"}
+
+	cleanupConsumedChunkFields(ck)
+
+	kwd, ok := ck["important_kwd"].([]string)
+	if !ok {
+		t.Fatalf("important_kwd should be []string, got %T", ck["important_kwd"])
+	}
+	// Executor fallback splits on comma/semicolon/CJK-comma and drops empties:
+	// "kw1,kw2;kw3，kw4" -> ["kw1","kw2","kw3","kw4"], NOT the component's
+	// ["kw1","kw2;kw3，kw4"].
+	want := []string{"kw1", "kw2", "kw3", "kw4"}
+	if len(kwd) != len(want) {
+		t.Fatalf("executor important_kwd = %v, want %v (multi-delimiter, empties dropped)", kwd, want)
+	}
+	for i := range want {
+		if kwd[i] != want[i] {
+			t.Errorf("executor important_kwd[%d] = %q, want %q", i, kwd[i], want[i])
+		}
+	}
+	if _, exists := ck["keywords"]; exists {
+		t.Error("keywords source field should be consumed/removed")
+	}
+}
+
+// TestCleanupConsumedChunkFields_ImportantKwdDropsEmptyParts documents that the
+// executor fallback drops empty parts (e.g. the middle empty token in
+// "a,,b"), diverging from the component path which PRESERVES it as ["a","","b"].
+// Together with the component CommaOnly test this locks the intentional
+// divergence: same input, different important_kwd arrays per layer.
+func TestCleanupConsumedChunkFields_ImportantKwdDropsEmptyParts(t *testing.T) {
+	ck := map[string]any{"text": "hello", "keywords": "a,,b"}
+
+	cleanupConsumedChunkFields(ck)
+
+	kwd, ok := ck["important_kwd"].([]string)
+	if !ok {
+		t.Fatalf("important_kwd should be []string, got %T", ck["important_kwd"])
+	}
+	// Executor drops the empty middle part: ["a","b"], NOT ["a","","b"].
+	want := []string{"a", "b"}
+	if len(kwd) != len(want) || kwd[0] != "a" || kwd[1] != "b" {
+		t.Fatalf("executor important_kwd = %v, want %v (empty parts dropped)", kwd, want)
+	}
+}

internal/utility/split.go

@@ -46,10 +46,17 @@ func nonEmpty(parts []string) []string {
 }
 
 // SplitKeywords splits a keywords string by common (ASCII + CJK) delimiters,
-// dropping empty elements. Returns nil for an empty input. It is the single
-// authority for materializing the important_kwd array from the keywords
-// string, shared by the Tokenizer component (in-pipeline) and the executor's
-// persist-schema mapping. Mirrors Python task_executor.run_dataflow:879.
+// dropping empty elements. Returns nil for an empty input. It is the authority
+// for materializing the important_kwd array in the executor's persist-schema
+// mapping (rag/flow/task_executor.py run_dataflow:879), which tolerates mixed
+// delimiters from older upstream producers.
+//
+// NOTE: the in-pipeline Tokenizer component does NOT use this function. For
+// DSL byte-compatibility it splits keywords on the ENGLISH COMMA ONLY via
+// strings.Split(kw, ",") and PRESERVES empty elements (mirroring Python
+// rag/flow/tokenizer/tokenizer.py:153 "a,,b".split(",")), so the two paths
+// intentionally diverge. See tokenizeChunks in
+// internal/ingestion/component/tokenizer.go.
 func SplitKeywords(keywords string) []string {
 	if keywords == "" {
 		return nil