From 06463f6a35d0e2ea46c89b0bb8b988752d7da9f2 Mon Sep 17 00:00:00 2001 From: Jack Date: Tue, 18 Aug 2026 16:16:02 +0800 Subject: [PATCH] fix(deepdoc): preserve cross-page continuation rows on column-count mismatch (#18416) --- .../deepdoc/parser/pdf/table/table_merge.go | 100 ++++++++------- .../parser/pdf/table/table_merge_test.go | 117 ++++++++++++++---- .../deepdoc/parser/pdf/table/table_spans.go | 7 ++ .../parser/pdf/table/table_spans_test.go | 34 +++++ 4 files changed, 192 insertions(+), 66 deletions(-) diff --git a/internal/deepdoc/parser/pdf/table/table_merge.go b/internal/deepdoc/parser/pdf/table/table_merge.go index 233ba91688..f429cbd76d 100644 --- a/internal/deepdoc/parser/pdf/table/table_merge.go +++ b/internal/deepdoc/parser/pdf/table/table_merge.go @@ -122,31 +122,46 @@ func MergeTablesAcrossPages(tables []pdf.TableItem, medianHeights map[int]float6 // production path); Grid-less tables fall back to the cells path // and must be left untouched to avoid regression. // - // Guard: all merged pages must share the anchor's column count. A - // jagged cross-page stack (continuation page with a different number - // of columns) would feed ConstructTable a non-uniform grid, causing - // CalSpans / CleanupOrphanColumns / RowsToHTML to misalign or - // silently drop continuation columns and possibly delete a - // legitimate anchor column. In that case we skip the rebuild and - // keep the anchor-only Grid — the same safe degrade as the - // len(anchor.Grid)==0 path (continuation rows dropped, but - // structurally valid HTML). + // The anchor and continuation pages form ONE logical table, but TSR + // can detect a slightly different number of columns per page (or even + // per row within a page). A non-uniform grid must NOT cause the + // continuation rows to be dropped — doing so silently deletes an + // entire continuation page from the output. + // + // We stack the unpadded per-page grids first, so the zero-coordinate + // padding cells never enter the Y-shift math in stackGrids / + // gridYExtent, then align the rebuilt grid to a shared column model: + // the maximum column count seen across all rows of all grids, padding + // shorter rows by index. Column i of a continuation page maps to + // column i of the anchor because they are the same logical column of + // one cross-page table, so padding keeps the grid uniform + // (CalSpans / CleanupOrphanColumns / RowsToHTML never see a jagged + // grid) while preserving every row. if len(anchor.Grid) > 0 && len(contGrids) > 0 { - // Stack the per-page grids the same way stackGrids expects them. - allGrids := make([][][]pdf.TSRCell, 0, 1+len(contGrids)) - allGrids = append(allGrids, anchor.Grid) - allGrids = append(allGrids, contGrids...) - // Every row of every merged grid (anchor included) must share one - // column count, otherwise stacking yields a non-uniform grid that - // CalSpans / CleanupOrphanColumns / RowsToHTML would misalign or - // silently drop. uniformColCount checks each row of each grid, not - // just the first row of each continuation grid, so a continuation - // whose interior rows are narrower than its first row is caught. - // It returns 0 (skip rebuild, keep anchor-only Grid) on any - // mismatch — the same safe degrade as len(anchor.Grid)==0. - if uniformColCount(allGrids...) > 0 { + allGrids := append([][][]pdf.TSRCell{anchor.Grid}, contGrids...) + uniCols := 0 + for _, g := range allGrids { + for _, row := range g { + if len(row) > uniCols { + uniCols = len(row) + } + } + } + keep := true + for _, g := range allGrids { + if len(g) == 0 { + // Degenerate grid with no rows: degrade to anchor-only so + // we don't build a malformed grid. + keep = false + break + } + } + if keep { + // Stack the unpadded grids first so the padded zero-coordinate + // cells stay out of the Y-shift calculation, then align the + // rebuilt grid to the shared column model. if rebuilt := stackGrids(allGrids...); len(rebuilt) > 0 { - anchor.Grid = rebuilt + anchor.Grid = padGridCols(rebuilt, uniCols) } } } @@ -208,26 +223,27 @@ func gridYExtent(g [][]pdf.TSRCell) (minY, maxY float64) { return minY, maxY } -// uniformColCount returns the shared column count if every non-empty row of -// every given grid has the same number of columns, or 0 otherwise. Unlike a -// first-row-only check, it inspects each row of each grid, so a grid whose -// interior rows are narrower than its first row is reported non-uniform. Empty -// rows are skipped so legitimate empty rows do not cause a false mismatch. -func uniformColCount(grids ...[][]pdf.TSRCell) int { - cols := -1 - for _, g := range grids { - for _, row := range g { - if len(row) == 0 { - continue - } - if cols == -1 { - cols = len(row) - } else if len(row) != cols { - return 0 - } - } +// padGridCols returns a copy of grid with every row extended to width uniCols +// by appending zero-valued cells. Grids shorter than uniCols keep their +// existing cells at the same column indices; column i of a continuation page +// maps to column i of the anchor because they are the same logical column of +// one cross-page table. Rows are never added or removed, so no content is +// lost when per-page (or per-row) column counts differ. +func padGridCols(grid [][]pdf.TSRCell, uniCols int) [][]pdf.TSRCell { + if uniCols <= 0 { + return grid } - return cols + out := make([][]pdf.TSRCell, len(grid)) + for i, row := range grid { + if len(row) >= uniCols { + out[i] = row + continue + } + nr := make([]pdf.TSRCell, uniCols) + copy(nr, row) + out[i] = nr + } + return out } // shiftGridY returns a copy of g with every cell's Y0/Y1 shifted by dy. diff --git a/internal/deepdoc/parser/pdf/table/table_merge_test.go b/internal/deepdoc/parser/pdf/table/table_merge_test.go index 29ae8fc2d4..d374bca369 100644 --- a/internal/deepdoc/parser/pdf/table/table_merge_test.go +++ b/internal/deepdoc/parser/pdf/table/table_merge_test.go @@ -1,6 +1,7 @@ package table import ( + "fmt" "testing" pdf "ragflow/internal/deepdoc/parser/pdf/type" @@ -231,21 +232,15 @@ func TestMergeTablesAcrossPages_RebuildsGridAcrossPages(t *testing.T) { } } -// TestMergeTablesAcrossPages_JaggedContinuationFallsBackToAnchorGrid verifies -// that when a continuation page's grid is not column-uniform with the anchor -// (a jagged cross-page stack), MergeTablesAcrossPages does NOT rebuild a -// non-uniform Grid. Instead it keeps the anchor-only Grid, so ConstructTable -// emits a structurally valid (if continuation-dropping) table rather than -// malformed HTML. This is the same safe degrade as the len(anchor.Grid)==0 -// path, and keeps the merge decision (and the appended continuation Cells) -// unchanged. -// -// Each case below is a jagged stack the OLD guard (which only compared the -// first row of each continuation grid) would have wrongly allowed to rebuild. -// The first case differs on the first row; the second matches on the first -// row but narrows on an interior row — exactly the gap the per-row -// uniformColCount check closes. -func TestMergeTablesAcrossPages_JaggedContinuationFallsBackToAnchorGrid(t *testing.T) { +// TestMergeTablesAcrossPages_JaggedContinuationPreservesRows verifies that +// when a continuation page's grid is not column-uniform with the anchor (a +// jagged cross-page stack), MergeTablesAcrossPages still preserves every +// continuation row. It aligns both per-page grids to a shared column model +// (the max column count, padding shorter rows by index) and stacks all rows, +// instead of dropping the continuation page's Grid. Regression test for the +// bug where a non-uniform cross-page grid silently deleted an entire +// continuation page from the output. +func TestMergeTablesAcrossPages_JaggedContinuationPreservesRows(t *testing.T) { pageGrid := func(rows [][]string) [][]pdf.TSRCell { g := make([][]pdf.TSRCell, len(rows)) for r, row := range rows { @@ -311,17 +306,32 @@ func TestMergeTablesAcrossPages_JaggedContinuationFallsBackToAnchorGrid(t *testi if len(merged) != 1 { t.Fatalf("expected 1 merged table, got %d", len(merged)) } - // Columns differ (anchor 3 vs continuation jagged) → rebuild must - // be skipped → Grid stays anchor-only, NOT a 4-row jagged grid. - if len(merged[0].Grid) != len(tc.anchorRows) { - t.Fatalf("jagged continuation must fall back to anchor-only Grid (want %d rows), got %d", len(tc.anchorRows), len(merged[0].Grid)) + // Columns differ (anchor 3 vs continuation jagged) → aligned to + // uniCols=3, rows still stacked: anchor + continuation (no drop). + wantRows := len(tc.anchorRows) + len(tc.contRows) + if len(merged[0].Grid) != wantRows { + t.Fatalf("jagged continuation must preserve all rows (want %d), got %d", wantRows, len(merged[0].Grid)) } - // Anchor rows preserved; continuation NOT stacked into the Grid. + // Aligned width is the max column count (3) for every row. + for r, row := range merged[0].Grid { + if len(row) != 3 { + t.Errorf("row %d: aligned grid width must be max cols (3), got %d", r, len(row)) + } + } + // Anchor rows preserved first. if merged[0].Grid[0][0].Text != tc.anchorRows[0][0] || merged[0].Grid[1][0].Text != tc.anchorRows[1][0] { - t.Errorf("anchor rows corrupted after jagged fallback: %s / %s", merged[0].Grid[0][0].Text, merged[0].Grid[1][0].Text) + t.Errorf("anchor rows corrupted after alignment: %s / %s", merged[0].Grid[0][0].Text, merged[0].Grid[1][0].Text) } - // Continuation Cells are still appended (pre-fix behaviour) — the - // merge decision is unchanged; only the Grid stays uniform. + // Continuation rows appended in page order, padded by index. + base := len(tc.anchorRows) + for r, crow := range tc.contRows { + for c, txt := range crow { + if merged[0].Grid[base+r][c].Text != txt { + t.Errorf("continuation cell lost after alignment: Grid[%d][%d]=%q want %q", base+r, c, merged[0].Grid[base+r][c].Text, txt) + } + } + } + // Continuation Cells are still appended (merge decision unchanged). hasCont := false for _, c := range merged[0].Cells { if c.Text == tc.contCellText { @@ -330,12 +340,71 @@ func TestMergeTablesAcrossPages_JaggedContinuationFallsBackToAnchorGrid(t *testi } } if !hasCont { - t.Errorf("continuation Cells should still be appended even when Grid rebuild is skipped") + t.Errorf("continuation Cells should still be appended after alignment") } }) } } +// TestMergeTablesAcrossPages_MixedColumnCountsPreservesAllRows reproduces the +// 中加纯债 cross-page table: page 0 has 27 rows × 6 cols and page 1 has 35 +// rows × 7 cols (TSR detects one extra spurious column on page 1). The merged +// table must contain ALL 62 rows (27 + 35) at the shared width of 7 columns. +func TestMergeTablesAcrossPages_MixedColumnCountsPreservesAllRows(t *testing.T) { + gridWithTag := func(rows, cols int, tag string) [][]pdf.TSRCell { + g := make([][]pdf.TSRCell, rows) + for r := 0; r < rows; r++ { + g[r] = make([]pdf.TSRCell, cols) + for c := 0; c < cols; c++ { + g[r][c] = pdf.TSRCell{ + X0: float64(c) * 100, Y0: float64(r) * 30, + X1: float64(c)*100 + 100, Y1: float64(r)*30 + 30, + Text: fmt.Sprintf("%s_r%d_c%d", tag, r, c), + } + } + } + return g + } + pg0 := pdf.TableItem{ + Positions: []pdf.Position{{PageNumbers: []int{0}, Left: 0, Right: 600, Top: 0, Bottom: 810}}, + Scale: 1.0, + Grid: gridWithTag(27, 6, "p0"), + } + pg1 := pdf.TableItem{ + Positions: []pdf.Position{{PageNumbers: []int{1}, Left: 0, Right: 700, Top: 0, Bottom: 1050}}, + Scale: 1.0, + Grid: gridWithTag(35, 7, "p1"), + } + + merged := MergeTablesAcrossPages([]pdf.TableItem{pg0, pg1}, nil) + if len(merged) != 1 { + t.Fatalf("expected 1 merged table, got %d", len(merged)) + } + // All 62 rows (27 + 35) must survive the cross-page merge. + if len(merged[0].Grid) != 62 { + t.Fatalf("merged Grid must contain all rows from both pages (want 62), got %d", len(merged[0].Grid)) + } + // Shared width is the max column count (7) for every row. + for r, row := range merged[0].Grid { + if len(row) != 7 { + t.Errorf("row %d: aligned grid width must be max cols (7), got %d", r, len(row)) + } + } + // Anchor rows first, continuation rows appended, both complete. + if merged[0].Grid[0][0].Text != "p0_r0_c0" { + t.Errorf("first anchor row lost: %s", merged[0].Grid[0][0].Text) + } + if merged[0].Grid[26][5].Text != "p0_r26_c5" { + t.Errorf("last anchor row lost: %s", merged[0].Grid[26][5].Text) + } + if merged[0].Grid[27][0].Text != "p1_r0_c0" { + t.Errorf("first continuation row lost: %s", merged[0].Grid[27][0].Text) + } + if merged[0].Grid[61][6].Text != "p1_r34_c6" { + t.Errorf("last continuation row lost: %s", merged[0].Grid[61][6].Text) + } +} + // TestMergeTablesAcrossPages_ThreePageCumulativeShift verifies that with three // consecutive pages the per-page grids stack cumulatively: each continuation // page sits strictly below the previous page's last row, and the Y shift diff --git a/internal/deepdoc/parser/pdf/table/table_spans.go b/internal/deepdoc/parser/pdf/table/table_spans.go index d7860199cb..bf145ac310 100644 --- a/internal/deepdoc/parser/pdf/table/table_spans.go +++ b/internal/deepdoc/parser/pdf/table/table_spans.go @@ -42,6 +42,13 @@ func CalSpans(rows [][]pdf.TSRCell) (map[[2]int][2]int, map[[2]int]bool) { if strings.Contains(cell.Label, "spanning") { continue } + // Cells without position data (e.g. the zero-coordinate cells + // padded by MergeTablesAcrossPages to align per-page column + // counts) must not define column/row geometry, or they would drag + // boundaries to the origin and corrupt span detection. + if cell.X0 == 0 && cell.X1 == 0 && cell.Y0 == 0 && cell.Y1 == 0 { + continue + } if cell.X0 < colLeft[j] { colLeft[j] = cell.X0 } diff --git a/internal/deepdoc/parser/pdf/table/table_spans_test.go b/internal/deepdoc/parser/pdf/table/table_spans_test.go index 29db5251d6..f298744b54 100644 --- a/internal/deepdoc/parser/pdf/table/table_spans_test.go +++ b/internal/deepdoc/parser/pdf/table/table_spans_test.go @@ -43,3 +43,37 @@ func TestCalSpans_NonSpanningCellsNotPolluted(t *testing.T) { t.Logf("spans: %v, covered: %v", spans, covered) } + +// TestCalSpans_IgnoresZeroPositionPaddedCells guards against a regression +// where MergeTablesAcrossPages pads a continuation page's grid with +// zero-coordinate cells (X0=X1=Y0=Y1=0) to align per-page column counts. +// Those padding cells must not define column geometry: without the +// zero-position skip, the padded column's left boundary is dragged to the +// origin, its center lands inside the neighbouring column's X range, and the +// neighbour is falsely reported as spanning into the padded column. +func TestCalSpans_IgnoresZeroPositionPaddedCells(t *testing.T) { + rows := [][]pdf.TSRCell{ + { + {X0: 0, Y0: 0, X1: 100, Y1: 30, Text: "a"}, + {X0: 100, Y0: 0, X1: 200, Y1: 30, Text: "b"}, + {X0: 0, Y0: 0, X1: 0, Y1: 0, Text: ""}, // zero-position padding cell + }, + { + {X0: 0, Y0: 35, X1: 100, Y1: 65, Text: "c"}, + {X0: 100, Y0: 35, X1: 200, Y1: 65, Text: "d"}, + {X0: 200, Y0: 35, X1: 300, Y1: 65, Text: "e"}, + }, + } + + spans, _ := CalSpans(rows) + + // Column 1 (cell "b" at [0,1], X=100-200) must NOT span into the padded + // column 2 just because the padding cell pulled column 2's center left. + if s, ok := spans[[2]int{0, 1}]; ok { + t.Errorf("cell [0,1] should NOT span into the zero-position padded column, got %v", s) + } + // Sanity: real adjacent columns keep their own geometry. + if s, ok := spans[[2]int{0, 0}]; ok { + t.Errorf("cell [0,0] should not span, got %v", s) + } +}