Skip to content

fix: correct graphics-to-captions assignment in reading order#166

Open
indoorhill wants to merge 11 commits into
docling-project:mainfrom
indoorhill:fix-148-graphics-to-captions
Open

fix: correct graphics-to-captions assignment in reading order#166
indoorhill wants to merge 11 commits into
docling-project:mainfrom
indoorhill:fix-148-graphics-to-captions

Conversation

@indoorhill

Copy link
Copy Markdown

_find_to_captions fix (#148)

Change Description

Rewrites _find_to_captions to fix incorrect caption<->figure assignment.

The old multi-pass graphic gathering + deduplication could silently drop a caption flanked by graphics on both sides, and let a one-sided caption greedily claim a neighbor's graphic, orphaning the caption between them. This change is the same interface and intended logic as the original implementation.

Notably, this fix

  • resolves the reported issue
  • drastically simplifies the logic of _find_to_captions
    • only ~12 lines and removes _remove_overlapping_indexes
  • improves caption<->graphic accuracy
  • improves runtime performance

Performance - ds4sd/docling-dpbench (test_readingorder)

metric before after
caption 0.893 0.927
reading order 0.982 0.982
footnotes 0.944 0.944

Sidenote

I use PyCharm and I noticed that the .gitignore didn't filter JetBrains IDEs project folder, so I added .idea/ to the ignore.

Issue resolved by this Pull Request

Resolves #148

Checklist

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

@github-actions

github-actions Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

DCO Check Failed

Hi @indoorhill, your pull request has failed the Developer Certificate of Origin (DCO) check.

This repository supports remediation commits, so you can fix this without rewriting history — but you must follow the required message format.


🛠 Quick Fix: Add a remediation commit

Run this command:

git commit --allow-empty -s -m "DCO Remediation Commit for Ethan Hill <5614778+indoorhill@users.noreply.github.com>

I, Ethan Hill <5614778+indoorhill@users.noreply.github.com>, hereby add my Signed-off-by to this commit: 9e89b7199f5341e6e0373b82595f8979c008b026
I, Ethan Hill <5614778+indoorhill@users.noreply.github.com>, hereby add my Signed-off-by to this commit: 74311cdc8063bd8e6433249c265b869ed02886fd
I, Ethan Hill <5614778+indoorhill@users.noreply.github.com>, hereby add my Signed-off-by to this commit: d7a24c898679de2d7843f9ffedd3fa9ef5d079f4
I, Ethan Hill <5614778+indoorhill@users.noreply.github.com>, hereby add my Signed-off-by to this commit: 4aad0647cf0db2a7adac1d9bd6b4b75571a5555b
I, Ethan Hill <5614778+indoorhill@users.noreply.github.com>, hereby add my Signed-off-by to this commit: a80affc08f9a2243b3b34d0ba58e7a94a6afc865
I, Ethan Hill <5614778+indoorhill@users.noreply.github.com>, hereby add my Signed-off-by to this commit: 76189231db71c60286ca0b0baa92a41dfdaa478e
I, Ethan Hill <5614778+indoorhill@users.noreply.github.com>, hereby add my Signed-off-by to this commit: f43ff6fe4112fe41bd25540706c6060a486e4d51
I, Ethan Hill <5614778+indoorhill@users.noreply.github.com>, hereby add my Signed-off-by to this commit: 21f6617ee83ac13c9dd7031b9362bc9641d9756f"
git push

🔧 Advanced: Sign off each commit directly

For the latest commit:

git commit --amend --signoff
git push --force-with-lease

For multiple commits:

git rebase --signoff origin/main
git push --force-with-lease

More info: DCO check report

@mergify

mergify Bot commented Jun 30, 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)(?:\(.+\))?(!)?:

…ly.github.com>

I, Ethan Hill <5614778+indoorhill@users.noreply.github.com>, hereby add my Signed-off-by to this commit: 9e89b71
I, Ethan Hill <5614778+indoorhill@users.noreply.github.com>, hereby add my Signed-off-by to this commit: 74311cd
I, Ethan Hill <5614778+indoorhill@users.noreply.github.com>, hereby add my Signed-off-by to this commit: d7a24c8

Signed-off-by: Ethan Hill <5614778+indoorhill@users.noreply.github.com>
Signed-off-by: Ethan Hill <5614778+indoorhill@users.noreply.github.com>
@indoorhill indoorhill changed the title Fix-148: graphics to captions Fix: graphics to captions bug, issue #148 Jun 30, 2026
Signed-off-by: Ethan Hill <5614778+indoorhill@users.noreply.github.com>
@indoorhill indoorhill changed the title Fix: graphics to captions bug, issue #148 fix: correct graphics-to-captions assignment in reading order Jun 30, 2026
@codecov

codecov Bot commented Jul 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@cau-git

cau-git commented Jul 21, 2026

Copy link
Copy Markdown
Member

@indoorhill thanks for this contribution, it looks promising. Can I ask you to create one extra proof please:

  • Create a draft PR on the docling repo, where you only pin the git branch-version of this PR under tool.uv.sources in pyproject.toml and update the uv.lock
  • Re-run the ground-truth generation on the test suite:
    CI=1 DOCLING_GEN_TEST_DATA=1 uv run pytest tests/ -s -v
  • Check if any of the test ground truth in docling has modified output (as in, a diff in the GT files is present)

Many thanks.

Replace strict pairwise adjacency with run-scanning in both directions from
each caption, then greedily assign by (distance, preceding-first, cid). The
nearest graphic wins, ties break toward the preceding graphic; pictures and
tables are treated identically. This stops a nearer/lower-cid table from
stealing a picture's caption and stops captions from being dropped in dense
figure clusters.

Tests distilled from the 2203.01017v2.pdf ground truth (docling).
_find_to_captions re-sorted page_elements by cid before pairing. cids are
assigned in parse order, so that sort dropped unrelated elements (a table, a
page number, a page_footer) between a caption and the graphic it belongs to,
orphaning 7 of 15 pictures in 2203.01017v2.pdf. page_elements already arrives
in reading order with each caption next to its graphic, so use it as-is.

Adds reproductions captured from a real conversion of that PDF (page 1 and the
page-13 figure cluster).
@indoorhill

Copy link
Copy Markdown
Author

@cau-git , nice callout. Running the tests that way caught some issues. It does ultimately produce a difference in the GT generated, but i think that the difference is actually more correct than it was prior. I will add a comment with a breakdown of why that is here once I finalize my new edits/refactors.

- replace the manual probe/step/while walk with reversed/enumerate slices fed
  to itertools.takewhile
- drop the redundant matched_graphics set (to_captions keys already track used
  graphics)
- remove the now-unused defaultdict import

Behavior-preserving: identical output on 20k randomized differential cases vs
the prior implementation and on the real 2203.01017v2.pdf conversion.
Sort candidates on (distance, side) via itemgetter and rely on the stable sort
to keep ties in reading order, instead of sorting the full tuple where the
parse-order cids appeared to participate. Proven equivalent: 0 differing
results across 50k randomized cases. Makes explicit that reading-order
proximity drives pairing and cids never break a tie.
@indoorhill

indoorhill commented Jul 23, 2026

Copy link
Copy Markdown
Author

alright @cau-git, here is the change. The old groundtruth was missing some caption links that this now fills in/completes for 2203.01017v2.pdf. The rewrite pairs captions by scanning graphic runs in reading order, and on this file it links figure captions the current groundtruth leaves unattached, notably Figure 8 (picture 7) and Figure 11 (picture 10), the first figure in each dense appendix cluster.

I spot checked two of 2203.01017v2.pdfs figure pages (13 and 14) by rendering them and drawing each picture to its assigned caption. The new links land on the right figures, so the diff just reflects the groundtruth now being more complete. At least from what i can tell

@cau-git

cau-git commented Jul 24, 2026

Copy link
Copy Markdown
Member

@indoorhill thanks for the check! Can you post that test PR to https://github.com/docling-project/docling/pulls so we can inspect the test delta too? (not with the intent to be merged, just for verification. You can make it a draft PR).

@indoorhill

indoorhill commented Jul 25, 2026

Copy link
Copy Markdown
Author

@cau-git done :)

hopefully thats what you were looking for, let me know if not

@cau-git

cau-git commented Jul 26, 2026

Copy link
Copy Markdown
Member

@indoorhill I assume you are relating to this: docling-project/docling#3872

The reason why I asked for this is that I wanted to see the changed GT files. Could you please commit the changed GT files you created with the docling-ibm-models update into this draft PR for inspection purposes? Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

reading_order_rb.py incorrectly assigns captions to graphics

2 participants