Skip to content

M9: stream structure-tree checks without a total-element cutoff - #926

Merged
echelon merged 2 commits into
storytold:mainfrom
cyfung1031:fix/a11y-structure-streaming
Oct 11, 2026
Merged

echelon merged 2 commits into
storytold:mainfrom
cyfung1031:fix/a11y-structure-streaming

Conversation

@cyfung1031

Copy link
Copy Markdown
Contributor

Summary

The structure-tree checks (alternate text, tables, lists and headings) first copied the whole structure tree into a vector of elements, and stopped adding elements at 2,000,000. Elements past that point were never checked, so a missing figure or an irregular table late in a large file went unreported. This change reads the tree once with an explicit stack and reports each element as it opens and closes; each rule keeps only the state it needs. The cap is removed. The report still lists the first 200 findings per rule and counts the rest.

Problem and root cause

structure::Tree::read materialises every structure element (type, parent, children, annotation references and cloned attribute dictionaries) before any rule runs, and Reader::kids stops at MAX_ELEMENTS = 2_000_000. That cap is a correctness cap, not a presentation limit: elements after it are absent from all 13 structure rules. check() also built the tree on every run, even when no structure rule was selected.

Scope and non-goals

  • In scope: the 13 rules that read the structure tree (crates/a11y/src/structure.rs), their call site in crates/a11y/src/lib.rs, the tree-existence flag used by four page rules in crates/a11y/src/content.rs, and their tests.
  • Non-goals: heading extraction caps (MAX_HEADINGS and the visit, depth and title guards in headings.rs), page-content decode limits, PDF/UA policy, finding wording, the 200-finding presentation limit (unchanged), the public API (unchanged), and dependencies (unchanged).

Rule inventory

Every rule that reads the structure tree, and what this change does with it:

Rule Class Converted
figures-alt-text, other-elements-alt-text, nested-alt-text local: the element and its open ancestors yes
table-rows, table-cells, list-items, lbl-lbody local: the element and its parent's type yes
table-summary local: the element's own attributes yes
heading-nesting local: one value, the previous heading level yes
alt-text-associated, alt-text-hides-annotation cross-reference: content and annotations folded up from the element's subtree when it closes yes
table-headers, table-regularity table state: header cells, and row widths with row spans carried into later rows yes
whole-tree rules none n/a

Four page rules (tagged-content, tagged-annotations, tagged-multimedia, tagged-form-fields) read only whether the catalog has a structure tree. They now receive that as a boolean and never traverse the tree.

Nothing is left on the old path. MAX_ELEMENTS is removed.

Before and after behaviour

  • Before: 6396e53 makes the element limit injectable through Tree::read_with_limit, which read calls with MAX_ELEMENTS, so behaviour is unchanged. Its test elements_past_the_limit_are_not_read records the cut-off: with a limit of 5, a figure after ten paragraphs is absent, and with a limit of 100 it is found.
  • After: 4b25bc2 removes the cap. figures_and_tables_past_the_old_element_cut_off_are_checked puts a figure without alternate text and an irregular table after 5,000 paragraphs, and both are reported. The same commit adds tests for: document order of findings (nested alternate text closes before its ancestors, and the findings still list in document order); the 200-finding bound with an exact count of the rest; cycles, shared kids and missing kids; the depth limit; arrays nested 300 deep; a marked-content list of 20,000 items; huge spans; and row spans that carry into later rows.
  • Existing tests are unchanged, and all 22 a11y tests pass. A randomized test (4,000 tables) compares the streamed row-span computation with a dense column-by-column reference.

Time and memory

Five interleaved rounds after one warm-up, medians reported. Each run is /usr/bin/time -l on a small harness that calls pdfcraft_a11y::check on one file and writes each rule's status and findings. The harness is not part of this change. Peak footprint is the peak memory footprint line; net is that peak minus the median peak of load-only runs of the same binary (open the file, no check).

Scenario Command Before: time, peak, net After: time, peak, net Output hash Rounds, input
tagged: about 300,000 elements a11y_bench <pdf> check <out> under /usr/bin/time -l 0.43 s, 261.4 MiB, 248.9 MiB 0.48 s, 207.1 MiB, 194.6 MiB match 5 rounds, 12.2 MB
figures: about 300,000 figures without alternate text same 0.83 s, 270.6 MiB, 231.8 MiB 0.74 s, 182.5 MiB, 143.0 MiB match 5 rounds, 24.9 MB
tables: one table of 3,000 rows by 100 cells, with row spans same 0.41 s, 207.0 MiB, 196.7 MiB 0.39 s, 142.5 MiB, 132.3 MiB match 5 rounds, 9.7 MB

Full SHA-256 of each output: tagged 225b34f8f0c7b236edf8d4aac692c5839c5d622ac901a326e906293927516769; figures 2d5ae5ae5d0100696894bcd88ff361355a1338350c79835f99426c452c791bac; tables 303db6e35b5dd2f62ab2220a08b20e9d5b15e4f0f4521a8d4dbe7d9ce59f35d9. Each is the same on both sides.

Peak footprint falls by 21% to 33% on all three inputs. Median wall time rises about 12% on the tagged input (0.48 s against 0.43 s) and falls by 11% and 5% on the other two. The walk allocates a small location record for each kid, which is the likely cause of the tagged result; the cause was not profiled.

Per element, the walk retains nothing once the element closes. While an element is open it holds one frame, about 100 bytes with its type name, so the stack grows with nesting depth, which is capped at 256. Three other structures are not per open element:

  • The seen set of indirect kids is unchanged from before. It holds one reference key per indirect kid reached: about 8 bytes plus hash-table overhead.
  • Table row spans: at most two runs per span cell that still covers a later row, 24 bytes each. They are released when the table closes, and the count is bounded by the span cells in open tables.
  • Findings: at most 200 messages per rule are built and kept. Later findings are counted, and their text is never built.

The remaining net footprint is mostly the document's object cache. Document::try_get keeps every object it loads, as it did before, and both versions read the same objects.

Output identity

  • The base binary and the final binary were compared on 544 PDFs: 480 synthetic tagged PDFs, four PDFs from the repository's test data and a local engine test file, and 60 larger synthetic trees of 1,000 to 2,500 elements whose rules exceed the 200-finding limit. All 544 outputs (every rule's status and finding list) are byte-identical. Each side produced 15,657 finding lines and three "… and N more" summaries.
  • The three timed inputs produce byte-identical outputs on both sides (hashes above).
  • Both timed binaries were rebuilt at the final base and tree and are byte-identical to the binaries that were timed.

Local checks run

  • cargo fmt --all -- --check: exit 0.
  • cargo clippy -p pdfcraft-a11y --all-targets -- -D warnings: exit 0.
  • cargo test -p pdfcraft-a11y: 22 passed, 0 failed.
  • cargo test -p pdfcraft-automation --test automation accessibility_check_report_and_fixes_through_tools: 1 passed, 0 failed. The engine and automation crates compile as part of this test.
  • Release builds of the harness for both sides, and the timed runs described above.
  • Not run: cargo test --workspace, cargo xtask ci, cargo xtask fuzz, and cargo xtask layers. No dependency or layering changed: Cargo.toml and Cargo.lock are untouched.

Checks deferred to CI

  • Full workspace tests and clippy, and cargo xtask ci (assets, parity, layers, WASM check). The local disk and CPU budget does not cover them.
  • Windows, Linux, FreeBSD and WASM builds. The desktop UI and the command-line crate are not compiled locally. Both use the a11y API, which is unchanged.
  • Fuzzing (cargo xtask fuzz). The hostile-input cases above are unit tests.

Malformed and hostile input safety

  • No unsafe, and no unwrap, expect or panic outside tests. The crate's deny lints are unchanged and clippy passes. The walk uses an explicit stack, not Rust recursion. Depth is capped at 256, as before.
  • Cycles, shared kids, missing kids and a kid reached through itself terminate, and each kid is read once (cycles_shared_kids_and_missing_kids_are_read_once).
  • Depth: sections nested 250 deep are checked and 300 deep are not (nesting_is_read_to_the_depth_limit_and_no_further). Arrays nested 300 deep terminate (arrays_nested_deeper_than_the_limit_are_not_read).
  • Spans: ColSpan and RowSpan are clamped to 10,000. Negative, zero, real and name values read as 1. A row of 2,000 cells each spanning 10,000 columns allocates no per-column storage (huge_spans_do_not_size_the_table).
  • Long lists: a paragraph holding 20,000 marked-content items is read in full (a_long_list_of_marked_content_counts_as_content).
  • Time: the walk is one pass over elements and edges. Table row spans are merged once per row, and each merge sorts only the spans of that row and the carried spans.

Independence

  • Base: origin/main at b8ddf71e30959dc8a5ca4b66c40b8fcaea68cf76. The branch is rebased onto it with no conflicts. Commits: 6396e53 (test seam) and 4b25bc2 (streaming walk).
  • No dependency on any other planned change, and no commits from one.
  • Overlap gate with M2: stream page content past the 256 MiB cap (item 1 of storytold/pdfcraft#623) #797: its files are crates/render/src/raster.rs and vendor/ (README and the hayro-interpret and hayro-syntax sources). This change touches crates/a11y/src/content.rs, crates/a11y/src/lib.rs, crates/a11y/src/structure.rs and crates/a11y/src/tests.rs. No path overlaps. The behaviour is separate: that change concerns renderer and vendored parser limits, while this one changes how the accessibility checker walks the structure tree.
  • Informational open changes: M7: keep embedded CMap ranges compact instead of truncating expansion #837 (crates/fonts/src/pdf.rs, crates/edit/src/text.rs), M4: make page-label generation lazy across large documents #845 (crates/organize/src/labels.rs, crates/organize/src/lib.rs, crates/organize/src/tests.rs) and M4: traverse large bookmark trees lazily without dropping later items #876 (crates/automation, crates/engine/src/lib.rs, crates/organize/src/outline.rs, crates/organize/src/lib.rs, crates/organize/src/tests.rs, crates/render/src/inspect.rs, crates/ui-egui/src/panels.rs, parity/acrobat-features.toml). None of these paths is changed here. lib.rs and tests.rs also exist under other crates, but those are different files.
  • The rebase picked up four upstream commits (82b1093 to b8ddf71) that touch crates/annot, crates/ui-egui and parity/acrobat-features.toml. None of those paths is in this change, and the a11y and automation tests were rerun after the rebase.

Environment

  • macOS (Darwin 23.6.0) on Apple silicon, 8 cores shared with other sessions.
  • Timed runs: the 1-minute load average was 6.0 to 6.4 during the final pass, below the discard threshold of 16, so no run was discarded.
  • An earlier timed pass was lost to a system sleep and was rerun. The numbers above are from the final pass.

🤖 Generated with Claude Code

cyfung1031 and others added 2 commits October 11, 2026 06:45
Tree::read keeps MAX_ELEMENTS; Tree::read_with_limit takes the limit, so a unit test can reach the cut-off with a few elements. The characterisation test records today's behaviour: a figure past the limit is not checked.

Co-Authored-By: Claude Haiku 5.5 <noreply@anthropic.com>
The structure checks built the whole tree as a Vec of elements, and stopped adding elements at 2,000,000, so later elements were never checked. One walk now reads the tree with an explicit stack and reports each element as it opens and closes. Each rule keeps only what it needs about the open elements and, for tables, the row spans that reach into later rows. Findings stay in document order, and the report still shows the first 200 per rule.

Co-Authored-By: Claude Haiku 5.5 <noreply@anthropic.com>
@echelon

echelon commented Oct 11, 2026

Copy link
Copy Markdown
Member

Thank you, @cyfung1031! This has landed on main: "M9: stream structure-tree checks without a total-element cutoff".

Please join us on Discord if you haven't already: https://discord.gg/artcraft

h-a-n-a-s-h-i pushed a commit to feraremag2680/pdfcraft that referenced this pull request Oct 11, 2026
…moved

Conflicts, each resolved so both sides survive:

- crates/a11y/src/structure.rs: main (storytold#926) rewrote the structure rules as
  a streaming visitor. The branch's wordings are ported onto it: the
  `{ty} element[ on page {p}]: {what}` frames via Finding::nested, the
  IRREGULAR wording with {min}/{max}, and the two heading sentences.
- crates/engine/src/lib.rs: the branch's Label arms, plus main's new
  FillAnnotation (comment properties) and MoveObjects ("Move object" /
  "Move objects", added to EDIT_LABELS; es.tsv already has them).
- crates/ui-egui/src/editing.rs: main's move_error message with the
  branch's edit_label.
- es.tsv, parity/acrobat-features.toml: both sides' entries; the parity
  entry has main's shortcut evidence and sentence plus the branch's.
- ATTRIBUTION.toml: main's, with es.tsv's sha256 recomputed.
- engine tests: main's new MoveObjects undo assertion compares as_str().

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants