fix(table): add generic merged-cell reconciliation

[dsolankii]

Issue resolved by this Pull Request:
Resolves #3698

Description

This PR fixes a TableFormer merged-cell reconstruction issue where merged rows and columns could be mishandled in both TableFormer V1 and TableFormer V2, regardless of whether do_cell_matching is enabled or disabled.

The implementation is intentionally conservative. It validates candidate table structures before accepting them and preserves the incumbent output when a repair would introduce invalid topology, text loss, unsafe metadata changes, or same-shape text corruption.

What this PR changes

1. Adds topology validation for table structures

Adds reusable topology validation for table cells.

The validation guards against:

• Invalid spans
• Overlapping cells
• Cells outside the grid
• Unsafe topology regressions
• OTSL regeneration issues

Relevant files:

• docling/models/stages/table_structure/table_topology.py
• tests/test_table_topology.py

2. Adds conservative table-structure reconciliation

Adds a production-used reconciliation path for merged-cell repair.

The reconciler now handles:

• Column reconciliation from text geometry
• Row-boundary repair from repeated vertical text-band evidence
• Row-span repair from empty-cell/span evidence
• Final structure selection between incumbent and repaired candidates

The large reconciler was split into focused modules so the repo max-lines hook passes.

Relevant files:

• docling/models/stages/table_structure/table_structure_reconciler.py
• docling/models/stages/table_structure/table_structure_columns.py
• docling/models/stages/table_structure/table_structure_row_reassignment.py
• docling/models/stages/table_structure/table_structure_row_boundary.py
• docling/models/stages/table_structure/table_structure_row_spans.py
• docling/models/stages/table_structure/table_structure_reconciler_common.py

3. Adds protected structure selection and acceptance gates

Adds final acceptance gates before a repaired table can replace the incumbent table.

The acceptance layer rejects candidates that introduce:

• Text-token regression
• Invalid topology
• Topology issue regression
• Same-shape header metadata regression
• Same-shape text-slot regression

This protects against cases such as:

• Dropping existing column_header / row_header metadata
• Accepting same-shape text swaps like A | B -> B | A

Relevant files:

• docling/models/stages/table_structure/table_structure_acceptance.py
• docling/models/stages/table_structure/table_structure_candidate_selection.py
• tests/test_table_structure_acceptance.py
• tests/test_table_structure_candidate_selection.py

4. Wires reconciliation into TableFormer V1 and V2

The reconciliation path is wired into both TableFormer V1 and TableFormer V2.

Relevant files:

• docling/models/stages/table_structure/table_structure_model.py
• docling/models/stages/table_structure/table_structure_model_v2.py

Why this is generic

This PR does not hardcode the reported PDF or its content.

It does not rely on:

• PDF filename
• Document title
• Specific text values
• Fixed row count
• Fixed column count
• Issue-specific labels

The repair logic is based on:

• Table geometry
• Topology validation
• Text preservation
• Model row/column offsets
• Conservative candidate acceptance gates

Tests added

Regression fixture

• tests/data/pdf/merged_span_detectable_workflow_test.pdf

Regression tests for #3698

• tests/test_table_structure_3698_regression.py

This test is marked with:

pytestmark = pytest.mark.ml_pdf_model

It covers:

• TableFormer V1
• TableFormer V2
• do_cell_matching=True
• do_cell_matching=False

Unit and behavior tests

• tests/test_table_topology.py
• tests/test_table_structure_reconciler.py
• tests/test_table_structure_acceptance.py
• tests/test_table_structure_candidate_selection.py

Local validation

uv run prek run --files $(git diff --cached --name-only --diff-filter=ACMRT)
Passed

uv run pytest tests/test_table_structure_acceptance.py tests/test_table_structure_candidate_selection.py tests/test_table_structure_reconciler.py tests/test_table_topology.py -q
53 passed

uv run pytest tests/test_table_structure_3698_regression.py -q -m ml_pdf_model
4 passed

Manual downloaded PDF smoke test

V1 + do_cell_matching=True: detected 1 table, topology_valid=True
V1 + do_cell_matching=False: detected 1 table, topology_valid=True
V2 + do_cell_matching=True: detected 1 table, topology_valid=True
V2 + do_cell_matching=False: detected 1 table, topology_valid=True

Notes for reviewers

This PR keeps reconciliation conservative.

If a candidate table structure is unsafe, loses text, introduces invalid topology, drops same-shape header metadata, swaps same-shape cell text, or does not clearly improve over the incumbent, the original model output is preserved.

---

[github-actions]

✅ DCO Check Passed

Thanks @dsolankii, all your commits are properly signed off. 🎉

[mergify]

Merge Protections

🔴 1 of 2 protections blocking · waiting on 👀 reviews

	Protection	Waiting on
🔴	Require two reviewer for test updates	👀 reviews
🟢	Enforce conventional commit	—

🔴 Require two reviewer for test updates

Waiting for

• #approved-reviews-by >= 2

This rule is failing.

When test data is updated, we require two reviewers

• #approved-reviews-by >= 2

---

Show 1 satisfied protection

🟢 Enforce conventional commit

Make sure that we follow https://www.conventionalcommits.org/en/v1.0.0/

• title ~= ^(fix|feat|docs|style|refactor|perf|test|build|ci|chore|revert)(?:\(.+\))?(!)?:

[dsolankii]

This PR adds one regression PDF fixture for #3698:

tests/data/pdf/merged_span_detectable_workflow_test.pdf

The fixture is required because the bug reproduces through the full TableFormer PDF conversion path across V1/V2 and do_cell_matching=True/False. The new tests use this fixture to prevent regressions in merged-row/merged-column reconstruction.

I understand this triggers the repository’s test-data protection requiring two approvals.

[geoHeil]

Requesting changes. The PR is much broader than the stated fix, has a reproducible lint blocker, and the acceptance gates still allow table metadata/text corruption cases that the PR says it protects against.

[codecov]

Codecov Report

❌ Patch coverage is 86.17021% with 299 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...ng/models/stages/table_structure/table_topology.py	82.96%	101 Missing ⚠️
.../stages/table_structure/table_structure_columns.py	84.56%	71 Missing ⚠️
...able_structure/table_structure_row_reassignment.py	73.83%	56 Missing ⚠️
...ages/table_structure/table_structure_acceptance.py	91.46%	21 Missing ⚠️
...tages/table_structure/table_structure_row_spans.py	89.09%	18 Missing ⚠️
...ble_structure/table_structure_reconciler_common.py	82.05%	14 Missing ⚠️
...es/table_structure/table_structure_row_boundary.py	94.02%	12 Missing ⚠️
...e_structure/table_structure_candidate_selection.py	94.44%	4 Missing ⚠️
...stages/table_structure/table_structure_model_v2.py	87.50%	1 Missing ⚠️
...ages/table_structure/table_structure_reconciler.py	99.15%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

[dsolankii]

Thanks @geoHeil for the detailed review. I pushed an update addressing the blocking points.

Changes made

• Split table_structure_reconciler.py into focused modules so the max-lines hook now passes.
• Removed the dead candidate-scoring pipeline that was only referenced by tests and was not wired into TableFormer output.
• Kept the production-used reconciliation, topology validation, structure selection, and acceptance-gate path.
• Fixed the same-shape header metadata guard so dropping existing column_header / row_header metadata is rejected.
• Added same-shape slot-level text preservation so text swaps like A | B -> B | A are rejected.
• Added regression tests for both acceptance-gate cases.
• Marked the PDF conversion regression with pytestmark = pytest.mark.ml_pdf_model.

Local validation

uv run prek run --files $(git diff --cached --name-only --diff-filter=ACMRT)
Passed

uv run pytest tests/test_table_structure_acceptance.py tests/test_table_structure_candidate_selection.py tests/test_table_structure_reconciler.py tests/test_table_topology.py -q
53 passed

uv run pytest tests/test_table_structure_3698_regression.py -q -m ml_pdf_model
4 passed

I also tested a separate downloaded manual PDF smoke case outside the pytest fixture:

V1 + do_cell_matching=True: detected 1 table, topology_valid=True
V1 + do_cell_matching=False: detected 1 table, topology_valid=True
V2 + do_cell_matching=True: detected 1 table, topology_valid=True
V2 + do_cell_matching=False: detected 1 table, topology_valid=True

Please let me know if you want any further cleanup or a different split of the modules.

[geoHeil]

Re-reviewed current head e3b83bb. The previous findings are addressed: the line-cap hook now passes, the dead candidate pipeline was removed, the PDF regression test is marked ml_pdf_model, and the two acceptance regressions now have focused tests. CI is green too.

Still requesting changes for one new blocker: the current same-shape acceptance guard can reject a legitimate same-row-count overspan repair, so part of the V1 path can stay a no-op while the lower-level fallback test passes.

[geoHeil]

One meta request before the next push: please do a few more self-review iterations before handing this back. This still reads like a broad AI-generated patch: lots of new modules, lots of heuristics, and tests that sometimes prove the helper layer but not the production path.\n\nIdeally run 3-5 review/refinement passes with your assistant or GitHub Copilot review and explicitly ask it to find cases where the new code is wired but ineffective, too broad for #3698, or only tested one layer below the real TableFormer path. Then trim what is not needed for the issue and add top-level regression coverage for the behavior that is supposed to ship.

[dsolankii]

Thanks @geoHeil — I did several additional self-review passes before this push, specifically looking for cases where the new code was wired but ineffective, too broad for #3698, or tested below the production path.

The main issue I found was that same-shape span/topology repairs could be generated but then rejected by the acceptance layer as text_slot_regression, causing candidate selection to preserve the incumbent.

I updated this narrowly:

• Same-shape text-slot changes are still rejected by default.
• The allowance is only enabled for known span/topology repair candidates:
  • topology_repair
  • overspan_geometry_fallback
  • row_span_reconciliation
• The allowance only applies when row/column shape is unchanged and span topology actually changes.
• All other acceptance gates still run.

I also added top-level reconcile_table_structure() coverage instead of only helper-layer coverage:

• V1 production flags accept the same-row-count overspan repair.
• Unsafe same-shape A | B -> B | A text swaps are still rejected.
• Same-shape topology_repair survives acceptance/selection.
• Same-shape row_span_reconciliation survives acceptance/selection.
• V2 production flags are covered directly.

I also removed the confirmed test-only/dead reassign_text_preserving_rows helper and its helper-only tests.

Validation run

uv run pytest -q \
  tests/test_table_structure_reconciler.py \
  tests/test_table_structure_reconciler_top_level.py \
  tests/test_table_topology.py \
  tests/test_table_structure_acceptance.py \
  tests/test_table_structure_candidate_selection.py

uv run pytest -q tests/test_table_structure_3698_regression.py -m ml_pdf_model

uv run pytest -vv tests/test_table_structure_3698_regression.py -m ml_pdf_model

uv run prek run --files $(git diff --cached --name-only --diff-filter=ACMRT)

Results

54 passed for focused table-structure tests
4 passed for the ml_pdf_model #3698 regression matrix
4 passed again with -vv, covering:
  - V1 + do_cell_matching=True
  - V1 + do_cell_matching=False
  - V2 + do_cell_matching=True
  - V2 + do_cell_matching=False

prek passed

I also re-ran the real #3698 PDF regression fixture, merged_span_detectable_workflow_test.pdf, before pushing.

[geoHeil]

Re-reviewed current head b3b9656. The previous top-level no-op blocker is addressed: the V1 production-flag test now proves the same-row-count overspan repair is selected, and my focused local checks pass (54 passed plus changed-file prek).

Still requesting changes for one remaining safety hole in the new escape hatch: it allows any same-shape text-slot change once span topology changes, so the old text-swap protection is only preserved for identical-span swaps.

[geoHeil]

This escape hatch is still too broad. The new unsafe-swap test only covers identical spans, but this condition disables the text-slot regression guard for any whitelisted candidate whose span topology changes. That means a same-shape candidate can preserve the token multiset, change spans, and still move text to different logical slots while being accepted.

Minimal shape of the failure: baseline has A at (0,0)-(1,1) and B at (0,1)-(1,2) in a 2x2 grid; candidate has B spanning (0,0)-(1,2) and A at (1,0)-(2,1). Both topologies are valid, token counts are preserved, span topology changed, and with allow_same_shape_text_slot_change=True the acceptance report is accepted=True. That is still a text-location corruption.

Please tighten this to allow only the specific slot movements the span repair is supposed to perform, or add a top-level regression where a whitelisted span-changing candidate also swaps/moves unrelated text and is rejected.

[dsolankii]

Thanks @geoHeil — you were right that the same-shape escape hatch was still too broad.

I tightened this on the acceptance side so allow_same_shape_text_slot_change=True is no longer treated as a full waiver of the same-shape slot-map guard.

A whitelisted span/topology repair now still has to prove that baseline non-empty full-cell text is uniquely matchable and only changed by a nested span repair. Duplicate-text movement and token-preserving merge/split corruption are rejected conservatively.

I also added top-level regressions for:

• A whitelisted span-changing candidate that moves unrelated A/B text
• Duplicate-text ambiguous movement
• Token-preserving merge/split corruption

The existing positives still pass:

• V1 production flags accept the same-row-count overspan repair
• Unsafe identical-span same-shape A | B -> B | A swap is still rejected
• Same-shape topology_repair survives acceptance/selection
• Same-shape row_span_reconciliation survives acceptance/selection
• V2 production flags are covered directly

I also marked PDFs as binary in .gitattributes so the regression fixture is not treated as text by diff-check.

Validation

uv run prek run --files \
  .gitattributes \
  docling/models/stages/table_structure/table_structure_acceptance.py \
  tests/test_table_structure_reconciler_top_level.py

git diff --check main...HEAD

uv run pytest -q tests/test_table_structure_reconciler_top_level.py

uv run pytest -q \
  tests/test_table_structure_reconciler.py \
  tests/test_table_structure_reconciler_top_level.py \
  tests/test_table_structure_acceptance.py \
  tests/test_table_structure_candidate_selection.py \
  tests/test_table_topology.py

uv run pytest -q tests/test_table_structure_3698_regression.py -m ml_pdf_model

uv run pytest -vv tests/test_table_structure_3698_regression.py -m ml_pdf_model

Results

prek passed
git diff --check main...HEAD passed
8 passed for top-level reconciler tests
57 passed for focused table tests
4 passed for the real #3698 PDF regression
4 passed again with -vv, covering V1/V2 and do_cell_matching=True/False

[geoHeil]

Re-reviewed current head c33338c. CI is green, prek passes locally, and the focused table tests pass locally (57 passed). The previous span-changing text-swap hole is mostly addressed, but there is still one real blocker: the same-shape allowance can accept invented candidate text.

Brutally honest slop read: this is still far too much mechanism for one regression PDF. The latest correctness issue comes from patching guard on top of guard instead of having one crisp invariant: same-shape repair may only preserve existing text ownership or perform a provable span-only expansion/shrink for the same cell text, with no extra candidate text. After this blocker, I would still shrink the arbitrary candidate scorer and the duplicated span-local tuple helpers; they read like generated glue, not a stable table API.

[dsolankii]

Thanks @geoHeil — you were right that the previous text-safety guard was still too permissive.

I replaced the global source-token allowance with a local text-ownership check. A candidate can no longer add text merely because that token appears somewhere in the source cells. Candidate-only text now has to be locally supported by source geometry for that candidate cell; otherwise the candidate is rejected and the incumbent is preserved.

This keeps reconciliation conservative:

• baseline-owned text must remain present
• surplus candidate text must be locally source-supported, not globally token-supported
• unsupported invented text is rejected
• source-token laundering is rejected
• A/B swaps, duplicate ambiguity, and merge/split corruption remain rejected
• same-shape span movement still has slot/span ownership checks

I also cleaned up the patch shape:

• removed fake helper re-exports from the reconciler module
• moved helper tests to import from owner modules directly
• removed duplicate/shadowed helper code and unused imports
• removed repeated monkeypatched top-level invariant tests that are now covered by focused acceptance tests plus one production-path unsafe-candidate test
• kept the real PDF matrix and production-path coverage

I investigated replacing the candidate scorer with deterministic ordering, but that changed the V2 regression result from 10 rows to 9 rows. I restored the scorer and replaced the inline magic values with named constants instead, so this patch does not broaden behavior there.

Validation:

uv run pytest -q tests/test_table_structure_acceptance.py
# 12 passed

uv run pytest -q tests/test_table_structure_reconciler_top_level.py
# 5 passed

uv run pytest -q \
  tests/test_table_structure_reconciler.py \
  tests/test_table_structure_reconciler_top_level.py \
  tests/test_table_structure_acceptance.py \
  tests/test_table_structure_candidate_selection.py \
  tests/test_table_topology.py
# 60 passed

uv run pytest -q tests/test_table_structure_3698_regression.py -m ml_pdf_model
# 4 passed

uv run pytest -vv tests/test_table_structure_3698_regression.py -m ml_pdf_model
# 4 passed, covering V1/V2 and do_cell_matching true/false

uv run prek run --files ...
# passed

git diff --check
# passed

make validate
# passed