From e95c81326eb566eb6006a4dab70f334ac4619c66 Mon Sep 17 00:00:00 2001 From: Jack Date: Thu, 6 Aug 2026 15:50:14 +0800 Subject: [PATCH] test(chunker): lock non-text segments as standalone on merge (closes #17889) (#17896) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Background Issue #17889 asks that, when merging adjacent segments, the chunker first checks each segment's type and only merges **text** segments — **table**, **image**, and any other non-text type must each remain a standalone chunk and must never be merged with a neighbouring segment. ## Why this PR closes #17889 (no Go code change required) After tracing the Go TokenChunker, the requirement is **already satisfied** on the structured (JSON / chunks) path. The type-aware rule is enforced at three layers in `internal/ingestion/component/chunker/`: - `common.go:138` `itemDocType` derives the type from `doc_type_kwd` (`"table"` -> `"table"`, `"image"` -> `"image"`, anything else -> `"text"`). It does **not** depend on the `ck_type` field being populated, so the type survives even when only `doc_type_kwd` is set (e.g. upstream Title/Group/Hierarchy chunks). - `token.go:756` `chunkFromItem` emits a non-text item as a single standalone chunk before the merge loop ever runs. - `token.go:1050` `mergeByTokenSizeFromJSON` forces any non-text chunk standalone (`if ck.CKType != "text"`); and `token.go:991` starts a *fresh* text chunk after a non-text chunk, so text on either side of a table/image is never merged across it. The only path without type information is the raw markdown/text/html string path (`PayloadFormatMarkdown/Text/HTML`), where the input is by contract an untyped string and `applyChildrenDelim` hard-codes `CKType: "text"` so merging is correct. There is no non-text segment to merge there, so this is out of #17889's scope (which is about the merge logic). ## Why the Python side is deferred The Python `naive` parser path does not thread a `ck_type` through to `merge_paragraphs` / `naive_merge` / `naive_merge_with_images` (`rag/nlp/__init__.py`): its parsers emit flat `(text, pos)` sections plus a parallel `section_images` list, and the type-aware `_merge_cks` rule (`rag/nlp/__init__.py:1749`) is only wired into the docx path. Propagating `ck_type` end-to-end across every Python parser is a large refactor, so it is intentionally **not** part of this PR. The Go engine is the active ingestion path, and it already honors the rule. ## This PR Adds a regression-lock (characterization) test, not a fix: - `TestTokenChunker_InvokeJSONPayload_KeepsNonTextStandalone` feeds a `[text, table, text, image, text]` structured payload and asserts it produces exactly five standalone chunks in the order `text, table, text, image, text` — proving tables/images stay standalone and text on either side is not merged across them. Verified green: ``` bash build.sh --test -run TestTokenChunker_InvokeJSONPayload_KeepsNonTextStandalone ./internal/ingestion/component/chunker/... --- PASS: TestTokenChunker_InvokeJSONPayload_KeepsNonTextStandalone (0.07s) ``` ## Related - Issue #17889 - PR #17808 (chunking refactor, merged) - Contract doc #17799 --- .../ingestion/component/chunker/token_test.go | 67 +++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/internal/ingestion/component/chunker/token_test.go b/internal/ingestion/component/chunker/token_test.go index 85588aa417..8da9c5d474 100644 --- a/internal/ingestion/component/chunker/token_test.go +++ b/internal/ingestion/component/chunker/token_test.go @@ -227,6 +227,73 @@ func TestTokenChunker_InvokeJSONPayload(t *testing.T) { } } +// TestTokenChunker_InvokeJSONPayload_KeepsNonTextStandalone is the +// regression lock for #17889: when merging adjacent segments, only +// "text" segments may be merged; "table"/"image" (any non-text type) +// must each stay a standalone chunk and must not be merged with a +// neighbouring segment. +// +// Go already enforces this via itemDocType (common.go:138, derives the +// type from doc_type_kwd), chunkFromItem (token.go:756, emits a non-text +// item as a single standalone chunk) and mergeByTokenSizeFromJSON +// (token.go:1050 forces non-text standalone; token.go:991 starts a fresh +// text chunk after a non-text chunk so text on either side of a +// table/image is never merged across it). This test pins the behaviour +// so a future refactor cannot silently start folding tables/images into +// text chunks. +func TestTokenChunker_InvokeJSONPayload_KeepsNonTextStandalone(t *testing.T) { + c, err := NewTokenChunker(map[string]any{ + "delimiter_mode": "delimiter", + "delimiters": []string{"\n"}, + }) + if err != nil { + t.Fatalf("NewTokenChunker: %v", err) + } + // text, table, text, image, text — in document order. + items := []map[string]any{ + {"text": "Alpha section text content", "doc_type_kwd": "text"}, + {"text": "caption
", "doc_type_kwd": "table"}, + {"text": "Beta section text content", "doc_type_kwd": "text"}, + {"text": "[image]", "doc_type_kwd": "image"}, + {"text": "Gamma section text content", "doc_type_kwd": "text"}, + } + out, err := c.Invoke(context.Background(), nil, map[string]any{ + "name": "doc.md", + "output_format": "json", + "json": items, + }) + if err != nil { + t.Fatalf("Invoke: %v", err) + } + chunks, ok := out["chunks"].([]map[string]any) + if !ok { + t.Fatalf("chunks: want []map[string]any, got %T", out["chunks"]) + } + // Each segment must remain its own chunk: 5 in, 5 out. If a table or + // image were merged into an adjacent text chunk this count would drop. + if len(chunks) != 5 { + t.Fatalf("chunks: want 5 (every segment standalone), got %d: %+v", len(chunks), chunks) + } + wantTypes := []string{"text", "table", "text", "image", "text"} + for i, ch := range chunks { + got, _ := ch["doc_type_kwd"].(string) + if got != wantTypes[i] { + t.Errorf("chunk %d: doc_type_kwd = %q, want %q (full chunk: %+v)", i, got, wantTypes[i], ch) + } + } + // The two text segments on either side of the table/image must remain + // distinct — they must NOT be merged across the non-text segments. + if got, _ := chunks[0]["text"].(string); !strings.Contains(got, "Alpha") { + t.Errorf("chunk 0 text = %q, want it to contain Alpha", got) + } + if got, _ := chunks[2]["text"].(string); !strings.Contains(got, "Beta") { + t.Errorf("chunk 2 text = %q, want it to contain Beta", got) + } + if got, _ := chunks[4]["text"].(string); !strings.Contains(got, "Gamma") { + t.Errorf("chunk 4 text = %q, want it to contain Gamma", got) + } +} + // TestTokenChunker_InvokeDeterministic runs a 20-item structured // payload 10 times under the race detector and asserts the chunk // list is identical every time.