feat(fonts): flag provably undecodable text layers with a structured warning - #892
feat(fonts): flag provably undecodable text layers with a structured warning#892tobocop2 wants to merge 1 commit into
Conversation
be6f2e9 to
2d1da3d
Compare
…warning A common subsetting pattern (Type0/Identity-H, Identity ordering, CIDToGIDMap /Identity, no ToUnicode, embedded CIDFontType2 stripped of both cmap and post) leaves a file with no glyph-to-Unicode mapping at all. Extraction can only echo the raw glyph indices, and callers could not tell that mojibake from a clean extraction. FontInfo::has_undecodable_text_layer() proves that severed shape, reading the embedded program's own table directory so a cmap the decoder never inspected still counts, and the text extractor surfaces it as a page-scoped undecodable_text_layer structured warning, once per page/font, without changing text output. Recoverable fonts (live embedded cmap, usable post names, usable ToUnicode, non-Identity ordering, explicit CIDToGIDMap stream, unparseable program) are never flagged. Every page-parameterized extraction API (extract_text and the conversion APIs, extract_chars, extract_spans_with_config, the word APIs) attributes the warning to the page it was asked about, via a diagnostic page context kept separate from the MCID scope stack so public span data is untouched; auxiliary extractors with no named page stay silent rather than misreport a default. The per-page check is memoized by font identity so same-named subset instances are judged independently, and WarningSink::push_once keeps the warning idempotent across repeated passes and across APIs. Verified with a 577-document differential sweep (text and HTML output byte-identical to main) and a corpus precision probe (7 documents flagged, all with provably severed subsets, zero false positives). Fixes yfedoseev#876. Signed-off-by: Tobias Perelstein <5562156+tobocop2@users.noreply.github.com>
2d1da3d to
f370186
Compare
|
@tobocop2 first: thank you for this — the diagnosis in #876 and the engineering here are genuinely excellent (the non-destructive design, the byte-identical differential sweep, the careful "only the fully-severed case" predicate). And I owe you an honest note: my earlier comment on #876 said "a PR would be very welcome" and pre-endorsed the diagnostic direction. That was a premature maintainer commitment on my part, made before we'd settled the scope — the bind that creates here is on us, not you. After sitting with it, we've decided to solve the underlying problem a different way, and I want to lay out the difference plainly rather than just close this. The fork: a judgment vs. a fact. This PR ships a judgment — Why the fact rather than the judgment:
Your use case is fully served, and arguably better: instead of a boolean flag, your pipeline routes on So I'm going to decline this PR in favor of #893, with real credit to you: #893 exists because your #876 nailed the condition and the trap (the offset-recovery dead end). I've credited the diagnosis in the CHANGELOG/PR. Thank you for pushing on this — the analysis is what made the right design obvious, and the premature-invite misstep was mine to own. |
|
sounds good to me. thanks! |
What this fixes
Implements the diagnostic agreed in #876 (fixes #876). A common subsetting pattern (Type0/Identity-H, Identity ordering,
CIDToGIDMap /Identity, no ToUnicode, embedded font stripped ofcmapandpost) leaves a page with no glyph-to-Unicode mapping at all. Extraction can only echo raw glyph indices, and callers had no way to tell that mojibake from a clean extraction.The diagnostic
FontInfo::has_undecodable_text_layer()detects exactly that severed shape. When such a font is used on a page, extraction emits a newundecodable_text_layerstructured warning — page-scoped, once per page/font, with the font name and the §9.10.2 reference.Text output is unchanged. The raw CID sequence stays available to callers who want it; the warning only adds the provable fact that no Unicode mapping exists in the file.
Staying silent on recoverable fonts
The predicate is structural and conservative. It proves
cmapabsence from the embedded program's own table directory rather than trusting the decoder's cached view, so a program the lazy accessor never inspected (e.g. a CIDFontType2 delivered as an OpenType FontFile3) is judged by what its tables actually say. Fonts with a live embeddedcmap(the byte-as-GID cases of #99/#760), usablepostglyph names, a usable ToUnicode, a non-Identity ordering, an explicitCIDToGIDMapstream, or an unparseable program are never flagged — when severedness can't be proven, it says nothing.Emission and page attribution
Every page-parameterized extraction API surfaces the warning —
extract_textand the conversion APIs,extract_chars, andextract_spans_with_config— each attributing it to the page it was asked about, via a diagnostic page context kept deliberately separate from the MCID scope stack (whose values are public span data and stay untouched). Auxiliary extractors with no named page (annotation appearance streams, font enumeration) stay silent rather than misreport a default page.WarningSink::push_oncekeeps the same page/font fact to a single entry across repeated passes and across APIs, and the per-page check is memoized by font identity, so the predicate runs at most once per font per page and adds no measurable work to theTfhot path.Bindings
No binding changes needed:
structured_warnings()serializes the category via its stable string ("undecodable_text_layer"), so Python and the other consumers of that surface pick the new category up automatically. The new Rust-side helpers (push_structured_warning_once,WarningSink::push_once) match the existing Rust-only push surface.Test plan
extract_chars,extract_spans_with_config, andextract_textall report the true page and collapse to one warning. Fixtures are three fully synthetic blank-glyph fonts (604 B–1.2 KB) built with fontTools; the PDFs are built in-test.undecodable.pdffrom [Feature]: Tell callers when a page's text cannot be read, instead of returning garbled text that looks correct #876 extracts byte-identically to released 0.3.74 (81,7('67$7(6',675,&7&2857) and raises the warning on page 0.pdffontsconfirms no Unicode map) and one arXiv paper whose embedded ArialMT subsets all lackcmapandpost(verified table-by-table with fontTools;pdftotextemits the same mojibake there). Zero false positives, and page attribution is exact.make checkgreen (fmt, clippy-D warningsincl.--features python, full test suite); clippy also clean on default features.