fix: drop malformed bounding boxes

[b-hahn]

This change handles the case where bounding boxes predicted by VLMs have a width or a height <=0. Previously, this would cause the pipeline to crash while saving the PIL image. Now, malformed bounding boxes are simply ignored, resulting in an empty provenance field while keeping the extracted text.

Fixes docling-project/docling#2763

[github-actions]

✅ DCO Check Passed

Thanks @b-hahn, all your commits are properly signed off. 🎉

[mergify]

Merge Protections

Your pull request matches the following merge protections and will not be merged until they are valid.

🔴 Require two reviewer for test updates

This rule is failing.

When test data is updated, we require two reviewers

• #approved-reviews-by >= 2

🟢 Enforce conventional commit

Wonderful, this rule succeeded.

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 Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[kklein]

@dolfim-ibm @cau-git:

I think that @b-hahn from yesterday would require another approval of the CI runs on your end. :)

[dolfim-ibm]

@b-hahn you identified the right function, i.e. load_from_doctags(), to fix the issue. As we discussed, instead of making an empty prov: [] we would like to drop the element completely if it has zero size.

[cau-git]

@dolfim-ibm we agreed on the proposed solution here. It’s legit for some elements to have missing prov

[kklein]

@dolfim-ibm and @cau-git : How do you suggest to move forward now?

[simon376]

I think I found another issue that might relate to this, unfortunately I can't provide a stack trace since it is hard to reproduce and I didn't fully log everything on the server, and I couldn't find this error message in the code yet. Maybe you can have a look at this, too:
Pipeline VlmPipeline failed: Coordinate 'lower' is less than 'upper'

[dolfim-ibm]

@b-hahn for the moment we should be good with the option which provides the empty prov: [].

[cau-git]

@b-hahn @kklein just reflecting the desired solution below. Let's first go only with your change in extract_bounding_box, then observe before introducing conditions that could potentially skip valid content.

[cau-git]

I would remove this condition so captions are created also when the bbox is not present, as before.

[b-hahn]

Done.

[cau-git]

I would remove this condition so that _add_text is called also with a None bounding box, as before.

[b-hahn]

Done.

[dosubot]

Related Documentation

Checked 8 published document(s) in 1 knowledge base(s). No updates required.

^{How did I do? Any feedback?}  

[ceberam]

LGTM, it works as expected, thus approved.

I would only add the following suggestion. Turn this variable into a module constant, so that we remember the value choice we made. We can also add some docstrings. Something like:

_DOCTAGS_BBOX_MIN_DIMENSION: Final = 1 / 500
"""Minimum bounding box dimension threshold for DocTags import.

This constant represents the minimum width or height (as a fraction of the image
dimensions) that a bounding box must have to be considered valid during DocTags
import. The value ``1/500 = 0.002 = 0.2%`` means that any bounding box with width or
height less than 0.2% of the image width or height will be discarded.
"""

[ceberam]

@b-hahn we just need your sign-off in the second commit (af71bb5) to merge this PR.

[ceberam]

@b-hahn we just need your sign-off in the second commit (af71bb5) to merge this PR.

@b-hahn ^^^ were you able to have a look at this? ^^^
Also, consider the remark here #454 (review)

[b-hahn]

@ceberam Added a module-level constant and signed off all commits. Let me know if you need anything else to see this through.