Skip to content

fix(otsl): keep cells when an unclosed cell precedes a closed one - #4916

Merged
dolfim-ibm merged 1 commit into
docling-project:mainfrom
kevin9327:fix-otsl-closed-cell-swallows-open-cells
Oct 21, 2026
Merged

dolfim-ibm merged 1 commit into
docling-project:mainfrom
kevin9327:fix-otsl-closed-cell-swallows-open-cells

Conversation

@kevin9327

Copy link
Copy Markdown
Contributor

parse_otsl_output (used by the Granite Vision table-structure stage and the MinerU helper) accepts both closed cells (<fcel>x</fcel>) and unclosed ones (<fcel>x<fcel>y<nl>). When an unclosed cell came before a closed cell of the same tag, the closed-tag branch matched first. Its non-greedy .*? then ran to the first </fcel> and swallowed the tokens in between:

parse_otsl_output("<otsl><fcel>a<fcel>b<nl><fcel>c</fcel><fcel>d</fcel><nl></otsl>")
# before: seq ['fcel', 'fcel', 'nl'], 1 row, cells ['a<fcel>b<nl><fcel>c', 'd']
# after:  seq ['fcel', 'fcel', 'nl', 'fcel', 'fcel', 'nl'], 2x2, cells ['a', 'b', 'c', 'd']

With the fix, closed-tag text cannot contain another OTSL token, so the unclosed-cell branch handles those cells instead. Output that is fully closed or fully unclosed parses the same as before.

Checklist:

  • Documentation has been updated, if necessary.
  • Examples have been added, if necessary.
  • Tests have been added, if necessary.

Review evidence:

  • tests/test_otsl_utils.py::test_closed_cell_does_not_swallow_preceding_open_cells fails on main and passes with the fix. The second test checks that fully closed output is unchanged.
  • I ran the existing parse_otsl_output tests in tests/test_table_structure_granite_vision.py (-k parse, 13 tests) against the fix with the model imports stubbed out, since I did not install the ML extras, and they all pass. I did not run the full suite.
  • ruff format --check and ruff check pass on the changed files.

AI assistance: I used an AI coding assistant (Grok Bot) to help find and write this fix. I reviewed the change and ran the tests above myself.

A closed-tag match ran non-greedily to the first matching closing tag, so
<fcel>a<fcel>b<nl><fcel>c</fcel> was read as one cell whose text was
'a<fcel>b<nl><fcel>c', dropping a row and two cells. Closed-tag text now
stops at any OTSL token.

Signed-off-by: kevin9327 <[email protected]>
@github-actions

Copy link
Copy Markdown
Contributor

✅ DCO Check Passed

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

@mergify

mergify Bot commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

Merge Protections

🟢 Merge protection satisfied — ready to merge.

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)(?:\(.+\))?(!)?:

@github-actions github-actions Bot added ai:ci-safe AI triage found no CI-safety concern (advisory) bug Something isn't working table structure labels Oct 20, 2026
@github-actions

Copy link
Copy Markdown
Contributor

AI triage (advisory)

Checked commit 9485ceb1db. The analysis read the diff only. It did not run code from this pull request.

The PR changes the closed-tag pattern in parse_otsl_output so closed-cell text cannot contain another OTSL token. An unclosed cell that comes before a closed cell no longer swallows the tokens between them. The PR adds two tests in tests/test_otsl_utils.py.

Topic labels added: bug, table structure.

CI safety: no concern found. A maintainer must still approve CI.

The ai:* labels are advisory. They do not approve CI and they do not add tests:full. A new push runs this check again.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI first review (advisory): no blocker found

This review covers the full diff at 9485ceb1db. It read the code and did not run it.

I found no defect. The closed-tag branch now stops at any OTSL token, so an unclosed cell before a closed one no longer merges cells. Fully closed and fully unclosed output parses as before.

A maintainer makes the merge decision. This review never approves the pull request.

@github-actions github-actions Bot added the ai:review-lgtm AI first review found no blocker or major issue (advisory) label Oct 20, 2026
@codecov

codecov Bot commented Oct 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@dolfim-ibm
dolfim-ibm merged commit 7b9a6b1 into docling-project:main Oct 21, 2026
33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai:ci-safe AI triage found no CI-safety concern (advisory) ai:review-lgtm AI first review found no blocker or major issue (advisory) bug Something isn't working table structure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants