Engineering case study · Document models · Open source
When Row Insertion Corrupts a Valid Table in Docling Core
Minimal reproduction · Regression evidence · Upstream PR #943

In this case study
Nut | 20 became Nut | 10 after a call to insert an unrelated row. The operation returned normally. The number 20 was still present, one row lower, beside an empty label. A check that counts rows or looks for the original strings would miss the corruption. The unit of correctness here is the association between a label and its value.
A three-row reproduction
The original table has a Stock cell spanning two columns, then two ordinary data rows. Calling data.insert_row(1, ["Washer", "30"]) inserts before zero-based row 1. The following are exact text projections of data.grid from the reproduction. Stock appears twice in the original and expected grids because the same cell covers both slots. "" is the empty text in an uncovered slot.
Original grid Actual on tested upstream Expected / proposed patch
Stock | Stock Bolt | Stock Stock | Stock
Bolt | 10 Washer | 30 Washer | 30
Nut | 20 Nut | 10 Bolt | 10
"" | 20 Nut | 20
The reproduction constructs TableData directly on Docling Core 2.105.0 at 2dc4c9f511269eb39be5aad84b838f7bb179142e. It needs neither a PDF nor a model. The affected methods belong to the document model’s row-editing API. They are insert_row, insert_rows, and add_row through its delegation to insertion. Issue #942 and runnable reproduction
Cell storage and the materialized grid
table_cells holds one object per stored cell. Each object describes a rectangle through half-open row and column offsets. A column span covers multiple columns with one object, a row span covers multiple rows, and a sparse table can have uncovered positions. A cell list can also be out of row-major order. grid materializes slots from those offsets. In the reproduction, six slots are represented by five cell objects. Grid implementation at the tested base
The old code spliced at effective_index * self.num_cols. The original renumbering loop then derived every cell’s row range from its list position.
for index, cell in enumerate(self.table_cells):
new_index = index // self.num_cols
cell.start_row_offset_idx = new_index
cell.end_row_offset_idx = new_index + 1
At insertion boundary 1 with two columns, the splice starts at list index 2. The first cell of row 1, Bolt, is already at index 1 because the header consumed just one entry. The loop assigns Bolt to row 0, where it overwrites the left slot of Stock when the grid is materialized. Nut and 10 end up in row 2. Column offsets stay in place, so the damage looks like plausible table data. The same loop also forces every row range to have length one, even for a vertically spanning cell. Original splice and renumbering
Move coordinates and preserve the cells
The proposed repair computes the effective insertion boundary, including after=True. It checks all row widths, the boundary, and any intersecting vertical span, then constructs every new TableCell before changing an existing one. New cells occupy successive rows in input order. The update to existing cells shifts the two row offsets together.
for cell in self.table_cells:
if cell.start_row_offset_idx >= effective_index:
cell.start_row_offset_idx += len(rows)
cell.end_row_offset_idx += len(rows)
Both endpoints move by the same amount, preserving the extent. Cells entirely above the boundary keep their offsets. The code reuses existing objects, keeping their text, flags, bounding boxes, span sizes, column offsets, and RichTableCell references. A referenced document group and its content remain in place, along with the existing table provenance. New strings acquire table coordinates without an invented source bounding box. Submitted insertion implementation
A boundary strictly inside an existing row range raises ValueError. For a cell spanning [0, 2), inserting at 1 would put a supplied full-width row through a region already owned by that cell. Extending the merge could cover the new values. Splitting it would require a rule for who gets the existing content. Both outer boundaries are valid when no other cell spans across them. A wrong width, invalid cell value, invalid position or span conflict leaves the table unchanged for insert_rows. insert_row delegates to it and add_row uses it through insert_row. add_rows still appends one row at a time, so the batch guarantee does not apply to that API.
The patch also preserves list replacement. It finds the first existing list entry starting at or below the boundary and splices a new list there, keeping the relative order of the old entries. The splice position is a storage choice. Offsets determine the grid even when the list is unordered. A caller holding the previous list sees no new entries in that list, though its shared cell objects can still have updated row offsets. An earlier local variant used in-place list insertion. The final audit caught that difference and added an explicit compatibility test.
The patch preserves ordinary rectangular behavior in the tested cases. It does not repair already-corrupted tables or validate arbitrary malformed coordinates. The small caption fixture correction follows from the append contract. To create a one-row table it now starts with zero rows and appends once. The old fixture started with one declared empty row, appended, then reset the count. Three-file commit, including the fixture correction
Regression evidence and its limits
- 27 of 27 focused insertion cases passed on each of Python 3.10.22, 3.12.14 and 3.14.10. The same test file collected against unmodified upstream on Python 3.12.14 produced 19 failures and eight passes. The eight passing cases capture existing behavior that the patch also needs to preserve. The audit checked the imported
TableDatasource path to prevent accidentally testing the patched module twice. Submitted regression tests - 400 generated rectangular cases produced identical model output across the baseline, insertion patch, removal patch and combined checkout. The probes covered all four orientations, zero dimensions, cell flags and bounding boxes. This was a local compatibility check. The generated cases and runner are unpublished, and the result has no independent verification.
- The same 47 tests failed in both local Windows full-suite runs. The baseline had 1,081 passes and the insertion variant had 1,108. Each had nine skips and one expected failure. The failed test identifiers matched, and the recorded runs did not reach an insertion or append method in any of those failed cases. The environment excluded the unavailable Java grammar build and an optional Saxon backend that had crashed. Full Linux CI and production data remain unverified. Test commands and environment notes in PR #943
The public issue can reproduce the failure independently of the test suite. To compare the focused tests, fetch the PR head, verify the SHA, and make clean checkouts of the submitted commit and its parent. From each checkout, use the local environment to run the test file from the submitted checkout.
uv sync --frozen --all-extras --all-packages
uv run --no-sync python -c 'import inspect; from docling_core.types.doc import TableData; print(inspect.getfile(TableData))'
uv run --no-sync pytest -q --import-mode=importlib /absolute/path/to/submitted/tests/test_table_row_insertion.py
The printed module path should belong to the checkout being tested. The submitted commit and the final local audit commit have the same Git tree. Reviewed submission and pinned base
Why the row-removal patch matters
PR #927 corrects the equivalent dense-list assumption in remove_rows. Its removal algorithm uses offsets to drop, shrink or shift cells. The insertion-only patch leaves the removal regression failing, and the removal-only variant leaves the same 19 insertion failures. The two patches address separate operations.
The local compatibility audit combined the insertion patch with #927 at e8a761c98976feadafa8137db3196ff6d37df3c6. Each of 200 seeded synthetic cases inserts one to three rows at a boundary in a table with shuffled cell origins and horizontal spans, removes exactly those inserted rows, and compares the complete serialized model to its starting state. Both patches together restored 200/200. The baseline and each single-patch variant restored 18/200 apiece. A separate rich-cell fixture restored the complete document, including its group reference and provenance, only in the combined run. PR #943 reports the 200-case result. The generated cases and runner are unpublished, so these local results have no independent verification. The audit used no real-document sample. Reported result in PR #943 · Pinned removal patch
The combination retained one observed removal limitation. Deleting one row of an existing two-row cell can shorten its offset range to [0, 1) while leaving row_span=2. This probe had the same result with #927 alone. The insertion PR leaves removal and that metadata behavior untouched. The round-trip corpus removes newly inserted rows and does not exercise deletion through an original vertical span.
What the upstream checks establish
I asked for a baseline comparison and an isolated run with #927 before approving submission. The public issue records the smallest failure. The PR proposes one conventionally titled, signed-off commit that changes the insertion implementation, adds a focused test module and corrects the caption fixture. The Developer Certificate of Origin is a contributor certification about the right to submit the work under the project’s license and the public record of the contribution. The automated DCO check verifies the sign-off. Community contribution guide · DCO 1.1
Status checked on November 3, 2026. #943 is open at 8c8cd3b8371910769f9f23a81b47dbf1bff3e40b. Lint and typing passed, as did DCO and DCO Advisor. GitHub records the full workflow as action_required, pending maintainer authorization. Merge protection calls for two reviewer approvals under the test-update rule. GitHub showed zero human reviews. Automated AI triage classified the change and flagged #927 as related. A separate advisory AI first review assessed the full diff and reported no blocker. Both read the code without executing it, and neither provided human maintainer approval. #927 was also open at the tested head. PR and reviews · Fast checks · Full workflow · Automated AI triage · Advisory AI first review
What I would test first next time
I would start with a pair whose fields all survive a mutation, then assert that the fields still belong together. For a coordinate-backed table, I would test the materialized grid, the stored offsets and the unchanged payloads. Each layer catches a different class of error. Sparse rows, shuffled origins, horizontal and vertical spans, and edits exactly on or inside a boundary belong in that fixture set from the start.
I would also run the same new tests on the pinned upstream implementation and keep checks for observable behaviors such as object and list identity. An explicit rejection gives a caller a chance to choose a valid boundary. A successful call that silently moves 10 to a different record gives the caller no reason to look. Automated review can flag a suspicious diff. The minimal reproduction, comparison against upstream and pending maintainer checks answer separate questions.
When Valid JSON Isn't Enough examines a similar distinction in AI tool calling, where a structurally valid response can still encode the wrong action.