feat(reading-order): ReadingOrder::Structure - fix tagged table order from the structure tree - #877
Conversation
…om tags `extract_spans_with_reading_order` offered only geometric orders (TopToBottom, ColumnAware), so a Tagged PDF's declared reading order went unused and a geometric XY-cut guessed at tables it had no need to guess at. On real documents that guess is wrong: a tagged wind-speed table read column-wise and dropped its header cells. Add `ReadingOrder::Structure`. It keeps the geometric order as the baseline and uses the structure tree (ISO 32000-1 §14.8.2.3) to fix ONLY the spans it can fix unambiguously - TABLE cells (`OrderedContent::in_table`): those are reordered IN PLACE by their pre-order structure rank, so a wide table reads row-major and complete, while every non-table span keeps its geometric position. Restricting to tables is deliberate. Applying the tree traversal to the WHOLE page also reorders flowing prose, where the tree's section order can legitimately differ from visual order (it de-prioritises page artifacts, for one) - a change, not an improvement. Measured on a 470-doc corpus: whole-page structure order was a wash (88 docs better / 82 worse on the reference); table-scoped was 43 better / 13 worse (and the worse ones are pages the reference reader itself extracts empty). When the tree is absent or not trustworthy for ordering (untagged, or `/MarkInfo /Suspects true`, gated by `struct_tree_trustworthy`), this is exactly `ColumnAware` - always safe to request. The geometric XY-cut is factored into `order_spans_column_aware`, shared by both branches. tests/structure_reading_order.rs: a 2x2 table with far-apart columns whose tags declare row-major order; Structure reads the rows, and the untagged twin falls back to the geometric order byte-for-byte. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Anthony J. (Tony) Bufort <ajbufort@ajbconsulting.us>
yfedoseev
left a comment
There was a problem hiding this comment.
🔴 Changes requested — the feature is correct, but the test doesn't prove it fires
Nice, well-scoped feature: ReadingOrder::Structure reorders only table-cell spans by their structure-tree pre-order rank (row-major for Table→TR→TD/TH) on top of the ColumnAware baseline, leaves non-table spans untouched, and correctly falls back to geometric when the tree is untrusted (/Suspects, untagged) — spec-faithful (§14.8) and non-breaking. No correctness bug in the logic.
Blocker — the marquee test may be vacuous. tests/structure_reading_order.rs:
structure_falls_back_to_geometry_when_untaggedsays in a comment: "On this simple 2×2 the geometric order happens to also be row-major."- If that's true, then
structure_order_reads_the_tagged_rows_not_the_columns(asserts[ALPHA, BRAVO, CHARLIE, DELTA]) would pass even if theStructurebranch were a no-op, because plainColumnAwarealready yields row-major on that fixture. The test never showsStructurediffers fromColumnAware.
Please make the tagged test prove the reorder happened: on the same tagged bytes, compute both orders and assert they differ, e.g.
let col = extract_spans_with_reading_order(0, ReadingOrder::ColumnAware);
let str_ = extract_spans_with_reading_order(0, ReadingOrder::Structure);
assert_eq!(col_labels, ["ALPHA","CHARLIE","BRAVO","DELTA"]); // column-major
assert_eq!(str_labels, ["ALPHA","BRAVO","CHARLIE","DELTA"]); // row-major via structure
assert_ne!(col_labels, str_labels);(Widen the column x-gap in the fixture if needed so XY-cut actually goes column-major first.)
Also (non-blocking but worth it):
- Binding reachability —
src/python.rsandsrc/wasm.rsreject"structure"("Expected 'top_to_bottom' or 'column_aware'"); addSome("structure") => ReadingOrder::Structureso the feature is usable from Python/WASM (per this repo's all-bindings convention). - Coverage — nested-table, TH-header-row, and multi-span-per-cell cases are explicit design claims but untested; one of each would harden it.
Happy to approve once the tagged test asserts Structure != ColumnAware. Thanks @ajbufort!
Addresses review on yfedoseev#877. The marquee test was vacuous: the old 2x2 fixture's geometric order was already row-major, so structure_order_reads_the_tagged_rows_not_the_columns would have passed even with a no-op Structure branch. XY-Cut deliberately does NOT split short table cells into columns (it would corrupt cell data), so it cannot be coaxed column-major on a clean grid. Instead the fixture now DRAWS the table rows out of logical sequence (logical row 3 sits highest), a shape real generators produce: geometry follows visual Y (rows 3,4,1,2) while the struct tree declares the true row order. The load-bearing test now asserts ColumnAware and Structure produce DIFFERENT orders and only Structure recovers the row-major sequence, so it can no longer pass if Structure is a no-op. The untagged test still pins the exact geometric fallback. Adds the coverage the review asked for - one test each for: - a multi-span cell (both spans stay contiguous at the cell's rank), - a /TH header row (header cells order like /TD, before a visually-higher data row), - a nested table (cells ordered by structure pre-order: outer cell, then nested). Binding reachability: src/python.rs and src/wasm.rs now accept "structure" (previously rejected as unknown), matching the all-bindings convention, with updated docstrings and error messages. Signed-off-by: Anthony J. (Tony) Bufort <ajbufort@ajbconsulting.us>
|
Thanks for the sharp catch - you're right that the old test was vacuous. Pushed The blocker (test now proves the reorder fires). XY-Cut deliberately does not split short table cells into columns (per the
Binding reachability. Coverage. Added one test each for the three design claims:
Gate: |
yfedoseev
left a comment
There was a problem hiding this comment.
Approved — reviewed and regression-passed; in the merge queue. (Formal approval to supersede the earlier review state and record sign-off.)
Description
extract_spans_with_reading_ordercurrently offers only geometric orders —TopToBottomandColumnAware. So a Tagged PDF's declared reading order goes unused, and the geometric XY-cut is left to guess at tables it has no need to guess at.On real documents that guess is wrong. A tagged wind-speed table on a product spec sheet reads column-wise under
ColumnAwareand drops its header cells entirely:Per ISO 32000-1:2008 §14.8.2.3, a pre-order traversal of the structure hierarchy is authoritative for reading order in a Tagged PDF — independent of glyph geometry. The library already computes it (
extract_textuses it for tagged docs), but the span API had no way to ask for it.What this adds
ReadingOrder::Structure. It keeps the geometric order as the baseline and uses the structure tree to fix only the spans it can fix unambiguously — table cells (OrderedContent::in_table): those are reordered in place by their structure rank, so a wide table reads row-major and complete, while every non-table span keeps its geometric position.Restricting to tables is deliberate, and measured. Applying the tree traversal to the whole page also reorders flowing prose — where the tree's section order can legitimately differ from visual order (it de-prioritises page artifacts, for instance). On a 470-document corpus, whole-page structure order was a wash (88 better / 82 worse against an independent reference); table-scoped was 43 better / 13 worse, and the 13 are pages the reference reader itself extracts empty.
When the tree is absent or not trustworthy for ordering (untagged, or
/MarkInfo /Suspects true— already gated by the existingstruct_tree_trustworthy),Structureis exactlyColumnAware, so it is always safe to request.The geometric XY-cut is factored into a private
order_spans_column_aware, shared by theColumnAwareandStructurebranches (no behaviour change toColumnAware).Type of change
New feature (additive enum variant; existing variants unchanged).
Testing performed
tests/structure_reading_order.rs:A 2×2 table with far-apart columns whose tags declare row-major order —
Structurereads the rows.The untagged twin of the same file —
Structurefalls back to the geometric order byte-for-byte.cargo test --features rendering— full suite green (8,862 tests).cargo clippy --features rendering -- -D warnings— clean.cargo fmt— clean.Checklist
cargo test)cargo fmt)cargo clippy -- -D warnings)