Skip to content

fix(file-parsers): stop deleting non-BMP characters when sanitizing parsed text - #6442

Merged
waleedlatif1 merged 2 commits into
stagingfrom
pdf-text-extraction-bounds
Aug 8, 2026
Merged

fix(file-parsers): stop deleting non-BMP characters when sanitizing parsed text#6442
waleedlatif1 merged 2 commits into
stagingfrom
pdf-text-extraction-bounds

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Summary

  • sanitizeTextForUTF8 stripped unpaired surrogates with a bare [\uD800-\uDFFF] class. That matches UTF-16 code units, so it removed both halves of every valid surrogate pair — silently deleting all emoji, CJK Extension B, and mathematical alphanumerics from every parser's output. Now matches only genuinely unpaired surrogates.
  • Surface PDF truncation inline. Callers read only content, so a bounded PDF was indistinguishable from a complete one. It now carries the same [... ... ...] marker csv and xlsx already used, moved into a shared truncationNotice() helper.
  • Removed two unused exports (sanitizeTextArray, hasInvalidUTF8Characters) — the latter carried the same surrogate bug.

Before / after:

"Hello 😀 emoji"    →  "Hello  emoji"      →  "Hello 😀 emoji"
"CJK ExtB: 𠮷野家"   →  "CJK ExtB: 野家"     →  "CJK ExtB: 𠮷野家"

Type of Change

  • Bug fix

Testing

New utils.test.ts covers pair preservation, unpaired-surrogate removal, an unpaired surrogate adjacent to a valid pair, and a UTF-8 round-trip; verified all four fail against the previous implementation. Confirmed the csv/xlsx notices are byte-identical to the strings they produced before, and that PDF output is unchanged for documents within budget (including a 2000-page fixture). 85 parser + 599 consumer tests pass.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

…arsed text

The unpaired-surrogate strip used a bare [\uD800-\uDFFF] class, which matches
UTF-16 code units and so removed both halves of every valid surrogate pair —
deleting all emoji, CJK Extension B, and mathematical alphanumerics from parsed
output. Match only genuinely unpaired surrogates instead.

Also surface PDF truncation inline. Callers read only `content`, so a bounded
PDF was indistinguishable from a complete one; it now carries the same
`[... ... ...]` marker csv and xlsx already use, via a shared helper.

Drop two unused exports, one of which carried the same surrogate bug.
@vercel

vercel Bot commented Aug 8, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Aug 8, 2026 8:54pm

Request Review

@cursor

cursor Bot commented Aug 8, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
The sanitization fix changes text stored from every file parser (data correctness), while PDF truncation behavior is user-visible but bounded by existing limits and tests.

Overview
Fixes sanitizeTextForUTF8 so it only strips unpaired surrogates instead of every UTF-16 code unit in the surrogate range, which had been silently removing emoji, CJK Extension B, and similar characters from all parser output. Adds truncationNotice() for a shared [... detail ...] marker and removes unused sanitizeTextArray / hasInvalidUTF8Characters.

CSV and XLSX now use that helper for row/size limits (same marker text as before). PDF appends an inline truncation notice when extraction hits parser limits and there is real text; pagesRead only counts pages that actually contributed content so empty pages do not inflate the notice. Text-free multi-page PDFs stay empty with no marker.

Reviewed by Cursor Bugbot for commit b66c26e. Configure here.

@greptile-apps

greptile-apps Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR corrects Unicode sanitization so valid surrogate pairs are preserved and standardizes inline truncation notices across file parsers.

  • Preserves emoji and other non-BMP characters while removing genuinely unpaired surrogates.
  • Adds an inline PDF truncation marker for content-only consumers.
  • Reuses a shared truncation-notice formatter for CSV and XLSX output.
  • Adds Unicode sanitization and bounded PDF extraction tests.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
apps/sim/lib/file-parsers/utils.ts Replaces blanket surrogate removal with unpaired-surrogate matching, removes unused exports, and adds the shared truncation marker formatter.
apps/sim/lib/file-parsers/pdf-parser.ts Tracks extracted-page coverage and exposes parser-limit truncation in returned content while retaining metadata.
apps/sim/lib/file-parsers/pdf-parser.test.ts Extends bounded-extraction coverage for inline notices, normal PDFs, and text-free multi-page PDFs.
apps/sim/lib/file-parsers/utils.test.ts Covers valid surrogate pairs, malformed surrogates, UTF-8 round trips, control characters, and truncation formatting.
apps/sim/lib/file-parsers/csv-parser.ts Replaces the local row-limit marker construction with the byte-equivalent shared helper.
apps/sim/lib/file-parsers/xlsx-parser.ts Replaces duplicated row and size truncation marker construction with the shared helper.

Reviews (2): Last reviewed commit: "fix(file-parsers): keep text-free PDFs e..." | Re-trigger Greptile

Comment thread apps/sim/lib/file-parsers/pdf-parser.ts Outdated
Comment thread apps/sim/lib/file-parsers/pdf-parser.ts
…t were read

Gate the truncation notice on the sanitized, trimmed body: a text-free
multi-page PDF collapses to a lone separator, so the previous length check let
a notice turn a document callers treat as empty into one that looks like it
holds content.

Also stop counting a page the budget cut off before it yielded anything, so the
notice no longer reports one more page than was shown.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit b66c26e. Configure here.

@waleedlatif1
waleedlatif1 merged commit 9395f0e into staging Aug 8, 2026
30 checks passed
@waleedlatif1
waleedlatif1 deleted the pdf-text-extraction-bounds branch August 8, 2026 21:00
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.

1 participant