Skip to content

fix(jats): preserve table footnotes - #4818

Open
AryanRG wants to merge 2 commits into
docling-project:mainfrom
AryanRG:fix/jats-table-footnotes-4796
Open

AryanRG wants to merge 2 commits into
docling-project:mainfrom
AryanRG:fix/jats-table-footnotes-4796

Conversation

@AryanRG

@AryanRG AryanRG commented Oct 14, 2026 •

Copy link
Copy Markdown

JATS conversion drops direct table-wrap-foot/fn notes. This change attaches each nonempty note as one FOOTNOTE owned and referenced by its table, preserving source order, formatting, hyperlinks, inline formulas and literal citations such as [8]. Adjacent paragraphs retain a word boundary, including when the next paragraph already starts with whitespace.

Refs #4796. This covers table notes only; author notes, nested fn-group containers, bare table-wrap-foot paragraphs and xref resolution remain separate work. Existing fn-group behavior is unchanged.

Released-core compatibility

Table notes use an empty FOOTNOTE wrapper with inline child runs. This keeps the note visible with released docling-core 2.100.0, whose table serializer skips referenced footnote wrappers but still emits their traversed children. Text child runs retain the FOOTNOTE label so label filtering and chunk metadata classify them as note content; formula runs retain FORMULA.

The change does not modify pyproject.toml or uv.lock.

Checklist:

  • Added synthetic regressions for parser behavior and released-core exports.
  • Added label-filter and chunk-metadata coverage.
  • Regenerated and reviewed the affected reference outputs.
  • Kept the change inside the JATS backend and its tests.

Review evidence:

  • Python 3.10.22 with released docling-core 2.100.0: tests/test_backend_jats.py plus tests/test_backend_jats_table_footnotes.py — 108 passed.
  • The focused table-footnote file alone — 22 passed.
  • make validate — passed, including Ruff, formatter, ty, Tach, module coverage and max-lines checks.
  • git diff --check — clean.
  • Before the released-core compatibility correction, the public CI head had nine JATS failures caused by table-note content being omitted from Markdown/HTML. The same focused cases pass with this update.
  • The full repository suite was not run locally.

Exactly six existing references change: JSON, Markdown and indented text for pntd.0008301.nxml and pone.0234687.nxml. They represent 20 notes across six tables. The pone JSON and indented-text semantic changes are child-run label changes from TEXT to FOOTNOTE; pntd also gains the empty-wrapper/inline-group structure needed for released-core export. Markdown changes place each source note in its own paragraph after the table and before following content. No table data, note text, formatting, links, formulas, ownership or source order changes.

Known compatibility limit: with released core, excluding only the FOOTNOTE label removes note text runs but can leave an inline formula because formulas intentionally retain the FORMULA label. A core serializer with subtree-aware table-footnote handling can filter the complete note. Default Markdown/HTML export, ordering and duplication behavior are covered here.

Reference-data changes require two approving reviewers under the repository contribution policy.

Signed-off-by: Aryan Rajesh Ganjoo <51299383+AryanRG@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown
Contributor

✅ DCO Check Passed

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

@mergify

mergify Bot commented Oct 14, 2026

Copy link
Copy Markdown
Contributor

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

@codecov

codecov Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 85.71429% with 5 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
docling/backend/xml/jats_backend.py 85.71% 3 Missing and 2 partials ⚠️

📢 Thoughts on this report? Let us know!

Signed-off-by: Aryan Rajesh Ganjoo <51299383+AryanRG@users.noreply.github.com>
@AryanRG
AryanRG marked this pull request as ready for review October 15, 2026 14:30
@github-actions github-actions Bot added ai:ci-safe AI triage found no CI-safety concern (advisory) bug Something isn't working xml issue related to supported schema-specific XML formats labels Oct 15, 2026
@github-actions

Copy link
Copy Markdown
Contributor

AI triage (advisory)

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

The PR makes the JATS backend keep direct table-wrap-foot/fn notes. Each nonempty note becomes a FOOTNOTE item owned and referenced by its table. The PR also adds a new test file and updates two JATS reference outputs.

Topic labels added: bug, xml.

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

1 related pull request(s) in the same code
PR State Signal Assessment
#4809 open changes the same code related: Both PRs change table handling in jats_backend.py and _add_table. #4809 adds subscript and superscript support inside table cells. This PR adds table footnotes. A maintainer can merge both, but must resolve a likely conflict in _add_table.

Reference data changes

Change Files
structure 2
text 4
  • structure tests/data/jats/groundtruth/pntd.0008301.nxml.json: footnote: 0 → 6
  • structure tests/data/jats/groundtruth/pone.0234687.nxml.json: footnote: 0 → 71
  • text tests/data/jats/groundtruth/pntd.0008301.nxml.itxt: +199 / -190 lines
  • text tests/data/jats/groundtruth/pntd.0008301.nxml.md: +6 / -0 lines
  • text tests/data/jats/groundtruth/pone.0234687.nxml.itxt: +563 / -475 lines
  • text tests/data/jats/groundtruth/pone.0234687.nxml.md: +34 / -0 lines

Assessment: expected. The .json files gain FOOTNOTE items (6 and 71), which matches the new table-note feature. The .md files only gain added lines. The large .itxt changes fit the shifted item numbers from the new items. The test also strips the trailing space after empty footnote: wrappers.

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 ad7506810f. It read the code and did not run it.

The change keeps table notes visible by using an empty FOOTNOTE wrapper with the note text in child runs. This is a workaround for the table serializer in docling-core 2.100.0. I could not read the installed docling_core source, so I did not check the claim about how its serializers treat referenced footnotes.

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

]
# Released core table serializers skip referenced footnote wrappers.
# Keep all content in child runs so it is also exported by traversal.
footnote = doc.add_text(

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.

question: Empty-text FOOTNOTE wrapper depends on one docling-core serializer behavior

Each note is a FOOTNOTE item with text="". The real text sits in child runs, and table_item.footnotes points to the empty wrapper. The code comment says this only works because released core skips referenced wrappers but still walks their children. pyproject.toml allows docling-core>=2.98.0,<3.0.0. The PR evidence covers only 2.100.0.

Please show two things:

  1. The oldest allowed core (2.98.0) still exports these notes in Markdown and HTML.
  2. A core release that prints table.footnotes text by reference does not add an empty line or duplicate the note. In that case the child runs would print a second time.

If one of these fails, either raise the core lower bound or add a test that fails on the problem. If you cannot show both, say that this layout is a deliberate workaround.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thanks, I checked both points against ad75068.

  1. Oldest allowed core (2.98.0). With docling-core[chunking]==2.98.0, tests/test_backend_jats_table_footnotes.py passes: 22 passed. The chunking extra is needed for the HierarchicalChunker import and matches the project's test dependency. I also exported 12 synthetic JATS cases containing 15 table notes—plain text, literal xrefs, labels, styled/linked/formula runs, multiple paragraphs, and multiple tables—to Markdown and HTML, before and after a JSON round trip. Every expected note fragment appears exactly once, with no empty footnote div or whitespace-only line.

  2. A core that prints table.footnotes by reference. The released table serializers I checked (2.98.0, 2.100.0, and 2.101.1) and current docling-core main do not call serialize_footnotes. The closest candidate is docling-core#675 (open at 11bb2a7, based on core 2.86.0, below Docling's current 2.98.0 floor, so the current converter cannot import against it). Running the JATS backend directly against that head, note text is not duplicated: its hook reads only the wrapper's empty text, while traversal emits the child runs once. It does add empty output, though: one <div class="footnote"></div> per note in HTML, and a whitespace-only line in Markdown when a table has two or more notes.

So the oldest supported release works, but the future-serializer case is only partly clean. The empty-text wrapper is a deliberate workaround for current released serializers, which skip referenced footnote items while still walking their children. Raising the lower bound would not help because no checked release has the by-reference behavior. If that behavior lands in core, this layout should be revisited. I can add a guard test here that fails on an empty footnote div or whitespace-only line if that would be useful.

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

This branch has not been deployed

No deployments
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 xml issue related to supported schema-specific XML formats

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant