fix(table): decide MergeSplitTokens per table, not per row

merge_split_test.go

@@ -115,6 +115,52 @@ func TestMergeSplitTokensLeavesRealColumnsAlone(t *testing.T) {
 	}
 }
 
+// TestMergeSplitTokensKeepsTableRectangular is the property that makes
+// the feature safe to use on a real table.
+//
+// A column boundary belongs to the table, not to one row. If the merge
+// decision were made per-row, a header band that happens to contain the
+// split would collapse while the data rows below it did not — leaving
+// rows with different column counts, so the header's second date would
+// sit above the first column of figures. A sheared grid is worse than
+// the split it set out to fix.
+func TestMergeSplitTokensKeepsTableRectangular(t *testing.T) {
+	// Row 0 has the split ("Dec"+"31,"); row 1 does not — its two cells
+	// are a genuine column apart.
+	chars := []Char{
+		// row 0, y 560-568: adjacent glyphs across the boundary
+		{Text: "c", X0: 110, X1: 118, Y0: 560, Y1: 568},
+		{Text: "3", X0: 118.02, X1: 126, Y0: 560, Y1: 568},
+		// row 1, y 540-548: a wide gutter across the same boundary
+		{Text: "A", X0: 92, X1: 100, Y0: 540, Y1: 548},
+		{Text: "9", X0: 160, X1: 168, Y0: 540, Y1: 548},
+	}
+	cells := [][]BBox{
+		{{X0: 90, X1: 118, Y0: 559, Y1: 569}, {X0: 118, X1: 180, Y0: 559, Y1: 569}},
+		{{X0: 90, X1: 118, Y0: 539, Y1: 549}, {X0: 118, X1: 180, Y0: 539, Y1: 549}},
+	}
+	rows := [][]string{{"Dec", "31,"}, {"A", "9"}}
+
+	gotRows, gotCells := mergeSplitTokens(rows, cells, chars, 3)
+
+	if len(gotRows) != 2 {
+		t.Fatalf("got %d rows, want 2", len(gotRows))
+	}
+	if len(gotRows[0]) != len(gotRows[1]) {
+		t.Fatalf("rows have different column counts (%d vs %d) — the grid sheared: %q",
+			len(gotRows[0]), len(gotRows[1]), gotRows)
+	}
+	if len(gotCells[0]) != len(gotCells[1]) {
+		t.Errorf("cell rows have different lengths (%d vs %d)", len(gotCells[0]), len(gotCells[1]))
+	}
+	// The boundary split a token in row 0, so it goes for the whole table —
+	// row 1 merges too, keeping the grid rectangular.
+	want := [][]string{{"Dec31,"}, {"A9"}}
+	if !reflect.DeepEqual(gotRows, want) {
+		t.Errorf("rows = %q, want %q", gotRows, want)
+	}
+}
+
 // TestMergeSplitTokensIsOptIn pins the default. pdfplumber produces the
 // same splits (verified against pdfplumber 0.11.9), so turning this on
 // by default would silently break the parity this package promises.

page.go

@@ -724,27 +724,81 @@ func mergeSplitTokens(rows [][]string, cells [][]BBox, chars []Char, tol float64
 	if tol <= 0 {
 		tol = 3
 	}
+	cols := 0
+	for _, r := range rows {
+		if len(r) > cols {
+			cols = len(r)
+		}
+	}
+	if cols < 2 {
+		return rows, cells
+	}
+
+	// A column boundary is a property of the TABLE, not of one row. Decide
+	// once, over every row, whether boundary ci|ci+1 cuts a token — then
+	// apply that decision uniformly.
+	//
+	// Doing this per-row instead would merge the header band (where the
+	// split occurs) while leaving the data rows alone, so rows would end
+	// up with different column counts and the grid would shear: the
+	// header's second date would sit above the first column of figures.
+	// A ragged table is a worse outcome than the split it set out to fix.
+	drop := make([]bool, cols)
+	for ci := 0; ci+1 < cols; ci++ {
+		for ri := range cells {
+			if ci+1 >= len(cells[ri]) {
+				continue
+			}
+			l, r := cells[ri][ci], cells[ri][ci+1]
+			if l.IsZero() || r.IsZero() {
+				continue
+			}
+			if rows[ri][ci] == "" || rows[ri][ci+1] == "" {
+				continue
+			}
+			if boundarySplitsToken(chars, l, r, tol) {
+				drop[ci] = true
+				break
+			}
+		}
+	}
+	anyDrop := false
+	for _, d := range drop {
+		if d {
+			anyDrop = true
+			break
+		}
+	}
+	if !anyDrop {
+		return rows, cells
+	}
+
 	outRows := make([][]string, len(rows))
 	outCells := make([][]BBox, len(cells))
-
 	for ri := range rows {
 		var rowText []string
 		var rowCells []BBox
-		for ci := range rows[ri] {
+		for ci := 0; ci < cols; ci++ {
+			text := ""
+			if ci < len(rows[ri]) {
+				text = rows[ri][ci]
+			}
 			cell := BBox{}
 			if ri < len(cells) && ci < len(cells[ri]) {
 				cell = cells[ri][ci]
 			}
-			text := rows[ri][ci]
-
-			// Try to append to the previous cell rather than start a new
-			// one. Only ever merges leftwards, so a run of fragments
-			// collapses into a single cell in one pass.
-			if n := len(rowText); n > 0 && text != "" && rowText[n-1] != "" &&
-				!cell.IsZero() && !rowCells[n-1].IsZero() &&
-				boundarySplitsToken(chars, rowCells[n-1], cell, tol) {
-				rowText[n-1] += text
-				rowCells[n-1] = rowCells[n-1].Union(cell)
+			// drop[ci-1] means the boundary to this cell's LEFT is gone,
+			// so it belongs to the cell before it.
+			if ci > 0 && drop[ci-1] && len(rowText) > 0 {
+				n := len(rowText) - 1
+				rowText[n] += text
+				if !cell.IsZero() {
+					if rowCells[n].IsZero() {
+						rowCells[n] = cell
+					} else {
+						rowCells[n] = rowCells[n].Union(cell)
+					}
+				}
 				continue
 			}
 			rowText = append(rowText, text)