feat(connectors): hand source files to the document pipeline instead of extracting them - #6821
Conversation
…of extracting them A connector that extracted text itself stranded the document on a second, weaker parser. The shared pipeline routes PDFs to OCR — the only way a scanned page is readable at all — and owns every other format's parser, but its OCR branch is gated on `mimeType === 'application/pdf'` and connector documents were stored as `text/plain`, so a connector PDF could never reach it. The same file dragged into the UI was read by OCR; synced through a connector it got the local parser. `ExternalDocument` can now carry the source file itself, and SharePoint and OneDrive hand over anything the knowledge base can parse rather than extracting it. The sync engine stores those bytes under the file's own name and type, so the pipeline parses them exactly as it would an upload of the same file. Formats that are already text stay on the text path: HTML still reduces to plain text and the rest are UTF-8 decodes, so nothing already indexed changes representation. The MIME type is derived from the extension rather than the source's own declaration, so a provider that omits or mislabels it cannot strand a PDF on the non-OCR path. Re-syncing an existing document now rewrites `mimeType` too, which is what lets one stored as connector-extracted text stop declaring `text/plain`. This removes the duplicate extraction path rather than leaving both in place: `extractConnectorText` is text-only, and the guard against fabricated content moves to the pipeline where parsing now happens. That guard still matters — `DocParser` and `PptxParser` never throw, returning a placeholder sentence or scraped archive bytes on a legacy binary or an image-only deck — so a `degraded` result now fails the document with the same actionable message it produced before, naming the modern container for legacy formats. The in-flight byte budget already accounted for this: `estimateOpSizeBytes` reads the true source size from listing metadata, so batching reserved against the real file all along and merely over-reserved while only text was stored.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview
The sync engine stores either the source file (preserving extension in storage keys via Document processing centralizes unreadable/degraded parser output (including empty OCR) with Tests were updated for the new connector contract and new coverage for merge, stored extensions, and unreadable documents. Reviewed by Cursor Bugbot for commit d3dfab0. Configure here. |
Greptile SummaryThe PR routes binary connector documents through the shared document pipeline while preserving the existing text-file path.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains; both previously reported issues are fixed in the current code.
|
| Filename | Overview |
|---|---|
| apps/sim/connectors/utils.ts | Separates text decoding from source-file handoff and maps every forwarded binary format to an explicit MIME type. |
| apps/sim/connectors/onedrive/onedrive.ts | Hydrates supported binary documents as original source files while retaining the existing text extraction behavior. |
| apps/sim/connectors/sharepoint/sharepoint.ts | Hydrates supported binary documents as original source files while retaining size-limit and transport-failure handling. |
| apps/sim/lib/knowledge/connectors/sync-engine.ts | Merges hydrated source-file fields, stores the correct artifact representation, and rewrites MIME type during updates. |
| apps/sim/lib/knowledge/documents/parser-extension.ts | Resolves connector artifact extensions against the parser registry, completing the fix for parser-only Office and OpenDocument formats. |
| apps/sim/lib/knowledge/documents/document-processor.ts | Applies empty and degraded parser-output rejection in the shared processing path. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
Provider[SharePoint or OneDrive] --> Hydrate[Hydrate connector document]
Hydrate --> Kind{Pipeline-parsed format?}
Kind -->|No: text or HTML| Extract[Decode or reduce to plain text]
Extract --> StoreText[Store as text/plain with .txt key]
Kind -->|Yes: PDF or Office| Source[Attach original source bytes, filename, and MIME]
Source --> StoreFile[Store under source extension and MIME]
StoreText --> Pipeline[Shared document pipeline]
StoreFile --> Pipeline
Pipeline --> Route{PDF MIME?}
Route -->|Yes| OCR[OCR with parser fallback]
Route -->|No| Parser[Registered file parser]
OCR --> Guard[Reject empty or degraded output]
Parser --> Guard
Guard --> Embed[Chunk and embed]
Reviews (3): Last reviewed commit: "fix(knowledge): carry the MIME type thro..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
… file parsers Moving connector parsing into the pipeline exposed a gap on the OCR branch. OCR reads a scanned page with no recoverable text as empty, and the empty-content guard lived inside the file-parser path, so such a document chunked to nothing and reported success — the same silently-complete-but-useless outcome the guard exists to prevent. The check now sits above the parser choice and covers OCR too. Also preserves a source file's extension when its name is too long for a storage key. The extension is what picks the parser; a truncated name would still parse correctly by falling back to the display name, but only by luck.
Ten of the formats a connector now hands over — docm, dotx, xlsm, xlsb, xltx, pptm, potx, odt, ods and odp — parse fine but are deliberately not offered as upload types. `resolveStoredArtifactExtension` gated on the upload allowlist, so it rejected every one of them and processing failed with `Unsupported file type`. They worked before only because the connector extracted them itself and stored the result as text. The question the gate is asking is whether a parser can read the stored object, which the parser registry answers; the upload allowlist answers a different question about what we accept from a user. Also matches the sibling comment style in the object literal it sits in, and teaches two test mocks the newly imported symbol.
|
@cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
A listing stub is built before the file is fetched and declares `text/plain` for everything, so a hydrated PDF kept claiming plain text at the top level. Nothing broke today only because storage reads `sourceFile.mimeType` — which is exactly what makes it a trap: anything later reaching for `extDoc.mimeType`, the obvious field, silently loses the OCR routing this change exists to restore. The merge is now `mergeHydratedDocument` rather than an inline spread, so what hydration must carry is a stated contract with a test behind it instead of a literal that is easy to under-specify — which is how the field was missed.
|
@cursor review |
There was a problem hiding this comment.
✅ 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 d3dfab0. Configure here.
1 participant
Connector PDFs never reached OCR. This makes connectors hand their source files to the shared document pipeline instead of extracting text themselves, so a synced file is parsed exactly the way an upload of that same file is.
Why they bypassed OCR
parseDocumentgates OCR onmimeType === 'application/pdf'. Connector documents were stored astext/plain, so the branch could never fire.That mimeType was not wrong — it truthfully described what was stored. The problem was what we chose to store. The connector downloaded the PDF, extracted text with the local parser, and discarded the original before the pipeline that knows about OCR ever saw it.
The result was two parsing implementations, with connectors on the weaker one:
unpdfconnectors/utils.tsWhat changed
ExternalDocumentgains an optionalsourceFilecarrier (bytes + fileName + mimeType as one object, so they cannot disagree). SharePoint and OneDrive populate it for any format the knowledge base can parse; the sync engine stores those bytes under the file's own name and type.Formats that are already text stay on the existing path — HTML still reduces to plain text, everything else is a UTF-8 decode — so nothing already indexed changes representation. Only the formats that were being parsed twice move.
Two details that matter for correctness:
mimeType. The update path previously never set it, so a document first stored as connector-extracted text would have kept declaringtext/plainforever and never routed to OCR even after this change.This removes the duplicate path rather than leaving both:
extractConnectorTextis text-only now, andConnectorTextExtractionError/extractionFailedSkipReasonare deleted rather than left unused.The guard that had to move with it
DocParserandPptxParsernever throw. On a legacy OLE binary or a deck with no text they return a placeholder sentence or scraped archive bytes, flaggeddegraded. That guard lived in the connector; since parsing moved, it moves too —parseWithFileParsernow treatsdegradedexactly like empty output and fails the document with the same actionable message, naming the modern container for legacy formats (Re-save it as DOCX). Without this the change would have regressed into indexing placeholder text.Constraints checked rather than assumed
estimateOpSizeBytesalready reads the true source size from listing metadata, so batching has always reserved against the real file and merely over-reserved while only text was stored.chunkOpsByByteBudgetexplicitly lets a single oversized file form its own chunk, so the per-file cap exceeding the in-flight budget is not a blocker.contentHashis connector-provided and independent of representation, so nothing re-syncs spuriously.defaultProvider: 'local'when unconfigured, so those deployments get today's behavior; deployments that pay for OCR now actually get it for connector files.Trade-offs
Storage grows — raw files instead of extracted text. OCR is an external per-PDF call (1,000-page cap, batched) that does not appear to flow through the metered model-cost path, so the spend is worth confirming before this reaches large libraries. Existing connector documents need a re-sync to convert; they are not migrated by this change.
Testing
vitest run connectors/ lib/knowledge/ lib/uploads/ lib/file-parsers/ app/api/knowledge/— 1,988 passed (129 files)unreadable-document.test.tspins the relocateddegradedguard; verified it fails without the guard (2 of 4 red)application/pdfso OCR can route it, an Office file is delivered as its source file, and a text file is still extracted in-connectortsgo --noEmitclean apart from two pre-existing errors on staging (mssql,@sim/testing)bun run check:audits— 29/29