Add attachment text extraction (agentextract/attachments)#6
Open
gulatikrishav wants to merge 12 commits into
Open
Add attachment text extraction (agentextract/attachments)#6gulatikrishav wants to merge 12 commits into
gulatikrishav wants to merge 12 commits into
Conversation
New subpath export agentextract/attachments (attachextract.ts): a type-keyed handler registry, never-trust-one-signal routing with BOM-aware sniffing, and charset-correct decoding (header/BOM/jschardet/utf-8/latin1) feeding the direct-text handler (plain/csv/tsv/markdown/calendar/headers/json/xml/yaml). Safety gates: 10MB size cap, handler timeout, and fail-open status mapping so attacker-controlled bytes never crash the caller. HTML/PDF/DOCX/EML handlers to follow.
html-to-text (script/style dropped, table text kept), unpdf per-page extraction with a scanned-PDF empty-detection heuristic, and mammoth extractRawText — each lazy-loaded on first use so a text-only Lambda never pays to load them, and so the ESM-only unpdf loads from this CJS module. Magic-byte sniffing (%PDF, PK+.docx) added to the router. Fail-open now reaches the 'failed' status on unparseable input. Tests use real .pdf/.docx fixtures minted with textutil/cupsfilter.
Documents are still text-extracted with no OCR, but now labeled when they likely hold unread image content, so a future OCR pass can target exactly the right attachments and pages: - extracted_empty now means truly zero text (fixes a false-positive that flagged a short-but-real PDF as empty); sparse-but-real text is kept and flagged lowTextDensity instead. - PDF reports pageCount + emptyPageCount (likely image pages), even on extracted_empty, so OCR knows how many pages to render. - DOCX flags lowTextDensity when it carries embedded media but little text. Adds real fixtures: sparse/blank PDFs and a docx with an embedded image.
message/rfc822 / .eml attachments are parsed with mailparser (its own html<->text synthesis disabled so our HTML handler owns that path): the email's subject + text body become extractedText, with the html body flattened through the shared flattenHtml helper when there's no text part. Each inner attachment recurses back through the pipeline at depth+1 as a first-class attachment (own size cap, routing, fail-open), collected under children. MAX_NESTING_DEPTH=1 bounds it: an email nested in an email is body-read but its own attachments are not descended into. Synthetic .eml fixtures only (plain, html-only, nested-attachment, eml-in-eml).
- PDF/DOCX density comments corrected (below-threshold + has-media -> flagged likely image-heavy; not 'text-heavy'); PDF lowTextDensity note now captures both triggers (any empty page OR low overall density). - PDF/EML join comments say 'blank line' (\n\n), not 'a newline'. - RoutedBy reframed as an audit trail (which signal decided the route). - Removed stale '(later step)'/'(added later)' incremental-build references. - Restored the file-header scope/rationale block; restored the timeout caveat (can't cancel sync CPU work, only stops awaiting). - Trimmed redundant comments; added one for errorMessage. Comments only — 203 tests green, no behavior change.
…al-traffic eval Eval'd routing against 6,714 production attachment records; four fixes: - Byte-verify text/html claims: a PDF or zip mislabeled text/plain used to latin1-decode into garbage reported as 'extracted' (a silent quality failure). Provably-binary bytes (no BOM, NUL/non-printable head) now void the claim; magic bytes rescue pdf/docx, anything else becomes a labeled skip. Zero-byte and BOM'd UTF-16 text attachments keep their handler (covered by tests). - Route html-ish text/* subtypes (text/x-amp-html: 32 real records) through the html handler instead of decoding markup as raw text. - Reclassify message/global as eml: it's the internationalized (SMTPUTF8) equivalent of message/rfc822 — a full email, not a headers blob. - Add message/delivery-status + message/feedback-report (DSN/ARF reports) to the text handler. Also parse the content type once in extractAt (was parsed three times). 8 new tests; 211 green. Co-authored-by: Cursor <cursoragent@cursor.com>
…ache
- Add a legacy OLE .doc handler (word-extractor) alongside docx; route by
application/msword, .doc extension, and OLE magic (extension-gated, since
.xls/.ppt/.msg share the signature). New tests + sample.doc fixture.
- Route RTF to a labeled skip instead of decoding control words as text
(text/rtf content-type path + {\rtf sniff magic). New test.
- Drop the version stamp (EXTRACTION_VERSION / extractionVersion): invalidation
is field-presence based, not version based.
- extracted_empty now returns extractedText '' (present-empty) instead of
undefined, mirroring the body extractor so a field-presence cache treats an
empty attachment as cached rather than re-extracting forever.
- XLSX handler (exceljs): flatten each sheet to "=== name ===" + tab-joined rows. exceljs over SheetJS since it has no known parse-time CVEs on untrusted bytes. cell.text extracts the shown value (formula result, formatted date), not the raw number/formula; empty cells/rows are dropped. Routes by content-type, .xlsx extension, and zip magic (extension-gated, like docx). New tests + fixtures. - Cap extracted text at MAX_OUTPUT_CHARS (1M): input is byte-capped but output isn't, and a big spreadsheet/HTML table can balloon into megabytes headed for S3 + the search index. Truncate surrogate-safely and set a `truncated` flag. New tests. - Tighten comments to plain what/why.
Routing: - Magic-verify a confident binary claim before parsing: a PDF/doc/docx/xlsx whose bytes don't match the claim is re-sniffed instead of failing in the wrong parser (recovers real text from mislabeled attachments). - Tell docx and xlsx apart from content, not just the extension: both are zips, but scan for the OOXML main part (word/document.xml vs xl/workbook.xml). Fixes a spreadsheet mislabeled/named .docx routing to mammoth and failing. Charset: - Floor now prefers valid utf-8, then jschardet's guess (even below the gate), then latin1 — undeclared single-byte legacy text no longer gets U+FFFD-destroyed. - If a decode still yields U+FFFD (a wrong charset, e.g. jschardet reading big5 as GB2312) and the bytes aren't valid utf-8, fall back to latin1 — never emit unrecoverable replacement chars. EML: - Cap nested children at MAX_EML_CHILDREN and parse them in bounded batches, so one email packed with attachments can't fan out unlimited parsers at once. Tests for each; corrupt-body fixtures updated to carry the magic/part markers that content-detection now requires.
Haakam21
requested changes
Jul 7, 2026
Haakam21
left a comment
Haakam21
left a comment
Contributor
There was a problem hiding this comment.
Attachment extraction should not live behind a separate agentextract/attachments subpath export. Please also expose it at the top level of the package, through an additional function on the main entry point, so consumers can reach it via import { extractAttachment } from 'agentextract' alongside the existing body-extraction API rather than a second import path.
Keeping the heavy parsers lazy-loaded (as this PR already does per-handler) means the top-level export costs nothing until extractAttachment is actually called, so the bundle-size motivation for the subpath doesn't require hiding the function on its own subpath. The subpath can stay if you like, but the primary, documented way to call this should be the top-level function.
Requesting changes for that API surface adjustment. The rest of the implementation looks solid — see my separate review notes on the charset-hint decoding edge cases (a wrong charset= param can currently override a BOM / mangle valid UTF-8), which are worth addressing but are independent of this API change.
Re-export extractAttachment (plus detectRoute and the public types) from
agentextract.ts so consumers can reach it via `import { extractAttachment }
from 'agentextract'` alongside extractEmailBody, not only through the
'agentextract/attachments' subpath. The subpath export stays for callers
that want it in isolation.
Each handler already lazy-loads its own heavy parser, so this re-export
costs nothing until extractAttachment is actually called. Document the
top-level import as the primary path in the README.
A wrong Content-Type charset= hint could override an in-band BOM or mangle valid UTF-8: the hint was trusted first. Reorder the fallback chain to BOM -> valid-UTF-8 -> hint -> statistical detection -> latin1 floor, so a definitive in-band signal wins over a stale/wrong declared charset (e.g. a UTF-16 file mislabeled charset=windows-1252 no longer decodes as garbage). Guard the UTF-8 shortcut on NUL bytes so BOM-less UTF-16 ASCII (valid utf-8 byte-wise, but really UTF-16) doesn't slip through it; routing already skips NUL-heavy text upstream, so this keeps resolveCharset correct on its own. Drop the now-redundant isUtf8 check at the floor. Add regression tests for the two reachable cases: BOM-beats-hint and UTF-8-beats-hint.
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
2 participants
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
Adds a standalone attachment text-extraction module, published as the
agentextract/attachmentssubpath export so its heavy parsers stay out of the body extractor's bundle. One entry point —extractAttachment(input) → ExtractionResult— routes an attachment to the right handler, extracts its text, and returns a labeled result. It never throws on bad/attacker-controlled input; failures come back asfailed/skipped_*statuses.Formats
html-to-text(drops script/style/img)unpdf, plus image-density signals (lowTextDensity/pageCount/emptyPageCount) for a future OCR passmammoth) and legacy .doc (word-extractor)exceljs) — each sheet flattened to tab-joined rowsmailparser) — nested email, depth-capped and child-count/concurrency boundedRouting & robustness
routedByword/document.xmlvsxl/workbook.xml), not the extension.docx, etc.) is re-sniffed instead of failing in the wrong parser, recovering real texttruncatedflag), 20 s handler timeout, 1-level eml nesting, 100 eml children'', notundefined)Testing
Dependencies
Adds
unpdf,mammoth,exceljs,word-extractor,iconv-lite,jschardet,html-to-text,mailparser(+@types). Heavy parsers are lazy-loaded per handler.