Skip to content

fix: expand typographic ligatures instead of dropping them (#172) - #185

Open
viwe-monai wants to merge 2 commits into
firecrawl:mainfrom
viwe-monai:fix/pdf-ligature-expansion
Open

viwe-monai wants to merge 2 commits into
firecrawl:mainfrom
viwe-monai:fix/pdf-ligature-expansion

Conversation

@viwe-monai

@viwe-monai viwe-monai commented Sep 30, 2026 •

Copy link
Copy Markdown

What

Expand Unicode typographic ligatures (U+FB00–U+FB06) to their constituent ASCII letters in clean_text(), instead of dropping them entirely.

Code point Glyph Expansion
U+FB00 ff ff
U+FB01 fi fi
U+FB02 fl fl
U+FB03 ffi ffi
U+FB04 ffl ffl
U+FB05/06 ſt/st st

Why

Fixes #172 — PDFs using typographic ligatures silently corrupt words in the output: classifies → classi es, affected → a ected.

How it was verified

  • Added ligatures_expanded test covering all six code points
  • Existing layout_invisibles_stripped and join_controls_preserved tests pass (no changes to their behavior)

Follow-ups deliberately left out

  • A fix in pdf_inspector's ToUnicode mapping would prevent the characters from being dropped at the parsing level. This anydoc-level fix is a safety net regardless of the upstream behavior.

Summary by cubic

Expands Unicode typographic ligatures (U+FB00–U+FB06) to their ASCII letters instead of dropping them, fixing silent word corruption in PDF extraction (classifies → classi es, affected → a ected).

  • Applies expansion in both clean_text() and the PDF extraction path (expand_ligatures() in src/formats/pdf.rs), since PDF output bypasses the shared normalizer.
  • Adds a regression test covering all six code points; layout_invisibles_stripped and join_controls_preserved behavior is unchanged.
  • A pdf_inspector-level ToUnicode fix is deliberately out of scope; this anydoc-side change is a safety net regardless of upstream behavior.

Written for commit 312b6c6. Summary will update on new commits.

Review in cubic

…#172)

Unicode ligatures (U+FB00-FB06: ff, fi, fl, ffi, ffl, st) were
dropped entirely during text extraction, silently corrupting words
(classifies -> 'classi es', affected -> 'a ected').

Expand them to their constituent ASCII letters in clean_text() and
add a regression test covering all six code points.

Fixes firecrawl#172

@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.

All reported issues were addressed across 1 file

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

Fix all with cubic | Re-trigger cubic

Comment thread src/shared/text.rs
Comment thread src/shared/text.rs
… review)

Address review feedback:
- P2: The PDF path (formats::pdf::to_markdown) bypasses clean_text().
  Add expand_ligatures() in the PDF module and apply it to the
  extracted markdown before returning.
- P3: Add U+FB05 and U+FB06 test coverage to ligatures_expanded.
@viwe-monai

Copy link
Copy Markdown
Author

Thanks for catching both issues! Fixed in 312b6c6:

P2 (PDF path bypasses clean_text): You are right — to_markdown_bytes routes Format::Pdf directly to formats::pdf::to_markdown, which returns pdf_inspector's markdown without any shared normalization. Added a dedicated expand_ligatures() in src/formats/pdf.rs and applied it to the extracted markdown before returning. The clean_text() expansion remains as defense-in-depth for the document-model path.

P3 (missing U+FB05/FB06 coverage): Added fa\u{fb05} → "fast" and ju\u{fb06} → "just" to ligatures_expanded.

This branch has not been deployed

No deployments
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.

Ligature characters (fi/fl/ffi) are dropped instead of expanded when extracting PDF text

1 participant