Skip to content

feat(knowledge): read a PDF's text layer before paying for OCR - #6850

Merged
waleedlatif1 merged 6 commits into
stagingfrom
feat/pdf-ocr-triage
Aug 19, 2026
Merged

feat(knowledge): read a PDF's text layer before paying for OCR#6850
waleedlatif1 merged 6 commits into
stagingfrom
feat/pdf-ocr-triage

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Every PDF is sent to OCR — an external, per-document call — even though most carry an embedded text layer that costs nothing to read. This reads the text layer first and reserves OCR for the documents that actually need it.

The evidence

Across a real corpus of 2,693 PDFs that exist both ways (locally extracted and OCR'd):

Local extraction produced nothing 0
Local within 70% of OCR's character volume 2,515 (93%)
Local materially under-extracted 178 (7%)

Separately, of ~1,480 PDFs in that library only ~9% genuinely lacked a text layer. So the large majority of OCR calls were buying nothing.

One caveat stated plainly: local extraction returned more characters than OCR on average (116K vs 14K), which is not local winning — pdfjs emits positioning noise where OCR returns clean markdown. Character volume demonstrates coverage, not fidelity. That is why the threshold is tunable and the routing is logged.

How a text layer is judged

Three independent failure modes, none of which catches the others. Taken from prior art rather than invented — pdfmux documents a text-density check, and docling#2963 describes the corrupted-layer cases:

  1. No text — a scan.
  2. Sparse text — present but too thin to be the document, e.g. a header or stamp on a scanned page. Measured per page, since a total-character floor cannot tell a short memo from a long scan.
  3. Text that is not language — a broken encoding, or the raw character ids a CID-keyed font emits when it has no ToUnicode map (/31 /8 /18 /12). This one matters here: it produces plenty of characters, so a length check reads it as healthy, and it is common in documents from older generators and subset fonts — much of the contract and procurement material that reaches a knowledge base.

KB_PDF_MIN_CHARS_PER_PAGE (default 100) moves the balance toward cost or fidelity without a deploy. A typeset page carries roughly 1,500–3,000 characters and a scan carries none, so the gap is wide and the threshold does not need to be precise.

Beyond cost

This narrows an availability dependency. Previously an OCR outage failed every PDF; now it only reaches the minority that cannot be read locally. That materially reduces the blast radius of the one failure mode this design deliberately does not paper over with a silent fallback.

Known limitation

The judgement is per document, so a file mixing typeset pages with scanned inserts can average above the threshold and keep its partial text. Per-page routing would catch it and needs per-page extraction we do not currently have. Called out rather than hidden — it is the natural next refinement.

One behavioural change worth review attention

assertKnowledgeOpaqueModelInputSafe now runs after the local read rather than before it, because the local read is what decides whether OCR is needed at all.

The guarantee is unchanged: bytes never reach an external model when the projection is refused. The refusal simply fires at the outbound request instead of the storage read, and its test now asserts against fetch accordingly. This is consistent with the case directly above it in that file, which establishes that local parsing is not model egress. It is also the better ordering — a secret-bearing PDF with a usable text layer is now indexed locally instead of being refused for an external call that never happens.

Testing

  • vitest run lib/knowledge/ lib/uploads/ lib/file-parsers/ connectors/ app/api/knowledge/2,020 passed (131 files)
  • pdf-text-layer.test.ts — 9 cases: typeset document, scan, sparse text, CID escapes, replacement characters, prose containing slashes and digits, accented and non-Latin text, unknown page count, and threshold scaling with length
  • pdf-ocr-triage.test.ts — 4 routing cases: text layer used with no OCR call, and fallthrough on scan, CID escapes, and an unparseable PDF. Verified the first fails when the triage is disabled
  • bun run check:audits — 29/29
  • tsgo --noEmit — no errors in changed files

Checklist

  • Self-reviewed
  • No secrets or customer data introduced

Every PDF went to OCR, an external per-document call, even though most carry an
embedded text layer that costs nothing to read. Across a real corpus of 2,693
documents, local extraction produced text for every PDF that OCR could also read,
so the great majority of those calls bought nothing.

A PDF's text layer is now read first and used when it is good enough, leaving OCR
for the documents that actually need it. Three ways a layer fails, none of which
catches the others: there is no text at all (a scan), the text is too sparse to
be the document, or there is plenty of text that is not language — a broken
encoding, or the raw character ids a CID-keyed font emits with no ToUnicode map,
which is common in exactly the contract and procurement material that reaches a
knowledge base and which a length check alone reads as healthy.

Beyond the cost, this narrows an availability dependency: an OCR outage no longer
touches every PDF, only the minority that cannot be read locally. The threshold is
env-tunable so the balance can be moved toward cost or fidelity without a deploy.

Known limitation: the judgement is per document, so a file mixing typeset pages
with scanned inserts can average above the threshold and keep its partial text.
Per-page routing would catch it and needs per-page extraction this does not have.

The opaque-input refusal now asserts against the outbound request rather than the
storage read: local parsing is not model input, so bytes are read before the
projection is checked and still never leave the worker when it refuses.
@vercel

vercel Bot commented Aug 19, 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 19, 2026 6:26pm

Request Review

@cursor

cursor Bot commented Aug 19, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes core knowledge document parsing, routing, and when bytes reach external OCR APIs, including a reordering of model-input provenance checks that affects refusal vs local indexing behavior.

Overview
PDF ingestion now tries the embedded text layer before any external OCR, using assessPdfTextLayer to reject empty, truncated, sparse, CID-gibberish, or unreadable layers so most PDFs index via file-parser without calling Mistral/Azure.

When OCR is still needed, Azure Mistral follows the same chunked PDF path as Mistral through shared ocrPdfInChunks (page-cap splitting, concurrency, and fail-the-whole-document if any chunk fails), instead of refusing oversized PDFs or indexing partial/empty API envelopes.

Opaque model-input safety (assertKnowledgeOpaqueModelInputSafe) runs after the local read attempt, so secret-bearing PDFs with a usable layer can index locally while outbound fetch to OCR remains blocked when projection is unsafe (tests now assert no fetch, not no download).

Reviewed by Cursor Bugbot for commit 0dabda6. Configure here.

@greptile-apps

greptile-apps Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR reads an embedded PDF text layer before using external OCR and includes complete-document safeguards for local extraction and chunked OCR.

  • Routes usable embedded text directly into normal file-parser processing.
  • Sends empty, sparse, corrupted, or truncated text layers to OCR.
  • Shares ordered, all-or-nothing PDF chunk processing across Mistral and Azure Mistral.
  • Adds routing, text-quality, provenance, Azure chunking, and partial-failure regression coverage.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
apps/sim/lib/knowledge/documents/document-processor.ts Adds local PDF text-layer triage, moves the provenance assertion to the OCR egress boundary, and makes shared OCR chunk processing all-or-nothing.
apps/sim/lib/knowledge/documents/pdf-text-layer.ts Defines text-layer usability checks for truncation, text density, CID escapes, and unreadable encoding.
apps/sim/lib/knowledge/documents/pdf-ocr-triage.test.ts Covers local-versus-OCR routing, Azure page-cap chunking, malformed-PDF fallback, empty responses, and partial chunk failure.
apps/sim/lib/knowledge/documents/pdf-text-layer.test.ts Exercises the text-layer classifier across ordinary, sparse, truncated, malformed, and multilingual content.
apps/sim/lib/knowledge/documents/document-processor-secret-provenance.test.ts Updates provenance coverage to assert that refused opaque PDF bytes never reach the external OCR request.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  PDF[PDF in object storage] --> Local[Parse embedded text layer locally]
  Local --> Assess{Text layer usable?}
  Assess -->|Yes| Parse[Use file-parser content]
  Assess -->|No| Guard[Verify opaque model input is safe]
  Guard --> OCR{Configured OCR provider}
  OCR --> Mistral[Mistral OCR]
  OCR --> Azure[Azure Mistral OCR]
  Mistral --> Split[Split PDF into bounded chunks]
  Azure --> Split
  Split --> Complete{Every chunk recovered?}
  Complete -->|Yes| Stitch[Stitch chunks in page order]
  Complete -->|No| Fail[Fail document for retry]
  Parse --> Chunk[Chunk and embed document]
  Stitch --> Chunk
Loading

Reviews (6): Last reviewed commit: "fix(knowledge): fail a PDF whose OCR onl..." | Re-trigger Greptile

Comment thread apps/sim/lib/knowledge/documents/document-processor.ts Outdated
Comment thread apps/sim/lib/knowledge/documents/document-processor.ts Outdated
… threshold env var

Two corrections to the text-layer triage.

A parser limit stops extraction partway and reports `truncated`. Such a result has
plenty of text by volume, so every volume-based check read it as healthy and the
document was indexed as a fragment with the remainder silently missing from
search. Truncation is now judged before anything that measures volume, and sends
the document to OCR, which reads it whole.

The characters-per-page threshold is a plain constant again. It read
`process.env` directly rather than going through the env module, and the tunable
was not worth having: a typeset page carries roughly 1,500-3,000 characters and a
scan carries none, so the value sits in a wide gap where no realistic tuning
changes an outcome. A constant is one less piece of configuration that can be set
wrong, and if the threshold is ever wrong the fix is to change it.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

Comment thread apps/sim/lib/knowledge/documents/pdf-text-layer.ts
…text

The density check counted pages with a second, independent read of the file. The
two could disagree: a count that failed reported no pages, the check fell back to
treating the document as a single page, and a long scan carrying only a header
looked dense enough to skip OCR and be indexed as that header.

`parseBuffer` already reports the page count from the parse that produced the
text, so the two can no longer diverge, and the redundant second open of the file
goes away with it.
@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 47cd6e1. Configure here.

Both OCR providers cap how many pages a single request may carry, and both were
handling that cap differently: one split the document to fit, the other rejected
any document over it. A long PDF could therefore be ingested on one provider and
not at all on the other, for a limit that belongs to a request rather than to a
document.

The splitting, concurrency, ordering and partial-failure rule now live in one
place that both providers call, so they cannot drift apart again. A chunk that
fails is dropped rather than failing the document — losing one section of a long
document beats losing all of it — and every chunk failing still throws.

Also drops the unpdf mock from the triage tests. It was masking real behaviour:
the page count now comes from the parse metadata, so the mock was no longer
needed, and while it was in place a test asserting the old page-cap refusal
passed against both the old and new code.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

Comment thread apps/sim/lib/knowledge/documents/document-processor.ts
Comment thread apps/sim/lib/knowledge/documents/document-processor.ts Outdated
…nest

Two regressions from chunking the Azure path.

Splitting loads the document, which an encrypted or malformed PDF refuses, and
that failure was deciding whether the file reached OCR at all. Those are exactly
the documents the triage routes here — no readable text layer — and the provider
may well accept bytes a local parser will not, so a failed split now sends the
document whole and leaves the page cap to the provider, as it did before it was
chunked.

An Azure response carrying no pages fell back to the raw API payload as content.
Chunked, that payload counted as recovered text and was stitched into the
document; unchunked, it satisfied the empty-content check written to catch this.
No pages is now no content, so the chunk counts as failed and the document
reports it.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

Comment thread apps/sim/lib/knowledge/documents/document-processor.ts

@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 fc8af91. Configure here.

A chunked OCR run dropped any chunk that failed and returned the rest as a
normal success, so the document was marked complete with whole page ranges
absent from search and nothing downstream could tell the difference.

That contradicted the rule this change set already applies to a truncated text
layer, which is sent to OCR precisely because indexing a fragment while
reporting success is the failure being removed. A document is now indexed whole
or not at all: any missing chunk fails it, leaving it visible with a reason and
eligible for the stuck-document sweep, which can retry and produce a complete
result. Each chunk has already exhausted its own retries, so a missing one is a
real failure rather than a blip.

The page-cap test mocked fetch with a single Response object, whose body can
only be read once — the second chunk was failing on "Body already read" and the
lenient path hid it. It now returns a fresh response per call.
@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 0dabda6. Configure here.

@waleedlatif1
waleedlatif1 merged commit cb6c842 into staging Aug 19, 2026
24 checks passed
@waleedlatif1
waleedlatif1 deleted the feat/pdf-ocr-triage branch August 19, 2026 18:31
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