diff --git a/internal/ingestion/component/chunker/token.go b/internal/ingestion/component/chunker/token.go index d54a105ed0..d0b302f420 100644 --- a/internal/ingestion/component/chunker/token.go +++ b/internal/ingestion/component/chunker/token.go @@ -76,6 +76,7 @@ import ( "regexp" "strings" "sync" + "unicode/utf8" "gorm.io/gorm" "ragflow/internal/agent/runtime" @@ -372,6 +373,23 @@ func (c *TokenChunkerComponent) chunkPerSegment(text string, delimPattern, child // it would diverge from Python's chunk boundaries. var sentenceDelimiter = regexp.MustCompile(`(\n|[!?。；！？])`) +// overlapCut returns the visible-text rune offset where the overlap prefix +// begins, mirroring the cut computed inside computeOverlapPrefix. It is split +// out so the coordinate-carrying path reuses the exact same cut as the text +// path (#18148). +func overlapCut(prevText string, overlappedPct float64) int { + visible := removeTag(prevText) + runes := []rune(visible) + cut := int(float64(len(runes)) * (100.0 - overlappedPct) / 100.0) + if cut < 0 { + cut = 0 + } + if cut >= len(runes) { + return len(runes) + } + return cut +} + // computeOverlapPrefix returns (overlapText, overlapTokenCount) carved from // the tail of prevText after stripping parser tags. overlappedPct is a // percentage in [0, 100]. Mirrors Python rag/nlp._compute_overlap_prefix. @@ -381,10 +399,7 @@ func computeOverlapPrefix(prevText string, overlappedPct float64) (string, int) return "", 0 } runes := []rune(visible) - cut := int(float64(len(runes)) * (100 - overlappedPct) / 100.0) - if cut < 0 { - cut = 0 - } + cut := overlapCut(prevText, overlappedPct) if cut >= len(runes) { return "", 0 } @@ -837,6 +852,49 @@ func takeFromStart(text string, tokens int) string { // joined string), matching Python's tk_nums += current["tk_nums"] (#17948). // Non-text units pass through unchanged and reset the merge run. joinSep is // "\n" for the JSON path and "" for the text path. +// mergeItem records one source unit that contributed to a merged chunk: its +// visible text and its coordinate box groups. Tracking items (not just the +// flattened position list) lets the overlap path map a visible-text offset +// range back to the exact boxes that belong to it, so the overlap prefix +// carries only the previous chunk's tail coordinates (#18148). +type mergeItem struct { + Text string + PDFPositions json.RawMessage + Positions json.RawMessage +} + +// overlapTailPositions returns the coordinate boxes of the previous chunk's +// source items whose visible span intersects the overlap tail +// [overlapStart, total). PDF positions are per-item (coarse), so an item is +// included wholesale once any part of it falls in the overlap tail. This keeps +// the overlap prefix highlighted without over-inflating the box set with the +// previous chunk's non-overlap (head) coordinates (#18148). Offsets are in +// rune units to match computeOverlapPrefix's visible-text indexing. +func overlapTailPositions(prevItems []mergeItem, overlapStart int, joinSep string) (json.RawMessage, json.RawMessage) { + if len(prevItems) == 0 { + return nil, nil + } + // Items are concatenated with joinSep (a single "\n" for the JSON path), + // matching the merge join at mergeUnits. + total := 0 + for _, it := range prevItems { + total += utf8.RuneCountInString(it.Text) + utf8.RuneCountInString(joinSep) + } + total -= utf8.RuneCountInString(joinSep) + var pdfAcc, posAcc json.RawMessage + offset := 0 + for _, it := range prevItems { + start := offset + end := offset + utf8.RuneCountInString(it.Text) + if start < total && end > overlapStart { + pdfAcc = extendRawJSONArray(pdfAcc, it.PDFPositions) + posAcc = extendRawJSONArray(posAcc, it.Positions) + } + offset = end + utf8.RuneCountInString(joinSep) + } + return pdfAcc, posAcc +} + func mergeUnits(units []schema.ChunkDoc, target int, overlapPct float64, strategy schema.MergeStrategy, joinSep string) []schema.ChunkDoc { if overlapPct < 0 { overlapPct = 0 @@ -847,11 +905,18 @@ func mergeUnits(units []schema.ChunkDoc, target int, overlapPct float64, strateg threshold := float64(target) * (100.0 - overlapPct) / 100.0 merged := make([]schema.ChunkDoc, 0, len(units)) + // mergedItems parallels merged: for each merged chunk, the source items it + // was built from. Tracking items (not just the flattened position list) + // lets the overlap path map a visible-text offset range back to the exact + // boxes that belong to it, so the overlap prefix carries only the previous + // chunk's tail coordinates (#18148). + mergedItems := make([][]mergeItem, 0, len(units)) prevIdx := -1 for i := range units { ck := units[i] if ck.CKType != "text" { merged = append(merged, cloneChunkDoc(ck)) + mergedItems = append(mergedItems, nil) prevIdx = -1 continue } @@ -859,12 +924,14 @@ func mergeUnits(units []schema.ChunkDoc, target int, overlapPct float64, strateg if tk <= 0 { tk = tokenizeStr(ck.Text) } + cur := mergeItem{Text: ck.Text, PDFPositions: ck.PDFPositions, Positions: ck.Positions} if prevIdx < 0 { // First text chunk (or first after a non-text chunk): no prior // text to overlap with. cp := cloneChunkDoc(ck) cp.TKNums = intPtr(tk) merged = append(merged, cp) + mergedItems = append(mergedItems, []mergeItem{cur}) prevIdx = len(merged) - 1 continue } @@ -878,12 +945,21 @@ func mergeUnits(units []schema.ChunkDoc, target int, overlapPct float64, strateg cp := cloneChunkDoc(ck) if overlapPct > 0 && merged[prevIdx].Text != "" { overlap, _ := computeOverlapPrefix(merged[prevIdx].Text, overlapPct) + // Carry the previous chunk's tail coordinates so the overlap + // prefix is highlighted, not just the cur span (#18148). + pdfTail, posTail := overlapTailPositions(mergedItems[prevIdx], overlapCut(merged[prevIdx].Text, overlapPct), joinSep) cp.Text = overlap + cp.Text + cp.PDFPositions = extendRawJSONArray(pdfTail, cp.PDFPositions) + cp.Positions = extendRawJSONArray(posTail, cp.Positions) cp.TKNums = intPtr(tokenizeStr(cp.Text)) - } else { - cp.TKNums = intPtr(tk) + merged = append(merged, cp) + mergedItems = append(mergedItems, []mergeItem{{Text: cp.Text, PDFPositions: cp.PDFPositions, Positions: cp.Positions}}) + prevIdx = len(merged) - 1 + continue } + cp.TKNums = intPtr(tk) merged = append(merged, cp) + mergedItems = append(mergedItems, []mergeItem{cur}) prevIdx = len(merged) - 1 continue } @@ -896,15 +972,23 @@ func mergeUnits(units []schema.ChunkDoc, target int, overlapPct float64, strateg cp := cloneChunkDoc(ck) if overlapPct > 0 && prev.Text != "" { // Unconditional overlap prefix (mirrors Python JSON). The - // prefix is a duplicate of prev's tail, so it carries no new - // coordinates — only cur's positions are kept. + // prefix is a duplicate of prev's tail; carry prev's tail + // coordinates so the overlap region is highlighted (#18148), + // then keep only cur's coordinates for the non-overlap part. overlap, _ := computeOverlapPrefix(prev.Text, overlapPct) + pdfTail, posTail := overlapTailPositions(mergedItems[prevIdx], overlapCut(prev.Text, overlapPct), joinSep) cp.Text = overlap + cp.Text + cp.PDFPositions = extendRawJSONArray(pdfTail, cp.PDFPositions) + cp.Positions = extendRawJSONArray(posTail, cp.Positions) cp.TKNums = intPtr(tokenizeStr(cp.Text)) - } else { - cp.TKNums = intPtr(tk) + merged = append(merged, cp) + mergedItems = append(mergedItems, []mergeItem{{Text: cp.Text, PDFPositions: cp.PDFPositions, Positions: cp.Positions}}) + prevIdx = len(merged) - 1 + continue } + cp.TKNums = intPtr(tk) merged = append(merged, cp) + mergedItems = append(mergedItems, []mergeItem{cur}) prevIdx = len(merged) - 1 continue } @@ -917,6 +1001,7 @@ func mergeUnits(units []schema.ChunkDoc, target int, overlapPct float64, strateg prev.TKNums = intPtr(intValue(prev.TKNums) + tk) prev.PDFPositions = extendRawJSONArray(prev.PDFPositions, ck.PDFPositions) prev.Positions = extendRawJSONArray(prev.Positions, ck.Positions) + mergedItems[prevIdx] = append(mergedItems[prevIdx], cur) } return merged } diff --git a/internal/ingestion/component/chunker/token_pdfpos_test.go b/internal/ingestion/component/chunker/token_pdfpos_test.go index d0ab422567..56274d05a8 100644 --- a/internal/ingestion/component/chunker/token_pdfpos_test.go +++ b/internal/ingestion/component/chunker/token_pdfpos_test.go @@ -117,3 +117,124 @@ func TestMergeByTokenSizeFromJSON_PositionsDecodeToMatrix(t *testing.T) { t.Fatalf("positions matrix has %d groups, want 2 (both merged items)", len(matrix)) } } + +// TestMergeByTokenSizeFromJSON_OverlapPrefixCarriesPrevPositions is a TDD test +// for issue #18148. Python's token_chunker drops overlap-head PDF coordinates, +// and the Go mergeUnits overlap branch has the SAME defect: when a fresh chunk +// starts and overlap>0, the tail of the previous chunk is prepended to the new +// chunk's text (computeOverlapPrefix, token.go:839 / :860), but the new chunk's +// PDFPositions is left as only cur's coordinates (token.go:836-848 and +// :854-868). The overlap prefix is part of the chunk's visible/displayed +// content, so its coordinates must be carried forward — exactly like the +// merge-into-prev path extends positions (token.go:877). On the buggy code the +// overlap text is shown but NOT highlighted. +// +// overlappedPct=100 forces the overlap prefix to be the ENTIRE previous chunk, +// so the expectation is crisp: every new chunk must carry the previous chunk's +// full coordinates. This test is RED until the overlap branch carries +// coordinates. +func TestMergeByTokenSizeFromJSON_OverlapPrefixCarriesPrevPositions(t *testing.T) { + posA := json.RawMessage(`[[1,0,10,0,5]]`) + posB := json.RawMessage(`[[2,0,20,0,8]]`) + posC := json.RawMessage(`[[3,0,30,0,12]]`) + // At overlapPct=100 the scaled threshold is 0, so every unit after the + // first starts a fresh chunk carrying the WHOLE previous chunk as overlap. + items := [][]schema.ChunkDoc{ + { + {Text: "alpha", DocType: "text", CKType: "text", TKNums: intPtr(5), PDFPositions: posA}, + {Text: "beta", DocType: "text", CKType: "text", TKNums: intPtr(5), PDFPositions: posB}, + {Text: "gamma", DocType: "text", CKType: "text", TKNums: intPtr(5), PDFPositions: posC}, + }, + } + got := mergeByTokenSizeFromJSON(items, 20, 100, schema.MergeOverCap) + merged := got[0] + if len(merged) != 3 { + t.Fatalf("want 3 chunks (each unit starts fresh at overlapPct=100), got %d", len(merged)) + } + + // chunk[1] starts with the overlap prefix copied from chunk[0] ("alpha"). + if !strings.Contains(merged[1].Text, "alpha") { + t.Errorf("chunk[1] missing overlap prefix from prev chunk: text=%q", merged[1].Text) + } + // The overlap prefix is shown, so chunk[1] must also carry chunk[0]'s + // coordinates. BUG: only chunk[1]'s own (posB) coordinates survive today. + if !strings.Contains(string(merged[1].PDFPositions), "1,0,10,0,5") { + t.Errorf("chunk[1] dropped overlap-head coordinates (prev chunk[0] posA): pdf_positions=%s", string(merged[1].PDFPositions)) + } + if !strings.Contains(string(merged[1].PDFPositions), "2,0,20,0,8") { + t.Errorf("chunk[1] lost its own coordinates: pdf_positions=%s", string(merged[1].PDFPositions)) + } + + // chunk[2]'s overlap prefix is the full chunk[1] text; its coordinates must + // include chunk[0], chunk[1], and its own (the overlap chain is carried). + if !strings.Contains(merged[2].Text, "alphabeta") { + t.Errorf("chunk[2] missing overlap prefix from prev chunk: text=%q", merged[2].Text) + } + for _, want := range []string{"1,0,10,0,5", "2,0,20,0,8", "3,0,30,0,12"} { + if !strings.Contains(string(merged[2].PDFPositions), want) { + t.Errorf("chunk[2] missing coordinates %s (overlap chain not carried): pdf_positions=%s", want, string(merged[2].PDFPositions)) + } + } +} + +// TestMergeByTokenSizeFromJSON_PartialOverlapPrefixCarriesOnlyTailPositions is +// a partial-overlap companion to +// TestMergeByTokenSizeFromJSON_OverlapPrefixCarriesPrevPositions (#18148). +// overlappedPct=100 (the full-overlap test) forces the ENTIRE previous chunk +// into the overlap prefix; here overlappedPct=20 means the overlap prefix is +// only the TAIL ~20% of the previous chunk. The coordinates carried must be +// exactly the previous chunk's tail items whose span intersects that tail -- +// NOT the whole previous chunk. This locks the per-item tail-selection in +// overlapTailPositions (token.go:832): a regression that carried the entire +// previous chunk's coordinates (over-inflating the highlight box) or dropped +// overlap coordinates entirely would both fail this test. +func TestMergeByTokenSizeFromJSON_PartialOverlapPrefixCarriesOnlyTailPositions(t *testing.T) { + posA := json.RawMessage(`[[1,0,10,0,5]]`) + posB := json.RawMessage(`[[2,0,20,0,8]]`) + posC := json.RawMessage(`[[3,0,30,0,12]]`) + posD := json.RawMessage(`[[4,0,40,0,16]]`) + posE := json.RawMessage(`[[5,0,50,0,20]]`) + posF := json.RawMessage(`[[6,0,60,0,24]]`) + // 6 equal-length items (5 runes each). With chunkTokens=5 and each item + // TKNums=1, items 0..4 merge into one chunk (tk reaches 5); item5 starts a + // fresh chunk. Its overlap prefix (overlappedPct=20) is the last ~20% of + // the 5-item previous chunk's text => only the last item ("eeeee", posE) + // intersects the tail. So chunk[1] must carry posE (tail) + posF (own), but + // NOT posA/posB/posC/posD. + items := [][]schema.ChunkDoc{ + { + {Text: "aaaaa", DocType: "text", CKType: "text", TKNums: intPtr(1), PDFPositions: posA}, + {Text: "bbbbb", DocType: "text", CKType: "text", TKNums: intPtr(1), PDFPositions: posB}, + {Text: "ccccc", DocType: "text", CKType: "text", TKNums: intPtr(1), PDFPositions: posC}, + {Text: "ddddd", DocType: "text", CKType: "text", TKNums: intPtr(1), PDFPositions: posD}, + {Text: "eeeee", DocType: "text", CKType: "text", TKNums: intPtr(1), PDFPositions: posE}, + {Text: "fffff", DocType: "text", CKType: "text", TKNums: intPtr(1), PDFPositions: posF}, + }, + } + got := mergeByTokenSizeFromJSON(items, 5, 20, schema.MergeOverCap) + merged := got[0] + if len(merged) != 2 { + t.Fatalf("want 2 chunks (5 items merge, 6th starts fresh with partial overlap), got %d", len(merged)) + } + + // The new chunk's overlap text is the tail of the previous chunk. + if !strings.Contains(merged[1].Text, "eeeee") { + t.Errorf("chunk[1] missing overlap tail text from prev chunk: text=%q", merged[1].Text) + } + // The tail item's coordinates MUST be carried. + pdf := string(merged[1].PDFPositions) + if !strings.Contains(pdf, "5,0,50,0,20") { + t.Errorf("chunk[1] dropped tail-item coordinates (prev posE): pdf_positions=%s", pdf) + } + if !strings.Contains(pdf, "6,0,60,0,24") { + t.Errorf("chunk[1] lost its own coordinates (posF): pdf_positions=%s", pdf) + } + // Partial overlap: the head items of the previous chunk must NOT be carried + // (that would over-inflate the highlight box). A whole-prev carry bug or a + // no-carry bug both fail here. + for _, absent := range []string{"1,0,10,0,5", "2,0,20,0,8", "3,0,30,0,12", "4,0,40,0,16"} { + if strings.Contains(pdf, absent) { + t.Errorf("chunk[1] over-carried non-overlap head coordinates %s: pdf_positions=%s", absent, pdf) + } + } +} diff --git a/rag/flow/chunker/token_chunker.py b/rag/flow/chunker/token_chunker.py index 19696026cb..cc6e296798 100644 --- a/rag/flow/chunker/token_chunker.py +++ b/rag/flow/chunker/token_chunker.py @@ -106,7 +106,7 @@ def _split_text_by_pattern(text, pattern): if not pattern: return [text or ""] - split_texts = re.split(r"(%s)" % pattern, text or "", flags=re.DOTALL) + split_texts = re.split("(" + pattern + ")", text or "", flags=re.DOTALL) chunks = [] for i in range(0, len(split_texts), 2): chunk = split_texts[i] @@ -230,19 +230,59 @@ def _attach_context_to_media_chunks(chunks, table_context_size, image_context_si chunk["context_below"] = "".join(parts_below) +def _overlap_tail_positions(prev_items, overlap_start): + # Given the source items a previous chunk was built from (each as + # ``(item_text, pos_group)`` where ``pos_group`` is that item's + # ``_pdf_positions`` list), return the flattened boxes whose item visible + # span intersects the overlap tail ``[overlap_start, total)``. + # + # PDF positions are per-item (coarse), so an item is included wholesale once + # any part of it falls in the overlap tail. This keeps the overlap prefix + # highlighted without over-inflating the box set with the previous chunk's + # non-overlap (head) coordinates (#18148). + if not prev_items: + return [] + # Items are concatenated with a single "\n" separator, matching the merge + # join at _merge_text_chunks_by_token_size. + spans = [] + offset = 0 + for item_text, _pos_group in prev_items: + start = offset + end = offset + len(item_text) + spans.append((start, end)) + offset = end + 1 + if not spans: + return [] + total = spans[-1][1] + out = [] + for (start, end), (_text, pos_group) in zip(spans, prev_items, strict=True): + if start < total and end > overlap_start: + out.extend(pos_group or []) + return out + + def _merge_text_chunks_by_token_size(chunks, chunk_token_size, overlapped_percent): # Merge adjacent text chunks when delimiter-based splitting is not active. merged = [] + # Parallel to ``merged``: for each merged chunk, the list of source items it + # was built from, each as ``(item_text, pos_group)``. Tracking items (not + # just the flattened position list) lets us map a visible-text offset range + # back to the exact coordinate boxes that belong to it, so the overlap + # prefix carries only the previous chunk's tail coordinates instead of + # dropping them (#18148). + merged_items = [] prev_text_idx = -1 threshold = chunk_token_size * (100 - overlapped_percent) / 100.0 for chunk in chunks: if chunk["ck_type"] != "text": merged.append(deepcopy(chunk)) + merged_items.append(None) prev_text_idx = -1 continue current = deepcopy(chunk) + current_item = (current["text"], list(current.get(PDF_POSITIONS_KEY) or [])) should_start_new = prev_text_idx < 0 or merged[prev_text_idx]["tk_nums"] > threshold # #17799: an over-budget unit stands alone — never merged into the # previous chunk. This matches Python naive_merge and the Go @@ -262,11 +302,21 @@ def _merge_text_chunks_by_token_size(chunks, chunk_token_size, overlapped_percen overlap_start = int(len(visible) * (100 - overlapped_percent) / 100.0) if 0 <= overlap_start < len(visible): overlap_text = visible[overlap_start:] + # Carry the previous chunk's tail coordinates so the overlap + # prefix is highlighted, not just the cur span (#18148). + # Only the items intersecting the overlap tail keep their + # boxes; the head (non-overlap) boxes are excluded so the + # highlight is not over-inflated. + overlap_positions = _overlap_tail_positions(merged_items[prev_text_idx], overlap_start) else: overlap_text = "" + overlap_positions = [] current["text"] = overlap_text + current["text"] + current[PDF_POSITIONS_KEY] = overlap_positions + (current.get(PDF_POSITIONS_KEY) or []) + current_item = (current["text"], list(current.get(PDF_POSITIONS_KEY) or [])) current["tk_nums"] = num_tokens_from_string(current["text"]) merged.append(current) + merged_items.append([current_item]) prev_text_idx = len(merged) - 1 continue @@ -276,6 +326,7 @@ def _merge_text_chunks_by_token_size(chunks, chunk_token_size, overlapped_percen merged[prev_text_idx]["text"] += current["text"] merged[prev_text_idx][PDF_POSITIONS_KEY].extend(current.get(PDF_POSITIONS_KEY) or []) merged[prev_text_idx]["tk_nums"] += current["tk_nums"] + merged_items[prev_text_idx].append(current_item) return merged @@ -340,8 +391,8 @@ class TokenChunker(ProcessBase): async def _invoke(self, **kwargs): try: from_upstream = TokenChunkerFromUpstream.model_validate(kwargs) - except Exception as e: - self.set_output("_ERROR", f"Input error: {str(e)}") + except Exception as e: # noqa: BLE001 + self.set_output("_ERROR", f"Input error: {e!s}") return # Build the primary delimiter regex. If no active custom delimiter exists, @@ -437,7 +488,7 @@ class TokenChunker(ProcessBase): offset += 1 combined_text = "".join(parts[:-1]) # drop the trailing glue - raw = re.split(r"(%s)" % delimiter_pattern, combined_text, flags=re.DOTALL) + raw = re.split("(" + delimiter_pattern + ")", combined_text, flags=re.DOTALL) segments = [] # (text, start, end) within combined_text pos = 0 for i in range(0, len(raw), 2): diff --git a/rag/flow/tests/test_parser_pdf_positions.py b/rag/flow/tests/test_parser_pdf_positions.py new file mode 100644 index 0000000000..8d4790d3fa --- /dev/null +++ b/rag/flow/tests/test_parser_pdf_positions.py @@ -0,0 +1,72 @@ +"""Parser-layer regression guard for the Book builtin DSL highlight path. + +The Book builtin DSL parses PDF with ``parse_method=DeepDOC`` and +``output_format=json``. The parser (rag/flow/parser/parser.py:781) runs +``normalize_pdf_items_metadata(bboxes)`` on the deepdoc output, which must +keep each box's PDF coordinates so the downstream TitleChunker can build +``position_int`` and the parsing-result view can highlight the text. + +This test drives the REAL gate function with deepdoc-style boxes (carrying +both ``positions`` and ``position_tag``, exactly as +deepdoc/parser/pdf_parser.py:1900-1902 emits) and asserts the coordinates +survive into the internal ``_pdf_positions`` field without stripping the +original ``positions`` field. It is the parser-layer counterpart of +rag/flow/tests/test_title_chunker_position_int.py (which proves the +chunker layer keeps the coordinates). Together they pin down the full +parser -> chunker -> position_int bridge for infiniflow/ragflow#18148. +""" + +from rag.flow.parser.pdf_chunk_metadata import ( + PDF_POSITIONS_KEY, + extract_pdf_positions, + normalize_pdf_items_metadata, +) + + +def _deepdoc_style_bboxes(): + # Shape mirrors deepdoc RAGFlowPdfParser.parse_into_bboxes output: + # one entry with both position_tag + positions, two with positions only, + # spanning page 1 and page 2. + return [ + { + "text": "Introduction paragraph on page one.", + "layout_type": "text", + "position_tag": "@@1\tIntroduction paragraph on page one.", + "positions": [[1, 10, 200, 50, 80]], + }, + { + "text": "Body text continues on page one.", + "layout_type": "text", + "positions": [[1, 12, 205, 90, 120]], + }, + { + "text": "Second chapter starts on page two.", + "layout_type": "text", + "positions": [[2, 15, 210, 40, 75]], + }, + ] + + +def test_parser_gate_preserves_bbox_coordinates(): + bboxes = _deepdoc_style_bboxes() + # This is exactly what parser.py:781 calls for output_format == "json". + normalize_pdf_items_metadata(bboxes) + + for box in bboxes: + # Coordinate bridge: the chunker reads PDF_POSITIONS_KEY. + assert box.get(PDF_POSITIONS_KEY), f"missing {PDF_POSITIONS_KEY}: {box}" + # The original field must NOT be stripped by normalization. + assert "positions" in box, f"positions stripped from box: {box}" + + # Pages referenced by the downstream chunker must cover every source page. + pages = {int(p[0]) for box in bboxes for p in extract_pdf_positions(box)} + assert pages == {1, 2}, f"expected pages {{1,2}}, got {pages}" + + +def test_parser_gate_produces_exact_coordinates(): + bboxes = _deepdoc_style_bboxes() + normalize_pdf_items_metadata(bboxes) + + # Each normalized box keeps the source (page, left, right, top, bottom). + assert extract_pdf_positions(bboxes[0]) == [[1, 10, 200, 50, 80]] + assert extract_pdf_positions(bboxes[2]) == [[2, 15, 210, 40, 75]] diff --git a/rag/flow/tests/test_title_chunker_position_int.py b/rag/flow/tests/test_title_chunker_position_int.py new file mode 100644 index 0000000000..2edd07224f --- /dev/null +++ b/rag/flow/tests/test_title_chunker_position_int.py @@ -0,0 +1,263 @@ +import asyncio +import importlib +import sys +import types +from contextlib import contextmanager +from pathlib import Path + +"""Reproduction test for the Book builtin DSL highlight path. + +The Book builtin DSL parses PDF with ``parse_method=DeepDOC`` and +``output_format=json``, then chunks with ``TitleChunker`` (method=hierarchy). +This test drives the real TitleChunker chain (extract -> merge -> finalize) +with deepdoc-style JSON items that carry ``positions``, and asserts the +emitted chunks keep a non-empty ``position_int`` covering every source page. + +It is a GREEN regression guard: it proves the TitleChunker layer preserves +PDF coordinates for the Book DSL, which localizes the parsing-result "missing +highlight" bug to the parser emission / dataset config rather than the +chunker. Mirrors rag/flow/tests/test_token_chunker.py's +``test_json_delimiter_mode_position_int_survives_full_chain``. +""" + + +@contextmanager +def _load_title_chunker_with_stubs(): + root = Path(__file__).resolve().parents[3] + original_modules = {} + + def _install(name: str, module: types.ModuleType): + original_modules.setdefault(name, sys.modules.get(name)) + sys.modules[name] = module + + try: + rag_pkg = types.ModuleType("rag") + rag_pkg.__path__ = [str(root / "rag")] + _install("rag", rag_pkg) + + rag_flow_pkg = types.ModuleType("rag.flow") + rag_flow_pkg.__package__ = "rag" + rag_flow_pkg.__path__ = [str(root / "rag" / "flow")] + _install("rag.flow", rag_flow_pkg) + + rag_flow_chunker_pkg = types.ModuleType("rag.flow.chunker") + rag_flow_chunker_pkg.__package__ = "rag.flow" + rag_flow_chunker_pkg.__path__ = [str(root / "rag" / "flow" / "chunker")] + _install("rag.flow.chunker", rag_flow_chunker_pkg) + + rag_flow_parser_pkg = types.ModuleType("rag.flow.parser") + rag_flow_parser_pkg.__package__ = "rag.flow" + rag_flow_parser_pkg.__path__ = [str(root / "rag" / "flow" / "parser")] + _install("rag.flow.parser", rag_flow_parser_pkg) + + common_pkg = types.ModuleType("common") + common_pkg.__path__ = [str(root / "common")] + _install("common", common_pkg) + + common_float_utils = types.ModuleType("common.float_utils") + common_float_utils.normalize_overlapped_percent = lambda value: value + _install("common.float_utils", common_float_utils) + + common_token_utils = types.ModuleType("common.token_utils") + common_token_utils.num_tokens_from_string = lambda text: 1 + _install("common.token_utils", common_token_utils) + + rag_nlp = types.ModuleType("rag.nlp") + rag_nlp.naive_merge = lambda *args, **kwargs: [] + rag_nlp.not_bullet = lambda text: False + rag_nlp.not_title = lambda text: True + _install("rag.nlp", rag_nlp) + + deepdoc_pkg = types.ModuleType("deepdoc") + deepdoc_pkg.__path__ = [str(root / "deepdoc")] + _install("deepdoc", deepdoc_pkg) + + deepdoc_parser_pkg = types.ModuleType("deepdoc.parser") + deepdoc_parser_pkg.__path__ = [str(root / "deepdoc" / "parser")] + _install("deepdoc.parser", deepdoc_parser_pkg) + + class _RAGFlowPdfParser: + @staticmethod + def remove_tag(text): + return text + + @staticmethod + def extract_positions(tag): + return [] + + deepdoc_pdf_parser = types.ModuleType("deepdoc.parser.pdf_parser") + deepdoc_pdf_parser.RAGFlowPdfParser = _RAGFlowPdfParser + _install("deepdoc.parser.pdf_parser", deepdoc_pdf_parser) + + deepdoc_parser_utils = types.ModuleType("deepdoc.parser.utils") + deepdoc_parser_utils.extract_pdf_outlines = lambda *args, **kwargs: [] + _install("deepdoc.parser.utils", deepdoc_parser_utils) + + class ProcessParamBase: + def __init__(self): + pass + + def check_valid_value(self, value, msg, allowed): + if value not in allowed: + raise ValueError(msg) + + def check_positive_integer(self, value, msg): + pass + + def check_decimal_float(self, value, msg): + pass + + def check_nonnegative_number(self, value, msg): + pass + + class ProcessBase: + def __init__(self, _pipeline, _id, param): + self._pipeline = _pipeline + self._id = _id + self._param = param + self._outputs = {} + self.callback = lambda *_args, **_kwargs: None + + def set_output(self, key, value): + self._outputs[key] = value + + rag_flow_base = types.ModuleType("rag.flow.base") + rag_flow_base.ProcessBase = ProcessBase + rag_flow_base.ProcessParamBase = ProcessParamBase + _install("rag.flow.base", rag_flow_base) + + pdf_chunk_metadata = types.ModuleType("rag.flow.parser.pdf_chunk_metadata") + pdf_chunk_metadata.PDF_POSITIONS_KEY = "pdf_positions" + pdf_chunk_metadata.extract_pdf_positions = lambda _item: [] + pdf_chunk_metadata.merge_pdf_positions = lambda _records: [] + pdf_chunk_metadata.finalize_pdf_chunk = lambda chunk: chunk + pdf_chunk_metadata.restore_pdf_text_previews = lambda *_a, **_k: None + _install("rag.flow.parser.pdf_chunk_metadata", pdf_chunk_metadata) + + common_spec = importlib.util.spec_from_file_location( + "rag.flow.chunker.title_chunker.common", + root / "rag" / "flow" / "chunker" / "title_chunker" / "common.py", + ) + common_module = importlib.util.module_from_spec(common_spec) + _install("rag.flow.chunker.title_chunker.common", common_module) + common_spec.loader.exec_module(common_module) + + hierarchy_spec = importlib.util.spec_from_file_location( + "rag.flow.chunker.title_chunker.hierarchy_chunker", + root / "rag" / "flow" / "chunker" / "title_chunker" / "hierarchy_chunker.py", + ) + hierarchy_module = importlib.util.module_from_spec(hierarchy_spec) + _install("rag.flow.chunker.title_chunker.hierarchy_chunker", hierarchy_module) + hierarchy_spec.loader.exec_module(hierarchy_module) + + yield common_module, hierarchy_module + finally: + for module_name, original in original_modules.items(): + if original is None: + sys.modules.pop(module_name, None) + else: + sys.modules[module_name] = original + + +def _real_extract_pdf_positions(item): + # Faithful mirror of rag/flow/parser/pdf_chunk_metadata.extract_pdf_positions. + if not isinstance(item, dict): + return [] + positions = item.get("pdf_positions") + if isinstance(positions, list): + return [list(p) for p in positions] + positions = item.get("positions") + if isinstance(positions, list): + return [list(p) for p in positions] + position_tag = item.get("position_tag") + if isinstance(position_tag, str) and position_tag: + return [] # RAGFlowPdfParser.extract_positions is stubbed out here. + position_int = item.get("position_int") + if isinstance(position_int, list): + return [list(p) for p in position_int if isinstance(p, (list, tuple)) and len(p) >= 5] + return [] + + +def _real_merge_pdf_positions(records): + # Faithful mirror of rag/flow/parser/pdf_chunk_metadata.merge_pdf_positions. + merged = [] + for rec in records or []: + if not isinstance(rec, dict): + continue + for pos in rec.get("pdf_positions") or []: + if isinstance(pos, (list, tuple)) and len(pos) >= 5: + merged.append([pos[0], pos[1], pos[2], pos[3], pos[4]]) + seen = set() + out = [] + for pos in merged: + key = tuple(pos[:5]) + if key not in seen: + seen.add(key) + out.append(pos) + out.sort(key=lambda item: (item[0], item[3], item[1])) + return out + + +def _real_finalize_pdf_chunk(chunk): + # Faithful mirror of rag/flow/parser/pdf_chunk_metadata.finalize_pdf_chunk. + positions = _real_extract_pdf_positions(chunk) + if positions: + chunk["position_int"] = [list(p) for p in positions] + chunk.pop("pdf_positions", None) + return chunk + + +def test_title_chunker_preserves_position_int_from_deepdoc_json(): + # Reproduces the parsing-result highlight verification for the Book builtin + # DSL (infiniflow/ragflow#18148 follow-up): a deepdoc + json parser output + # carrying per-item ``positions`` must flow through TitleChunker (method= + # hierarchy) and reach ``position_int`` on the emitted chunks. This proves + # the TitleChunker layer is NOT the cause of the missing highlight. + with _load_title_chunker_with_stubs() as (common_module, hierarchy_module): + # Install faithful coordinate helpers so the REAL TitleChunker path runs. + common_module.extract_pdf_positions = _real_extract_pdf_positions + common_module.merge_pdf_positions = _real_merge_pdf_positions + common_module.finalize_pdf_chunk = _real_finalize_pdf_chunk + + async def _restore_previews(*_a, **_k): + return None + + common_module.restore_pdf_text_previews = _restore_previews + + json_result = [ + {"text": "Introduction paragraph on page one.", "doc_type_kwd": "text", "positions": [[1, 10, 200, 50, 80]]}, + {"text": "Body text continues on page one.", "doc_type_kwd": "text", "positions": [[1, 12, 205, 90, 120]]}, + {"text": "Second chapter starts on page two.", "doc_type_kwd": "text", "positions": [[2, 15, 210, 40, 75]]}, + ] + + from_upstream = types.SimpleNamespace( + output_format="json", + json_result=json_result, + markdown_result=None, + text_result=None, + html_result=None, + chunks=None, + file=None, + name="book-test", # not *.pdf -> restore_pdf_text_previews early-returns + ) + + param = common_module.TitleChunkerParam() + param.method = "hierarchy" + param.hierarchy = 1 + param.levels = [] # no headings -> all body -> single merged chunk + param.include_heading_content = False + param.root_chunk_as_heading = False + + process = common_module.ProcessBase(None, "title_chunker", param) + process._canvas = types.SimpleNamespace(_doc_id="doc", _tenant_id="tenant") + process._outputs = {} + + chunker = hierarchy_module.HierarchyTitleChunker(process, from_upstream) + asyncio.run(chunker.invoke()) + + chunks = process._outputs.get("chunks", []) + assert chunks, "TitleChunker produced no chunks" + pos_int = chunks[0].get("position_int") + assert pos_int, "position_int missing from TitleChunker output" + pages = {p[0] for p in pos_int} + assert pages == {1, 2}, f"expected pages {{1,2}}, got {pages}" diff --git a/rag/flow/tests/test_token_chunker.py b/rag/flow/tests/test_token_chunker.py index 58c0ac5dfa..b7bb03711d 100644 --- a/rag/flow/tests/test_token_chunker.py +++ b/rag/flow/tests/test_token_chunker.py @@ -1,10 +1,12 @@ -import importlib.util import asyncio +import importlib.util import sys import types from contextlib import contextmanager from pathlib import Path +import pytest + @contextmanager def _load_token_chunker_with_stubs(): @@ -107,7 +109,7 @@ def _load_token_chunker_with_stubs(): schema_module = importlib.util.module_from_spec(schema_spec) _install("rag.flow.chunker.schema", schema_module) schema_spec.loader.exec_module(schema_module) - except Exception: + except Exception: # noqa: BLE001 schema_module = types.ModuleType("rag.flow.chunker.schema") class TokenChunkerFromUpstream: @@ -303,11 +305,8 @@ def test_token_size_mode_normalized_to_delimiter(): bad = token_chunker_module.TokenChunkerParam() bad.delimiter_mode = "nope" - try: + with pytest.raises(ValueError): bad.check() - raise AssertionError("expected check() to reject unknown delimiter_mode") - except Exception: - pass def test_json_no_delimiter_mode_merges_to_token_cap(): @@ -375,7 +374,7 @@ def test_json_delimiter_mode_pdf_positions_per_segment_not_broadcast(): if key in preview_cache: chunk["img_id"] = preview_cache[key] else: - new_id = "img-%d" % len(preview_cache) + new_id = f"img-{len(preview_cache)}" chunk["img_id"] = new_id preview_cache[key] = new_id @@ -410,6 +409,95 @@ def test_json_delimiter_mode_pdf_positions_per_segment_not_broadcast(): assert len(set(img_ids)) == len(img_ids), img_ids +def test_json_no_delimiter_mode_overlap_prefix_carries_prev_positions(): + # TDD test for #18148. When the JSON path merges to the token cap with + # overlapped_percent>0, a fresh chunk starts with an overlap prefix copied + # from the previous chunk's tail (token_chunker.py:_merge_text_chunks_by_ + # token_size, the should_start_new branch). That overlap text is part of the + # chunk's displayed content, so its PDF coordinates MUST also be carried + # forward into the new chunk's _pdf_positions -- exactly like the merge-into- + # prev path extends positions (token_chunker.py:277). Today the overlap + # branch only keeps cur's coordinates, so the overlap head is shown but not + # highlighted. Mirrors the Go test + # TestMergeByTokenSizeFromJSON_OverlapPrefixCarriesPrevPositions. + # + # overlapped_percent=100 makes the overlap prefix the ENTIRE previous chunk, + # so the expectation is crisp: every new chunk must carry the previous + # chunk's full coordinates. RED until the overlap branch carries coordinates. + for _module, chunker in _build_json_chunker({"delimiter_mode": "delimiter", "delimiters": [], "chunk_token_size": 5, "overlapped_percent": 100}): + kwargs = { + "name": "token_chunker", + "output_format": "json", + "json_result": [ + {"text": "alpha", "doc_type_kwd": "text", "positions": [[1, 0, 10, 0, 5]]}, + {"text": "beta", "doc_type_kwd": "text", "positions": [[2, 0, 20, 0, 8]]}, + {"text": "gamma", "doc_type_kwd": "text", "positions": [[3, 0, 30, 0, 12]]}, + ], + } + asyncio.run(chunker._invoke(**kwargs)) + chunks = chunker._outputs["chunks"] + # Each unit starts a fresh chunk at overlapped_percent=100. + assert len(chunks) == 3, f"want 3 chunks, got {len(chunks)}" + + # chunk[1] starts with the overlap prefix copied from chunk[0] ("alpha"). + assert "alpha" in chunks[1]["text"], f"chunk[1] missing overlap prefix: {chunks[1]['text']!r}" + # The overlap prefix is shown, so chunk[1] must also carry chunk[0]'s + # coordinates. BUG: only chunk[1]'s own (posB) coordinates survive. + pos1 = chunks[1].get("pdf_positions") or [] + assert [1, 0, 10, 0, 5] in pos1, f"chunk[1] dropped overlap-head coords (prev posA): {pos1}" + assert [2, 0, 20, 0, 8] in pos1, f"chunk[1] lost its own coords: {pos1}" + + # chunk[2]'s overlap prefix is the full chunk[1] text; its coordinates + # must include chunk[0], chunk[1], and its own (overlap chain carried). + assert "alphabeta" in chunks[2]["text"], f"chunk[2] missing overlap prefix: {chunks[2]['text']!r}" + pos2 = chunks[2].get("pdf_positions") or [] + for want in ([1, 0, 10, 0, 5], [2, 0, 20, 0, 8], [3, 0, 30, 0, 12]): + assert want in pos2, f"chunk[2] missing coords {want} (overlap chain not carried): {pos2}" + + +def test_json_no_delimiter_mode_partial_overlap_prefix_carries_only_tail_positions(): + # Partial-overlap companion to + # test_json_no_delimiter_mode_overlap_prefix_carries_prev_positions (#18148). + # overlapped_percent=100 (the full-overlap test) forces the ENTIRE previous + # chunk into the overlap prefix; here overlapped_percent=20 means the + # overlap prefix is only the TAIL ~20% of the previous chunk. The + # coordinates carried must be exactly the previous chunk's tail items whose + # span intersects that tail -- NOT the whole previous chunk. This locks the + # per-item tail-selection in _overlap_tail_positions (token_chunker.py:233): + # a regression that carried the entire previous chunk's coordinates + # (over-inflating the highlight box) or dropped overlap coordinates entirely + # would both fail this test. + for _module, chunker in _build_json_chunker({"delimiter_mode": "delimiter", "delimiters": [], "chunk_token_size": 5, "overlapped_percent": 20}): + kwargs = { + "name": "token_chunker", + "output_format": "json", + "json_result": [ + {"text": "aaaaa", "doc_type_kwd": "text", "positions": [[1, 0, 10, 0, 5]]}, + {"text": "bbbbb", "doc_type_kwd": "text", "positions": [[2, 0, 20, 0, 8]]}, + {"text": "ccccc", "doc_type_kwd": "text", "positions": [[3, 0, 30, 0, 12]]}, + {"text": "ddddd", "doc_type_kwd": "text", "positions": [[4, 0, 40, 0, 16]]}, + {"text": "eeeee", "doc_type_kwd": "text", "positions": [[5, 0, 50, 0, 20]]}, + {"text": "fffff", "doc_type_kwd": "text", "positions": [[6, 0, 60, 0, 24]]}, + ], + } + asyncio.run(chunker._invoke(**kwargs)) + chunks = chunker._outputs["chunks"] + # 5 items merge into one chunk (tk reaches 5); the 6th starts fresh with + # a partial overlap prefix. + assert len(chunks) == 2, f"want 2 chunks, got {len(chunks)}" + + # The new chunk's overlap text is the tail of the previous chunk. + assert "eeeee" in chunks[1]["text"], f"chunk[1] missing overlap tail text: {chunks[1]['text']!r}" + pos1 = chunks[1].get("pdf_positions") or [] + # The tail item's coordinates MUST be carried. + assert [5, 0, 50, 0, 20] in pos1, f"chunk[1] dropped tail-item coords (prev posE): {pos1}" + assert [6, 0, 60, 0, 24] in pos1, f"chunk[1] lost its own coords (posF): {pos1}" + # Partial overlap: the head items of the previous chunk must NOT be + # carried (that would over-inflate the highlight box). + for absent in ([1, 0, 10, 0, 5], [2, 0, 20, 0, 8], [3, 0, 30, 0, 12], [4, 0, 40, 0, 16]): + assert absent not in pos1, f"chunk[1] over-carried non-overlap head coords {absent}: {pos1}" + + def test_json_delimiter_mode_consecutive_delimiter_keeps_boundary(): # Regression for #17723: "A####B" with pattern "##" must yield ["A", "B"], # both boundary-adjacent segments preserved (the bug collapsed it to "A##B"). @@ -438,7 +526,7 @@ def test_text_delimiter_mode_one_no_atom_split(): "text": "aaa|bbb|ccc", } chunk_token_size = 1 - setattr(chunker._param, "chunk_token_size", chunk_token_size) + chunker._param.chunk_token_size = chunk_token_size asyncio.run(chunker._invoke(**kwargs)) chunks = chunker._outputs["chunks"] texts = [c["text"] for c in chunks]