Skip to content

feat(text): extract_spans_filtered_with_reading_order - reading order + OC/ink filtering#883

Merged
yfedoseev merged 3 commits into
yfedoseev:mainfrom
ajbufort:feature/spans-filtered-with-reading-order
Jul 19, 2026
Merged

feat(text): extract_spans_filtered_with_reading_order - reading order + OC/ink filtering#883
yfedoseev merged 3 commits into
yfedoseev:mainfrom
ajbufort:feature/spans-filtered-with-reading-order

Conversation

@ajbufort

Copy link
Copy Markdown
Contributor

The gap

Positioned-span extraction cannot filter, and filtered extraction cannot return positioned spans:

API reading order filtering returns
extract_spans_with_reading_order yes no spans
extract_text_filtered (fixed) yes assembled text
extract_spans_filtered no yes spans, but private

So a consumer that lays spans out itself - an HTML/XML emitter placing each span at its own rectangle - has no way to honour optional content. It needs both at once, and no public call offers them.

Why it matters

This is the default-policy split the optional_content module already documents: render_page honours /OCProperties/D, while span extraction treats everything as visible unless the caller names layers. Both are spec-defensible (ISO 32000-1 §8.11.3 NOTE 4), and the module note already tells callers wanting render/extract parity to pass matching excluded_layers - but with positioned spans there was nothing to pass them to.

Found in a web-crawl corpus: a PDF whose default-OFF layer holds a copy of the visible content renders one copy and extracts two - every word on the page duplicated. Measured on that file against poppler (which honours /D):

extracted poppler
tokens 1153 750
COUNTY 14 7
MOED 12 6
baltimorecity 24 12

A clean 2x. Passing compute_default_off_ocgs() through the new method gives 751 tokens, with each of those words landing exactly on poppler's count.

The change

The new method is the existing reading-order body with the raw-span source swapped for extract_spans_raw_filtered when a filter is supplied. extract_spans_with_reading_order now delegates to it with empty sets, and empty sets take the verbatim unfiltered path - so this is a strict superset of the existing API and no current caller can shift behaviour or pay any cost.

let hidden = optional_content::compute_default_off_ocgs(&doc);
let spans = doc.extract_spans_filtered_with_reading_order(
    0, ReadingOrder::ColumnAware, hidden, Default::default(),
)?;

Tests

  • excluding_default_off_layer_drops_the_duplicate_copy - the fixture models the real shape above: it asserts the unfiltered call yields both copies (the bug) and the filtered call yields one (the fix), so it cannot pass vacuously.
  • empty_filters_are_identical_to_the_unfiltered_call - across every reading order, pinning the strict-superset claim.
  • filtering_preserves_the_requested_reading_order.

Gate on this branch: cargo test (5735 lib + 3 new + the doctest), cargo clippy --all-targets -- -D warnings, cargo fmt --check, cargo doc - all clean. DCO signed.

… + OC/ink filtering

Positioned-span extraction cannot filter, and filtered extraction cannot return
positioned spans:

- `extract_spans_with_reading_order` takes no excluded_layers/inks.
- `extract_text_filtered` filters, but returns ASSEMBLED TEXT, not spans.
- `extract_spans_filtered` is private.

So a consumer that lays spans out itself - an HTML/XML emitter placing each span
at its own rectangle - has no way to honour optional content.

That matters because of the default-policy split this crate documents in
`optional_content`: `render_page` honours `/OCProperties/D`, while span extraction
treats everything as visible unless the caller names layers. Both are
spec-defensible (s8.11.3 NOTE 4), and the module note already tells callers
wanting render/extract parity to pass matching `excluded_layers` - but with
positioned spans there was no call to pass them to.

Found in a web-crawl corpus: a PDF whose default-OFF layer holds a COPY of the
visible content renders one copy and extracts TWO - every word on the page
duplicated. On that file, against poppler (which honours /D): 1153 tokens vs 750;
"COUNTY" 14 vs 7, "MOED" 12 vs 6, "baltimorecity" 24 vs 12 - a clean 2x. Passing
`compute_default_off_ocgs()` through this method brings it to 751 tokens, each of
those words landing exactly on poppler's count.

The new method is the existing reading-order body with the raw-span source
swapped for `extract_spans_raw_filtered` when a filter is supplied.
`extract_spans_with_reading_order` now delegates to it with empty sets, and empty
sets take the verbatim unfiltered path - so it is a strict superset and no
existing caller can shift.

Tests: the duplicate-layer fixture asserts unfiltered yields BOTH copies (the bug)
and filtered yields one (the fix); empty filters are identical to the unfiltered
call across every reading order; filtering preserves the requested order.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Anthony J. (Tony) Bufort <ajbufort@ajbconsulting.us>
@ajbufort
ajbufort requested a review from yfedoseev as a code owner July 16, 2026 21:48

@yfedoseev yfedoseev left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approve

Verified end-to-end: code review, real-PDF functional proof against poppler, and a full corpus regression sweep. This is a well-scoped, well-tested fix for a real correctness bug.

Design

The strict-superset framing holds up. The empty-set branch is the verbatim prior path (extract_spans_raw), and extract_spans_with_reading_order delegates with empty sets — so no existing caller can shift behaviour or pay a cost. Spec anchoring is correct (ISO 32000-1 §8.11.3; /OCProperties/D/OFF), and the render/extract-parity rationale matches the optional_content module's documented default-policy split.

Functional proof on real corpus PDFs (poppler = ground truth, honours /D)

Not just the synthetic fixture — the new method makes extraction agree exactly with what the page displays:

PDF default-off OCGs unfiltered filtered poppler
pdfjs/test/pdfs/issue11144_reduced.pdf {"Layer 3", "Notes!"} 10 0 0
pdfjs/test/pdfs/issue269_2.pdf 47 OCGs 72 1 1

The unfiltered path over-extracts hidden-layer content; passing compute_default_off_ocgs() lands on poppler's count exactly. That is precisely the bug described, reproduced independently on files not in this PR.

Regression: zero

Built main (eae0dfc9) and this branch (f9c81bae), extracted the 419-PDF corpus manifest through the Python binding in both, compared text/md/html:

compared 1257 files (text/md/html x 419 PDFs)
byte-identical: 1257   differing: 0

Empirically confirms the strict-superset claim — existing extraction is untouched.

Local gates

cargo build --lib, cargo clippy --lib -- -D warnings, and the 3 new tests all clean. The tests are genuinely non-vacuous: excluding_default_off_layer_drops_the_duplicate_copy asserts both the 2-copy bug and the 1-copy fix, so it cannot pass by accident.

CI

The 7 red checks are not this PR. All are maturin-action failing to download with HttpClientError: statusCode: 503 — transient infra, identical across every wheel/sdist job. Re-runnable; I'll re-run them when this reaches the front of the merge queue.

Notes (non-blocking)

  • No binding gap: the sibling extract_spans_with_reading_order isn't exposed to the language bindings either, so this stays consistent with the existing Rust-core surface.
  • Queued behind #862/#875/#878/#881 (one-at-a-time merges; limited CI runners).

@ajbufort

Copy link
Copy Markdown
Contributor Author

The 11 red checks are all binding-packaging/toolchain-setup jobs failing on infrastructure, not on this change:

  • Build wheels / Python wheel / Python wheel FIPS (9 of the 11): the failing step downloads a build artifact and gets HttpClientError: <!DOCTYPE html> - an HTML error page in place of the artifact.
  • Clojure on macos-latest: fails in the Set up Clojure step (toolchain install), before any code compiles.
  • Build source distribution: same packaging path.

Every job this change could actually affect passed: Clippy, Lint and Format Check, Build lib on all platforms/feature sets, and the full Python test suite (3.8-3.14, all OSes). 185 checks green, 38 skipped. This is a pure-Rust addition (src/document.rs + one test file, +217/-2) with no packaging surface, so the wheel/binding failures are unrelated and should clear on a re-run.

@yfedoseev
yfedoseev merged commit 3c380b0 into yfedoseev:main Jul 19, 2026
236 checks passed
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