mirror of
https://github.com/infiniflow/ragflow.git
synced 2026-08-20 15:25:04 +08:00
fix(deepdoc): preserve cross-page continuation rows on column-count mismatch (#18416)
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user