fix(layout): fix unreachable TABLE check in _handle_cross_type_overlaps#3701
Open
charlesaurav13 wants to merge 1 commit into
Open
fix(layout): fix unreachable TABLE check in _handle_cross_type_overlaps#3701charlesaurav13 wants to merge 1 commit into
charlesaurav13 wants to merge 1 commit into
Conversation
TABLE is a member of WRAPPER_TYPES, which is a subset of SPECIAL_TYPES,
so TABLE clusters always land in special_clusters and never in
regular_clusters. The inner condition
for regular in self.regular_clusters:
if regular.label == DocItemLabel.TABLE:
could therefore never be true, making the KEY_VALUE_REGION vs TABLE
deduplication path a complete no-op.
Fix by building the tables list from special_clusters (already done
below for the PICTURE case added in docling-project#3523) and moving it above both
loops so it is shared. Add an explicit skip for TABLE itself so a
TABLE cluster cannot trigger its own removal.
Adds two unit tests: one that confirms a near-identical KEY_VALUE_REGION
is dropped, and one that confirms a non-overlapping one is kept.
Fixes docling-project#3495
Signed-off-by: Saurav Pandey <sauravp1236@gmail.com>
Contributor
|
✅ DCO Check Passed Thanks @charlesaurav13, all your commits are properly signed off. 🎉 |
Contributor
Merge Protections🟢 Merge protection satisfied — ready to merge. Show 1 satisfied protection🟢 Enforce conventional commitMake sure that we follow https://www.conventionalcommits.org/en/v1.0.0/
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
_handle_cross_type_overlapshas a dead code path for KEY_VALUE_REGION vs TABLE deduplication:TABLEis a member ofWRAPPER_TYPES, which is a subset ofSPECIAL_TYPES, so TABLE clusters always land inspecial_clustersand never inregular_clusters. This inner condition can never fire, so a KEY_VALUE_REGION that nearly coincides with a TABLE is never removed.Fix
The
tableslist is already built fromspecial_clustersin the PICTURE block added by #3523. This PR moves that list above both loops so it's shared, then iterates it in the wrapper loop instead ofself.regular_clusters. Acontinueguard skips TABLE itself so it can't remove itself.No change to the PICTURE/TABLE path from #3523.
Tests
Two new tests added to
test_layout_postprocessor.py:Closes #3495