unstructured
af0b4ea9 - fix(utils): compare bounding boxes for elements with no page number (#4561)

Commit
10 days ago
fix(utils): compare bounding boxes for elements with no page number (#4561) ## Problem `unstructured.utils.catch_overlapping_and_nested_bboxes()` raises `IndexError` for element lists it should be able to compare. Size of the per-page lists is taken from the page number of the **last** element, while each box is bucketed by its own page number (a missing one counted as page 1): ```python num_pages = elements[-1].metadata.page_number or 0 pages_of_bboxes: list[list[Points]] = [[] for _ in range(num_pages)] ... for ix, element in enumerate(elements): page_number = element.metadata.page_number or 1 n_page_to_ix = page_number - 1 if element.metadata.coordinates: ... pages_of_bboxes[n_page_to_ix].append(box) ``` So the two disagree whenever the last element is not on the highest page, and an empty list produces zero pages for a loop that starts at page 1. Reproduction (`python scratch2_repro.py`): ``` catch_overlapping_and_nested_bboxes() on element lists: empty element list : RAISED IndexError: list index out of range no page numbers (2 elements) : RAISED IndexError: list index out of range pages 2 then 1 (last is page 1): RAISED IndexError: list index out of range pages 1 then 2 (ascending) : False, 0 overlapping case(s) ``` Expected and actual after the fix: ``` empty element list : False, 0 overlapping case(s) no page numbers (2 elements) : True, 1 overlapping case(s) # both are on page 1, so compared pages 2 then 1 (last is page 1): False, 0 overlapping case(s) # different pages, nothing to compare pages 1 then 2 (ascending) : False, 0 overlapping case(s) # unchanged ``` Real-world shape of the second case: elements from a partitioner that assigns no page number (`partition_text()`, `partition_html()`, `partition_xml()`, ...) — they exist, have coordinates when the source has them, and are all implicitly on page 1. ## Root cause `num_pages` was derived from one element (the last one) instead of from the page numbers actually present, and it used a different default (`0`) than the per-element bucketing code (`1`). ## Fix (smallest correct change) ```python # -- page-numbers are 1-based and an element without one is on page 1, like the per-element # -- default below, so an empty element-list has no pages to compare and every element above # -- page 1 gets its own page-bucket -- num_pages = max((element.metadata.page_number or 1 for element in elements), default=0) ``` - The highest page number present is exactly the number of buckets the loop can index. - For elements in ascending page order (the tested case, and the common case) `max(...)` equals the old value, so output for existing inputs is unchanged. - An empty element list yields zero buckets, the loop body never runs, and the function returns the documented "no cases" result `(False, [])` instead of raising. `test_utils.py` and `unstructured/utils.py` are the only files changed (plus the CHANGELOG entry). ## Tests Three tests in `test_unstructured/test_utils.py`, beside the other `catch_overlapping_and_nested_bboxes` tests: 1. `..._returns_no_cases_for_no_elements` — empty list → `(False, [])`. 2. `..._compares_elements_that_have_no_page_number` — two boxes at the same coordinates with no page number → `True` and `overlapping_case == "nested NarrativeText in Title"` (proves they are actually compared, not silently skipped). 3. `..._handles_pages_that_are_not_in_ascending_order` — page 2 then page 1 → `(False, [])`. Proof — RED with only the source change reverted (`git stash push -- unstructured/utils.py`): ``` $ python -m pytest "test_unstructured/test_utils.py::test_catch_overlapping_and_nested_bboxes_returns_no_cases_for_no_elements" \ "test_unstructured/test_utils.py::test_catch_overlapping_and_nested_bboxes_compares_elements_that_have_no_page_number" \ "test_unstructured/test_utils.py::test_catch_overlapping_and_nested_bboxes_handles_pages_that_are_not_in_ascending_order" -q page_number = element.metadata.page_number or 1 n_page_to_ix = page_number - 1 if element.metadata.coordinates: box = cast(Points, element.metadata.coordinates.to_dict()["points"]) > pages_of_bboxes[n_page_to_ix].append(box) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ E IndexError: list index out of range unstructured\utils.py:607: IndexError =========================== short test summary info =========================== FAILED test_unstructured/test_utils.py::test_catch_overlapping_and_nested_bboxes_returns_no_cases_for_no_elements FAILED test_unstructured/test_utils.py::test_catch_overlapping_and_nested_bboxes_compares_elements_that_have_no_page_number FAILED test_unstructured/test_utils.py::test_catch_overlapping_and_nested_bboxes_handles_pages_that_are_not_in_ascending_order 3 failed in 0.33s ``` GREEN after `git stash pop`: ``` $ python -m pytest <same three tests> -q 3 passed in 0.15s $ python -m pytest test_unstructured/test_utils.py -q 43 passed in 1.79s ``` Patch-level verification: `git worktree add --detach <tmp> origin/main`, `git apply <patch>`, `python -m pytest test_unstructured/test_utils.py -q` → **43 passed** (imports resolved from the patched worktree). ## Commands run (full record) ``` # environment (Windows, Python 3.11.0) — unchanged from the earlier PR python -m venv .venv .venv\Scripts\python.exe -m pip install -e ".[csv]" pytest pytest-mock .venv\Scripts\python.exe -m pip install "ruff==0.15.10" # same version as uv.lock / CI # tests pytest test_unstructured/test_utils.py -q # 43 passed pytest test_unstructured/chunking -q # 484 passed, 38 skipped pytest test_unstructured/documents -q # 193 passed, 2 failed (pre-existing) pytest test_unstructured/partition/test_xml.py -q # 31 passed (unrelated, sanity) # lint (CI gate: make check == check-ruff + check-version) ruff.exe check . # All checks passed! ruff.exe format --check . # 344 files already formatted ``` **Pre-existing failures** (reproduced earlier with a clean tree, unrelated to this patch): `documents/test_elements.py::DescribeElementMetadata::it_accommodates_either_a_pathlib_Path_or_str_for_its_filename_arg[file_path0]` (asserts a POSIX `file_directory` while `WindowsPath` stringifies with backslashes) and `documents/test_ontology_to_unstructured_parsing.py::test_parsed_ontology_can_be_serialized_from_html`. **Could not run** (stated honestly): `test_unstructured/partition/test_auto.py` — collection error `ModuleNotFoundError: No module named 'PIL'` (needs the image extra, out of scope for this minimal environment); `make check-version` — needs bash, unavailable in this Windows session (the invariant it checks still holds: CHANGELOG's newest section is `## 0.27.30`, `unstructured/__version__.py` is `0.27.30`, and this patch changes neither); mypy/pyright — not wired into `make check` or CI. ## Risks / notes - `catch_overlapping_and_nested_bboxes()` has no in-library caller (it is exercised by `test_unstructured/test_utils.py` and by downstream users of `unstructured.utils`), so the blast radius is small: no partitioner or chunker output changes. - For elements in ascending page order the computed page count is identical to the old value; only the previously-crashing inputs change (they now produce the documented "no overlapping cases" result or a real comparison). - Same-shaped fixes have landed before, e.g. the CHANGELOG entry "`unstructured_elements_to_ontology` handles empty and malformed input: returns an empty `Document` for empty input (instead of raising `IndexError`)". - The CHANGELOG entry was added to the existing `## 0.27.30` / `### Fixes` list without a version bump, matching fixes like commit `3253c2b`; bump to `0.27.31` if the convention is one version per PR. ## AI-assistance note Produced with AI assistance (DeepSeek Harness agent): the agent found the defect while probing the module, wrote the reproduction and the regression tests, and ran every command quoted above on this machine. No maintainer has reviewed the change; treat the analysis as agent-authored and verify independently. <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/Unstructured-IO/unstructured/pull/4561?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="View guided diff" src="https://www.cubic.dev/buttons/review-in-cubic-light.svg"></picture></a> <!-- End of auto-generated description by cubic. --> Signed-off-by: simpleqt <2015900818@qq.com>
Author
Parents
Loading