Skip to content

feat(extractor): expose from_font_without_tounicode on TextItem - #195

Open
mangeshraut712 wants to merge 1 commit into
firecrawl:mainfrom
mangeshraut712:cursor/feat-tounicode-textitem-flag-7813
Open

feat(extractor): expose from_font_without_tounicode on TextItem#195
mangeshraut712 wants to merge 1 commit into
firecrawl:mainfrom
mangeshraut712:cursor/feat-tounicode-textitem-flag-7813

Conversation

@mangeshraut712

@mangeshraut712 mangeshraut712 commented Aug 1, 2026

Copy link
Copy Markdown

Summary

Fixes #122.

classifyPdf / encoding heuristics can already hint at garbled text, but consumers of extractTextWithPositions had no deterministic per-run signal that a glyph stream came from a font lacking /ToUnicode. Identity-H / Type3 pages without a map still report as text-based with high confidence while decoding garbage.

Changes

  • Add TextItem.from_font_without_tounicode
    • Set for Type0 Identity-H/V and Type3 fonts with no /ToUnicode entry
    • Not set for standard Type1/TrueType (Helvetica etc. are fine without ToUnicode)
    • Broken-but-present ToUnicode stays false (use existing encoding-issue heuristics for validity)
  • Propagate through merge / subscript merge (OR across merged runs)
  • Add tounicode_missing_width_fraction(items) for page-level coverage (issue option 3)
  • Wire napi + Python (pdf_inspector.pyi)

Test plan

  • cargo fmt
  • cargo clippy --lib -- -D warnings
  • cargo test --lib → 716 passed
  • cargo test --test integration_tests → 145 passed
  • shinagawa_identity_h.pdf → flag true + width fraction > 0.5
  • Synthetic Type0 with broken ToUnicode present → flag false
  • Minimal Helvetica Type1 → flag false

Notes

Does not compete with #193 (Form underline), #184 (wasm strategy), or #182 (bindings refactor). Field addition is additive for napi/Python.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Expose a deterministic flag on each text run to indicate when it was rendered with a font that lacks a required /ToUnicode map (Type0 Identity-H/V or Type3), plus a helper to summarize page-level coverage. This enables reliable OCR routing and avoids guessing from garbled text.

  • New Features
    • Added TextItem.from_font_without_tounicode; set for Type0 Identity-H/V and Type3 fonts missing /ToUnicode, not set for standard Type1/TrueType; remains false if a (broken) map exists.
    • Preserved across run merges and sub/superscript merges (OR across merged runs).
    • Added tounicode_missing_width_fraction(items) to measure width affected on a page.
    • Exposed in napi and Python (pdf_inspector.pyi) bindings; tests added for Identity-H, broken-map, and Type1 cases.

Written for commit ed99f5f. Summary will update on new commits.

Review in cubic

Type0 Identity-H/V and Type3 runs that lack /ToUnicode now set a
per-item flag so consumers can route OCR without guessing from
character statistics. Also adds tounicode_missing_width_fraction for
page-level coverage. Wired through napi and Python bindings.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

3 issues found across 27 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/tables/detect_heuristic.rs">

<violation number="1" location="src/tables/detect_heuristic.rs:109">
P3: Merged table-detection runs always discard the missing-ToUnicode signal, including when `first_item` or any absorbed item is flagged. Preserve it with an OR across the merged source items, matching the extractor merge behavior.</violation>
</file>

<file name="src/extractor/mod.rs">

<violation number="1" location="src/extractor/mod.rs:84">
P2: RTL merged runs can disappear from this coverage metric because their existing widths are negative and this clamp converts them to zero. Use absolute width here so missing-ToUnicode RTL content contributes to both numerator and denominator.</violation>

<violation number="2" location="src/extractor/mod.rs:86">
P2: Mixed-font merged runs overstate missing-map coverage: OR propagation marks the whole merged bounding width as missing. Preserve missing-source width before merging, or compute this metric from pre-merge runs, so a short flagged fragment does not make an entire line report as missing.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic

Comment thread src/extractor/mod.rs
if !matches!(item.item_type, ItemType::Text) || item.text.trim().is_empty() {
continue;
}
let w = item.width.max(0.0);

@cubic-dev-ai cubic-dev-ai Bot Aug 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: RTL merged runs can disappear from this coverage metric because their existing widths are negative and this clamp converts them to zero. Use absolute width here so missing-ToUnicode RTL content contributes to both numerator and denominator.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/extractor/mod.rs, line 84:

<comment>RTL merged runs can disappear from this coverage metric because their existing widths are negative and this clamp converts them to zero. Use absolute width here so missing-ToUnicode RTL content contributes to both numerator and denominator.</comment>

<file context>
@@ -68,6 +68,32 @@ fn extract_text_from_doc(doc: &Document) -> Result<String, PdfError> {
+        if !matches!(item.item_type, ItemType::Text) || item.text.trim().is_empty() {
+            continue;
+        }
+        let w = item.width.max(0.0);
+        total += w;
+        if item.from_font_without_tounicode {
</file context>
Suggested change
let w = item.width.max(0.0);
let w = item.width.abs();
Fix with cubic

Comment thread src/extractor/mod.rs
}
let w = item.width.max(0.0);
total += w;
if item.from_font_without_tounicode {

@cubic-dev-ai cubic-dev-ai Bot Aug 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: Mixed-font merged runs overstate missing-map coverage: OR propagation marks the whole merged bounding width as missing. Preserve missing-source width before merging, or compute this metric from pre-merge runs, so a short flagged fragment does not make an entire line report as missing.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/extractor/mod.rs, line 86:

<comment>Mixed-font merged runs overstate missing-map coverage: OR propagation marks the whole merged bounding width as missing. Preserve missing-source width before merging, or compute this metric from pre-merge runs, so a short flagged fragment does not make an entire line report as missing.</comment>

<file context>
@@ -68,6 +68,32 @@ fn extract_text_from_doc(doc: &Document) -> Result<String, PdfError> {
+        }
+        let w = item.width.max(0.0);
+        total += w;
+        if item.from_font_without_tounicode {
+            missing += w;
+        }
</file context>
Fix with cubic

is_italic: first_item.is_italic,
is_underline: first_item.is_underline,
is_strikeout: first_item.is_strikeout,
from_font_without_tounicode: false,

@cubic-dev-ai cubic-dev-ai Bot Aug 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P3: Merged table-detection runs always discard the missing-ToUnicode signal, including when first_item or any absorbed item is flagged. Preserve it with an OR across the merged source items, matching the extractor merge behavior.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/tables/detect_heuristic.rs, line 109:

<comment>Merged table-detection runs always discard the missing-ToUnicode signal, including when `first_item` or any absorbed item is flagged. Preserve it with an OR across the merged source items, matching the extractor merge behavior.</comment>

<file context>
@@ -106,6 +106,7 @@ pub(crate) fn merge_adjacent_items(items: &[TextItem]) -> (Vec<TextItem>, Vec<Ve
                 is_italic: first_item.is_italic,
                 is_underline: first_item.is_underline,
                 is_strikeout: first_item.is_strikeout,
+                from_font_without_tounicode: false,
                 item_type: first_item.item_type.clone(),
                 mcid: first_item.mcid,
</file context>
Fix with cubic

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.

Expose per-font ToUnicode presence/validity so consumers can detect garbled text layers

1 participant