Repository navigation
Make recognized scan text searchable and save it in PDF copies - #21
Conversation
Scanned PDFs are pictures of pages with no text, so search, selection, highlights and Ask AI had nothing to work with. Folio now offers "Recognize text" on a scan, reads the pages on the device with Tesseract (bundled, offline, English), and feeds the words back to pdf.js as the page's own text. - Detection: a page counts as a scan when it has no text, or a picture covers most of it and the embedded text covers little (a stamp, a header). Recognized words are added to the PDF's own text, and words that repeat it are dropped. - Runs in the background starting at the page being read, one or two workers, and is saved per document so a scan is only read once. Password-protected files are never stored. - Specks are filtered out: low-confidence short words, and symbols that are unsure or tiny next to the line's text. Line size comes from Tesseract's row height, since word boxes swell around specks. - Ask AI sees the recognized text and is told it came from OCR. - New layout for this feature: components/ocr, hooks/ocr, lib/ocr, with tests beside each file. - Adds Vitest (160 tests) and runs it in CI on all three platforms; the smoke test now recognizes a generated scan. Fixes the lockfile so native optional packages keep their libc markers. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D6GG856opX3UVvPvBq8EWK
Taken from a generated test scan (scripts/fixtures/scanned-sample.pdf), for the pull request description. Kept in their own commit so they can be dropped if images shouldn't live in the repository. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D6GG856opX3UVvPvBq8EWK
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D6GG856opX3UVvPvBq8EWK
Resolves the conflict in scripts/smoke-test.mjs: main added a changelog check and this branch added the scanned-page check; both stay, with the changelog check first so the slow recognition step remains last. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D6GG84S8tDUVUetZpeTA1V
- Save a copy now carries the recognized text. pdf-lib (loaded only when a copy is saved) lays the words over
each scanned page as invisible text, squeezed per word to the width of its ink so selection lines up in other
readers, and lists the recognized pages in the document info. A copy opened again is not offered recognition,
even for a drawing whose few words look like a scan. It is left out, with a message, for password-protected
files and digitally signed ones (rewriting would break the signature), and if adding it fails the copy is still
saved. Checked against Poppler and Apple's PDFKit as well as pdf.js.
- A search that is open when recognition finishes is run again once find has been reset, instead of having to be
typed again. The request is read from the find controller before it is cleared and replayed as a fresh forward
search, and only if the find bar is still showing it.
- Search results put a space where a line ends ("identification. Invoice") and read each page the way find does
(disableNormalization), so offsets stay right with ligatures.
- Ask AI is still told the text came from OCR for a copy that carries it.
- Large sheets stay one capped picture: on a synthetic A1 sheet, reading at 200 dpi or in 300 dpi tiles found no
more words than 100 dpi (84-85%) and took up to three times as long. The reasoning is recorded next to the
constants, and a test pins the dpi quoted in the README (Letter 300, A4 282, A3 200, A1 100).
- OcrSession: removes the unused focus field and covers its cancellation and shutdown paths.
Tests: 291 (was 160), run on Node 20 and 22. The OCR and search code is at 100% of statements, branches,
functions and lines, and mutations of it are caught. New: the real pdf.js find controller and a pdf-lib round
trip (src/testing/pdfjs.ts), component and hook tests under jsdom. The smoke test saves a copy of the scan and
reopens it. New dev dependencies: pdf-lib, jsdom.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D6GG84S8tDUVUetZpeTA1V
README: Save a copy now includes the recognized text (and when it can't), a search is run again when recognition finishes, OCR is no longer listed as missing, and the resolution large sheets are read at is stated. AGENTS.md: how to test against the real pdf.js and pdf-lib, and keeping logic out of the big files. THIRD_PARTY_NOTICES.md: pdf-lib and its dependencies. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D6GG84S8tDUVUetZpeTA1V
…onestly The checks that keep Save a copy from rewriting an encrypted or signed file had only been run against stand-ins. They now also run against what the real pdf.js reports for real files: two encrypted PDFs written by PDFKit (one needs a password, one has only an owner password and opens without one) and PDFs built with a blank and a filled-in signature field. That showed pdf.js cannot tell a blank signature field from a signed one, so the guard now treats any signature field as a reason to leave the text out, and the message says "signature field" instead of claiming a digital signature. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D6GG84S8tDUVUetZpeTA1V
main moved on again (v1.10.3, contribution rules, saved marks on pages not drawn yet, asking before replacing a document). Resolved: - src/App.tsx: kept both sets of refs and helpers. A saved copy now reports marks the way main does (`bytes !== doc.data`, since main adds unrendered marks to the copy only), and still adds the recognized text. - scripts/smoke-test.mjs: kept main's checks first (they use the sample document), then the scan checks. The saved-copy check now captures the download with Electron's will-download event, as main's saved-marks check does. - AGENTS.md: the code layout and testing notes sit before main's new Pull requests section. - CONTRIBUTING.md: step 3 asks for `npm test`, `npm run build`, and `npm run smoke`. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D6GG84S8tDUVUetZpeTA1V
…out the text The check also catches files that are encrypted but open without a password (only an owner password is set), where "password-protected" would be wrong. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D6GG84S8tDUVUetZpeTA1V
Picks up "Keep local test runs from touching the clipboard or showing the app". AGENTS.md: its closing paragraph follows the checklist line, which keeps `npm test`. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D6GG84S8tDUVUetZpeTA1V
- scripts/copy-assets.test.ts compared .gitignore lines after splitting on "\n", which fails where files are checked out with CRLF (Windows): it saw "public/ocr\r". - The smoke test opened the scanned files with Ctrl+O. On the Linux runner the shortcut never reached the app after the text box check, so no file chooser appeared. They are now opened from the library by clicking "Open a PDF", which does not depend on where the keyboard focus is. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D6GG84S8tDUVUetZpeTA1V
Picks up v1.10.4 (reduced PDF memory use, trimmed bundled resources). Resolved:
- scripts/copy-pdfjs-assets.mjs: main's clean-start and quickjs-eval filter, plus the OCR runtime copy.
- src/App.tsx: main's toast timer ref with the duration parameter, and main's { bytes, annotated } copy, which
gets the recognized text added before it is saved.
- src/AiPanel.tsx: both sets of imports.
- src/pdf.ts: exports pdfjs again, which OcrSession needs for the drawing operator numbers.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D6GG84S8tDUVUetZpeTA1V
mberman84
left a comment
There was a problem hiding this comment.
Thanks for the follow-ups. The latest head (e71576c) passes the existing tests, the build, and the Windows smoke test, including recognizing a scanned page and reopening the saved copy. Two bugs still need fixing before this can merge, plus some cleanup.
1. Stopping and restarting can stop the new run
cancel() sets the phase back to offer right away, so start() can run again while the first run is still starting its workers. Both runs share #cancelled and #workers, so when the old run finishes it calls #stopWorkers() and terminates the new run's worker (and start() clears #cancelled, so the old run carries on as if it hadn't been stopped). See OcrSession.ts start() and cancel().
Each run needs its own cancellation and its own workers (for example, a run token or generation counter that cancel() bumps, with each run only stopping the workers it created).
This test fails on the current head:
it("restarting during startup does not let an old run stop the new worker", async () => {
const oldStartup = deferred();
const newRecognition = deferred();
const oldWorker = { recognize: vi.fn(async () => nothing), terminate: vi.fn(async () => {}) };
const newWorker = { recognize: vi.fn(async () => { await newRecognition.promise; return nothing; }), terminate: vi.fn(async () => {}) };
mocks.createWorker
.mockImplementationOnce(async () => { await oldStartup.promise; return oldWorker; })
.mockResolvedValueOnce(newWorker);
const session = new OcrSession(makePdf([makePage({ image: true })]), "k");
const oldRun = session.start(() => 1);
await vi.waitFor(() => expect(mocks.createWorker).toHaveBeenCalledTimes(1));
session.cancel();
const newRun = session.start(() => 1);
await vi.waitFor(() => expect(newWorker.recognize).toHaveBeenCalledTimes(1));
oldStartup.resolve();
await oldRun;
const terminatedByOldRun = newWorker.terminate.mock.calls.length;
const phaseDuringNewRun = session.state.phase;
newRecognition.resolve();
await newRun;
expect(terminatedByOldRun).toBe(0);
expect(phaseDuringNewRun).toBe("running");
});2. A failed scan is reported as success in mixed documents
The run ends with failures === total ? error : done, but total counts every page, including pages that already have text and are skipped. In a document with one scanned page and one typed page, if the scanned page fails, failures is 1 and total is 2, so the phase becomes done and the user never sees the error or the retry button. Compare failures against the pages that were actually sent for recognition (and consider reporting a partial failure rather than done).
This test fails on the current head:
it("a failed scanned page in a mixed document reports failure", async () => {
const worker = makeWorker();
worker.recognize.mockRejectedValue(new Error("bad scanned page"));
const session = new OcrSession(makePdf([makePage({ image: true }), makePage({ items: TYPED })]), "k");
await session.start(() => 1);
expect(session.pages.size).toBe(0);
expect(session.state.phase).toBe("error");
});Please add both tests with the fixes.
3. Cleanup
- Screenshots: drop the
Add screenshots of text recognitioncommit. CONTRIBUTING.md asks for screenshots to be attached to the pull request, not committed, so please attach them here instead. - Unrelated changes: the contribution and architecture rule edits should come out of this pull request; one pull request fixes one problem.
- Scope: issue #15 asked about OCRmyPDF and choosing a language. Bundled, English-only recognition is fine as a first step from our side, but please say so in the description and note that other languages are a follow-up.
If we haven't heard back by Monday, November 2, we'll make these changes on your branch ourselves so this can ship. Thanks again for the work here.
…leanup - OcrSession: cancel() bumps a generation number and each run only acts on, and only stops the workers of, its own generation, so stopping and restarting while the first run is still starting can't terminate the new run's worker or let the old run carry on. - A scanned page that fails is reported against the pages actually sent to the recognizer, not every page, so a failed scan in a mixed document no longer ends as "done". Some failures now also end as an error (the pages that were read keep their text, and retrying reads only the others). - Removes the committed screenshots (CONTRIBUTING.md asks for them to be attached to the pull request instead) and puts AGENTS.md and CONTRIBUTING.md back as they are on main: the rule edits were unrelated to this change. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D6GG84S8tDUVUetZpeTA1V
Picks up LaTeX rendering in AI chat and the print fix. ci.yml runs both `npm test` and `npm run test:chat-math`; package.json keeps both sets of scripts and dependencies; package-lock.json is main's with this branch's packages added (generated with npm 11, because npm 10 fails on the combined tree), with the libc markers kept. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D6GG84S8tDUVUetZpeTA1V
|
@mberman84 thanks, all of it is done on 1. Stop and restart. 2. Failed scan in a mixed document. Failures are now counted against the pages actually sent to the recognizer. Your test is included and passes. I also took your partial-failure suggestion: if any scanned page fails the run ends as an error ("Couldn't read 1 of 3 scanned pages.") with the retry button; pages that were read keep their text and a retry reads only the others. 3. Cleanup.
Tests: 301 pass, the OCR code stays at 100% coverage, and CI (this repo's |
mberman84
left a comment
There was a problem hiding this comment.
Thanks for fixing the earlier stop/restart and mixed-document failure cases; the regression tests for both pass. I reviewed the current head a17da1d and found three remaining issues detailed inline: the lockfile blocks clean installs, search snippets can retain pre-OCR text, and a worker can leak when another worker fails startup.
Validation on Windows in isolated snapshots: a fresh npm ci fails with the missing Mammoth dependency tree. After repairing dependencies only in the temporary snapshot, all 301 existing tests, the build, and the full Windows smoke test pass, including recognition and reopening a searchable saved copy. Two additional focused regression probes reproduce the stale-snippet and late-worker cleanup bugs. No source changes were made to the project checkout.
Please fix these three issues and add the regression tests before merge. Cross-platform CI must also pass at the final head.
Reviewed with gpt-6.1-sol through the Codex harness in T3 Code.
|
@mberman84 Could you please re-review this PR? Your three findings were addressed in d07c694, and the full CI suite passed on Windows, macOS and Linux: https://github.com/damogallagher/folio/actions/runs/41202132162. Thank you! |
mberman84
left a comment
There was a problem hiding this comment.
I verified the three fixes from the previous review. On macOS, a clean install, all 303 tests and the build passed at d07c694. I also confirmed that the fork CI run passed on all three platforms at that commit.
I tested a local merge with current main (c72f8cf). The document-open path no longer compiles, and an existing Folio profile silently drops recognized text when it tries to save a page. Ask AI also moved its prompt construction to the main process. Please update the branch, fix the three integration points below, and include the v4 upgrade regression. The updated head needs upstream CI on Windows, macOS and Linux before merge.
|
Two things I found while test-merging this branch (at d07c694) into current 1. Existing users won't get the 2. It no longer merges cleanly. Merging into current A smaller point for the maintainers: the per-page |
|
@joswalt Thanks for catching these. Commit 56d8cea merges current main, resolves the AI panel conflict, and upgrades the database to v5. Regression tests cover upgrades from v3 and shipping v4, preserving notes and v4 Recent entries while saving and restoring OCR pages. The OCR store should also be included when the stored-data controls in #37 are implemented; this PR leaves Clear all’s existing behavior unchanged. @mberman84 Could you please re-review and test the updated PR? It also fixes the aborted document-open lifecycle and carries a validated OCR flag through both AI providers. The full suite passed on Windows, macOS and Linux in the fork: https://github.com/damogallagher/folio/actions/runs/41242880567. The upstream PR workflow still requires maintainer approval: https://github.com/Forward-Future/folio/actions/runs/41242885738. Thank you both! |
mberman84
left a comment
There was a problem hiding this comment.
The updated branch addresses the earlier document-open, database migration, and Ask AI issues. I also verified all three platform jobs passed in the fork on 56d8cea.
Please rebase onto current main and build on the page OCR shipped in 1.12.0 so the same page is recognized only once. The inline findings cover starting OCR on a mixed document, recovering from a worker startup failure, and packaging the CPU fallback.
Please also test saving a reordered two-page scan with a different word on each page. The exporter currently indexes the output PDF by the original page number. Map both the word positions and the recognized-page metadata to the output order so each word stays on its own page. Once the changes are ready, we can run upstream CI on all three platforms.
…eature/ocr-scanned-pdfs
…eature/ocr-scanned-pdfs # Conflicts: # scripts/test-reader-security.mjs # src/App.tsx
|
P2 — Refresh the text layer after OCR on image-only pages. At |
…eature/ocr-scanned-pdfs # Conflicts: # package-lock.json # package.json # src/App.tsx # src/recent.ts
|
Fixed in 4a0a26f. Image-only pages now rebuild their text layer through PDF.js after recognition, so its search highlighter uses the same text spans as Find. I removed the old manual span rendering and added an app regression that searches for and highlights a newly recognized word before reopening the PDF. Locally on macOS, the build, 250 unit tests, and full app smoke test passed, including selection on a rotated scan and mixed-document OCR. The updated cross-platform checks are pending. |
…eature/ocr-scanned-pdfs
…eature/ocr-scanned-pdfs
mberman84
left a comment
There was a problem hiding this comment.
Please make two changes before merge.
- When someone searches while a scan is still being recognized, zero matches can look like a final result. Please show that results are incomplete until recognition finishes, or refresh matches as pages become ready. Check this by searching for a word before its page is recognized and leaving the search open.
- Please make the Clear all confirmation say “Remove all recent files and all saved recognized text from this device?” It also clears recognized text for PDFs that are no longer in Recent. Choosing Cancel should preserve both the recent files and their saved text.
|
Thanks, @mberman84. I've addressed both points in 1d92925:
The fork CI run passed on Windows, macOS and Linux at this commit, including the existing OCR regressions. The upstream PR CI is still awaiting maintainer approval. Could you please re-review when you have a chance? |
|
Hi @mberman84, I've merged the latest The fork CI run passed on Windows, macOS and Linux, including the OCR and thumbnail tests and desktop smoke checks. GitHub now reports the PR as mergeable. The upstream PR CI is still awaiting maintainer approval. Could you please re-review when you have a chance? Thanks! |
…eature/ocr-scanned-pdfs # Conflicts: # package.json # scripts/smoke-test.mjs
…eature/ocr-scanned-pdfs
|
Re-reviewed at a18613a. Both points from 1d92925 are fixed:
The PR conflicts with
Once it's conflict-free I'll approve the CI run and do a final pass. Generated by Claude Code |
mberman84
left a comment
There was a problem hiding this comment.
The earlier review requests are addressed. Changing languages reloads the matching OCR cache. Clearing saved scan text preserves annotations and chats. Search refreshes when recognition finishes. The export test verifies that pages whose text cannot be embedded remain eligible for recognition.
I checked the recognized-text warning through both AI providers after resolving the merge conflicts. I also reproduced an invisible OCR status box intercepting clicks on the PDF and verified the fix in the reader. The retry button still works. Windows, macOS, and Linux passed on 846bd16.
What this adds
Folio already performs on-device OCR on scanned PDF pages to make their text selectable. This PR builds on that existing recognition pipeline so the recognized text is useful throughout the reader and beyond the current session:
Recognition remains on-device. English is available by default; other supported languages use the scan language packs provided by main. Ask AI is optional and uses its selected provider. Signed or encrypted files are not rewritten to add a text layer; the original OCR behavior and its limitations still apply.
Validation
This change was prepared with OpenAI Codex using the Codex desktop harness.