Repository navigation
Conversation
The JATS backend (figures, tables) and the AsciiDoc backend (pictures, tables, code blocks) created the caption with doc.add_text() and no parent, so every caption became a child of the body. The captioned item stays in its section, while the caption is listed after the last section of the document. Pass the item's parent to the caption, so the caption is a sibling that comes just before its item, as the Office backends do since docling-project#4190. Signed-off-by: SK Arin <162553727+ska2704@users.noreply.github.com>
|
✅ DCO Check Passed Thanks @ska2704, all your commits are properly signed off. 🎉 |
Merge Protections🔴 1 of 2 protections blocking · waiting on 👀 reviews
🔴 Require two reviewer for test updatesWaiting for
This rule is failing.When test data is updated, we require two reviewers
Show 1 satisfied protection🟢 Enforce conventional commitMake sure that we follow https://www.conventionalcommits.org/en/v1.0.0/
|
github-actions
Bot
added
ai:ci-safe
AI triage (advisory)Checked commit Fixes caption parenting in the JATS and AsciiDoc backends: captions created by Topic labels added: CI safety: no concern found. A maintainer must still approve CI. 5 related pull request(s) in the same code
Reference data changes
Assessment: expected. The fix moves captions from The |
github-actions
Bot
added
the
ai:review-lgtm
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
AI first review (advisory): no blocker found
This review covers the full diff at d5ad100c9f. It read the code and did not run it.
The changes set the parent node for captions in the JATS and AsciiDoc backends so that captions remain siblings of their items. I verified the parent assignment and document structure across pictures, tables, and code blocks in both backends.
A maintainer makes the merge decision. This review never approves the pull request.
| table_text: str = f"{label}{' ' if label and caption else ''}{caption}" | ||
| table_caption: TextItem | None = ( | ||
| doc.add_text(label=DocItemLabel.CAPTION, text=table_text) | ||
| doc.add_text(label=DocItemLabel.CAPTION, text=table_text, parent=parent) |
There was a problem hiding this comment.
Small edge case in _add_table: the caption is created before the data is not None check. parse_table_data returns None when the table contains a nested table, so in that case the caption gets added without its table. Before this change that lone caption ended up at the end of body, but now it sits inside the section with nothing next to it. Maybe move the doc.add_text call inside the if data is not None: block so the caption is only added together with the table? A JATS table-wrap whose table contains another table would make a quick test for it.
There was a problem hiding this comment.
You're right that the caption is created before the data is not None check. That order is the same as on main, and I think it's the better one to keep. When parse_table_data returns None the table is skipped, and the caption is the only text left from that table-wrap. Moving add_text into the if would drop it from the output entirely, Markdown included, where main still prints it (as the last line of the document).
For a table-wrap whose table contains another table, the tree on this branch is:
section_header: 'One'
text: 'Before.'
caption: 'Table 1. Nested.'
text: 'After.'
section_header: 'Two'
text: 'Para two.'
On main the same caption is the last child of body, after section "Two". So the lone caption now sits where the table was instead of at the end. Real support for nested tables is the TODO in parse_table_data, and I'd rather keep that out of this PR. If the maintainers prefer to drop the caption when the table is skipped, that's a small change and I can add a test for it.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
The JATS backend (figures and tables) and the AsciiDoc backend (pictures, tables and code block titles) created captions with
doc.add_text()and noparent, so each caption became a child ofbody. The captioned item stayed in its section, and the captions were listed after the last section of the document.The fix passes the item's parent to the caption. The caption is now a sibling that comes just before its item, the same layout the Office backends use since #4190 and the HTML backend uses for table captions. In AsciiDoc the parent expression was repeated for the caption and the item, so it is now computed once in
_add_picture_itemand_add_block.I found this while checking tree invariants over the test inputs of the non-PDF backends (each child points back to its parent, no orphans, and every caption has the same parent as its item, or is its child). After this change the only remaining failure in that check is the EPUB sample, which goes through the HTML backend and is covered by #5036 / #5037.
Issue resolved by this Pull Request:
Resolves #5038
Checklist:
Review evidence:
Before, for a JATS article with a figure and a table in section "One", followed by section "Two":
After:
AsciiDoc with
.Titlebefore an image, a table and a listing block gives the same before/after shape.Reference data: regenerated with
DOCLING_GEN_TEST_DATA=1for the three JATS samples (elife-56337,pntd.0008301,pone.0234687). I compared old and new JSON with a script: the texts, tables and pictures are identical, and only the captionparentrefs and thechildrenlists change (13 figure captions and 9 table captions move frombodyto their section). The.itxtdiffs are the same move. No Markdown reference changed, the AsciiDoc ground truth is Markdown only, so no AsciiDoc data changed.Tests:
test_backend_jats_captions.py::test_jats_captions_stay_in_the_section_of_their_figure_and_tableandtest_backend_asciidoc.py::test_block_title_caption_stays_in_the_section_of_its_block(picture, table, code). All 4 fail onmainwith the caption parent#/bodyand pass with the fix. The JATS test is in its own module becausetest_backend_jats.pyis at 1494 lines and the max-lines hook stops at 1500.uv run pytest tests/test_backend_jats*.py tests/test_backend_asciidoc.py: 155 passed.uv run pytest tests/test_backend_*.py tests/test_groundtruth_diff.py -n 4: 1530 passed, 14 skipped. The 1 failure and 4 errors (test_backend_webp, the pdfium tests,test_conversion_result_json) also happen on unchangedmainin my environment, becausertreeand the OCR model extras are not installed there.make validate: all hooks pass (ruff, ty, tach, max-lines, license header).Not run: the full
make testwith all extras (PDF/OCR/VLM model tests). Those code paths do not call the two changed backends.#4291 also edits caption code in
jats_backend.py(the caption text), but different lines, so the two should not conflict.