The two wide-array tests assert item counts within a range near the
budget, not an exact value (charging entries in the entry guard shifts
the boundary by one or two). Fix the stale comments that claimed exact
counts.
Co-authored-by: Abimael Martell <abimaelmartell@users.noreply.github.com>
Both arrays were cloned in full before the budget check, so a crafted
oversized /Fields or /Kids array forced an O(n) allocation and copy
regardless of the cap. resolve_array already returns a borrow tied to the
document and the walker only needs a shared &Document, so iterate the
borrowed arrays directly — the early break now bounds how many entries
are even touched, before any per-array allocation.
Co-authored-by: Abimael Martell <abimaelmartell@users.noreply.github.com>
The budget counted only distinct visited nodes, so /Fields or /Kids
arrays full of invalid (non-reference) or duplicate entries never grew
visited and ran to completion regardless of size — the node budget did
not actually cap traversal work.
Introduce FieldWalkBudget tracking both visited nodes and total entries
examined; charge every array entry (valid, invalid, or duplicate) and
stop once either hits MAX_FORM_FIELD_NODES. Adds a regression test with a
huge /Kids array of duplicate + null entries.
Co-authored-by: Abimael Martell <abimaelmartell@users.noreply.github.com>
Checking the budget before insertion capped the visited set, but callers
still iterated every remaining entry of a wide /Fields or /Kids array
after the budget was exhausted — each walk returned immediately, yet the
O(N) sibling iteration let a single multi-million-entry array burn
extraction CPU unbounded. Break out of both the top-level and recursive
loops once visited reaches the cap, making the budget a true
traversal-work cap. Adds a top-level wide-/Fields regression test.
Co-authored-by: Abimael Martell <abimaelmartell@users.noreply.github.com>
The node-budget guard inserted each field ID into the visited set before
checking the budget, so the check triggered an early return but never
actually capped the set. A field with a huge /Kids array kept inserting
post-budget IDs, letting visited (memory and work) grow with the crafted
input rather than stopping at MAX_FORM_FIELD_NODES.
Check depth and budget before inserting, so visited can never exceed the
cap. Adds a wide-tree regression test.
Co-authored-by: Abimael Martell <abimaelmartell@users.noreply.github.com>
The visited-set guard stops cyclic /Kids graphs, but a long *acyclic*
chain of distinct fields still recurses to the chain length and overflows
the stack (a ~1.6MB PDF with 20k linked fields aborts pdf2md, exit 134)
before the 100k node budget is reached.
Add an explicit recursion depth cap (100 levels — far above any legitimate
form hierarchy) so stack usage is bounded independently of node count.
Adds a deep-acyclic-chain regression test.
Co-authored-by: Abimael Martell <abimaelmartell@users.noreply.github.com>
A crafted PDF whose AcroForm field lists itself (or another ancestor) in
/Kids caused walk_form_fields to recurse indefinitely, overflowing the
stack and aborting pdf2md (exit 134) — an application-level DoS from a
~730-byte input.
Track visited field object IDs to break /Kids cycles, and cap total
field-node traversal at 100k nodes to bound pathologically large trees.
Adds regression tests for self-cycle and mutual-cycle field graphs.
Co-authored-by: Abimael Martell <abimaelmartell@users.noreply.github.com>
* fix(tables): exclude script attachments and tiny numeric fragments from detection
Split out of #242 (draft) so it can be reviewed on its own evidence.
Display equations with sub/superscripts form phantom small-font table
regions: the subscripts cluster with nearby small text (footnotes, axis
labels) into fake multi-column grids. Two guards:
- Script attachment: a small-font item horizontally adjacent to a
larger-font item at a genuine baseline offset is a sub/superscript,
not a table cell, and is excluded from candidates. A real baseline
offset is required so a small cell beside a larger same-baseline
label is never filtered. Attachment targets are indexed by Y and
scanned through a bounded window rather than a full-page sweep.
The body-font pass applies the same exclusion but only for
heading-sized anchors (>= 1.15x base), so body-size table cells
beside slightly larger labels are untouched.
- Tiny numeric fragments: a <=2-row grid whose every cell is a bare
1-2 digit number carries no tabular information. Restricted to the
small-font pass, where the pattern is overwhelmingly exponent
clusters; body-font numeric grids are unaffected.
Corpus impact: 20 of 186 documents, measured against a control build of
main so main's own drift is excluded. Table rows fall in 17 of 18
inspected documents and no content is lost — 2103_07786 drops all 21
rows, every one a math fragment ('|X 42 43|1|||'); Stijn_SB_doc drops
173 rows of footnote text that had been shredded into cells, with word
count slightly UP and footnote markers intact. M2019_mordeste gains 11
rows from a 3-column grid re-detected as 2-column, neither clearly
better nor worse.
Note the 20 documents is far more than the 3 that per-change ablation
suggested: that figure measured sole-cause attribution inside the
original combined PR, where other heuristics changed the same files and
masked this one. Reach and sole-cause are different measurements.
805 unit + 148 integration tests pass, clippy clean, in-repo snapshots
unchanged.
* fix(tables): suppress script column evidence instead of dropping candidates
Reworked after reviewing the corpus diffs: the first approach removed
sub/superscripts from table candidates entirely, which had two failure
modes beyond the intended fix.
- Legitimate cell content was displaced. citizen-sr-282 is a calculator
manual whose engineering-notation table lists M = 10^6, k = 10^3.
Those exponents are superscripts, so they were dropped from the table
and resurfaced elsewhere in the reading order ('9 mega 6 kilo = 10 3
milli').
- Removing items changed the candidate geometry, so different spurious
structure could form from what remained.
Scripts are now kept as candidates and excluded only from the geometry:
they cannot create a column (find_column_boundaries), cannot qualify a
region on their own (find_table_regions / _strict), but are still
assigned to cells. Column alignment is validated against ALL items
including scripts — validating only the non-script subset would let a
region manufacture alignment by ignoring its awkward items, which is
what block-diagram pages did.
Corpus: 20 of 186 documents, net -359 table rows, no content lost.
Token-level comparison shows the only text changes are merges in the
right direction: 'X' + '10' becomes 'X10', 'L' + 'g' becomes 'Lg' —
subscripts joining their base instead of floating free.
Remaining artifact: MCF5235RM (and its _nxp duplicate) gains a small
spurious table from a block-diagram label line, and M2019_mordeste
gains 11 rows from a 3-column grid re-detected as 2-column. Both are
borderline regions where the previous output was also wrong; documented
rather than tuned away.
805 unit + 148 integration tests pass, clippy clean.
* fix(tables): use a heading-anchored script mask in the body-font pass
Cubic review of #264: a single script mask computed with a 0.0 anchor
was applied to both passes, including body-font region qualification and
geometry. The body pass is supposed to require a heading-sized anchor —
that distinction existed before the geometry rework and was lost in it.
Why it matters: body-pass candidates are themselves body-sized
(0.85..1.05x base). A cell at the low end of that band, say 8.5pt,
sitting beside a 10.5pt label clears the inherent 'anchor >= 1.2x cell'
rule (10.2) and so was flagged as a script attachment. At body sizes a
slightly larger neighbour is a bold label or column header, not the base
of a superscript, so flagging it stripped real cells out of the region
evidence and column geometry and could lose the table entirely.
Two masks now: the small-font pass keeps the 0.0 anchor, the body pass
requires >= 1.15x base. Note the threshold only bites below base size —
for a cell at base, 1.2x-of-cell already exceeds 1.15x-of-base — which
is exactly the 0.85..1.0x band cubic identified.
Corpus: 20 documents, net -367 table rows (was -359 with the single
mask), so the body pass now keeps 8 rows of real table it had been
discarding. 958 tests pass, clippy clean.
* fix(markdown): reject headings that end on a relational verb
A heading candidate ending in 'equals', 'denotes', 'implies' and the
like is the first half of a sentence, not a title. This shows up when a
block dissolves and strands its lead-in ahead of the formula it
introduced — opendataloader 01030000000144 produced
## Note that the exact error equals
M - Q(h) = e - 2.7525... = -0.0342....
Deliberately a very short list. Broader variants were tried and
measured, then rejected:
- Function words (of/and/for/the): a heading that WRAPS across lines
ends on exactly those. Destroyed real IRS Publication 17 headings —
'Casualty and' -> 'Casualty and Theft Losses', 'Rule 10. You Must Be
at' -> '... At Least Age 25'. 52 documents affected, -619 headings.
- Copulas and auxiliaries (is/are/be/have): same failure. 'Rule 15.
Your AGI Must Be', 'What Medical Expenses Are' and 'When Can a Roth
IRA Be' are real wrapped headings, while 'the tax burden should be'
is a genuine fragment. The trailing word cannot separate them; that
needs the next line's context, which this text-only predicate lacks.
The verbs kept never end a heading in any register, so they are safe
without context. Standalone the guard is a no-op on both benchmarks
(0 documents on opendataloader, 4 on pdf-evals with no net heading
change) — its value is as a companion to the table filter in this PR,
which is what strands these lead-ins.
Combined effect on opendataloader (200 docs, vs a control build of
main), where the table filter alone regressed:
table filter + this guard
overall -0.0003 +0.0003
mhs -0.0019 +0.0003
doc ...144 -0.063 +0.053
doc ...144 mhs -0.203 +0.028
* review: gate the dangling-verb veto on sentence case, drop 'yields'
Cubic review of a5a6e8f — both findings valid.
1. 'yields' is also a plural noun. 'Bond Yields', 'Crop Yields' and
'Dividend Yields' are real section titles in financial documents,
which this corpus contains. Removed from the list; my claim that
these verbs 'never end a heading in any register' was wrong for it.
2. A wrapped title-case heading whose first line ends on one of these
verbs would be suppressed if the heading preprocessor failed to
merge it.
Both are fixed by the same gate, which is the discriminator I was
missing: case. A heading is title case ('Bond Yields', 'The Theorem
Implies'); a stranded lead-in is sentence case ('Note that the exact
error equals', 'the method yields'). The veto now applies only when
every content word is NOT capitalized, so titles are spared regardless
of their final word.
This is also why the earlier function-word and copula variants failed:
they had no way to tell 'Rule 15. Your AGI Must Be' from 'the tax
burden should be'. Case separates those two as well.
No measured cost. opendataloader is unchanged from the previous
revision — overall +0.0003, mhs +0.0003, doc 01030000000144 still
0.732 -> 0.785 — and pdf-evals still 20 documents. 963 tests pass,
clippy clean.
* review: exempt section-numbered lines from the dangling-verb veto
Valid ordering bug. heading.rs consults is_heading_fragment at line 282
and only applies its numbered-prefix allowance at line 288, so the veto
pre-empted it: '1. What the model implies' is sentence case and ends on
a listed verb, so it was discarded before numbering could vouch for it.
Numbering is independent evidence of a heading, so the veto now skips
any line opening with a section number.
Acceptance is deliberately a little broader than heading::parse_numbering
(which requires a trailing delimiter) because '2.3 Section Title' is
written without one, and being permissive in a veto exemption can only
avoid suppressing headings. Two guards keep it from swallowing prose:
- a bare single number needs a delimiter ('1.' yes, '3 apples' no)
- roman numerals always need one, since a leading 'I' is the pronoun far
more often than a section number
Not reused from convert::starts_with_section_number, which deliberately
demands two components because it bypasses isolation checks — that would
reject the reviewer's single-'1.' case.
No measured change: opendataloader still overall +0.0003 / mhs +0.0003
with doc 01030000000144 at 0.732 -> 0.785, pdf-evals still 20 documents,
target case still suppressed. 964 tests pass, clippy clean.
* review: share roman_value so the veto exemption matches the parser
Valid. My numbering predicate accepted tokens heading::parse_numbering
rejects — lowercase 'iv)', alphabetical 'd)', over-long 'MMMM.' — because
it case-folded and allowed D and M. Anything the parser rejects is not
numbering, so exempting it let ordinary list items bypass the
dangling-verb veto and reach font-based heading promotion.
Rather than restate the grammar, roman_value is now pub(super) and the
exemption calls it, so the two cannot drift. Its rules apply as written:
uppercase I/V/X/L/C only, at most 8 characters, positive total.
Decimal numbering keeps its slightly broader acceptance (bare '2.3' with
no trailing delimiter), which is deliberate and documented — that form is
common in real headings and being permissive in a veto exemption cannot
manufacture a heading, only decline to suppress one. The roman case is
different because single letters collide with alphabetical list markers.
No measured change: opendataloader overall +0.0003 / mhs +0.0003, doc
01030000000144 still 0.732 -> 0.785. 964 tests pass, clippy clean.
* test: cover the roman length bound with a nine-character token
Valid P3. The 'MMMM.' case fails on the unsupported M, not on length, so
the 8-character bound in roman_value had no coverage and could regress
silently. Added a nine-'I' token, which is rejected only by the bound,
plus an eight-'I' token that must stay exempt to pin the boundary from
both sides.
* fix(tables): stop dropping body-band scripts from the candidate set
Valid: the body-font pass filtered scripts out of body_candidates
itself, so body_script_flags and its two downstream uses were dead. The
mask filters region_evidence and feeds detect_table_in_region's is_script
closure, but neither ever saw a script item because the candidate set no
longer contained any.
Consequences: a body-band sub/superscript attached to a heading-sized
anchor was dropped from the table outright rather than assigned to a
cell, so its text was lost — the opposite of what both the
body_script_flags comment ('they stay candidates') and the
detect_table_in_region docstring ('they remain eligible for cell
assignment') describe, and inconsistent with the small-font pass.
Root cause: the geometry rework removed the candidate-level filter from
the small-font pass, but the body one had been reflowed onto a single
line by rustfmt so the same edit missed it. Adding body_script_flags in
a later review then wired a mask that the surviving filter made
unreachable.
No measured change on either benchmark — pdf-evals still 20 documents
and -367 table rows, opendataloader still overall +0.0003 / mhs +0.0003
with one document changed — because the combination it affects (a
body-sized script attached to a heading-sized anchor) does not occur in
either corpus. The fix is for correctness and consistency between the
two passes, not for a score.
964 tests pass, clippy clean.
* fix(tounicode): skip subset GID remap for CIDFontType0 (CFF) descendants
The sequential-GID repair in try_remap_subset_cmap assumes CIDs are glyph
indices that a subsetter can renumber. That holds for CIDFontType2
(TrueType) but not for CIDFontType0 (CFF), where CIDs are resolved through
the CFF charset, so a valid ToUnicode CMap stays valid after subsetting.
For CFF fonts the corrupting path was unavoidable: CIDToGIDMap is
CIDFontType2-only (PDF 32000-1:2008, 9.7.4.2), so the branch that repairs
the CMap correctly can never be taken, and any CFF font whose /W array
starts at a low CID fell through into remap_to_sequential. Japanese
Adobe-Japan1 documents extracted as long runs of a single unrelated kanji.
Guard both repair paths on a CIDFontType2 descendant, placed before the
CIDToGIDMap branch so a CIDToGIDMap wrongly attached to a CFF font by a
malformed producer is ignored too.
On a National Diet Library proceedings PDF: 1233 U+FFFD in 69099 chars
before, 0 in 68331 after; character 3-gram recall against a hand-written
ground truth 0.354 -> 0.605. The PDF from #118 (CIDFontType2) extracts
byte-identically before and after.
Two existing tests build descendant dicts without a /Subtype and set
CIDToGIDMap, which is CIDFontType2-only, so the fixtures now say what they
already meant. Without that, test_try_remap_skipped_when_w_covers_cmap
would keep passing while no longer exercising the W-coverage logic.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(tounicode): resolve indirect /Subtype, skip only explicit non-CIDFontType2
Addresses review feedback on the guard added in the previous commit.
- /Subtype may be an indirect reference, and as_name() does not dereference
it. A genuine CIDFontType2 font storing /Subtype indirectly would have been
read as "not CIDFontType2", returning early and losing the repair it needs —
reintroducing the corruption this PR fixes, for those fonts. Resolve the
reference through the document before comparing.
- Bail out only when /Subtype is explicitly a non-CIDFontType2 name. A missing
or unresolvable /Subtype now keeps the pre-existing behaviour instead of
silently disabling the repair. As a result the two existing tests no longer
need fixture changes, and this commit reverts those; the diff against main
is now additive only.
- The CFF regression test now attaches a real CIDToGIDMap stream rather than
/Identity, which get_cid_to_gid_map treats as "no map". With the stream, the
test also fails if the guard is moved back below the CIDToGIDMap branch —
verified by moving it and watching it fail.
- Added test_try_remap_resolves_indirect_subtype.
cargo fmt --check, cargo clippy -- -D warnings and cargo test (862 tests) pass.
The Diet PDF still extracts with 0 U+FFFD and the #118 PDF is still unchanged.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix: recover from a corrupted startxref pointer
Fixes#228.
A PDF whose startxref pointer has been corrupted to point at the wrong
byte offset — a single flipped digit, which is what damaged writers
emit in the wild — was entirely unprocessable: every entry point
(classify_pdf, extract_pages_markdown, process_pdf) raised "Invalid
PDF structure", even though the file's object data, real xref table,
and trailer were all completely intact just past the wrong pointer.
Both pypdf and pdfium recover from this by locating the real table
directly instead of trusting the pointer; lopdf doesn't.
Added a new repair candidate (alongside the existing
missing-%%EOF-marker and stripped-leading-bytes repairs in
repair_pdf_container_candidates): scan the buffer for the real,
standalone `xref` keyword and append a corrected trailing
`startxref`/`%%EOF` block. lopdf's own get_xref_start always reads the
*last* `%%EOF` in the final 512 bytes of the buffer and the
`startxref` value immediately before it, so the appended block
transparently supersedes the corrupted one already in the file — no
in-place byte surgery on content the original writer produced.
Scoped to classic (non-stream) xref tables, matching the reported
repro and the common case; a corrupted pointer into a cross-reference
*stream* (`N 0 obj << /Type /XRef ...>>`, some PDF 1.5+ writers) would
need the containing object's number, not just a byte offset — out of
scope here.
Verified against the issue's exact repro (a valid one-page PDF with a
single corrupted byte in its startxref offset): before this fix,
process_pdf/classify_pdf/extract_pages_markdown all raised "Invalid
PDF structure"; after, both the page count and the real extracted text
("Order Detail Report by Account", "WIDGET ASSEMBLY", the dollar
amount) come back correctly. New regression test added.
Full suite (859 tests, 1 new) passes; cargo clippy --all-targets
-- -D warnings unchanged at 28 pre-existing/unrelated errors.
* fix: validate xref table shape and scan in a single reverse pass
Addresses cubic-dev-ai's review of #230.
- P2 (correctness/safety): the recovery candidate trusted the last
standalone "xref" token unconditionally, without confirming it's
actually a cross-reference table. A coincidental "xref" substring
inside unrelated content — a stream, a string, uncompressed
metadata — could get "repaired" against a bogus offset, letting
lopdf load successfully against garbage instead of returning a
clean error: a real failure turned into silent data corruption on
the fallback path. Added looks_like_xref_subsection_header, which
confirms a plausible classic xref subsection header (`<start-id>
<count>`, e.g. "0 6" — the shape every real classic table starts
with) actually follows the candidate token before accepting it.
find_last_valid_xref_table_start now walks backward from the end of
the buffer until it finds a token that both stands alone *and*
validates, rather than accepting the first (rightmost) standalone
match unconditionally.
- P2 (performance): the old scan re-invoked
`buf[..search_end].windows(4).rposition(...)` on a shrinking prefix
every time a candidate token failed the boundary check, which is
quadratic on a pathological buffer with many non-standalone "xref"
occurrences. Rewrote as a single reverse byte-index walk — O(n)
regardless of how many false candidates it has to reject along the
way.
Added direct unit tests on the byte-level scan (more precise than
constructing adversarial full PDFs, and the coincidental-match
scenario can't be represented in an integration-test fixture anyway
since reportlab compresses page content by default): a coincidental
standalone "xref" with no subsection header is rejected; a real
classic table is found; a coincidental match positioned *after* the
real table in the buffer doesn't shadow it; "xref" as a substring of
"startxref" still doesn't match. The original #228 repro (corrupted
startxref pointer, real table otherwise intact) is unaffected —
verified manually in addition to the existing integration test.
Full suite (863 tests, 5 new) passes; cargo clippy --all-targets
-- -D warnings unchanged at 28 pre-existing/unrelated errors.
* fix: reject xref subsection count runs with trailing garbage
looks_like_xref_subsection_header validated that a count run of digits
followed the whitespace separator, but never checked what came after
it. A coincidental "xref\n0 6garbage" in stream/literal content would
still validate as a real subsection header shape and get repaired
against a bogus offset.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Co-authored-by: Abimael Martell <1450169+abimaelmartell@users.noreply.github.com>
* fix: extract_pages_markdown's needs_ocr now agrees with classify_pdf
Fixes#227.
extract_pages_markdown_mem computed its per-page needs_ocr entirely
from text-quality signals: decoding/garble issues, empty markdown, GID
fonts, garbage-text ratio. It had no awareness of the page's image
content at all — so a page that is fundamentally a full-page scan
with a little genuine native text drawn over it (a header, a stamp, a
cover-sheet annotation) extracts that text cleanly, trips none of the
text-quality checks, and reports needs_ocr=false — while
classify_pdf/detect_pdf_type correctly see the dominant background
image and flag the same page as needing OCR. Two public APIs
answering the same question, silently disagreeing, in the unsafe
direction (skipping OCR on a page that needs it).
Exposed detector::analyze_page_images at crate visibility (was
private) and call it per page in extract_pages_markdown_mem's loop —
the same "large background image" signal (>50% page coverage) that
already powers has_template_image in classify_pdf/detect_pdf_type,
rather than reimplementing image-area detection a second time with
its own thresholds that could drift out of sync again. When it's
true, the page is flagged needs_ocr (with OCR_REASON_SCANNED added
to ocr_reasons_by_page, matching how the same signal is already
reported elsewhere) and its markdown is blanked, exactly like the
existing text-quality-triggered needs_ocr paths already do — no
special-casing added for "cleanly-extracted-but-still-a-scan" text.
Verified against the issue's exact repro (a full-page raster with one
native text line drawn over it, built via reportlab/pillow): before
this fix, extract_pages_markdown_bytes reported page 0
needs_ocr=False with the header line as markdown while
classify_pdf_bytes correctly flagged pages_needing_ocr=[0]; after,
both agree needs_ocr=True and the page's markdown is empty. Confirmed
no regression on a normal text-based fixture (nexo-price-en.pdf:
needs_ocr stays False, full markdown returned). New Rust regression
test added exercising both APIs against the same fixture.
Full suite (860 tests, 1 new) passes; cargo clippy --all-targets
-- -D warnings unchanged at 28 pre-existing/unrelated errors.
* fix: gate has_template_image behind the same OCR signals classify_pdf uses
extract_pages_markdown_mem was treating has_template_image alone as
sufficient to force needs_ocr=true and discard the page's markdown, but
classify_pdf/detect_pdf_type never treats that raw signal alone as
needing OCR. A text page with a full-bleed watermark, letterhead, or
large figure would get its clean markdown wrongly blanked and routed
to OCR.
Added page_template_image_needs_ocr(), mirroring the two distinct
signals classify_pdf actually uses to decide a template-image page
needs OCR: the looks_like_scan gate (image_count <= 1, few text ops,
low alphanumeric diversity) used for Mixed-type routing, and the
insufficient-text-volume signal (text_operator_count < 10) that routes
a page with a dominant background image and only a couple of native
text calls to PdfType::ImageBased independent of looks_like_scan.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix: match per-page OCR threshold and add missing vector-text signal
Two follow-up findings on the has_template_image gate added in the
previous commit:
1. insufficient_text used a hard-coded threshold of 10 text operators,
but Mixed-type per-page routing (the actual per-page decision this
function tries to agree with) uses config.min_text_ops_per_page
(default 3). The higher 10 threshold was borrowed from a *different*
classify_pdf code path — the effective_min_ops floor used only for
whole-document ImageBased/Scanned classification, a cross-page
aggregate this per-page function can't replicate anyway. Using the
lower per-page threshold removes a real disagreement window
(3-9 text ops with high alphanumeric diversity) without breaking
the #227 regression fixture (text_ops=1, still well under 3).
2. extract_pages_markdown_mem never checked has_vector_text at all,
even though Mixed-type per-page routing always sends
vector-outlined-text pages to OCR (outlined glyphs can't be
extracted as text). A page with massive path ops plus a short
genuine caption could extract that caption cleanly, slipping past
the existing empty/garbage-text checks. Added
page_has_vector_text() and wired it into needs_ocr the same way
has_template_image is.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* perf: compute template-image and vector-text OCR signals in one pass
page_template_image_needs_ocr and page_has_vector_text each called
analyze_page_content independently, so every requested page's content
streams (page + XObjects) and image coverage were decompressed and
scanned twice per page with one result discarded each time.
detect_from_document avoids this by caching its per-page PageAnalysis;
extract_pages_markdown_mem had no such cache.
Merged both into page_ocr_signals(), a single analyze_page_content
call returning both signals as a tuple.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Co-authored-by: Abimael Martell <1450169+abimaelmartell@users.noreply.github.com>
* fix(extractor): supply built-in metrics for non-embedded base-14 fonts
PDFs may legally omit /Widths for non-embedded standard fonts (Times,
Helvetica, Courier, Symbol, ZapfDingbats) — the spec requires the reader
to supply the metrics. We returned None, so every glyph advanced 0 and
each text item got width 0, silently breaking every gap-based heuristic
downstream: space synthesis, sub/superscript detection, table column
detection, heading merging.
- src/extractor/base14.rs: Adobe Core-14 AFM width tables keyed by
Unicode char, plus the standard Symbol/ZapfDingbats encoding vectors
(their glyphs sit at byte positions unrelated to Latin text, so widths
must resolve through the built-in encoding, not cp1252)
- Width resolution order: Differences -> built-in encoding -> the same
cp1252-style fallback the text decoder uses, so a code's advance always
matches the character we emit for it
- Type3 visual sizing: PK bitmap fonts (dvips) use FontMatrix
[1 0 0 -1 0 0] with nominal sizes like 0.12pt; scale by FontBBox height
x |matrix_y|. Applied in the page-stream and Form XObject paths.
Indirect numeric array elements are resolved before use.
Effect on Shannon's 'A Mathematical Theory of Communication' (1998
dvips/Distiller, the reported case): glued sentences 95 -> 5. Corpus
impact: 12 of 184 eval documents, e.g. Data-Processing-Agreement
recovers a paragraph that a phantom table had shredded into cells.
Layout heuristics tuned on the same document (indent-based paragraph
breaks, heading reclassification, table script filtering) are held back
for a separate PR — they change ~98 further documents and need to be
justified against the corpus, not against one PDF.
* review: narrow Type3 rescaling to self-inconsistent fonts; dedup + test all width tables
Addresses cubic review on #241, plus a follow-up from a local cubic run.
- Type3 visual scaling was applied to every Type3 font whose FontBBox
height x |matrix_y| deviated >5% from 1.0. FontBBox is the glyph box,
not the em box, so a conventional 1/1000-matrix font with a
descender..ascender bbox (~700 units) computed 0.7 and had every
reported size shrunk by 30% — corrupting the drop-cap, heading-tier,
sub/superscript and table heuristics this is meant to fix.
First attempt gated on the matrix being unit-scale, but a local cubic
run pointed out that wrongly excludes valid non-standard matrices (a
0.005 matrix with a full-em bbox legitimately needs a 5x scale). The
product is the right discriminator, not the matrix: a self-consistent
font lands near 1.0 because the matrix is the reciprocal of the
glyph-space em, so only a wildly inconsistent one (dvips/PK bitmap
fonts sit at ~159) is renormalized. Band widened to [0.25, 4.0].
Corpus effect: 12 -> 7 documents change. The 5 that drop out were
being wrongly rescaled — including Data-Processing-Agreement, whose
phantom-table fix turned out to come from this bug rather than from
the width fallback, so it is correctly given up.
- base14: all 14 width tables now covered by the sort-invariant test via
an ALL_TABLES registry, not a hand-picked subset.
- base14: identical tables share one static (all four Courier variants
are monospace 600; the oblique Helvetica variants match their upright
forms), removing 5 duplicate copies.
* test: refresh Shannon snapshot after merging main
CI checks out a merge of the PR head with main, and main advanced 8
commits since this branch was cut — including #201 (contextual digit
runs), #240 and #253 (markdown fixes). Those change extraction output,
so a snapshot generated on the unmerged branch could not match; the
Test job failed on the merge commit while passing on the branch itself.
The merged behaviour is better: the footnote marker '2' before
'Hartley, R. V. L.' is now recovered instead of dropped.
950 tests pass on the merged tree, clippy clean.
AGENTS.md was stale (179+ PDFs, missing the semantic-quality bullet).
Both files now match: ~200-PDF corpus, and iteration guidance to prefer
subset runs (bench.py test -q / -s <name>) with the full suite as the
final pre-commit check.
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
strip_pdf_comments tracked parenthesis nesting to protect string
literals, but ignored backslash escapes. An escaped \) desynced the
depth counter, after which a % glyph inside a string was stripped as a
top-level comment, corrupting the stream for Content::decode and
silently truncating the page's text.
Treat \ inside a string literal as escaping the next byte, so \(,
\), and \\ never touch the nesting depth.
## Bug 1: Python type stubs missing 5 fields/classes
The pdf_inspector.pyi file was out of sync with the actual Python
bindings exposed via #[pyo3(get)] in src/python.rs. This breaks
IDE autocompletion and type checking (PyCharm, VS Code/Pylance, mypy)
for all Python users.
Added:
- PdfResult.ocr_reasons_by_page (python.rs:35)
- PageOcrReasons class with page and
easons fields (python.rs:67-86)
- RegionText.ocr_reason (python.rs:136)
- PageMarkdown.ocr_reason (python.rs:193)
- PagesExtractionResult.ocr_reasons_by_page (python.rs:226)
## Bug 2: PdfResult.pages_needing_ocr indexing undocumented
PdfResult.pages_needing_ocr is 1-indexed (per python.rs:30) but
neither the .pyi stubs nor docs/python.md annotated this, while the
same field on PdfClassification was annotated as 0-indexed. Users
mixing both APIs would get wrong page numbers.
## Bug 3: README.md duplicate bullet character
The Markdown features table listed * twice in bullet prefixes.
The first should be ullet (U+2022), matching the actual source code in
src/markdown/mod.rs:34 which uses: ullet, dash, sterisk, circle, illed-circle, open-circle.
## Bug 4: docs/python.md missing fields in type reference
The Types section was missing PageOcrReasons, RegionText class
definition, ocr_reason fields, and ocr_reasons_by_page fields.
## Evidence
Cross-referenced every #[pyo3(get)] attribute in src/python.rs
against the .pyi declarations and docs/python.md type reference.
* chore(ci): bump GitHub Actions to current majors
Node 20 action runtimes are deprecated on GitHub runners; bump every
first-party action to its latest major across all workflows:
- actions/checkout v4/v6 -> v7
- actions/cache v4 -> v6
- actions/upload-artifact v4 -> v7, download-artifact v4 -> v8
- actions/setup-node v6 -> v7, setup-python v5 -> v7
- actions/upload-pages-artifact v3 -> v5, deploy-pages v4 -> v5
Third-party pins (dtolnay/rust-toolchain, Swatinem/rust-cache,
setup-zig, setup-bun, taiki-e/install-action, maturin-action) are
already on their latest majors.
* chore(ci): pin all actions to full commit SHAs
Mutable @vN tags can be retagged; in the publish workflows that code
runs with OIDC credentials before npm/PyPI/crates.io publishes. Pin
every action (first- and third-party) to its release commit SHA with
the version in a trailing comment.
dtolnay/rust-toolchain infers the toolchain from its ref name, so the
SHA-pinned invocations pass an explicit toolchain: stable input.
Adds x86_64-unknown-linux-musl, aarch64-unknown-linux-gnu, and
aarch64-unknown-linux-musl to the napi build targets so
@firecrawl/pdf-inspector works on Alpine and ARM64 Linux deployments.
- gnu arm64 cross-compiles with --use-napi-cross (old-glibc sysroot),
musl targets with -x (zig + cargo-zigbuild), per the napi-rs template
- new platform packages carry npm libc metadata (glibc/musl)
- smoke-test job runs napi/test.mjs on all six targets before publish
(Alpine containers for musl, ubuntu-24.04-arm runners for ARM64)
- bump to 1.12.0 to trigger publishing of all platform packages
Closes#216
* fix(extractor): don't flag gid Differences names covered by ToUnicode
Pages were marked as having unresolvable gid-encoded fonts whenever any
font's /Differences array used gidNNNN glyph names, and when every page
carried such a font the whole document's markdown was suppressed.
LibreOffice exports do exactly this: subset fonts get /gidNNNN names in
Differences alongside a complete ToUnicode CMap that decodes them, so
ordinary text documents lost their entire markdown output even though
extraction decoded every glyph.
Track the character codes behind the gid names and only flag the font
when its ToUnicode CMap addresses none of them. Partially mapped codes
stay unflagged: an emoji ZWJ sequence maps whole on its first code, and
the remaining component-glyph codes are subset leftovers, not damage.
Fonts without ToUnicode, or whose CMap ignores the gid codes, are
flagged as before, and the downstream garbage/encoding checks still
catch partial breakage.
* fix(extractor): require a usable ToUnicode mapping to clear the gid flag
A mapping to U+FFFD (or an empty string) is rejected by extraction as
an invalid CMap result, so it must not count as decodable when deciding
whether gid-named Differences codes are resolvable.
Single-word bold section headings ('Replace', 'Trash', 'Instructions')
required a paragraph break before AND after, but headings hug their
section's first paragraph — the break-after almost never exists. A
standalone all-bold single word (>=4 chars, paragraph break before or
page top) now classifies; mixed bold lead-ins ('Note: ...') stay
excluded via all_bold.
opendataloader-bench: 0.8567 -> 0.8575, MHS 0.773 -> 0.776; docs 145
+0.118, 069 +0.112 (net of one cover-page layout shuffle at -0.066
where the new output is semantically closer to GT). pdf-evals: 66
snapshots, composite 0.5864 -> 0.5883, sole >0.02 mover positive.
p1244/thermo fixture snapshots regenerated ('Instructions' un-fuses
from its body paragraph — the intended behavior).
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* fix(headings): digit-only lines do not define heading tiers
A large bold page number (14pt folio over 11pt body) claimed tier 0:
every real heading demoted one level document-wide, and the bold-size
fallback (which requires an empty tier list) was blocked for documents
whose headings match body size.
Bench-neutral (MHS scores relative hierarchy); pdf-evals: 18 docs get
their heading levels back (#### -> ###), semantic composite +0.0006,
no percentile down.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(headings): exclude digit-only lines from the bold fallback tier pass too
The exclusion in the main pass wasn't enough: with the page-number
tier gone, the bold fallback re-collected the same bold folio. Also
regenerates the thermo-freon12 snapshot (cosmetic churn on the
scrambled legend fixture).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
IntechOpen-family academic PDFs embed Computer Modern math symbol
subsets whose glyphs are misnamed after Latin lookalikes (equal →
/onequarter, plus → /thorn, parens → /eth //Thorn) — and the generated
ToUnicode faithfully propagates the wrong names, so formulas decode as
'S ¼ kB þ 1' instead of 'S = kB + 1'.
Remap the observed misnames, gated strictly on the TeXCMMathsSymbols
base font (subset prefix stripped) so genuine fractions and thorns in
text fonts are untouched. Known limitation: a sibling subset misnames
the slash as /onequarter too, so an occasional '/' renders as '=' —
still strictly better than the previous mojibake.
Affects 4 bench PDFs (028/031 +0.001-0.004 NID) and zero pdf-evals
snapshots.
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* fix(headings): rescue wrapped bold headings on interleaved column pages
Report pages whose columns can't be detected (6pt gutters) interleave
both columns' lines, which breaks every whitespace signal the bold
heading heuristic relies on: para_threshold inflates to ~3x line
height and a wrapped heading's own internal line gap defeats isolation.
'9.5. Adapting to the New Normal: Changing / Business Models' merged
into the following paragraph.
Four changes:
- merge_wrapped_bold_heading_groups: 2-3 consecutive all-bold
body-size lines merge into one line when the group is isolated
(column-locally, judged by x-overlapping lines only) or starts with
a section number.
- Section-numbered all-bold lines ('9.5. ...') classify as headings
without the standalone/isolation score gate.
- Line unfusing extends to uppercase-start continuations, gated on a
bold-style mismatch between the runs (a bold heading beside regular
body text) — same-style label rows stay joined.
- The unfuse line-side wordiness requirement drops to 2 words so a
wrapped heading's short last line ('Business Models') still splits
from the neighboring column.
opendataloader-bench: overall 0.8554 -> 0.8576, MHS 0.769 -> 0.777;
docs 037 +0.161, 111 +0.157, 039 +0.091, 198 +0.028, none down.
pdf-evals: 63 snapshots, composite 0.5952 -> 0.5964, sole >0.02 mover
positive. thermo-freon12 snapshot regenerated (cosmetic churn on an
already-scrambled 3-column legend).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(headings): review follow-ups — multi-component section numbers, wholly-bold line gate
Single '1. ' prefixes are ordered list items and no longer bypass
isolation; the uppercase unfuse requires the whole line bold (a
heading), not merely its last run, so mixed bold-label/value rows
stay joined.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* fix(layout): unfuse independent column runs sharing a baseline
Two-column report pages with charts fused headings into the adjacent
column's body text: the columns' ~6pt gutter is below what histogram
valley detection can safely use, so the page grouped single-column and
same-baseline items from both columns joined into one line ('6.2.
Expectations for Re-Hiring Employees' + mid-sentence text, killing
MHS and NID on the whole survey-report doc family).
Three changes:
- Line grouping splits same-baseline runs separated by a wide void
(>3x font size, >=30pt) when the incoming run starts lowercase
(mid-sentence continuation from another column) and both sides are
multi-word prose. TOC page numbers, dot leaders, and table cells
(numbered/capitalized) stay joined.
- Column detection is blind to chart-region text (tight 2pt bounds —
wider padding ate rows adjacent to charts), via a chart-aware line
grouping variant wired from the markdown pipeline.
- validate_and_build_columns computes its vertical span from
histogram-eligible items only, so full-width captions no longer sink
the overlap ratio for partial-page column regions.
opendataloader-bench: overall 0.8532 -> 0.8554, MHS 0.761 -> 0.769;
doc 038 +0.434, no regressions. pdf-evals: 34 snapshots change,
semantic composite wash (0.5749 -> 0.5748), no per-doc mover >0.015.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(layout): review follow-ups — chart-aware band-split grouping, single chart scan per page
Band-split pages now route through the chart-aware grouping too, and
the band loop reuses the precomputed page_chart_map instead of
re-scanning the rect list per page.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* fix(headings): bold-size fallback tiers when nothing clears the ratio gate
Books often set section headings barely above body size (11pt bold
over 10pt text). Nothing cleared the 1.2x heading-tier ratio gate, so
tiers stayed empty and every bold heading defaulted to H2 — H1 was
unreachable for the whole document.
When no size clears the gate, build tiers from bold line sizes >=1.05x
body, and let tier matches through detect_header_level down to that
ratio. Documents with real (>=1.2x) tiers are untouched.
Bench-neutral by construction (the MHS metric scores relative
hierarchy, not absolute levels); pdf-evals semantic composite +0.004
on the 11 affected docs with all percentiles up.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(headings): require boldness for sub-gate tier matches
Review follow-up: fallback tiers come from bold lines, so honoring
them for non-bold text at the same size would promote captions.
detect_header_level now takes is_bold and only matches tiers below
the 1.2x gate for bold lines; >=1.2x matches stay bold-agnostic.
Also restores the >=1.2x tier-match loop the refactor dropped.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(headings): judge line boldness by character mass
Review follow-up: a heading with an unbold section-number prefix
('4. ' + bold title) failed the first-item boldness test. Judge the
line by bold character mass instead, at all three call sites.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
