From 336428ac6f36110bb4acd4044f5ecb9cc994c043 Mon Sep 17 00:00:00 2001 From: eeshsaxena Date: Wed, 5 Aug 2026 17:34:09 +0530 Subject: [PATCH] fix(tables): make calc_bbox_intersection safety_margin actually apply The 'expanded boxes intersect' guard in calc_bbox_intersection could never be true: e.g. max(right edges) + margin <= max(left edges) never holds because every box's right edge exceeds its left. So the safety_margin branch was dead and the function only returned the intersection of the un-expanded boxes; safety_margin (passed as 5 when building table cells in ml.py) had no effect, and boxes within the margin but not strictly overlapping returned None. Expand each box by safety_margin and intersect the expanded boxes, matching the tolerance used by Element.overlaps(). Two existing test cases whose expectations were 'adjusted' to None to fit the broken behavior now assert the correct margin-expanded intersections, plus a far-apart case that stays None. --- .../tables/table_transformers/geometry.py | 29 ++++++------------- .../tables/transformers/test_geometry.py | 12 +++++--- 2 files changed, 17 insertions(+), 24 deletions(-) diff --git a/src/openparse/tables/table_transformers/geometry.py b/src/openparse/tables/table_transformers/geometry.py index 83e4195..5946e8a 100644 --- a/src/openparse/tables/table_transformers/geometry.py +++ b/src/openparse/tables/table_transformers/geometry.py @@ -13,26 +13,15 @@ def calc_bbox_intersection(bbox1, bbox2, safety_margin=5.0): ): raise ValueError("Bounding boxes must have non-zero width and height.") - # Expand bounding boxes - x1_expanded_min = min(bbox1[0], bbox2[0]) - safety_margin - y1_expanded_min = min(bbox1[1], bbox2[1]) - safety_margin - x2_expanded_max = max(bbox1[2], bbox2[2]) + safety_margin - y2_expanded_max = max(bbox1[3], bbox2[3]) + safety_margin - - # Check if expanded boxes intersect - if ( - x2_expanded_max <= max(bbox1[0], bbox2[0]) - or x1_expanded_min >= min(bbox1[2], bbox2[2]) - or y2_expanded_max <= max(bbox1[1], bbox2[1]) - or y1_expanded_min >= min(bbox1[3], bbox2[3]) - ): - return None - - # Calculate and return the actual intersection based on original boxes - x1 = max(bbox1[0], bbox2[0]) - y1 = max(bbox1[1], bbox2[1]) - x2 = min(bbox1[2], bbox2[2]) - y2 = min(bbox1[3], bbox2[3]) + # Expand each box by the safety margin, then intersect the expanded boxes, + # so two boxes that are within `safety_margin` of each other are treated as + # overlapping. This matches the tolerance used by ``Element.overlaps`` and + # makes ``safety_margin`` actually take effect: expanding a box on both + # sides shifts each edge of the intersection outward by the margin. + x1 = max(bbox1[0], bbox2[0]) - safety_margin + y1 = max(bbox1[1], bbox2[1]) - safety_margin + x2 = min(bbox1[2], bbox2[2]) + safety_margin + y2 = min(bbox1[3], bbox2[3]) + safety_margin # Only return the intersection if it's valid if x2 > x1 and y2 > y1: diff --git a/src/tests/tables/transformers/test_geometry.py b/src/tests/tables/transformers/test_geometry.py index faeb619..b04bd3e 100644 --- a/src/tests/tables/transformers/test_geometry.py +++ b/src/tests/tables/transformers/test_geometry.py @@ -9,12 +9,16 @@ [ # Test case 1: Intersecting without margin ((10, 10, 20, 20), (15, 15, 25, 25), 0, (15, 15, 20, 20)), - # Test case 2: Intersecting with margin - adjusted to expect None for a point intersection - ((10, 10, 20, 20), (20, 20, 30, 30), 5, None), # Adjusted expectation + # Test case 2: Boxes touching at a corner are within the margin, so the + # margin-expanded intersection is returned. + ((10, 10, 20, 20), (20, 20, 30, 30), 5, (15, 15, 25, 25)), # Test case 3: Not intersecting, no margin ((10, 10, 20, 20), (25, 25, 35, 35), 0, None), - # Test case 4: Not intersecting, with margin - ((10, 10, 20, 20), (26, 26, 36, 36), 5, None), + # Test case 4: Boxes 6 apart are within a margin of 5 on each side, so + # the expanded intersection is returned. + ((10, 10, 20, 20), (26, 26, 36, 36), 5, (21, 21, 25, 25)), + # Test case 5: Boxes far apart stay non-intersecting even with a margin. + ((0, 0, 10, 10), (100, 100, 110, 110), 5, None), ], ) def test_calc_bbox_intersection(bbox1, bbox2, safety_margin, expected):