firecrawl / firecrawl/pdf-inspector

extract_pages_markdown returns an empty page when the page contains a single U+FFFD

Open
#202 0 comments 1 reaction 0 assignees View on GitHub
Dominant language
Rust
Stars
19.2k
Forks
1.3k
Avg merge
9h 21m
Merged PRs (30d)
51

Description

Hi! Thanks for pdf-inspector — the speed and the fact that it needs no ML models is
exactly what we were looking for, and `pages_needing_ocr` is a genuinely nice interface to
build on. We hit one behaviour while evaluating it that I don't think is intended, and it
looked worth writing up carefully rather than just working around.

## Summary

If a page's text contains **one** U+FFFD replacement character, `extract_pages_markdown`
returns `markdown: ""` for that page — the entire page, including all the text that
decoded perfectly well. The page is also reported as `needs_ocr: true` with
`ocr_reason: "suspected_garbled_text"`.

The interesting part is that this appears to contradict a policy the code already
implements deliberately: the page-level replacement-character analysis requires real
evidence before it routes a page to OCR (a density threshold, repeated spans, or a long
run), and there is a test named `test_text_quality_allows_isolated_replacement_character`.
An isolated U+FFFD passes that check correctly, and the markdown is built in full — it is
then discarded by a later, separate check.

## Why this matters for a consumer

A single unmappable glyph is common in office documents: a `≤`, a diameter sign or a
ballot box is frequently drawn from a subset symbol font with no ToUnicode CMap, while the
rest of the page is ordinary text. On our evaluation corpus (2,117 pages of Word-produced
business documents, mostly German) this affected **120 pages across 69 documents — 5.7% of
all pages**. Each of those pages had 900–5,000 characters of correctly-decoded text and
returned an empty string.

It is also silent: no error, no partial content, and `markdown == ""` is hard to
distinguish from a legitimately blank page without a second parser to compare against.

## Reproduction

`repro.pdf` is 1,980 bytes, one page, generated by the script below (stdlib only) — no third-party
content. It is ~700 characters of plain Helvetica prose plus **one** character drawn from
a composite (Identity-H) font with no ToUnicode CMap.

```bash
pip install pdf-inspector
python make_repro.py repro.pdf
```

make_repro.py — stdlib only, no dependencies

```python
"""Build a 2 KB, single-page PDF: ordinary text plus ONE character that cannot be
mapped to Unicode. No dependencies, no third-party content.

The page is deliberately healthy: ~700 characters of plain prose in Helvetica.
One character in the middle is drawn from a second, composite (Identity-H) font
with no ToUnicode CMap -- a subset symbol font, as a word processor emits when
the body font lacks a glyph (<=, a diameter sign, a ballot box). That one CID
cannot be mapped, so extraction yields a single U+FFFD.
"""
from __future__ import annotations

import sys

LINES = [
"Section 4 - Storage and handling",
"",
"Samples are kept in the cold room and logged on arrival. The retention",
"period is twelve months from the date of receipt unless the method",
"states otherwise. Containers must be labelled with the sample number,",
"the date, and the name of the person who took the sample.",
"",
"Stock solutions are stable for twelve months when stored at",
"@ -18 C. Check the seal before use and discard any container",
"whose label is no longer legible. Record the check in the logbook.",
"",
"Filters are supplied in two sizes. Use the larger one for turbid",
"samples and the smaller one for everything else. Rinse the funnel",
"with deionised water between samples to avoid carry-over.",
]
SYMBOL_LINE = 8 # the line whose leading '@' is replaced by the unmappable glyph

def content_stream() -> bytes:
out = ["BT", "/F1 11 Tf", "14 TL", "56 760 Td"]
for i, line in enumerate(LINES):
if i == SYMBOL_LINE:
out += ["/F2 11 Tf", "<00A3> Tj",
"/F1 11 Tf", f"({line[1:]}) Tj", "T*"]
else:
out += [f"({line}) Tj", "T*"]
out.append("ET")
return "\n".join(out).encode("latin-1")

def build() -> bytes:
stream = content_stream()
objects = [
b"<< /Type /Catalog /Pages 2 0 R >>",
b"<< /Type /Pages /Kids [3 0 R] /Count 1 >>",
b"<< /Type /Page /Parent 2 0 R /MediaBox [0 0 595 842] "
b"/Resources << /Font << /F1 5 0 R /F2 6 0 R >> >> /Contents 4 0 R >>",
b"<< /Length " + str(len(stream)).encode() + b" >>\nstream\n"
+ stream + b"\nendstream",
b"<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica "
b"/Encoding /WinAnsiEncoding >>",
b"<< /Type /Font /Subtype /Type0 /BaseFont /AAAAAA+SymbolMT "
b"/Encoding /Identity-H /DescendantFonts [7 0 R] >>",
b"<< /Type /Font /Subtype /CIDFontType2 /BaseFont /AAAAAA+SymbolMT "
b"/CIDSystemInfo << /Registry (Adobe) /Ordering (Identity) /Supplement 0 >> "
b"/FontDescriptor 8 0 R /DW 1000 >>",
b"<< /Type /FontDescriptor /FontName /AAAAAA+SymbolMT /Flags 4 "
b"/FontBBox [0 -220 1113 1005] /ItalicAngle 0 /Ascent 1005 /Descent -220 "
b"/CapHeight 662 /StemV 86 >>",
]
pdf = bytearray(b"%PDF-1.4\n")
offsets = []
for i, body in enumerate(objects, start=1):
offsets.append(len(pdf))
pdf += f"{i} 0 obj\n".encode() + body + b"\nendobj\n"
xref = len(pdf)
pdf += f"xref\n0 {len(objects) + 1}\n".encode()
pdf += b"0000000000 65535 f \n"
for off in offsets:
pdf += f"{off:010d} 00000 n \n".encode()
pdf += (f"trailer\n<< /Size {len(objects) + 1} /Root 1 0 R >>\n"
f"startxref\n{xref}\n%%EOF\n").encode()
return bytes(pdf)

if __name__ == "__main__":
path = sys.argv[1] if len(sys.argv) > 1 else "repro.pdf"
with open(path, "wb") as fh:
fh.write(build())
print(f"wrote {path}")
```

```python
import pdf_inspector as p

pg = p.extract_pages_markdown("repro.pdf").pages[0]
print(pg.needs_ocr, repr(pg.ocr_reason), len(pg.markdown or ""))
# True 'suspected_garbled_text' 0 <-- expected: False, None, ~682

print(list(p.classify_pdf("repro.pdf").pages_needing_ocr))
# [] <-- disagrees with the call above

r = p.process_pdf("repro.pdf")
print(len(r.markdown or ""), list(r.pages_needing_ocr))
# 682 [] <-- keeps the page, and its content is correct

items = p.extract_text_with_positions("repro.pdf")
text = "".join(i.text for i in items)
print(len(text), text.count("�"))
# 665 1 <-- the page decodes fine; exactly one U+FFFD
```

### Observed vs expected

| Call | Observed | Expected |
| --- | --- | --- |
| `extract_pages_markdown` | `markdown=""`, `needs_ocr=True`, `ocr_reason="suspected_garbled_text"` | the page's markdown, `needs_ocr=False` |
| `classify_pdf().pages_needing_ocr` | `[]` | `[]` (agrees with expectation) |
| `process_pdf().markdown` | full text (682 chars), including the U+FFFD | same |
| `extract_text_with_positions` | full text, one U+FFFD | same |

So three entry points currently give three different answers for the same page. If the
intended contract is "one bad glyph does not invalidate a page" — which the density
thresholds and that unit test suggest — then `extract_pages_markdown` is the odd one out.

## A note on the underlying glyph

Separately, and much more minor: the CID here genuinely cannot be mapped, so U+FFFD is a
fair thing to emit. We are not asking for it to be resolved — only that its presence not
cost the rest of the page. (Deterministically distinguishing these cases is what
[#122](https://github.com/firecrawl/pdf-inspector/issues/122) asks for, which would be a
nicer long-term answer than any character statistic.)

This also looks like the same *shape* as
[#200](https://github.com/firecrawl/pdf-inspector/issues/200) — content that survives
`extractTextWithPositions` and disappears during markdown conversion with no signal to the
caller — though the mechanism is different.

Where we think it happens (from reading v0.2.6 / main @ a15ec2d)

Feel free to ignore this if it is off-base — this is from reading, not from running your
test suite.

The page-level policy that correctly tolerates an isolated replacement character:

```rust
// src/text_quality.rs:353 — page_replacement_evidence_needs_ocr
let enough_bad_text = evidence.replacement_chars >= 12 && replacement_density_bps >= 500;
let repeated_bad_spans = evidence.replacement_spans >= 3 && replacement_density_bps >= 250;
let long_bad_run = evidence.longest_replacement_run >= 8 && replacement_density_bps >= 250;
enough_bad_text || repeated_bad_spans || long_bad_run
```

With one replacement character none of these fire, `has_text_quality_issue` is false, and
the markdown is built normally. Then:

```rust
// src/text_quality.rs:40 — detect_encoding_issues
if markdown.contains('\u{FFFD}') {
return true; // any single one, anywhere on the page
}
```

```rust
// src/lib.rs:543
let has_decoding_issue = has_text_quality_issue
|| (!md.is_empty() && (is_cid_garbage(&md) || detect_encoding_issues(&md)));
if has_decoding_issue { add_ocr_reason(..., OCR_REASON_SUSPECTED_GARBLED_TEXT); }
...
let needs_ocr = ocr_reason.is_some() || md.trim().is_empty() || has_gid || is_garbage_text(&md);
...
markdown: if needs_ocr { String::new() } else { md }, // src/lib.rs:561
```

So the calibrated decision is made first and then overridden by an uncalibrated
`contains()`. That would also explain why the unit tests pass: they exercise
`analyze_text_quality` directly, where the policy holds, rather than the composed
`extract_pages_markdown` path.

Two possible directions, whichever fits your design better:

1. Have the U+FFFD branch of `detect_encoding_issues` use the same evidence
`page_replacement_evidence_needs_ocr` already computes, instead of `contains()`. The
dollar-as-space and cipher heuristics would be unaffected.
2. Keep the flag but stop it emptying the page — return the markdown alongside
`needs_ocr: true` and let the caller decide. That also removes the disagreement with
`process_pdf`, which already keeps the content in this situation.

Happy to test a patch against our corpus and report back — we have the 120-page case
handy and can give you before/after counts quickly. Thanks again for the library, and for
reading this far!

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with src/text_quality.rs, including detect_encoding_issues and page_replacement_evidence_needs_ocr, then trace their use in src/lib.rs around lines 543-561. Reproduce the isolated-U+FFFD case with the supplied script and compare extract_pages_markdown with classify_pdf, process_pdf, and extract_text_with_positions. Done means a single replacement character no longer discards otherwise valid markdown, with behavior aligned to the existing text-quality test and policy.

Written by the indexing model from the issue text.

Assessment

Tech stack
python, rust
Domain
backend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
78/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.