fix: expand typographic ligatures instead of dropping them (#172) - #185
Open
viwe-monai wants to merge 2 commits into
Open
viwe-monai wants to merge 2 commits into
viwe-monai wants to merge 2 commits into
Conversation
…#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
There was a problem hiding this comment.
All reported issues were addressed across 1 file
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
… 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.
Author
|
Thanks for catching both issues! Fixed in 312b6c6: P2 (PDF path bypasses P3 (missing U+FB05/FB06 coverage): Added |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Expand Unicode typographic ligatures (U+FB00–U+FB06) to their constituent ASCII letters in
clean_text(), instead of dropping them entirely.Why
Fixes #172 — PDFs using typographic ligatures silently corrupt words in the output:
classifies→classi es,affected→a ected.How it was verified
ligatures_expandedtest covering all six code pointslayout_invisibles_strippedandjoin_controls_preservedtests pass (no changes to their behavior)Follow-ups deliberately left out
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).clean_text()and the PDF extraction path (expand_ligatures()insrc/formats/pdf.rs), since PDF output bypasses the shared normalizer.layout_invisibles_strippedandjoin_controls_preservedbehavior is unchanged.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.