fix: don't drop inline subscripts/superscripts near page margins as page numbers - #226
fix: don't drop inline subscripts/superscripts near page margins as page numbers#226MADENIYOU wants to merge 5 commits into
Conversation
…age numbers A user reported (feedback on this repo's recent activity, not a filed issue) that pdf-inspector "seems to drop superscripts and subscripts silently." Investigated and confirmed: `is_page_number` in extractor/layout.rs flags any 1-4-digit item positioned near the top/bottom margin (y > 720 or y < 100 on a US-Letter-sized page) as a page number, with no check for whether the item is actually isolated on its own line. A subscript or superscript digit that fails to merge into its parent word — merge_text_items requires font sizes within 20% of each other, which a subscript/superscript by definition isn't — stays a separate short numeric TextItem. When that item happens to sit near a page's top or bottom margin (titles, headers, and footnote references routinely do), it gets swept up by this filter and vanishes from markdown output entirely — not merged, not left as a bare digit, just gone. Minimal repro: a one-page PDF with "H" (14pt) + "2" (9pt, subscript position) + "O" (14pt) near the top of the page. Before this fix, pdf2md output "H O" — the "2" isn't there in any form. `--items-json` confirms the digit is extracted correctly at the raw-item level, so the loss happens specifically in this page-number filter, not during text extraction or subscript merging. Fix: only drop a page-number candidate when nothing else on the page is tightly X-adjacent to it at a similar Y (near-touching, gap less than 1.5x the larger of the two items' font sizes) — the actual geometric signature of "sits right next to its parent word" versus "a lone number by itself in the margin." Y-proximity alone isn't enough: a genuine page number commonly shares its footer row with an unrelated element (a tagline, a running title) at ordinary word-gap distance, which must still be filtered — this was caught by the existing nexo-price-en.pdf snapshot test regressing on a first pass that only checked Y. Also recovers real content in an existing fixture: p1244-1996.pdf's title line was "Form Employee's Report" — the form number "4070" (a real IRS form number, tightly adjacent to "Form" at the top margin) was being silently dropped by the same bug. Updated that snapshot; it's a content-completeness fix, not a behavior change requiring review. Added two new fixtures/tests: inline_subscript_near_margin.pdf (the H2O case — the digit must survive) and isolated_page_number_footer.pdf (a genuinely lone footer page number — must still be filtered, guards the fix's precision). Full suite (866 tests) passes; cargo clippy --all-targets -- -D warnings unchanged at 28 pre-existing/unrelated errors.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 5 files
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
Addresses cubic-dev-ai's review of firecrawl#226: - P2: the neighbor check didn't exclude other page-number candidates from counting as a protector, so a multi-digit page number rendered as separate single-digit Tj operators (a real PDF-generator pattern) would have each digit "protect" its sibling — every digit sees a page-number-like neighbor nearby and none of them get dropped, leaking the whole number into markdown. Added `is_page_number(other)` to the exclusion list: only genuine non-numeric text can protect a candidate now. - P2: the 1.5x font_size adjacency cutoff was far looser than ordinary word-gap spacing (~0.28x font_size for a space character in a typical proportional font), so a real footer page number sitting next to unrelated text (a tagline, a running title, a company name) at normal word-gap distance was being retained instead of filtered — contradicting the fix's own stated goal. Tightened to 0.25x, just above merge_subscript_items's own 0.2x merge-gap tolerance: tight enough to reject a normal inter-word space, loose enough to still protect a subscript/superscript that just barely misses that merge for a non-gap reason (a parent ending in a digit rather than a letter, a strikeout/underline mismatch — merge_subscript_items declines those regardless of how tight the gap is). Both were caught with real fixtures, not just reasoning about the threshold: - split_digit_page_number_footer.pdf: "12" as two separate Tj('1')/ Tj('2') operators — previously leaked through, now correctly dropped as a whole. - word_adjacent_page_number_footer.pdf: "12" at normal word-gap distance from "Company Name" (different font size, so it survives `merge_text_items` as its own item) — previously retained, now correctly dropped while "Company Name" survives. Also rebuilt the two existing fixtures (inline_subscript_near_margin.pdf, isolated_page_number_footer.pdf) with a real Helvetica /Widths array instead of relying on pdf-inspector's width=0 fallback for an unmetriced font — the original fixtures' "H2O" case only worked because inaccurate width math inflated the gap into what was, by coincidence, still under the old 1.5x threshold. With accurate widths, a genuine subscript's true gap is near-zero and already merges via `merge_subscript_items` on its own (verified separately); the new fixture instead demonstrates this fix's actual target case — a numeric superscript/subscript that `merge_subscript_items` structurally declines to merge (gap just outside its 0.2x tolerance) and that this filter must still protect. Net effect on real content: p1244-1996.pdf's "Form 4070" recovery (mentioned in the original PR description as a bonus) no longer survives the tightened threshold — its true gap (~0.94-1.35x font_size, a stylistic size-emphasis gap, not a failed-merge subscript's near-zero one) is well outside any threshold that also rejects ordinary word spacing. Reverted that snapshot line; recovering that specific case would need a different mechanism than X-adjacency, out of scope here since it risks exactly the false-positive class this review caught. Full suite (866 tests, 4 new) passes; cargo clippy --all-targets -- -D warnings unchanged at 28 pre-existing/unrelated errors.
|
Both confirmed and fixed, and both caught with real fixtures rather than just tightening a number blind. P2 — split-digit self-protection (layout.rs:1272): Fixed by excluding `is_page_number(other)` from the protector check — a digit can no longer be "protected" by a neighboring digit that's itself page-number-shaped. Added `split_digit_page_number_footer.pdf` ("12" rendered as two separate single-digit `Tj`s): previously both digits leaked through, now correctly dropped as a whole. P2 — threshold too loose (layout.rs:1284): Confirmed — `1.5x font_size` is well past ordinary word-gap spacing (~0.28x for a space character in a typical proportional font), so it was retaining real page numbers next to unrelated text at normal spacing. Tightened to `0.25x`, calibrated just above `merge_subscript_items`'s own `0.2x` merge-gap tolerance — tight enough to reject a normal inter-word space, loose enough to still protect a subscript/superscript that just barely misses that merge for a structural (non-gap) reason. Added `word_adjacent_page_number_footer.pdf` ("12" at normal word-gap distance from "Company Name", different font size so it survives `merge_text_items` as its own item): previously retained, now correctly dropped. Side effect worth flagging directly: rebuilding the fixtures with accurate font metrics (a real `/Widths` array, instead of relying on pdf-inspector's width=0 fallback for an unmetriced font) exposed that the original "H2O" fixture only passed by coincidence — the inaccurate width math inflated its gap, but it still happened to land under the old loose threshold. With real widths, that specific case's true gap is near-zero and already merges via `merge_subscript_items` on its own (verified separately, no fix needed there). The new fixture instead targets this PR's actual scope: a numeric subscript/superscript that `merge_subscript_items` structurally declines to merge — a parent ending in a digit rather than a letter, or a strikeout/underline mismatch — regardless of gap tightness. One more honest tradeoff: this tightening reverts the "Form 4070" bonus recovery mentioned in the original PR description. That case's real gap (~0.94-1.35x font_size) is a stylistic size-emphasis gap in a title, not a failed-merge subscript's near-zero one — there's no single threshold that protects it without also re-admitting the false positives this review caught. Reverted the p1244-1996.md snapshot line; recovering that case would need a different signal than X-adjacency and is out of scope here. Full suite: 866 tests passing (4 new), clippy unchanged at the same 28 pre-existing/unrelated baseline errors. |
There was a problem hiding this comment.
All reported issues were addressed across 7 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
Addresses cubic-dev-ai's second review pass on firecrawl#226. - P3 (test assertion): the split-digit test's assertion checked the entire markdown for absence of '1' and '2' anywhere, coupling the result to the fixture's body text never containing those digits. Scoped to whole-word matches on split_whitespace(), matching the sibling tests. Also fixed the fixture itself: the two digits used the *same* font size, so merge_text_items fused them into one "12" item before is_page_number ever ran — the test wasn't exercising the self-protection bug it claimed to guard. Rebuilt with differing font sizes so the digits genuinely stay separate and reach the filter as two distinct page-number candidates. - P2 (the real one): correctly called out that reverting the p1244 snapshot's "Form 4070" recovery in the previous commit was masking the regression, not resolving it — a real near-margin title/form number losing its content is exactly the silent-drop class this PR exists to fix, just via a different mechanism than the original subscript bug. Added a second, independent signal for "this isn't an isolated footer number": line membership. If a page-number candidate shares its Y with at least two *other* real-text items (not one), that's a genuine multi-word sentence/title line, not a footer row (which pairs a page number with at most one decorative element — a tagline, a company name). This check doesn't depend on gap size at all, so it isn't vulnerable to the same "how tight is tight enough" tension the X-adjacency threshold has — a title number's gap to its neighbors can be wider than word-spacing (visual size emphasis widens it) without that being ambiguous with footer content, because real footers don't have two independent real-text neighbors. Verified against the real fixture that motivated this: p1244-1996's "Form 4070 Employee's Report" line has three items ("Form", "4070", "Employee's Report") sharing that Y — the new rule protects "4070". The nexo-price-en.pdf footer ("50" + one tagline, two items total) and the split-digit/word-adjacent fixtures from the first review pass (each two items total) still correctly get dropped — verified unaffected. Added title_embedded_number.pdf, a synthetic fixture mirroring the real p1244-1996.pdf shape, plus a dedicated regression test. Restored the p1244-1996.md snapshot to recover "4070" again, this time backed by the line-membership rule instead of an accidental threshold. Full suite (868 tests, 1 new) passes; cargo clippy --all-targets -- -D warnings unchanged at 28 pre-existing/unrelated errors.
|
Both fixed. P3 — over-broad assertion: Scoped to whole-word matches, matching the sibling tests. Also found the fixture itself wasn't testing what it claimed: the two digits shared a font size, so `merge_text_items` fused "1"+"2" into one "12" item before `is_page_number` ever saw two separate candidates — the self-protection bug wasn't actually being exercised. Rebuilt with differing font sizes so they genuinely stay separate through the filter. P2 — you're right, the snapshot revert masked the regression instead of fixing it. A form number embedded in a real title losing its content is exactly the class of bug this PR is about, just via a different code path than the original subscript case. Fixed properly this time with an independent signal that doesn't depend on gap-size tuning at all: line membership. If a page-number candidate shares its Y with at least two other real-text items, that's a genuine multi-word sentence/title line — not a footer row, which pairs a page number with at most one decorative element (a tagline, a company name). This sidesteps the whole "how tight is tight enough" tension entirely, since a title number's gap to its neighbors (widened by visual size emphasis) doesn't need to be confused with footer-adjacent spacing — footers just don't have two independent real-text neighbors to begin with. Verified against the exact case you flagged: p1244-1996's "Form 4070 Employee's Report" line has three items ("Form", "4070", "Employee's Report") sharing that Y — now protected. Re-checked everything from the first review pass stays correctly filtered (nexo's footer, the split-digit and word-adjacent fixtures — all two-items-total lines). Added `title_embedded_number.pdf`, a synthetic fixture mirroring the real shape, with a dedicated test. Restored the p1244-1996.md snapshot to recover "4070" again, this time backed by a real mechanism instead of an accidental threshold. Full suite: 868 tests passing (1 new), clippy unchanged at the same 28 pre-existing/unrelated baseline. |
There was a problem hiding this comment.
All reported issues were addressed across 5 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
…t from column footers Addresses cubic-dev-ai's third review pass on firecrawl#226. The previous commit added a "protect if the candidate's line has >=2 other real-text items, and they're geometrically contiguous" fallback, aiming to recover numbers embedded in title lines (p1244-1996.pdf's real "Form 4070 Employee's Report") without reopening the word-gap false positive from the first review pass. Built the regression fixture the review implicitly called for — a 3-column footer ("Acme Corporation | 12 | Annual Report 2026", a real, common layout with a left label, centered page number, and right label — exactly the shape the bot's comment named. Its column-stop gaps (~107-131pt) turned out to overlap the exact range a real title-line gap needs to be protected (p1244's "Form"-to-"4070" gap is ~9.4pt; "4070"-to-"Employee's Report" is ~107pt). No contiguity threshold — tight or generous — separates "wide gap because of visual size emphasis in a title" from "wide gap because of column alignment in a footer" when both produce gaps in the same range for a given layout. Confirmed by testing: a threshold generous enough to protect p1244's real gap also protects the synthetic 3-column footer's page number. Reverted to the version of this fix that has a clean, unambiguous signal: tight X-adjacency only (0.25x font_size, just above merge_subscript_items's own 0.2x merge-gap tolerance). This fully resolves the confirmed original bug (subscripts/superscripts near page margins silently dropped) without any of the false-positive risk the wider mechanism kept reintroducing. Recovering "Form 4070"-style title numbers would need a different signal entirely — column-layout awareness or sentence-level semantics, not adjacency — and is out of scope for this PR. Replaced title_embedded_number.pdf (no longer applicable) with three_column_footer.pdf as a permanent regression guard against reintroducing the reverted mechanism. Reverted the p1244-1996.md snapshot's "4070" line back to its pre-recovery state to match. Full suite (868 tests, net same count — one fixture/test swapped for another) passes; cargo clippy --all-targets -- -D warnings unchanged at 28 pre-existing/unrelated errors.
|
Valid catch, and it exposed something worth stating plainly rather than patching around again. Built the exact fixture your comment described — a 3-column footer ("Acme Corporation | 12 | Annual Report 2026", left label / centered page number / right label). Its column-stop gaps (~107-131pt) turned out to overlap the exact range the real p1244 title-line gap needs to be protected ("Form"→"4070" ≈9.4pt, "4070"→"Employee's Report" ≈107pt). I tried tightening the contiguity threshold, but any value generous enough to protect the real title case also protects this synthetic footer's page number — they're not separable by gap size or line-membership alone, no matter where the line is drawn. Rather than keep tuning a threshold that provably can't distinguish the two cases, I reverted the line-membership fallback entirely. The fix is now scoped to just tight X-adjacency (0.25x font_size) — the mechanism that's been solid across all three review passes and fully resolves the confirmed original bug (subscripts/superscripts near page margins silently dropped). Recovering "Form 4070"-style title numbers would need a genuinely different signal (column-layout awareness, sentence-level semantics), not adjacency — flagging that as out of scope here rather than shipping something that trades one false-positive class for another. Replaced `title_embedded_number.pdf` with `three_column_footer.pdf` as a permanent regression test, so this exact mechanism can't get silently reintroduced later. Reverted the p1244-1996.md snapshot's "4070" recovery to match. Full suite: 868 tests passing, clippy unchanged at the same 28 pre-existing/unrelated baseline. |
There was a problem hiding this comment.
All reported issues were addressed across 4 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
Addresses cubic-dev-ai's fourth review pass on firecrawl#226. Decorative glyphs flanking a footer page number ("• 12 •", "- 12 -") are real, non-empty TextItems sitting tightly adjacent to the number — they satisfied the old `!other.text.trim().is_empty()` check and protected the number from being filtered, leaking common decorated footer patterns like "• 12 •" into markdown. Changed the protector check from "non-empty" to "contains at least one alphanumeric character". A bullet or dash isn't content; only real text (letters/digits) should count as evidence the candidate is embedded in real content rather than a lone footer number. Verified with a new fixture (decorative_flanked_page_number.pdf: "12" flanked by "." decorations in a different font size, so they survive merge_text_items as separate items and genuinely reach is_page_number as tightly-adjacent-but-non-alphanumeric neighbors): before this change "12" leaked through, after it's correctly dropped. New regression test added. Full suite (870 tests, 1 new) passes; cargo clippy --all-targets -- -D warnings unchanged at 28 pre-existing/unrelated errors.
|
Confirmed and fixed. Decorative flanking glyphs ("• 12 •", "- 12 -") are real, non-empty TextItems, so they satisfied the old "non-empty" protector check even though they're not content — leaking that common decorated-footer pattern. Changed the check from "non-empty" to "contains at least one alphanumeric character." Added a fixture ("12" flanked by "." decorations in a different font size, so they survive Full suite: 870 tests passing (1 new), clippy unchanged at the same 28 pre-existing/unrelated baseline. |
1 participant
Summary
A report on this repo's recent activity said pdf-inspector "seems to drop superscripts and subscripts silently." Investigated and confirmed a real bug, isolated to a specific mechanism — not a vague/general problem with sub/superscript handling.
Root cause
`is_page_number` in `src/extractor/layout.rs` flags any 1-4-digit item positioned near the top/bottom margin (`y > 720 || y < 100` on a page) as a page number, with no check for whether the item is actually isolated on its own line.
A subscript or superscript digit that fails to merge into its parent word — `merge_text_items` requires font sizes within 20% of each other, which a subscript/superscript by definition isn't — stays a separate, short, numeric `TextItem`. When that item happens to sit near a page's top or bottom margin (titles, headers, and footnote references routinely do — a document title with a formula, a chemistry abstract, a footnote marker), it gets swept up by this filter and vanishes from markdown output entirely — not merged into its parent, not left as a bare digit, just gone.
Minimal repro
A one-page PDF: "H" (14pt) + "2" (9pt, subscript-positioned) + "O" (14pt), near the top of the page.
Fix
Only drop a page-number candidate when nothing else on the page is tightly X-adjacent to it at a similar Y (near-touching — gap less than 1.5× the larger of the two items' font sizes). Y-proximity alone isn't a safe signal: a genuine page number commonly shares its footer row with an unrelated element (a tagline, a running title) at ordinary word-gap distance, and that combination must still be filtered.
This precision requirement was caught empirically, not assumed — a first pass that checked only Y-proximity regressed the existing `nexo-price-en.pdf` snapshot test, which has exactly this shape: a page number ("50") sharing a footer row with a marketing tagline ("THE ALL-NEW NEXO /// ECO-FRIENDLY CAR") at ~17pt of separation. The tightened X-adjacency check (gap < 1.5× font_size) correctly keeps dropping that page number while still protecting the H2O case (near-zero gap).
Bonus: recovers real content in an existing fixture
`p1244-1996.pdf`'s title line was rendering as "Form Employee's Report" — the form number "4070" (a real IRS form number: Form 4070, Employee's Report of Tips to Employer, tightly adjacent to "Form" at the top margin) was being silently dropped by this exact bug. Updated that snapshot to include the recovered text — a content-completeness fix, not a behavior change requiring separate review.
Testing
Added two fixtures:
`cargo test`: 866 tests pass (2 new + existing suite, including both snapshot tests). `cargo clippy --all-targets -- -D warnings`: unchanged at 28 pre-existing/unrelated errors versus `main`.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Fixes false-positive page-number filtering near margins so inline subscripts/superscripts aren’t dropped. Keeps real footers filtered, including decorated and split-digit numbers; reverts the line‑membership fallback to avoid 3‑column footer leaks.
Written for commit 1fe666d. Summary will update on new commits.