mirror of
https://github.com/infiniflow/ragflow.git
synced 2026-08-08 00:18:12 +08:00
## 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
This commit is contained in:
@@ -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": "<table>caption</table>", "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.
|
||||
|
||||
Reference in New Issue
Block a user