diff --git a/internal/agent/canvas/scheduler.go b/internal/agent/canvas/scheduler.go index 8fa38d6c1d..af5a58d06d 100644 --- a/internal/agent/canvas/scheduler.go +++ b/internal/agent/canvas/scheduler.go @@ -21,6 +21,7 @@ package canvas import ( "context" "fmt" + "sort" "strings" "time" @@ -696,13 +697,8 @@ func BuildWorkflow(ctx context.Context, c *Canvas) (*compose.Workflow[map[string // tracks start/end membership by these explicit wirings — without // them, Compile() returns "start node not set" / "end node not set". // - // Multi-terminal case: eino's END node is stricter than regular - // workflow nodes about repeated output mappings. Instead of wiring - // multiple terminals directly into END, route them through one - // synthetic merge node. The merge node consumes one terminal as its - // data input and treats the rest as exec-only dependencies, mirroring - // the same "first input carries data; the rest are dependencies" - // policy used in Pass 2. + // Multiple terminals use distinct field mappings into a gather node. + // Branch pruning lets it collect only the leaves selected in this run. // // A "start" node with no upstream gets an empty input from START so // eino registers it as a workflow entry point. FieldMapping is nil @@ -824,48 +820,29 @@ func wireWorkflowTerminals( return nil } - // Sub-workflows wire END without field mappings. These multi-terminal - // shapes commonly come from mutually exclusive branches (for example a - // loop body Switch choosing either continue or exit). We therefore - // create a small field-mapped gather node that forwards whichever - // branch actually produced output, instead of the outer workflow's - // dependency-based merge node that would incorrectly wait for every - // terminal to execute in the same run. - if !useFieldMapping { - gatherNode := wf.AddLambdaNode( - terminalMergeNodeID, - compose.InvokableLambda[map[string]any, map[string]any]( - func(_ context.Context, in map[string]any) (map[string]any, error) { - for _, terminalID := range terminals { - if v, ok := in[terminalID].(map[string]any); ok && v != nil { - return v, nil - } - } - return in, nil - }, - ), - compose.WithNodeName(terminalMergeNodeID), - ) - for _, terminalID := range terminals { - gatherNode.AddInput(terminalID, compose.ToField(terminalID)) - } - addEndInput(terminalMergeNodeID) - return nil - } - - mergeNode := wf.AddLambdaNode( + // Keep checkpoint mappings stable across builds. Every terminal has the + // same data/control role, regardless of component map enumeration order. + terminals = append([]string(nil), terminals...) + sort.Strings(terminals) + gatherNode := wf.AddLambdaNode( terminalMergeNodeID, compose.InvokableLambda[map[string]any, map[string]any]( func(_ context.Context, in map[string]any) (map[string]any, error) { + if !useFieldMapping { + for _, terminalID := range terminals { + if output, ok := in[terminalID].(map[string]any); ok && output != nil { + return output, nil + } + } + } return in, nil }, ), compose.WithNodeName(terminalMergeNodeID), ) - mergeNode.AddInput(terminals[0]) - for _, terminalID := range terminals[1:] { - mergeNode.AddDependency(terminalID) + for _, terminalID := range terminals { + gatherNode.AddInput(terminalID, compose.ToField(terminalID)) } - addEndInput(terminalMergeNodeID) + wf.End().AddInput(terminalMergeNodeID) return nil } diff --git a/internal/agent/canvas/terminal_resume_test.go b/internal/agent/canvas/terminal_resume_test.go new file mode 100644 index 0000000000..0b0e4fa770 --- /dev/null +++ b/internal/agent/canvas/terminal_resume_test.go @@ -0,0 +1,56 @@ +package canvas + +import ( + "context" + "reflect" + "testing" + "time" + + "github.com/cloudwego/eino/compose" +) + +// Rebuilding a paused workflow must preserve the terminal data mappings even +// when the component map enumerates its mutually exclusive leaves differently. +func TestWorkflowTerminalsResumeAcrossEnumerationOrder(t *testing.T) { + for _, active := range []string{"a", "b"} { + t.Run(active, func(t *testing.T) { + store, _ := newTestStore(t, time.Hour) + build := func(terminals []string) compose.Runnable[map[string]any, map[string]any] { + wf := compose.NewWorkflow[map[string]any, map[string]any]() + wf.AddLambdaNode("route", compose.InvokableLambda(func(_ context.Context, in map[string]any) (map[string]any, error) { + return in, nil + })).AddInput(compose.START) + for _, id := range []string{"a", "b"} { + wf.AddLambdaNode(id, compose.InvokableLambda(func(_ context.Context, _ map[string]any) (map[string]any, error) { + return map[string]any{"content": "finished " + id}, nil + })).AddInput("route") + } + wf.AddBranch("route", compose.NewGraphMultiBranch(func(_ context.Context, _ map[string]any) (map[string]bool, error) { + return map[string]bool{active: true}, nil + }, map[string]bool{"a": true, "b": true})) + if err := wireWorkflowTerminals(wf, terminals, "", true); err != nil { + t.Fatal(err) + } + run, err := wf.Compile(t.Context(), compose.WithCheckPointStore(store), compose.WithInterruptBeforeNodes([]string{active})) + if err != nil { + t.Fatal(err) + } + return run + } + first := build([]string{"a", "b"}) + _, err := first.Invoke(t.Context(), map[string]any{}, compose.WithCheckPointID("terminal-order")) + if !IsInterruptError(err) { + t.Fatalf("first run: want interrupt, got %v", err) + } + resumed := build([]string{"b", "a"}) + out, err := resumed.Invoke(t.Context(), nil, compose.WithCheckPointID("terminal-order")) + if err != nil { + t.Fatalf("resume: %v", err) + } + want := map[string]any{active: map[string]any{"content": "finished " + active}} + if !reflect.DeepEqual(out, want) { + t.Fatalf("output = %#v, want %#v", out, want) + } + }) + } +} diff --git a/internal/deepdoc/parser/pdf/parser.go b/internal/deepdoc/parser/pdf/parser.go index 663a25c5a1..23e84df68d 100644 --- a/internal/deepdoc/parser/pdf/parser.go +++ b/internal/deepdoc/parser/pdf/parser.go @@ -615,7 +615,7 @@ func (p *Parser) buildLayout(ctx context.Context, // PageHeight is already in PDF-point space — computed in processPage. result.Sections = lyt.BoxesToSections(boxes, result.PageHeight) result.Metrics.BoxesFinal = len(result.Sections) - result.Sections = tbl.MergeCaptions(result.Sections, result.Figures()) + result.Sections = tbl.MergeCaptions(result.Sections) return nil } diff --git a/internal/deepdoc/parser/pdf/parser_test.go b/internal/deepdoc/parser/pdf/parser_test.go index 78545b1afc..6b72953107 100644 --- a/internal/deepdoc/parser/pdf/parser_test.go +++ b/internal/deepdoc/parser/pdf/parser_test.go @@ -492,8 +492,7 @@ func TestTableSectionCaptionInHTML(t *testing.T) { }) // Step 2: mergeCaptions injects caption as a element inside - figures := pdf.CollectFigures(sections) - sections = tbl.MergeCaptions(sections, figures) + sections = tbl.MergeCaptions(sections) got := sections[0].Text if !strings.Contains(got, "") { diff --git a/internal/deepdoc/parser/pdf/table/caption_kind_test.go b/internal/deepdoc/parser/pdf/table/caption_kind_test.go index 59dc97a0a8..54d1b72e7a 100644 --- a/internal/deepdoc/parser/pdf/table/caption_kind_test.go +++ b/internal/deepdoc/parser/pdf/table/caption_kind_test.go @@ -62,8 +62,7 @@ func TestMergeCaptions_KeepsInterleavedParagraph(t *testing.T) { {Text: "
表1: 交通工具等级
Category
", LayoutType: pdf.LayoutTypeTable, Positions: []pdf.Position{{PageNumbers: []int{0}, Left: 159, Right: 393, Top: 130, Bottom: 336}}}, } - figures := pdf.CollectFigures(sections) - result := MergeCaptions(sections, figures) + result := MergeCaptions(sections) if len(result) != 3 { t.Fatalf("expected 3 sections (title + paragraph + table), got %d: %v", len(result), textsOf(result)) } @@ -78,6 +77,54 @@ func TestMergeCaptions_KeepsInterleavedParagraph(t *testing.T) { } } +// TestCaptionKind_CJKBodyParagraphNotCaption locks the fix for go_bug +// cjk-caption-false-positive: Python's is_caption pattern +// `[图表]+[ 0-9:：]{2,}` requires ≥2 marker chars after the 图/表 run, so a +// body paragraph that merely STARTS with 表/图 (Chinese or Japanese) is not a +// caption. The bare `^表`/`^图` used previously misclassified such paragraphs +// as captions, and MergeCaptions then either dropped them (no table nearby) +// or swallowed them into — content loss Python never has. +func TestCaptionKind_CJKBodyParagraphNotCaption(t *testing.T) { + for _, text := range []string{ + "表格是一种常见的数据组织形式，本文对其进行对比。", // Chinese: 表格… + "图中所示的方法在实践中得到广泛应用。", // Chinese: 图中… + "表現方法の比較について述べる。", // Japanese: 表現… + } { + if got := CaptionKind(pdf.Section{Text: text, LayoutType: pdf.LayoutTypeText}); got != "" { + t.Errorf("CaptionKind(%q) = %q, want \"\" (body paragraph is not a caption)", text, got) + } + } +} + +// TestCaptionKind_CJKCaptionMarkersStillMatch pins the other side of the +// cjk-caption-false-positive fix: real CJK captions (图/表 + number + +// separator) must still classify — the suffix requirement must not +// over-tighten detection. The kind follows the run's first character. +func TestCaptionKind_CJKCaptionMarkersStillMatch(t *testing.T) { + for _, tt := range []struct{ text, want string }{ + {"表1：交通工具等级", pdf.LayoutTypeTable}, + {"表 1 出差标准", pdf.LayoutTypeTable}, + {"图1 系统架构总览", pdf.LayoutTypeFigure}, + {"图表1 全球市场规模", pdf.LayoutTypeFigure}, + } { + if got := CaptionKind(pdf.Section{Text: tt.text, LayoutType: pdf.LayoutTypeText}); got != tt.want { + t.Errorf("CaptionKind(%q) = %q, want %q", tt.text, got, tt.want) + } + } +} + +// hasBox reports whether positions contains a box with the given top and +// bottom edges. Tests use it to assert that a merged caption's box reached the +// section it was merged into. +func hasBox(positions []pdf.Position, top, bottom float64) bool { + for _, p := range positions { + if p.Top == top && p.Bottom == bottom { + return true + } + } + return false +} + func textsOf(secs []pdf.Section) []string { out := make([]string, 0, len(secs)) for _, s := range secs { diff --git a/internal/deepdoc/parser/pdf/table/merge_captions.go b/internal/deepdoc/parser/pdf/table/merge_captions.go index aba8b986f3..70913420cf 100644 --- a/internal/deepdoc/parser/pdf/table/merge_captions.go +++ b/internal/deepdoc/parser/pdf/table/merge_captions.go @@ -8,14 +8,26 @@ import ( pdf "ragflow/internal/deepdoc/parser/pdf/type" ) -// captionText is a caption box's text plus its top edge, used to order -// multiple captions of one table in READING order (top→bottom) before -// concatenation. Section order is not guaranteed to match the PDF layout -// (e.g. 06's lower caption box precedes the upper one in sections), so the -// top coordinate is carried explicitly. +// captionText is one caption box: its text plus the box itself. type captionText struct { - top float64 text string + // pos is the caption box. Two things read it: the reading-order sort below, + // and the target's highlight geometry — merging a caption moves its TEXT + // into the target's chunk, so its boxes have to move with it, or the UI + // highlights the figure region only and never the caption line beside it. + pos []pdf.Position +} + +// top is the caption's top edge, used to order multiple captions of one target +// in READING order (top→bottom) before concatenation. Section order is not +// guaranteed to match the PDF layout (e.g. 06's lower caption box precedes the +// upper one in sections), so it is read from the box. A caption with no +// positions sorts last. +func (c captionText) top() float64 { + if len(c.pos) == 0 { + return 1e9 + } + return c.pos[0].Top } // captionSep returns the separator inserted before a caption whose text is @@ -35,7 +47,7 @@ func captionSep(text string) string { return "" } -func MergeCaptions(sections []pdf.Section, figures []pdf.Section) []pdf.Section { +func MergeCaptions(sections []pdf.Section) []pdf.Section { captions := make([]int, 0, 4) // Group caption texts by the target section index they attach to, so // multiple caption boxes for the SAME table/figure collapse into a single @@ -44,32 +56,44 @@ func MergeCaptions(sections []pdf.Section, figures []pdf.Section) []pdf.Section // (browsers, HTML→Markdown converters) silently drop — a real content loss. byTarget := make(map[int][]captionText) for i, s := range sections { + // An extracted figure/table box is already a parent candidate, never a + // caption: Python pops table/figure boxes out of self.boxes BEFORE its + // caption scan (pdf_parser.py _extract_table_figure), so they are never + // classified there. Without this skip, CaptionKind's text patterns would + // classify a figure whose embedded text starts with a caption marker + // (图1 …, Figure 1: …) as its own caption, and findNearestParent would + // match it to ITSELF (CollectFigures includes it — center distance 0) — + // the figure would become its own merge target and be removed with the + // caption, deleting the image from the output (go_bug + // figure-self-caption-deleted). + if s.LayoutType == pdf.LayoutTypeFigure || s.LayoutType == pdf.LayoutTypeTable { + continue + } captionType := CaptionKind(s) if captionType == "" { continue } - target := findNearestParent(i, s, sections, figures, captionType) + target := findNearestParent(s, sections, captionType) if target >= 0 { // Emit the caption inside the target table's HTML as a // element (matching Python's __html_table) and drop the standalone // caption section. Retaining the caption text closes the previous // content-loss go_bug table-html-emission-format; it is NOT a // table-assembly change (cell content/structure are untouched). - top := 1e9 - if len(s.Positions) > 0 { - top = s.Positions[0].Top - } - byTarget[target] = append(byTarget[target], captionText{top: top, text: s.Text}) + byTarget[target] = append(byTarget[target], captionText{text: s.Text, pos: s.Positions}) captions = append(captions, i) continue } // No merge target. A FIGURE caption is kept as its own section: a pure // image figure has no text section (BoxesToSections skips empty figure // boxes), so removing it would drop caption text that Python keeps - // (07_mixed_content 'Figure 1/2'). A TABLE caption without a table - // section is a DLA mislabel (e.g. rotate_270's rotated text labeled - // "table") — keep the historical removal so rotated-page text is not - // duplicated. + // (07_mixed_content 'Figure 1/2'). A TABLE caption with neither a table + // nor a figure parent — findNearestParent already fell back to figures, + // mirroring Python's nearest(tables)/nearest(figures) double search — is + // a DLA mislabel (e.g. rotate_270's rotated text labeled "table"); keep + // the historical removal so rotated-page text is not duplicated. Python + // likewise drops a caption only when BOTH searches fail, so the removal + // branch now fires only when no parent exists at all. if captionType != pdf.LayoutTypeFigure { captions = append(captions, i) } @@ -77,21 +101,24 @@ func MergeCaptions(sections []pdf.Section, figures []pdf.Section) []pdf.Section // Inject one combined per target. Captions of the same table are // ordered by top edge (reading order, top→bottom) before concatenation. for idx, entries := range byTarget { - sort.SliceStable(entries, func(i, j int) bool { return entries[i].top < entries[j].top }) + sort.SliceStable(entries, func(i, j int) bool { return entries[i].top() < entries[j].top() }) texts := make([]string, len(entries)) for i, e := range entries { texts[i] = e.text } if sections[idx].LayoutType == pdf.LayoutTypeTable { injectCaption(§ions[idx], texts) - continue + } else { + // Non-table target (figure): keep the historical raw-text + // concatenation. The element is table-specific; wrapping a + // figure section's text in it would emit meaningless HTML in a figure + // section (which carries an image, not a table). Figure captions are + // out of this PR's scope, so preserve their pre-existing behavior. + appendRawCaptions(§ions[idx], texts) } - // Non-table target (figure): keep the historical raw-text - // concatenation. The element is table-specific; wrapping a - // figure section's text in it would emit meaningless HTML in a figure - // section (which carries an image, not a table). Figure captions are - // out of this PR's scope, so preserve their pre-existing behavior. - appendRawCaptions(§ions[idx], texts) + // Either way the caption's text now lives in this section's chunk, so its + // boxes have to move with it (see extendCaptionPositions). + extendCaptionPositions(§ions[idx], entries) } // Remove caption sections in reverse order. n := len(sections) @@ -108,40 +135,111 @@ func MergeCaptions(sections []pdf.Section, figures []pdf.Section) []pdf.Section return out } -// findNearestParent finds the nearest figure (for figure caption) or -// table (for table caption) section by position proximity. -// captionType is "table" or "figure" (from captionKind). -// Returns the index in `sections` (for tables) or a virtual index mapping -// to `figures` (negative offset for figures). -func findNearestParent(captionIdx int, caption pdf.Section, sections []pdf.Section, figures []pdf.Section, captionType string) int { - find := func(targets []pdf.Section, skipIdx int) (int, float64) { - bestIdx := -1 - bestDist := 1e9 - for i, t := range targets { - if i == skipIdx { - continue // don't match caption to itself - } - if len(t.Positions) == 0 || len(caption.Positions) == 0 { - continue - } - tp := t.Positions[0] - cp := caption.Positions[0] - // Squared Euclidean distance (Python _extract_table_figure:1196). - // Caption is typically below. Use center-point distance. - cx := (tp.Left + tp.Right) / 2 - cy := (tp.Top + tp.Bottom) / 2 - ccx := (cp.Left + cp.Right) / 2 - ccy := (cp.Top + cp.Bottom) / 2 - dist := (cx-ccx)*(cx-ccx) + (cy-ccy)*(cy-ccy) - if dist < bestDist { - bestDist = dist - bestIdx = i +// maxCaptionGap is the squared centre distance (PDF points²) beyond which a +// caption is not attached to a candidate parent: 40000 — about 200pt (~7cm). +const maxCaptionGap = 40000.0 + +// occupiesPage reports whether a section's recorded positions place it on +// `page`, and whether the section carries page metadata at all. Page-local +// coordinates repeat on every page (every page has a caption band at roughly +// the same y), so proximity alone cannot decide which page a caption belongs +// to — both parent searches must consult this. +// +// hasPages==false means the section has NO page metadata (a section built by a +// caller that never set PageNumbers). The two callers treat that differently on +// purpose: findTables rejects it (its pre-existing behavior), while the figure +// search keeps it, because that search is also the fallback target for +// orphaned table captions — rejecting on missing metadata there would delete +// their text. +func occupiesPage(s pdf.Section, page int) (onPage, hasPages bool) { + for _, p := range s.Positions { + for _, pn := range p.PageNumbers { + hasPages = true + if pn == page { + onPage = true } } - return bestIdx, bestDist } + return onPage, hasPages +} - const maxCaptionGap = 40000.0 // PDF points (~7cm) — beyond this, don't attach. +// nearestFigureIn returns the index in `sections` of the figure closest to +// caption by page-local centre distance, or -1 when none is within +// maxCaptionGap. +// +// Candidates are read straight off `sections` and the section index is tracked +// directly. A position box is NOT a figure's identity: page-local coordinates +// repeat on every page, so two figures on different pages can carry the same +// box — the very repetition the page filter exists to resolve — and resolving a +// hit back by box would land on the wrong page's figure. +// +// requirePage restricts candidates to figures that occupy the caption's page. +// Without it the search is page-blind, and the nearest figure by page-local +// centre is usually on a DIFFERENT page. A figure with no page metadata stays a +// candidate in both modes, so a caller that must not lose text (the +// table-caption fallback) cannot be defeated by missing metadata — see +// occupiesPage. +func nearestFigureIn(sections []pdf.Section, caption pdf.Section, requirePage bool) int { + if len(caption.Positions) == 0 { + return -1 + } + capKnown, capPage := false, 0 + if requirePage { + if pn := caption.Positions[0].PageNumbers; len(pn) > 0 { + capKnown, capPage = true, pn[0] + } + } + cp := caption.Positions[0] + ccx := (cp.Left + cp.Right) / 2 + ccy := (cp.Top + cp.Bottom) / 2 + + // Candidates are ranked in two tiers so a figure recorded on the caption's + // own page always beats a page-less one: tier 0 is occupied-by-capPage, + // tier 1 is "no page metadata". A figure known to sit on a DIFFERENT page is + // skipped outright. Keeping tier 1 means a figure whose caller never set + // PageNumbers can still take a caption the table fallback must not drop; + // ordering the tiers means it cannot outbid a figure we can actually place. + // + bestIdx, bestDist, bestTier := -1, 1e9, 2 + for i, t := range sections { + if t.LayoutType != pdf.LayoutTypeFigure || len(t.Positions) == 0 { + continue + } + tier := 0 + if capKnown { + switch onPage, hasPages := occupiesPage(t, capPage); { + case !hasPages: + tier = 1 + case onPage: + tier = 0 + default: + continue + } + } + tp := t.Positions[0] + cx := (tp.Left + tp.Right) / 2 + cy := (tp.Top + tp.Bottom) / 2 + dist := (cx-ccx)*(cx-ccx) + (cy-ccy)*(cy-ccy) + if tier < bestTier || (tier == bestTier && dist < bestDist) { + bestTier, bestDist, bestIdx = tier, dist, i + } + } + if bestIdx < 0 || bestDist >= maxCaptionGap { + return -1 + } + return bestIdx +} + +// findNearestParent finds the nearest figure (for figure captions) or table +// (for table captions) section by position proximity. captionType is "table" +// or "figure" (from CaptionKind). A table caption first searches tables and, +// when none is in range, falls through to the figure search — mirroring +// Python's nearest(tables)/nearest(figures) double search and its `elif fk` +// fallback (pdf_parser.py _extract_table_figure), so a caption whose table is +// unreachable (wrong page, out of range, or absent) lands on the nearest +// figure instead of being dropped. Returns the index in `sections`, or -1 +// when no parent is in range. +func findNearestParent(caption pdf.Section, sections []pdf.Section, captionType string) int { // maxCaptionVGap is the vertical band within which a caption attaches to a // table regardless of its horizontal offset. A narrow caption (e.g. a short // Chinese label) sitting directly above a much wider table has a large dx to @@ -152,23 +250,6 @@ func findNearestParent(captionIdx int, caption pdf.Section, sections []pdf.Secti // via min-distance. Keep this in line with the vertical tolerance implied by // maxCaptionGap when dx≈0 (~200pt). const maxCaptionVGap = 200.0 - if captionType == pdf.LayoutTypeFigure && len(figures) > 0 { - idx, dist := find(figures, -1) // figures don't contain the caption itself - if idx >= 0 && dist < maxCaptionGap { - // Match by position coordinates, not PositionTag strings. - f := figures[idx] - for i, s := range sections { - if s.LayoutType != pdf.LayoutTypeFigure || len(s.Positions) == 0 || len(f.Positions) == 0 { - continue - } - sp, fp := s.Positions[0], f.Positions[0] - if sp.Left == fp.Left && sp.Right == fp.Right && - sp.Top == fp.Top && sp.Bottom == fp.Bottom { - return i - } - } - } - } if captionType == pdf.LayoutTypeTable { idx, dist, gapY := findTables(sections, caption) // Attach a vertically-adjacent caption even when it is horizontally @@ -178,6 +259,29 @@ func findNearestParent(captionIdx int, caption pdf.Section, sections []pdf.Secti if idx >= 0 && (dist < maxCaptionGap || gapY <= maxCaptionVGap) { return idx } + // No table in range: fall through to the figure search below — + // Python's `elif fk` — rather than leaving the caller to delete a + // caption whose text Python preserves on a nearby figure. + } + // Two-pass figure search (see nearestFigureIn). Page-local coordinates + // repeat on every page, so proximity alone picks a figure on the wrong + // page: Figure 1's caption on page 0, page-local centre (415,474), matched + // a formula figure on page 13 at (417,480) — dist²=45, ~7pt. + // + // A genuine FIGURE caption is restricted to its own page: when no figure + // shares that page it stays a standalone section, which MergeCaptions keeps + // for figure captions — no text is lost and the caption is never glued to + // an unrelated figure. The TABLE-caption fallback must not lose text + // either, so it retries without the page filter and takes the nearest + // figure anywhere, mirroring Python's nearest(figures), which has no page + // scope at all: there, placement is best-effort but preservation is not. + if idx := nearestFigureIn(sections, caption, true); idx >= 0 { + return idx + } + if captionType == pdf.LayoutTypeTable { + if idx := nearestFigureIn(sections, caption, false); idx >= 0 { + return idx + } } return -1 } @@ -249,19 +353,9 @@ func findTables(sections []pdf.Section, caption pdf.Section) (int, float64, floa // icbccs bug where a page-3 caption wrongly attached to a page-5 // table and was concatenated into the , duplicating it // ("请求参数 请求参数"). - onPage := false - for _, pp := range t.Positions { - for _, pn := range pp.PageNumbers { - if pn == capPage { - onPage = true - break - } - } - if onPage { - break - } - } - if !onPage { + // A table whose positions carry no page metadata is rejected, + // matching the pre-existing behavior (see occupiesPage). + if onPage, _ := occupiesPage(t, capPage); !onPage { continue } } @@ -306,6 +400,68 @@ func appendRawCaptions(target *pdf.Section, captions []string) { } } +// extendCaptionPositions appends the attached captions' boxes to the target's +// own, so the target chunk's highlight geometry covers the caption as well as +// the table or figure region it was merged into. The chunk TEXT already +// contains the caption (appendRawCaptions / injectCaption); without this the +// caption is never highlighted with the parent it belongs to. A table's +// caption box routinely sits a couple of points ABOVE the table box rather +// than overlapping it, so this bites tables too. +// +// Only the pages the target itself occupies are merged. A cross-page +// attachment (the table-caption fallback can land on a figure pages away), or +// a caption box that itself spans pages, must not make the target claim a page +// it does not occupy: Position pages drive the section's page set, its +// render/eviction window and its crop plan. So a caption position is trimmed +// to the pages it shares with the target, and dropped when it shares none. A +// caption with no page metadata at all carries no page claim and is merged +// as-is. +// +// Eligibility is anchored to the target's ORIGINAL pages, so a caption merged +// earlier in the loop cannot widen what a later one is allowed to claim. +// +// Appending (not prepending) keeps Positions[0] as the primary box, which +// reading-order sorting and the proximity searches elsewhere depend on. +func extendCaptionPositions(target *pdf.Section, entries []captionText) { + base := make([]pdf.Position, len(target.Positions)) + copy(base, target.Positions) + for _, e := range entries { + for _, p := range e.pos { + shared, constrained := sharedPageNumbers(p, base) + if constrained { + if len(shared) == 0 { + continue + } + p.PageNumbers = shared + } + target.Positions = append(target.Positions, p) + } + } +} + +// sharedPageNumbers returns the page numbers p and have have in common. +// constrained is false when p carries no page numbers at all — there is then +// nothing to intersect and the position is safe to merge unchanged. +func sharedPageNumbers(p pdf.Position, have []pdf.Position) (shared []int, constrained bool) { + if len(p.PageNumbers) == 0 { + return nil, false + } + seen := make(map[int]struct{}, len(p.PageNumbers)) + for _, b := range p.PageNumbers { + for _, h := range have { + for _, a := range h.PageNumbers { + if a == b { + if _, dup := seen[b]; !dup { + seen[b] = struct{}{} + shared = append(shared, b) + } + } + } + } + } + return shared, true +} + // injectCaption concatenates the given caption texts (already grouped per // target by MergeCaptions) into a SINGLE element inserted // immediately after the table's opening tag (matching Python's diff --git a/internal/deepdoc/parser/pdf/table/merge_captions_caption_test.go b/internal/deepdoc/parser/pdf/table/merge_captions_caption_test.go index cc4f6966df..325d046061 100644 --- a/internal/deepdoc/parser/pdf/table/merge_captions_caption_test.go +++ b/internal/deepdoc/parser/pdf/table/merge_captions_caption_test.go @@ -20,8 +20,7 @@ func TestMergeCaptions_EmitsCaptionTag(t *testing.T) { {Text: "Table 1: Revenue", LayoutType: pdf.DLALabelTableCaption, Positions: []pdf.Position{{PageNumbers: []int{0}, Left: 61, Right: 144, Top: 219, Bottom: 231}}}, } - figures := pdf.CollectFigures(sections) - result := MergeCaptions(sections, figures) + result := MergeCaptions(sections) if len(result) != 1 { t.Fatalf("expected 1 section (table with caption, standalone caption removed), got %d: %v", len(result), textsOf(result)) @@ -56,8 +55,7 @@ func TestMergeCaptions_LeftMarginCaptionAttaches(t *testing.T) { {Text: "The following table summarizes the quarterly sales performance", LayoutType: pdf.DLALabelTableCaption, Positions: []pdf.Position{{PageNumbers: []int{0}, Left: 61, Right: 144, Top: 150, Bottom: 170}}}, } - figures := pdf.CollectFigures(sections) - result := MergeCaptions(sections, figures) + result := MergeCaptions(sections) if len(result) != 2 { t.Fatalf("expected 2 sections (title + table-with-caption), got %d: %v", len(result), textsOf(result)) } @@ -90,8 +88,7 @@ func TestMergeCaptions_SingleCaptionPerTable(t *testing.T) { {Text: "The following table summarizes the quarterly sales performance", LayoutType: pdf.DLALabelTableCaption, Positions: []pdf.Position{{PageNumbers: []int{0}, Left: 200, Right: 380, Top: 350, Bottom: 370}}}, } - figures := pdf.CollectFigures(sections) - result := MergeCaptions(sections, figures) + result := MergeCaptions(sections) if len(result) != 1 { t.Fatalf("expected 1 section (table with combined caption), got %d: %v", len(result), textsOf(result)) } @@ -129,8 +126,7 @@ func TestMergeCaptions_ReadingOrderByTop(t *testing.T) { {Text: "The following table summarizes the quarterly sales performance", LayoutType: pdf.DLALabelTableCaption, Positions: []pdf.Position{{PageNumbers: []int{0}, Left: 107, Right: 501, Top: 105, Bottom: 118}}}, } - figures := pdf.CollectFigures(sections) - result := MergeCaptions(sections, figures) + result := MergeCaptions(sections) if len(result) != 1 { t.Fatalf("expected 1 section (table with combined caption), got %d: %v", len(result), textsOf(result)) } @@ -175,8 +171,7 @@ func TestMergeCaptions_TallTableCaptionNearEdgeAttaches(t *testing.T) { } { t.Run(c.name, func(t *testing.T) { sections := []pdf.Section{table, c.caption} - figures := pdf.CollectFigures(sections) - result := MergeCaptions(sections, figures) + result := MergeCaptions(sections) if len(result) != 1 { t.Fatalf("expected 1 section (table with caption), got %d: %v", len(result), textsOf(result)) } @@ -208,8 +203,7 @@ func TestMergeCaptions_CaptionOtherPageDoesNotAttach(t *testing.T) { Positions: []pdf.Position{{PageNumbers: []int{1}, Left: 183, Right: 412, Top: 64, Bottom: 82}}, } sections := []pdf.Section{table, caption} - figures := pdf.CollectFigures(sections) - result := MergeCaptions(sections, figures) + result := MergeCaptions(sections) // No table on the caption's page -> orphaned caption (dropped), table unchanged. if len(result) != 1 { t.Fatalf("expected 1 section (table unchanged, orphaned caption dropped), got %d: %v", len(result), textsOf(result)) @@ -233,8 +227,7 @@ func TestMergeCaptions_FigureCaptionRawText(t *testing.T) { {Text: "Figure 1: Revenue trend by quarter", LayoutType: pdf.DLALabelFigureCaption, Positions: []pdf.Position{{PageNumbers: []int{0}, Left: 100, Right: 400, Top: 510, Bottom: 525}}}, } - figures := pdf.CollectFigures(sections) - result := MergeCaptions(sections, figures) + result := MergeCaptions(sections) if len(result) != 1 { t.Fatalf("expected 1 section (figure with caption), got %d: %v", len(result), textsOf(result)) } @@ -266,8 +259,7 @@ func TestMergeCaptions_NarrowCaptionAttachesWideTable(t *testing.T) { {Text: "请求参数", LayoutType: pdf.DLALabelTableCaption, Positions: []pdf.Position{{PageNumbers: []int{2}, Left: 28, Right: 100, Top: 157, Bottom: 178}}}, } - figures := pdf.CollectFigures(sections) - result := MergeCaptions(sections, figures) + result := MergeCaptions(sections) if len(result) != 1 { t.Fatalf("expected 1 section (table with caption, standalone caption removed), got %d: %v", len(result), textsOf(result)) @@ -280,3 +272,450 @@ func TestMergeCaptions_NarrowCaptionAttachesWideTable(t *testing.T) { t.Errorf("caption text should appear exactly once (inside
), got %q", got) } } + +// TestMergeCaptions_CJKBodyParagraphKept locks go_bug +// cjk-caption-false-positive end-to-end: a Chinese/Japanese body paragraph +// starting with 表/图 must survive MergeCaptions as its own section (Python +// keeps it as body text) — neither dropped when no table is nearby, nor +// swallowed into a table's when one is. +func TestMergeCaptions_CJKBodyParagraphKept(t *testing.T) { + para := pdf.Section{Text: "表格是一种常见的数据组织形式，本文对其进行对比。", LayoutType: pdf.LayoutTypeText, + Positions: []pdf.Position{{PageNumbers: []int{0}, Left: 100, Right: 500, Top: 170, Bottom: 195}}} + t.Run("no table nearby", func(t *testing.T) { + sections := []pdf.Section{ + {Text: "产品分析报告", LayoutType: pdf.LayoutTypeTitle, + Positions: []pdf.Position{{PageNumbers: []int{0}, Left: 100, Right: 500, Top: 50, Bottom: 80}}}, + para, + } + result := MergeCaptions(sections) + if len(result) != 2 { + t.Errorf("body paragraph dropped; got %d sections: %v", len(result), textsOf(result)) + } + }) + t.Run("table nearby", func(t *testing.T) { + sections := []pdf.Section{ + {Text: "
data
", LayoutType: pdf.LayoutTypeTable, + Positions: []pdf.Position{{PageNumbers: []int{0}, Left: 100, Right: 500, Top: 200, Bottom: 400}}}, + para, + } + result := MergeCaptions(sections) + kept, swallowed := false, false + for _, s := range result { + if strings.Contains(s.Text, "表格是一种") { + kept = true + } + if s.LayoutType == pdf.LayoutTypeTable && strings.Contains(s.Text, "表格是") { + swallowed = true + } + } + if !kept || swallowed { + t.Errorf("body paragraph must stay a standalone section (not enter ); kept=%v swallowed=%v sections=%v", + kept, swallowed, textsOf(result)) + } + }) +} + +// TestMergeCaptions_FigureWithCaptionMarkerTextSurvives locks go_bug +// figure-self-caption-deleted: a figure section whose OWN text starts with a +// caption marker (embedded chart title, OCR'd caption inside the figure box) +// must not be classified as its own caption and deleted. Python pops figure +// boxes before its caption scan so this cannot happen there; before the fix, +// findNearestParent matched the figure to ITSELF (CollectFigures includes it, +// distance 0) and the whole image section vanished from the output. Covers +// the figure-kind (图1/Figure 1) and table-kind (Table 2) classification +// paths — the latter would also steal the figure's text into a nearby +// table's . +func TestMergeCaptions_FigureWithCaptionMarkerTextSurvives(t *testing.T) { + for _, text := range []string{ + "Figure 1: system architecture overview", + "图1 系统架构总览", + "Table 2: embedded chart title inside the figure region", + } { + sections := []pdf.Section{ + {Text: "Introduction", LayoutType: pdf.LayoutTypeTitle, + Positions: []pdf.Position{{PageNumbers: []int{0}, Left: 100, Right: 500, Top: 50, Bottom: 80}}}, + {Text: text, LayoutType: pdf.LayoutTypeFigure, Image: "img", + Positions: []pdf.Position{{PageNumbers: []int{0}, Left: 100, Right: 500, Top: 200, Bottom: 400}}}, + } + result := MergeCaptions(sections) + figureAlive := false + for _, s := range result { + if s.LayoutType == pdf.LayoutTypeFigure { + figureAlive = true + if s.Text != text { + t.Errorf("figure text mutated: got %q, want %q", s.Text, text) + } + } + } + if !figureAlive { + t.Errorf("figure %q removed from output (self-caption deletion); sections = %v", text, textsOf(result)) + } + } +} + +// TestMergeCaptions_FigureTextNotStolenIntoTable: same figure, but with a +// real table in range — before the fix, the figure's "Table 2: …" text was +// injected into that table's and the figure section deleted. +func TestMergeCaptions_FigureTextNotStolenIntoTable(t *testing.T) { + sections := []pdf.Section{ + {Text: "
data
", LayoutType: pdf.LayoutTypeTable, + Positions: []pdf.Position{{PageNumbers: []int{0}, Left: 100, Right: 500, Top: 200, Bottom: 400}}}, + {Text: "Table 2: embedded chart title inside the figure region", LayoutType: pdf.LayoutTypeFigure, Image: "chartimg", + Positions: []pdf.Position{{PageNumbers: []int{0}, Left: 120, Right: 480, Top: 420, Bottom: 600}}}, + } + result := MergeCaptions(sections) + figureAlive, stolen := false, false + for _, s := range result { + if s.LayoutType == pdf.LayoutTypeFigure { + figureAlive = true + } + if s.LayoutType == pdf.LayoutTypeTable && strings.Contains(s.Text, "embedded chart title") { + stolen = true + } + } + if !figureAlive { + t.Errorf("figure removed; sections = %v", textsOf(result)) + } + if stolen { + t.Errorf("figure text stolen into the table's : %v", textsOf(result)) + } +} + +// TestMergeCaptions_FigureCaptionKeptWithMarkerFigure locks the cascade form +// of figure-self-caption-deleted: a genuine DLA figure caption attaching to a +// figure whose own text starts with a caption marker must keep BOTH the +// image and the real caption text (before the fix, the figure became its own +// target, was deleted, and the attached caption text vanished with it). +func TestMergeCaptions_FigureCaptionKeptWithMarkerFigure(t *testing.T) { + sections := []pdf.Section{ + {Text: "Figure 4: deployment topology", LayoutType: pdf.LayoutTypeFigure, Image: "img4", + Positions: []pdf.Position{{PageNumbers: []int{0}, Left: 100, Right: 500, Top: 200, Bottom: 400}}}, + {Text: "图4 部署拓扑结构", LayoutType: pdf.DLALabelFigureCaption, + Positions: []pdf.Position{{PageNumbers: []int{0}, Left: 100, Right: 500, Top: 410, Bottom: 430}}}, + } + result := MergeCaptions(sections) + if len(result) != 1 || result[0].LayoutType != pdf.LayoutTypeFigure { + t.Fatalf("want the single figure section, got %d sections: %v", len(result), textsOf(result)) + } + if !strings.Contains(result[0].Text, "部署拓扑") { + t.Errorf("real figure caption text lost with the deleted figure: %q", result[0].Text) + } +} + +// TestMergeCaptions_TableCaptionFallsBackToFigure locks go_bug +// table-caption-orphan-dropped: Python attaches a caption to the nearest +// table OR figure (nearest(tables)/nearest(figures), `elif fk`) and drops it +// only when BOTH searches fail. Go used to search tables only and delete the +// caption when none was reachable — losing text Python keeps, for English +// body text AND for correctly-DLA-labeled "table caption" sections alike +// (language-independent; the figure here is deliberately neutral-text so the +// figure-self-deletion bug cannot mask this path). +func TestMergeCaptions_TableCaptionFallsBackToFigure(t *testing.T) { + for _, caption := range []pdf.Section{ + {Text: "Table 1 shows revenue by category.", LayoutType: pdf.LayoutTypeText, + Positions: []pdf.Position{{PageNumbers: []int{0}, Left: 100, Right: 500, Top: 510, Bottom: 530}}}, + {Text: "Quarterly revenue breakdown", LayoutType: pdf.DLALabelTableCaption, + Positions: []pdf.Position{{PageNumbers: []int{0}, Left: 100, Right: 500, Top: 510, Bottom: 530}}}, + } { + sections := []pdf.Section{ + {Text: "market overview chart", LayoutType: pdf.LayoutTypeFigure, Image: "img", + Positions: []pdf.Position{{PageNumbers: []int{0}, Left: 100, Right: 500, Top: 300, Bottom: 500}}}, + caption, + } + result := MergeCaptions(sections) + found := false + for _, s := range result { + if strings.Contains(s.Text, "revenue") { + found = true + } + } + if !found { + t.Errorf("table caption %q dropped although a figure exists (Python attaches it); sections = %v", + caption.Text, textsOf(result)) + } + } +} + +// TestMergeCaptions_EnglishTable1OpeningParagraphSwallowed pins the SHARED +// (Python-identical) heuristic boundary of the CJK fix: a body paragraph that +// STARTS with "Table N" is classified a caption by BOTH implementations +// (Python's re.match is start-anchored too) and consumed into the table's +// . The text is retained — misplacement parity with Python, not a +// Go regression; only the CJK false-positive class was fixed. +func TestMergeCaptions_EnglishTable1OpeningParagraphSwallowed(t *testing.T) { + sections := []pdf.Section{ + {Text: "
data
", LayoutType: pdf.LayoutTypeTable, + Positions: []pdf.Position{{PageNumbers: []int{0}, Left: 100, Right: 500, Top: 200, Bottom: 400}}}, + {Text: "Table 1 shows revenue by category for the fiscal year.", LayoutType: pdf.LayoutTypeText, + Positions: []pdf.Position{{PageNumbers: []int{0}, Left: 100, Right: 500, Top: 170, Bottom: 195}}}, + } + result := MergeCaptions(sections) + for _, s := range result { + if s.LayoutType == pdf.LayoutTypeTable && strings.Contains(s.Text, "Table 1 shows revenue") { + return + } + } + t.Errorf("expected the start-anchored English paragraph consumed into (Python parity); sections = %v", + textsOf(result)) +} + +// TestMergeCaptions_FigureCaptionOtherPageDoesNotAttach locks the figure-side +// page-scope guard (go_bug figure-caption-cross-page-attached). Page-local +// coordinates repeat on every page, so the figure search used to match on +// page-local distance alone: here the page-2 caption's page-local centre is +// CLOSER to the page-0 figure (dist²=13225) than to the page-2 figure it +// belongs to (dist²=21025). The caption was therefore glued to the page-0 +// figure — pages away — and its own figure was left with no caption at all. +func TestMergeCaptions_FigureCaptionOtherPageDoesNotAttach(t *testing.T) { + sections := []pdf.Section{ + {Text: "page zero chart", LayoutType: pdf.LayoutTypeFigure, Image: "img0", + Positions: []pdf.Position{{PageNumbers: []int{0}, Left: 250, Right: 350, Top: 250, Bottom: 350}}}, + {Text: "page two chart", LayoutType: pdf.LayoutTypeFigure, Image: "img2", + Positions: []pdf.Position{{PageNumbers: []int{2}, Left: 250, Right: 350, Top: 510, Bottom: 610}}}, + {Text: "Figure 9: latency by shard count", LayoutType: pdf.DLALabelFigureCaption, + Positions: []pdf.Position{{PageNumbers: []int{2}, Left: 250, Right: 350, Top: 400, Bottom: 430}}}, + } + result := MergeCaptions(sections) + + var onZero, onTwo bool + for _, s := range result { + if s.LayoutType != pdf.LayoutTypeFigure { + continue + } + hasCap := strings.Contains(s.Text, "latency by shard count") + switch s.Positions[0].PageNumbers[0] { + case 0: + onZero = hasCap + case 2: + onTwo = hasCap + } + } + if !onTwo { + t.Errorf("caption did not attach to its own (page 2) figure; sections = %v", textsOf(result)) + } + if onZero { + t.Errorf("caption wrongly attached to the page-0 figure (page-local distance only): %v", textsOf(result)) + } +} + +// TestMergeCaptions_FigureCaptionUnknownPageStillAttaches pins the deliberate +// asymmetry with findTables: a figure whose positions carry NO page metadata is +// kept as a candidate. This search is also the fallback target for orphaned +// table captions, so rejecting on missing metadata would DELETE their text. +func TestMergeCaptions_FigureCaptionUnknownPageStillAttaches(t *testing.T) { + sections := []pdf.Section{ + {Text: "metadata-less chart", LayoutType: pdf.LayoutTypeFigure, Image: "imgnp", + Positions: []pdf.Position{{Left: 100, Right: 500, Top: 200, Bottom: 400}}}, + {Text: "Table 3: yearly totals", LayoutType: pdf.DLALabelTableCaption, + Positions: []pdf.Position{{PageNumbers: []int{4}, Left: 100, Right: 500, Top: 410, Bottom: 430}}}, + } + result := MergeCaptions(sections) + for _, s := range result { + if s.LayoutType == pdf.LayoutTypeFigure && strings.Contains(s.Text, "yearly totals") { + return + } + } + t.Errorf("caption dropped instead of attaching to the page-less figure: %v", textsOf(result)) +} + +// TestMergeCaptions_TableFallbackSurvivesPageGuard locks the interaction +// between the figure-side page guard and the orphaned-table-caption fallback +// (go_bug table-caption-orphan-dropped). A table caption with no table in +// range falls back to the figure search; the page guard must not then reject +// every candidate and hand the caller a section to delete. Here the only +// figure is on another page, so the page-scoped pass finds nothing — the +// fallback must retry without the page filter and keep the text. Before that +// retry existed, this caption was deleted outright. +func TestMergeCaptions_TableFallbackSurvivesPageGuard(t *testing.T) { + sections := []pdf.Section{ + {Text: "far away chart", LayoutType: pdf.LayoutTypeFigure, Image: "img0", + Positions: []pdf.Position{{PageNumbers: []int{0}, Left: 100, Right: 400, Top: 300, Bottom: 500}}}, + {Text: "Table 5 shows the answer quality and retrieval performance across benchmarks.", + LayoutType: pdf.LayoutTypeText, + Positions: []pdf.Position{{PageNumbers: []int{7}, Left: 100, Right: 400, Top: 320, Bottom: 340}}}, + } + result := MergeCaptions(sections) + for _, s := range result { + if strings.Contains(s.Text, "answer quality and retrieval performance") { + return + } + } + t.Errorf("table caption text deleted instead of preserved on the fallback figure: %v", textsOf(result)) +} + +// TestMergeCaptions_FigureCaptionPositionMerged locks the highlight geometry: +// a caption merged into a figure carries its TEXT into the figure's chunk, so +// its box must be merged into the figure's Positions too — otherwise the UI +// highlights the figure region only and the caption line is never highlighted +// with the image. Positions[0] must stay the figure's own box (reading-order +// sorting and the proximity searches use it). +func TestMergeCaptions_FigureCaptionPositionMerged(t *testing.T) { + sections := []pdf.Section{ + {Text: "revenue chart", LayoutType: pdf.LayoutTypeFigure, Image: "img", + Positions: []pdf.Position{{PageNumbers: []int{0}, Left: 100, Right: 400, Top: 300, Bottom: 500}}}, + {Text: "Figure 1: revenue by quarter", LayoutType: pdf.DLALabelFigureCaption, + Positions: []pdf.Position{{PageNumbers: []int{0}, Left: 100, Right: 400, Top: 510, Bottom: 525}}}, + } + result := MergeCaptions(sections) + if len(result) != 1 { + t.Fatalf("expected 1 section (figure with caption), got %d: %v", len(result), textsOf(result)) + } + got := result[0] + if len(got.Positions) != 2 { + t.Fatalf("caption box not merged into the figure's positions: %+v", got.Positions) + } + if got.Positions[0].Top != 300 || got.Positions[0].Bottom != 500 { + t.Errorf("primary box must stay first, got %+v", got.Positions[0]) + } + if !hasBox(got.Positions, 510, 525) { + t.Errorf("caption box missing from the merged positions: %+v", got.Positions) + } +} + +// TestMergeCaptions_CrossPageFallbackDoesNotClaimPage pins the guard on the +// above: a table caption rescued by the page-blind fallback can land on a +// figure pages away. Its text is still preserved, but its box must NOT be +// merged into the figure's positions — Position pages drive the section's page +// set, its render/eviction window and its crop plan, so claiming a page the +// figure does not occupy would corrupt all three. +func TestMergeCaptions_CrossPageFallbackDoesNotClaimPage(t *testing.T) { + sections := []pdf.Section{ + {Text: "far chart", LayoutType: pdf.LayoutTypeFigure, Image: "img0", + Positions: []pdf.Position{{PageNumbers: []int{0}, Left: 100, Right: 400, Top: 300, Bottom: 500}}}, + {Text: "Table 9: yearly totals", LayoutType: pdf.DLALabelTableCaption, + Positions: []pdf.Position{{PageNumbers: []int{5}, Left: 100, Right: 400, Top: 320, Bottom: 340}}}, + } + result := MergeCaptions(sections) + for _, s := range result { + if s.LayoutType != pdf.LayoutTypeFigure { + continue + } + if len(s.Positions) != 1 { + t.Errorf("cross-page caption must not extend the figure's positions: %+v", s.Positions) + } + if !strings.Contains(s.Text, "yearly totals") { + t.Errorf("caption text must still be preserved: %q", s.Text) + } + } +} + +// TestMergeCaptions_IdenticalBoxOtherPageNotChosen locks the page identity of +// the figure the caption lands on. Two figures on DIFFERENT pages can share a +// page-local box (page-local coordinates repeat per page — the very reason the +// page filter exists). The search used to map its hit back to a section by +// comparing position boxes alone, which returned the earliest section with an +// equal box: the caption was then attached to the other page's figure. +func TestMergeCaptions_IdenticalBoxOtherPageNotChosen(t *testing.T) { + shared := func(pg int) []pdf.Position { + return []pdf.Position{{PageNumbers: []int{pg}, Left: 100, Right: 400, Top: 200, Bottom: 400}} + } + sections := []pdf.Section{ + {Text: "page zero chart", LayoutType: pdf.LayoutTypeFigure, Image: "i0", Positions: shared(0)}, + {Text: "page two chart", LayoutType: pdf.LayoutTypeFigure, Image: "i2", Positions: shared(2)}, + {Text: "Figure 7: numbers", LayoutType: pdf.DLALabelFigureCaption, + Positions: []pdf.Position{{PageNumbers: []int{2}, Left: 100, Right: 400, Top: 410, Bottom: 425}}}, + } + result := MergeCaptions(sections) + var zero, two bool + for _, s := range result { + has := strings.Contains(s.Text, "numbers") + switch s.Positions[0].PageNumbers[0] { + case 0: + zero = has + case 2: + two = has + } + } + if !two { + t.Errorf("caption did not attach to the page-2 figure; sections = %v", textsOf(result)) + } + if zero { + t.Errorf("caption attached to the page-0 figure that merely shares a box: %v", textsOf(result)) + } +} + +// TestMergeCaptions_SamePageFigureBeatsPageLessNearer locks the candidate +// ordering: a figure with no page metadata stays eligible so that a caption the +// table fallback must not drop can still land somewhere, but it must never +// outbid a figure we can actually place on the caption's page. +func TestMergeCaptions_SamePageFigureBeatsPageLessNearer(t *testing.T) { + sections := []pdf.Section{ + // page-less figure, much CLOSER to the caption than the same-page one: + // dist² 56 against the same-page figure's 6006, so distance alone would + // pick it. + {Text: "page-less chart", LayoutType: pdf.LayoutTypeFigure, Image: "np", + Positions: []pdf.Position{{Left: 100, Right: 400, Top: 500, Bottom: 540}}}, + {Text: "same-page chart", LayoutType: pdf.LayoutTypeFigure, Image: "sp", + Positions: []pdf.Position{{PageNumbers: []int{1}, Left: 100, Right: 400, Top: 400, Bottom: 500}}}, + {Text: "Figure 3: detail", LayoutType: pdf.DLALabelFigureCaption, + Positions: []pdf.Position{{PageNumbers: []int{1}, Left: 100, Right: 400, Top: 520, Bottom: 535}}}, + } + result := MergeCaptions(sections) + for _, s := range result { + if !strings.Contains(s.Text, "detail") { + continue + } + if !strings.Contains(s.Text, "same-page chart") { + t.Errorf("closer page-less figure outbid the caption's own-page figure: %v", textsOf(result)) + } + } +} + +// TestMergeCaptions_TableCaptionPositionMerged is the table half of the +// highlight-geometry fix: injectCaption puts the caption text inside the +// table's HTML, but a table's caption box routinely sits a few points ABOVE the +// table box rather than overlapping it, so without merging the boxes the UI +// highlights the table and never the caption line above it. +func TestMergeCaptions_TableCaptionPositionMerged(t *testing.T) { + sections := []pdf.Section{ + {Text: "
x
", LayoutType: pdf.LayoutTypeTable, + Positions: []pdf.Position{{PageNumbers: []int{0}, Left: 113, Right: 484, Top: 161, Bottom: 236}}}, + {Text: "Table 1: results", LayoutType: pdf.DLALabelTableCaption, + Positions: []pdf.Position{{PageNumbers: []int{0}, Left: 210, Right: 385, Top: 138, Bottom: 149}}}, + } + result := MergeCaptions(sections) + if len(result) != 1 { + t.Fatalf("expected 1 section (table with caption), got %d: %v", len(result), textsOf(result)) + } + got := result[0] + if !strings.Contains(got.Text, "") { + t.Fatalf("caption not injected into the table HTML: %q", got.Text) + } + if len(got.Positions) != 2 { + t.Fatalf("caption box not merged into the table's positions: %+v", got.Positions) + } + if got.Positions[0].Top != 161 || got.Positions[0].Bottom != 236 { + t.Errorf("primary box must stay first, got %+v", got.Positions[0]) + } + if !hasBox(got.Positions, 138, 149) { + t.Errorf("caption box missing from the merged positions: %+v", got.Positions) + } +} + +// TestMergeCaptions_MergedCaptionKeepsOnlySharedPages locks the page trimming +// on the highlight merge: a caption box that itself spans pages must not make +// the target claim a page it does not occupy. Position pages drive the +// section's page set, its render/eviction window and its crop plan, so merging +// a {0,5} caption box into a page-0 figure would corrupt all three. +func TestMergeCaptions_MergedCaptionKeepsOnlySharedPages(t *testing.T) { + sections := []pdf.Section{ + {Text: "spread chart", LayoutType: pdf.LayoutTypeFigure, Image: "img", + Positions: []pdf.Position{{PageNumbers: []int{0}, Left: 100, Right: 400, Top: 300, Bottom: 500}}}, + {Text: "Figure 5: spread", LayoutType: pdf.DLALabelFigureCaption, + Positions: []pdf.Position{{PageNumbers: []int{0, 5}, Left: 100, Right: 400, Top: 510, Bottom: 525}}}, + } + result := MergeCaptions(sections) + if len(result) != 1 { + t.Fatalf("expected 1 section (figure with caption), got %d: %v", len(result), textsOf(result)) + } + got := result[0] + if len(got.Positions) != 2 { + t.Fatalf("caption box not merged: %+v", got.Positions) + } + if pn := got.Positions[1].PageNumbers; len(pn) != 1 || pn[0] != 0 { + t.Errorf("merged caption must keep only the pages the figure occupies, got %v", pn) + } + if pn := got.Positions[0].PageNumbers; len(pn) != 1 || pn[0] != 0 { + t.Errorf("primary box changed: %v", pn) + } +} diff --git a/internal/deepdoc/parser/pdf/table/merge_captions_test.go b/internal/deepdoc/parser/pdf/table/merge_captions_test.go index d818498779..4f3b55e5f8 100644 --- a/internal/deepdoc/parser/pdf/table/merge_captions_test.go +++ b/internal/deepdoc/parser/pdf/table/merge_captions_test.go @@ -13,9 +13,8 @@ func TestMergeCaptions_Unit(t *testing.T) { {Text: "F", LayoutType: "figure", Positions: []pdf.Position{{PageNumbers: []int{0, 0}, Left: 40, Right: 60, Top: 30, Bottom: 45}}}, {Text: "C", LayoutType: "figure caption", Positions: []pdf.Position{{PageNumbers: []int{0, 0}, Left: 40, Right: 60, Top: 80, Bottom: 95}}}, } - figures := pdf.CollectFigures(sections) - result := MergeCaptions(sections, figures) + result := MergeCaptions(sections) // Caption removed. if len(result) != 1 { @@ -36,9 +35,8 @@ func TestMergeCaptions_TableCaption(t *testing.T) { {Text: "T", LayoutType: "table", Positions: []pdf.Position{{PageNumbers: []int{0, 0}, Left: 40, Right: 60, Top: 30, Bottom: 45}}}, {Text: "C", LayoutType: "table caption", Positions: []pdf.Position{{PageNumbers: []int{0, 0}, Left: 40, Right: 60, Top: 80, Bottom: 95}}}, } - figures := pdf.CollectFigures(sections) - result := MergeCaptions(sections, figures) + result := MergeCaptions(sections) if len(result) != 1 { t.Fatalf("expected 1 section after merge, got %d", len(result)) @@ -62,8 +60,7 @@ func TestMergeCaptions_EuclideanDistance(t *testing.T) { {PageNumbers: []int{0, 0}, Left: 0, Right: 100, Top: 70, Bottom: 80}, }}, } - figures := pdf.CollectFigures(sections) - result := MergeCaptions(sections, figures) + result := MergeCaptions(sections) // Caption merged into figure — verified by figure Text containing caption. if len(result) != 1 { t.Fatalf("expected 1 section after merge, got %d", len(result)) @@ -83,8 +80,7 @@ func TestMergeCaptions_FigureCaptionNoTargetKept(t *testing.T) { {Text: "Figure 2: system architecture overview", LayoutType: "figure caption", Positions: []pdf.Position{{PageNumbers: []int{0, 0}, Left: 40, Right: 160, Top: 300, Bottom: 315}}}, } - figures := pdf.CollectFigures(sections) // empty: no "figure" section present - result := MergeCaptions(sections, figures) + result := MergeCaptions(sections) if len(result) != 1 { t.Fatalf("figure caption with no parent must be kept, got %d sections", len(result)) } diff --git a/internal/deepdoc/parser/pdf/table/table_cells.go b/internal/deepdoc/parser/pdf/table/table_cells.go index cffdb4acbe..9b85b18d4d 100644 --- a/internal/deepdoc/parser/pdf/table/table_cells.go +++ b/internal/deepdoc/parser/pdf/table/table_cells.go @@ -436,10 +436,21 @@ func IsCaptionBox(text string, layoutType string) bool { // start-anchored with ^: an unanchored alternative wrongly classifies such a // paragraph as a caption and MergeCaptions then drops it (go_bug // table-text-interleaved-paragraph-dropped). -var reTableCaptionText = regexp.MustCompile(`^表|(?i)^Table\s+\d+`) +// +// The CJK branch mirrors Python's `[图表]+[ 0-9:：]{2,}`: the 图/表 run must be +// followed by ≥2 chars from [ 0-9:：], so a body paragraph that merely STARTS +// with 表/图 (表格是一种…, 表現方法…, 图中所示…) is not a caption. The bare +// `^表` previously classified any 表-prefixed CJK/Japanese body paragraph as a +// caption that MergeCaptions either swallowed into or dropped — +// content loss Python never has (go_bug cjk-caption-false-positive). Kind is +// split by the run's first character because findNearestParent needs a +// table/figure kind to pick which parent to search; Python has a single +// pattern and picks the nearest parent of either kind instead. +var reTableCaptionText = regexp.MustCompile(`^表[图表]*[ 0-9:：]{2,}|(?i)^Table\s+\d+`) -// reFigureCaptionText matches text patterns that indicate a figure caption. -var reFigureCaptionText = regexp.MustCompile(`^图|(?i)^Fig\.?\s*\d+|(?i)^Figure\s+\d+`) +// reFigureCaptionText matches text patterns that indicate a figure caption — +// the figure-kind split of the same Python pattern (see reTableCaptionText). +var reFigureCaptionText = regexp.MustCompile(`^图[图表]*[ 0-9:：]{2,}|(?i)^Fig\.?\s*\d+|(?i)^Figure\s+\d+`) // captionKind returns "table" if the section is a table caption, // "figure" if a figure caption, or "" if not a caption. @@ -460,10 +471,6 @@ func CaptionKind(s pdf.Section) string { if reFigureCaptionText.MatchString(t) { return pdf.LayoutTypeFigure } - // The chart/figure pattern is ambiguous (matches both) — fall back to isCaptionBox. - if IsCaptionBox(t, "") { - return pdf.LayoutTypeTable - } return "" } diff --git a/internal/deepdoc/parser/pdf/table/table_construct_test.go b/internal/deepdoc/parser/pdf/table/table_construct_test.go index e14b97cd1c..8dcb82dfc0 100644 --- a/internal/deepdoc/parser/pdf/table/table_construct_test.go +++ b/internal/deepdoc/parser/pdf/table/table_construct_test.go @@ -730,8 +730,7 @@ func TestMergeCaptions_NeedsCaptionLayoutType(t *testing.T) { {LayoutType: "text", Text: "公司领导班子成员、出差地", Positions: []pdf.Position{{Left: 100, Right: 500, Top: 180, Bottom: 198}}}, } - figures := pdf.CollectFigures(sections) - result := MergeCaptions(sections, figures) + result := MergeCaptions(sections) // BUG: "text" layout type is NOT matched by mergeCaptions (only "table caption"/"figure caption"). // The caption text survives as a separate section instead of being prepended to the table. for _, s := range result { diff --git a/internal/service/agent.go b/internal/service/agent.go index 0ef19bed7e..fba67b5ea5 100644 --- a/internal/service/agent.go +++ b/internal/service/agent.go @@ -2242,6 +2242,7 @@ func (s *AgentService) buildRunFunc(canvasID string, versionRow *entity.UserCanv if isResume && resumeID != "" { wfInput = map[string]any{"query": ""} } + runStartedAt := time.Now() workflowOutput, invokeErr := cc.Workflow.Invoke(ctx2, wfInput, invokeOpts...) err = invokeErr if errors.Is(err, context.Canceled) || errors.Is(ctx2.Err(), context.Canceled) { @@ -2255,56 +2256,27 @@ func (s *AgentService) buildRunFunc(canvasID string, versionRow *entity.UserCanv _ = s.runTracker.AttachCheckpoint(ctx2, runID, cpID) } - // Collect answer and references from the state snapshot. + // Collect thinking and references from the state snapshot. // node_finished events are already emitted per-node by the // statePost wrappers in scheduler.go. - var answer string var thinking string var legacyReference []interface{} - var downloads any - var attachment map[string]any now := float64(time.Now().UnixNano()) / 1e9 for _, bucket := range state.Snapshot() { - if v, ok := bucket["answer"].(string); ok && v != "" { - if answer == "" { - answer = v - } - } - if v, ok := bucket["content"].(string); ok && v != "" && answer == "" { - answer = v - } - if v, ok := bucket["result"].(string); ok && v != "" && answer == "" { - answer = v - } if v, ok := bucket["thinking"].(string); ok && v != "" && thinking == "" { thinking = v } if v, ok := bucket["reference"].([]interface{}); ok { legacyReference = append(legacyReference, v...) } - if v, ok := bucket["downloads"]; ok && !emptyDownloadValue(v) { - downloads = v - } - if v, ok := bucket["attachment"].(map[string]any); ok && len(v) > 0 { - attachment = v - } } referencePayload := agentRunReferencePayload(c, state, legacyReference) - assistantOutput := terminalCanvasOutput(c, state, workflowOutput, answer, downloads, attachment) - // The terminal Message output is authoritative. Do not let a result or - // content field from a non-terminal upstream node become the answer when - // the Message intentionally selected attachments only. - if terminalContent, ok := assistantOutput["content"].(string); ok { - answer = terminalContent - } else { - answer = "" - } - downloads = assistantOutput["downloads"] - if terminalAttachment, ok := assistantOutput["attachment"].(map[string]any); ok { - attachment = terminalAttachment - } else { - attachment = nil - } + assistantOutput := terminalCanvasOutput(c, state, workflowOutput, runStartedAt) + // The selected Message owns the answer, including empty text when it + // intentionally selected attachments only. + answer, _ := assistantOutput["content"].(string) + downloads := assistantOutput["downloads"] + attachment, _ := assistantOutput["attachment"].(map[string]any) // Release any deferred Agent node that was not consumed because the // downstream Message was skipped by an exception/branch path. runtime.CompleteAllDeferredNodes(ctx2) @@ -2347,41 +2319,6 @@ func (s *AgentService) buildRunFunc(canvasID string, versionRow *entity.UserCanv } return state, err } - if shouldTreatAsCompletedLoopRun(err, answer) { - appendAssistantHistory(state, partialAssistantOutput(answer, downloads, attachment)) - if persistErr := s.persistAgentRunSession(ctx, canvasID, userID, sessionID, messageID, userInput, answer, thinking, referencePayload, dsl, state, true); persistErr != nil { - s.markRunFailed(ctx2, runID, "persist session: "+persistErr.Error()) - return nil, canvas.NewInternalRunError( - fmt.Errorf("persist agent session: %w: %w", persistErr, ErrAgentStorageError), - ) - } - if !messageEventsEmitted && shouldEmitMessage { - emitAgentMessageEvents(emit, answer, thinking, referencePayload) - } - - if shouldEmitMessage { - meData, _ := json.Marshal(canvas.MessageEndEvent{ - Attachment: attachment, - Reference: referencePayload, - }) - emit("message_end", string(meData)) - } - - wfPayload := map[string]interface{}{ - "inputs": map[string]any{"query": userInput}, - "outputs": workflowOutputsFromTerminal(assistantOutput), - "elapsed_time": now - startedAt, - "created_at": now, - } - if u := usagePayload(); u != nil { - wfPayload["usage"] = u - } - wfData, _ := json.Marshal(wfPayload) - emit("workflow_finished", string(wfData)) - - s.markRunSucceeded(ctx2, runID) - return state, nil - } if failureText := deferredAgentStreamFailureText(err); failureText != "" { visibleAnswer := answer if visibleAnswer == "" && !messageEventsEmitted && shouldEmitMessage { @@ -2760,19 +2697,27 @@ func terminalCanvasOutput( c *canvas.Canvas, state *canvas.CanvasState, workflowOutput map[string]any, - answer string, - downloads any, - attachment any, + runStartedAt time.Time, ) map[string]any { terminalIDs := make([]string, 0) + messageIDs := make([]string, 0) if c != nil { for cpnID, component := range c.Components { - if len(component.Downstream) == 0 { + if strings.EqualFold(component.Obj.ComponentName, "Message") { + messageIDs = append(messageIDs, cpnID) + } + if c.NodeParents[cpnID] != "" || strings.EqualFold(component.Obj.ComponentName, "ExitLoop") { + continue + } + // Successful workflow output names the actual outer terminals, + // including macros whose declared downstream belongs to their body. + if _, ok := workflowOutput[cpnID]; ok || len(component.Downstream) == 0 { terminalIDs = append(terminalIDs, cpnID) } } } sort.Strings(terminalIDs) + sort.Strings(messageIDs) for _, cpnID := range terminalIDs { if output, ok := workflowOutput[cpnID].(map[string]any); ok && len(output) > 0 { return cloneCanvasOutput(output) @@ -2782,21 +2727,30 @@ func terminalCanvasOutput( snapshot := state.Snapshot() for _, cpnID := range terminalIDs { if output := snapshot[cpnID]; len(output) > 0 { - return cloneCanvasOutput(output) + created, _ := output["_created_time"].(string) + createdAt, err := time.Parse(time.RFC3339Nano, created) + if err == nil && !createdAt.Before(runStartedAt) { + return cloneCanvasOutput(output) + } } } + // A Message may have emitted a prompt before the next node paused. + // Persist that partial answer, but never replay an earlier turn. + var partial map[string]any + latest := runStartedAt + for _, cpnID := range messageIDs { + output := snapshot[cpnID] + created, _ := output["_created_time"].(string) + createdAt, err := time.Parse(time.RFC3339Nano, created) + if err == nil && !createdAt.Before(latest) { + partial, latest = output, createdAt + } + } + if partial != nil { + return cloneCanvasOutput(partial) + } } - if len(workflowOutput) > 0 { - return cloneCanvasOutput(workflowOutput) - } - fallback := map[string]any{"content": answer} - if !emptyDownloadValue(downloads) { - fallback["downloads"] = downloads - } - if !emptyAttachmentValue(attachment) { - fallback["attachment"] = attachment - } - return fallback + return map[string]any{} } func cloneCanvasOutput(input map[string]any) map[string]any { @@ -2959,14 +2913,6 @@ func deferredAgentStreamFailureText(err error) string { return strings.TrimSpace(deferred.FailureText()) } -func shouldTreatAsCompletedLoopRun(err error, answer string) bool { - if err == nil || answer == "" { - return false - } - msg := err.Error() - return strings.Contains(msg, "[GraphRunError] no tasks to execute") -} - func canvasInvokeError(err error) error { if err == nil { return nil diff --git a/internal/service/agent_terminal_output_test.go b/internal/service/agent_terminal_output_test.go new file mode 100644 index 0000000000..b860bd35e7 --- /dev/null +++ b/internal/service/agent_terminal_output_test.go @@ -0,0 +1,80 @@ +package service + +import ( + "reflect" + "testing" + "time" + + "ragflow/internal/agent/canvas" +) + +func TestTerminalCanvasOutputExcludesLoopBody(t *testing.T) { + c := &canvas.Canvas{ + Components: map[string]canvas.CanvasComponent{ + "Loop:x": {Obj: canvas.CanvasComponentObj{ComponentName: "Loop"}, Downstream: []string{"Message:done"}}, + "ExitLoop:x": {Obj: canvas.CanvasComponentObj{ComponentName: "ExitLoop"}}, + "Message:body": {Obj: canvas.CanvasComponentObj{ComponentName: "Message"}}, + "Message:done": {Obj: canvas.CanvasComponentObj{ComponentName: "Message"}}, + }, + NodeParents: map[string]string{"ExitLoop:x": "Loop:x", "Message:body": "Loop:x"}, + } + state := canvas.NewCanvasState("run", "session") + state.SetVar("ExitLoop:x", "_next", []string{"ExitLoop:x"}) + state.SetVar("Message:body", "content", "continue") + state.SetVar("Message:body", "_created_time", "2026-10-10T00:00:02Z") + state.SetVar("Message:done", "content", "finished") + state.SetVar("Message:done", "_created_time", "2026-10-10T00:00:01Z") + got := terminalCanvasOutput(c, state, nil, time.Date(2026, 10, 10, 0, 0, 0, 0, time.UTC)) + if got["content"] != "finished" { + t.Fatalf("output = %#v, want the outer terminal message", got) + } +} + +func TestTerminalCanvasOutputKeepsCurrentPartialMessage(t *testing.T) { + c := &canvas.Canvas{Components: map[string]canvas.CanvasComponent{ + "Message:old": {Obj: canvas.CanvasComponentObj{ComponentName: "Message"}, Downstream: []string{"input"}}, + "Message:current": {Obj: canvas.CanvasComponentObj{ComponentName: "Message"}, Downstream: []string{"input"}}, + "input": {Obj: canvas.CanvasComponentObj{ComponentName: "UserFillUp"}}, + }} + state := canvas.NewCanvasState("run", "session") + state.SetVar("Message:old", "content", "previous prompt") + state.SetVar("Message:old", "_created_time", "2026-10-09T00:00:00Z") + state.SetVar("Message:current", "content", "current prompt") + state.SetVar("Message:current", "_created_time", "2026-10-10T00:00:01Z") + got := terminalCanvasOutput(c, state, nil, time.Date(2026, 10, 10, 0, 0, 0, 0, time.UTC)) + if got["content"] != "current prompt" { + t.Fatalf("output = %#v, want the current partial message", got) + } +} + +func TestTerminalCanvasOutputUsesCurrentWorkflowBranch(t *testing.T) { + c := &canvas.Canvas{Components: map[string]canvas.CanvasComponent{ + "Message:a": {Obj: canvas.CanvasComponentObj{ComponentName: "Message"}}, + "Message:b": {Obj: canvas.CanvasComponentObj{ComponentName: "Message"}}, + }} + state := canvas.NewCanvasState("run", "session") + state.SetVar("Message:a", "content", "old branch") + for _, output := range []map[string]any{ + {"content": "current branch"}, + {"content": "", "downloads": []any{map[string]any{"url": "/file"}}}, + {"content": ""}, + } { + got := terminalCanvasOutput(c, state, map[string]any{"Message:b": output}, time.Now()) + if !reflect.DeepEqual(got, output) { + t.Fatalf("output = %#v, want %#v", got, output) + } + } +} + +func TestTerminalCanvasOutputDoesNotReplayPreviousTurn(t *testing.T) { + c := &canvas.Canvas{Components: map[string]canvas.CanvasComponent{ + "Message:a": {Obj: canvas.CanvasComponentObj{ComponentName: "Message"}}, + }} + state := canvas.NewCanvasState("run", "session") + state.SetVar("Message:a", "content", "previous answer") + state.SetVar("Message:a", "_created_time", "2026-10-09T00:00:00Z") + got := terminalCanvasOutput(c, state, nil, time.Date(2026, 10, 10, 0, 0, 0, 0, time.UTC)) + if len(got) != 0 { + t.Fatalf("output = %#v, want no output from the previous turn", got) + } +}