Fix cross-page merged tables dropping continuation rows (#18362)

internal/deepdoc/parser/pdf/table/table_merge.go

@@ -1,6 +1,7 @@
 package table
 
 import (
+	"math"
 	"sort"
 
 	pdf "ragflow/internal/deepdoc/parser/pdf/type"
@@ -47,6 +48,7 @@ func MergeTablesAcrossPages(tables []pdf.TableItem, medianHeights map[int]float6
 		}
 		anchor := tables[it.idx]
 		merged[it.idx] = true
+		var contGrids [][][]pdf.TSRCell
 
 		// Python nomerge_lout_no: tables whose box is followed by a
 		// caption/title/reference should not be merged cross-page.
@@ -102,6 +104,7 @@ func MergeTablesAcrossPages(tables []pdf.TableItem, medianHeights map[int]float6
 			// Merge: combine cells and positions.
 			anchor.Cells = append(anchor.Cells, tables[jt.idx].Cells...)
 			anchor.Positions = append(anchor.Positions, tables[jt.idx].Positions...)
+			contGrids = append(contGrids, tables[jt.idx].Grid)
 			if tables[jt.idx].Caption != "" {
 				if anchor.Caption != "" {
 					anchor.Caption += " "
@@ -113,6 +116,39 @@ func MergeTablesAcrossPages(tables []pdf.TableItem, medianHeights map[int]float6
 			anchorBtm = bp.Bottom
 			ap = anchor.Positions[len(anchor.Positions)-1]
 		}
+		// Rebuild the merged Grid from the per-page grids so ConstructTable
+		// emits rows from every merged page, not just the stale anchor
+		// (page-0) grid. Only when the anchor already had a Grid (the
+		// 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).
+		if len(anchor.Grid) > 0 && len(contGrids) > 0 {
+			anchorCols := len(anchor.Grid[0])
+			uniform := true
+			for _, cg := range contGrids {
+				if len(cg) == 0 || len(cg[0]) != anchorCols {
+					uniform = false
+					break
+				}
+			}
+			if uniform {
+				allGrids := make([][][]pdf.TSRCell, 0, 1+len(contGrids))
+				allGrids = append(allGrids, anchor.Grid)
+				allGrids = append(allGrids, contGrids...)
+				if rebuilt := stackGrids(allGrids...); len(rebuilt) > 0 {
+					anchor.Grid = rebuilt
+				}
+			}
+		}
 		result = append(result, anchor)
 	}
 	// Append unprocessed tables (those with empty Positions) so they
@@ -124,3 +160,65 @@ func MergeTablesAcrossPages(tables []pdf.TableItem, medianHeights map[int]float6
 	}
 	return result
 }
+
+// stackGrids concatenates per-page grids (each already built correctly by
+// processOneTable) into one grid for a cross-page-merged table. Continuation
+// pages are shifted in Y so their rows sit strictly below the anchor rows,
+// keeping Y-based downstream logic (span detection, ordering) monotonic.
+func stackGrids(grids ...[][]pdf.TSRCell) [][]pdf.TSRCell {
+	var out [][]pdf.TSRCell
+	prevMaxY := 0.0
+	for _, g := range grids {
+		if len(g) == 0 {
+			continue
+		}
+		minY, maxY := gridYExtent(g)
+		if prevMaxY > 0 {
+			// Place this page's rows below everything stacked so far, with a
+			// gap of at least one row height to avoid false row grouping.
+			shift := prevMaxY - minY + math.Max(maxY-minY, 1)
+			g = shiftGridY(g, shift)
+			maxY += shift
+		}
+		out = append(out, g...)
+		prevMaxY = maxY
+	}
+	return out
+}
+
+// gridYExtent returns the min/max Y0/Y1 across all cells of a grid.
+func gridYExtent(g [][]pdf.TSRCell) (minY, maxY float64) {
+	first := true
+	for _, row := range g {
+		for _, c := range row {
+			if first {
+				minY, maxY = c.Y0, c.Y1
+				first = false
+				continue
+			}
+			if c.Y0 < minY {
+				minY = c.Y0
+			}
+			if c.Y1 > maxY {
+				maxY = c.Y1
+			}
+		}
+	}
+	return minY, maxY
+}
+
+// shiftGridY returns a copy of g with every cell's Y0/Y1 shifted by dy.
+func shiftGridY(g [][]pdf.TSRCell, dy float64) [][]pdf.TSRCell {
+	out := make([][]pdf.TSRCell, len(g))
+	for i, row := range g {
+		nr := make([]pdf.TSRCell, len(row))
+		for j, c := range row {
+			nc := c
+			nc.Y0 += dy
+			nc.Y1 += dy
+			nr[j] = nc
+		}
+		out[i] = nr
+	}
+	return out
+}

internal/deepdoc/parser/pdf/table/table_merge_test.go

@@ -177,3 +177,249 @@ func TestMergeTablesAcrossPages_NoMedianHeights(t *testing.T) {
 		t.Errorf("expected 2 cells after merge, got %d", len(merged[0].Cells))
 	}
 }
+
+// TestMergeTablesAcrossPages_RebuildsGridAcrossPages verifies that after a
+// cross-page merge the merged table's Grid contains rows from BOTH pages,
+// not just the anchor (page-0) grid. This catches the regression where
+// ConstructTable reads the stale anchor Grid and drops all continuation rows.
+func TestMergeTablesAcrossPages_RebuildsGridAcrossPages(t *testing.T) {
+	pageGrid := func(rows [][]string) [][]pdf.TSRCell {
+		g := make([][]pdf.TSRCell, len(rows))
+		for r, row := range rows {
+			g[r] = make([]pdf.TSRCell, len(row))
+			for c := range row {
+				g[r][c] = pdf.TSRCell{
+					X0: float64(c) * 100, Y0: float64(r) * 30,
+					X1: float64(c)*100 + 100, Y1: float64(r)*30 + 30,
+					Text: row[c],
+				}
+			}
+		}
+		return g
+	}
+	pg0 := pdf.TableItem{
+		Positions: []pdf.Position{{PageNumbers: []int{0}, Left: 0, Right: 200, Top: 0, Bottom: 60}},
+		Scale:     1.0,
+		Grid:      pageGrid([][]string{{"a", "b"}, {"c", "d"}}),
+	}
+	pg1 := pdf.TableItem{
+		Positions: []pdf.Position{{PageNumbers: []int{1}, Left: 0, Right: 200, Top: 0, Bottom: 60}},
+		Scale:     1.0,
+		Grid:      pageGrid([][]string{{"e", "f"}, {"g", "h"}}),
+	}
+
+	merged := MergeTablesAcrossPages([]pdf.TableItem{pg0, pg1}, nil)
+	if len(merged) != 1 {
+		t.Fatalf("expected 1 merged table, got %d", len(merged))
+	}
+	// Anchor has 2 rows, continuation has 2 rows → merged Grid must be 4.
+	if len(merged[0].Grid) != 4 {
+		t.Fatalf("merged Grid must contain rows from both pages (want 4), got %d", len(merged[0].Grid))
+	}
+	// Continuation rows must appear after anchor rows, in page order.
+	if merged[0].Grid[0][0].Text != "a" || merged[0].Grid[2][0].Text != "e" {
+		t.Errorf("row order wrong after stacking: %s / %s", merged[0].Grid[0][0].Text, merged[0].Grid[2][0].Text)
+	}
+	// Continuation rows must be Y-shifted strictly below the anchor rows so
+	// Y-monotonic downstream logic (span detection, ordering) stays correct.
+	// Catches a regression where the shift is dropped but stacking is kept:
+	// row order would still be correct by page order, so only this assertion
+	// would fail.
+	if merged[0].Grid[2][0].Y0 <= merged[0].Grid[1][0].Y1 {
+		t.Errorf("continuation row was not shifted below the anchor rows (Grid[2][0].Y0=%v <= Grid[1][0].Y1=%v)",
+			merged[0].Grid[2][0].Y0, merged[0].Grid[1][0].Y1)
+	}
+}
+
+// TestMergeTablesAcrossPages_JaggedContinuationFallsBackToAnchorGrid verifies
+// that when a continuation page's grid has a different number of columns than
+// 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.
+func TestMergeTablesAcrossPages_JaggedContinuationFallsBackToAnchorGrid(t *testing.T) {
+	pageGrid := func(rows [][]string) [][]pdf.TSRCell {
+		g := make([][]pdf.TSRCell, len(rows))
+		for r, row := range rows {
+			g[r] = make([]pdf.TSRCell, len(row))
+			for c := range row {
+				g[r][c] = pdf.TSRCell{
+					X0: float64(c) * 100, Y0: float64(r) * 30,
+					X1: float64(c)*100 + 100, Y1: float64(r)*30 + 30,
+					Text: row[c],
+				}
+			}
+		}
+		return g
+	}
+	cells := func(rows [][]string) []pdf.TSRCell {
+		var cs []pdf.TSRCell
+		for r, row := range rows {
+			for c := range row {
+				cs = append(cs, pdf.TSRCell{
+					X0: float64(c) * 100, Y0: float64(r) * 30,
+					X1: float64(c)*100 + 100, Y1: float64(r)*30 + 30,
+					Text: row[c],
+				})
+			}
+		}
+		return cs
+	}
+	// Anchor: 3 columns. Continuation: 2 columns (jagged).
+	pg0 := pdf.TableItem{
+		Positions: []pdf.Position{{PageNumbers: []int{0}, Left: 0, Right: 300, Top: 0, Bottom: 60}},
+		Scale:     1.0,
+		Grid:      pageGrid([][]string{{"a", "b", "c"}, {"d", "e", "f"}}),
+		Cells:     cells([][]string{{"a", "b", "c"}, {"d", "e", "f"}}),
+	}
+	pg1 := pdf.TableItem{
+		Positions: []pdf.Position{{PageNumbers: []int{1}, Left: 0, Right: 200, Top: 0, Bottom: 60}},
+		Scale:     1.0,
+		Grid:      pageGrid([][]string{{"g", "h"}, {"i", "j"}}),
+		Cells:     cells([][]string{{"g", "h"}, {"i", "j"}}),
+	}
+
+	merged := MergeTablesAcrossPages([]pdf.TableItem{pg0, pg1}, nil)
+	if len(merged) != 1 {
+		t.Fatalf("expected 1 merged table, got %d", len(merged))
+	}
+	// Columns differ (3 vs 2) → rebuild must be skipped → Grid stays
+	// anchor-only (2 rows), NOT a 4-row jagged grid.
+	if len(merged[0].Grid) != 2 {
+		t.Fatalf("jagged continuation must fall back to anchor-only Grid (want 2 rows), got %d", len(merged[0].Grid))
+	}
+	// Anchor rows preserved; continuation NOT stacked into the Grid.
+	if merged[0].Grid[0][0].Text != "a" || merged[0].Grid[1][0].Text != "d" {
+		t.Errorf("anchor rows corrupted after jagged fallback: %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 so HTML is valid.
+	hasCont := false
+	for _, c := range merged[0].Cells {
+		if c.Text == "g" {
+			hasCont = true
+			break
+		}
+	}
+	if !hasCont {
+		t.Errorf("continuation Cells should still be appended even when Grid rebuild is skipped")
+	}
+}
+
+// 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
+// accumulates (page2 below page1 below page0). Catches a regression where
+// stackGrids resets prevMaxY to the anchor instead of carrying the prior
+// page's shifted bottom forward.
+func TestMergeTablesAcrossPages_ThreePageCumulativeShift(t *testing.T) {
+	pageGrid := func(rows [][]string) [][]pdf.TSRCell {
+		g := make([][]pdf.TSRCell, len(rows))
+		for r, row := range rows {
+			g[r] = make([]pdf.TSRCell, len(row))
+			for c := range row {
+				g[r][c] = pdf.TSRCell{
+					X0: float64(c) * 100, Y0: float64(r) * 30,
+					X1: float64(c)*100 + 100, Y1: float64(r)*30 + 30,
+					Text: row[c],
+				}
+			}
+		}
+		return g
+	}
+	// Three pages, 2 rows × 2 cols each, identical layout.
+	pages := []pdf.TableItem{
+		{Positions: []pdf.Position{{PageNumbers: []int{0}, Left: 0, Right: 200, Top: 0, Bottom: 60}}, Scale: 1.0, Grid: pageGrid([][]string{{"a", "b"}, {"c", "d"}})},
+		{Positions: []pdf.Position{{PageNumbers: []int{1}, Left: 0, Right: 200, Top: 0, Bottom: 60}}, Scale: 1.0, Grid: pageGrid([][]string{{"e", "f"}, {"g", "h"}})},
+		{Positions: []pdf.Position{{PageNumbers: []int{2}, Left: 0, Right: 200, Top: 0, Bottom: 60}}, Scale: 1.0, Grid: pageGrid([][]string{{"i", "j"}, {"k", "l"}})},
+	}
+
+	merged := MergeTablesAcrossPages(pages, nil)
+	if len(merged) != 1 {
+		t.Fatalf("expected 1 merged table, got %d", len(merged))
+	}
+	// 3 pages × 2 rows → 6 rows, in page order.
+	if len(merged[0].Grid) != 6 {
+		t.Fatalf("3-page merge must stack all rows (want 6), got %d", len(merged[0].Grid))
+	}
+	if merged[0].Grid[0][0].Text != "a" || merged[0].Grid[2][0].Text != "e" || merged[0].Grid[4][0].Text != "i" {
+		t.Errorf("row order wrong after 3-page stacking: %s / %s / %s",
+			merged[0].Grid[0][0].Text, merged[0].Grid[2][0].Text, merged[0].Grid[4][0].Text)
+	}
+	// Cumulative Y shift: page1 strictly below page0, page2 strictly below
+	// page1, and page2 below page1 (monotonic accumulation).
+	if merged[0].Grid[2][0].Y0 <= merged[0].Grid[1][0].Y1 {
+		t.Errorf("page1 not shifted below page0 (Grid[2][0].Y0=%v <= Grid[1][0].Y1=%v)",
+			merged[0].Grid[2][0].Y0, merged[0].Grid[1][0].Y1)
+	}
+	if merged[0].Grid[4][0].Y0 <= merged[0].Grid[3][0].Y1 {
+		t.Errorf("page2 not shifted below page1 (Grid[4][0].Y0=%v <= Grid[3][0].Y1=%v)",
+			merged[0].Grid[4][0].Y0, merged[0].Grid[3][0].Y1)
+	}
+	if merged[0].Grid[4][0].Y0 <= merged[0].Grid[2][0].Y0 {
+		t.Errorf("Y shift not cumulative: page2 (Y0=%v) must be below page1 (Y0=%v)",
+			merged[0].Grid[4][0].Y0, merged[0].Grid[2][0].Y0)
+	}
+}
+
+// TestMergeTablesAcrossPages_GridlessAnchorUnchanged verifies that when the
+// anchor has no Grid, MergeTablesAcrossPages skips the rebuild entirely and
+// leaves anchor.Grid empty (nil) so ConstructTable falls back to the Cells
+// path — the same pre-fix behaviour. This locks the no-regression promise the
+// fix relies on for Grid-less tables (and the 7 pre-existing tests that set no
+// Grid). The cross-page merge decision itself is unchanged: continuation
+// Cells and Positions are still appended.
+func TestMergeTablesAcrossPages_GridlessAnchorUnchanged(t *testing.T) {
+	cells := func(texts []string) []pdf.TSRCell {
+		cs := make([]pdf.TSRCell, len(texts))
+		for i, txt := range texts {
+			cs[i] = pdf.TSRCell{
+				X0: 0, Y0: float64(i) * 30, X1: 100, Y1: float64(i)*30 + 30,
+				Text: txt,
+			}
+		}
+		return cs
+	}
+	// Anchor has Cells but NO Grid; continuation also Grid-less.
+	pg0 := pdf.TableItem{
+		Positions: []pdf.Position{{PageNumbers: []int{0}, Left: 0, Right: 200, Top: 0, Bottom: 60}},
+		Scale:     1.0,
+		Cells:     cells([]string{"a", "b", "c", "d"}),
+	}
+	pg1 := pdf.TableItem{
+		Positions: []pdf.Position{{PageNumbers: []int{1}, Left: 0, Right: 200, Top: 0, Bottom: 60}},
+		Scale:     1.0,
+		Cells:     cells([]string{"e", "f", "g", "h"}),
+	}
+
+	merged := MergeTablesAcrossPages([]pdf.TableItem{pg0, pg1}, nil)
+	if len(merged) != 1 {
+		t.Fatalf("expected 1 merged table, got %d", len(merged))
+	}
+	// Guard: anchor has no Grid → rebuild skipped entirely → Grid stays empty.
+	if len(merged[0].Grid) != 0 {
+		t.Fatalf("Grid-less anchor must keep Grid empty (rebuild skipped), got %d rows", len(merged[0].Grid))
+	}
+	// Merge decision unchanged: all continuation Cells still appended.
+	have := map[string]bool{}
+	for _, c := range merged[0].Cells {
+		have[c.Text] = true
+	}
+	for _, want := range []string{"a", "b", "c", "d", "e", "f", "g", "h"} {
+		if !have[want] {
+			t.Errorf("merged Cells missing %q (merge decision changed for Grid-less anchor)", want)
+		}
+	}
+	// Positions from both pages present → the cross-page merge did happen.
+	pages := map[int]bool{}
+	for _, p := range merged[0].Positions {
+		for _, pn := range p.PageNumbers {
+			pages[pn] = true
+		}
+	}
+	if !pages[0] || !pages[1] {
+		t.Errorf("cross-page merge did not combine both pages' positions: %v", pages)
+	}
+}