Desktop: prepare large PDFs off the UI thread by cyfung1031 · Pull Request #1493 · storytold/pdfcraft

cyfung1031 · 2026-10-12T16:16:15Z

Review direction

Review request/result ownership and race handling first: preparation must stay isolated from Session, and a completed document must be committed exactly once on the UI thread. Then review the responsiveness claim: a live desktop app can continue processing frames and control requests while one background worker reads and prepares a document. This PR does not claim lower total open time, CPU work, or memory use; no time or memory effectiveness measurement is offered.

Summary

Split engine document opening into prepare and commit phases, then use a single bounded native desktop worker for live path, remote-document, dropped-file, and recovery opens. The existing synchronous engine API remains available. Password retries preserve their source metadata and are prepared off the frame thread.

The worker owns no App or Session references. It handles one active request and a FIFO queue of at most 16 waiting requests. UI state changes only when the app polls a successful result and commits it on the UI thread.

Correctness evidence

Engine focused prepare/commit, password, and XFA tests passed locally.
cargo check -p pdfcraft-ui-egui and cargo check --tests -p pdfcraft-ui-egui passed.
cargo clippy -p pdfcraft-ui-egui --lib -- -D warnings -A clippy::collapsible_match passed; the allow is for an unchanged warning on the base branch.
cargo fmt --all and git diff --check passed.
Adds a deterministic UI regression test that blocks preparation and checks control and existing-tab responsiveness before release. The test source compiled, but execution did not complete: a bounded local build stopped while compiling GUI dependencies before running tests.
No hosted CI was used or relied on.

Scope and limits

The implementation is native-desktop only. Windows, Linux, FreeBSD, and web builds/runs were not verified here. The PR establishes where preparation runs; it does not report a measured frame-time, total-open-time, or memory improvement.

Overlapping open PRs

#1476 touches engine/UI library entry points for speech features, but does not overlap the open-worker behavior.
Draft #1172 touches the open success path for its default-app prompt. This branch is independent and does not include that change. If Windows: Explorer PDF actions and default-app prompt #1172 lands first, carry its pdf_opened_for_default_prompt callback into finish_open.

No changes from either PR were incorporated.

Branch and commits

Based on origin/main at 7163ba3dee8becfff6871c9a65d0d8314314742b; head 355d4c5be83bf208565b9a7aaf295a3735ceed95.

76c42625 #307: split document open into prepare and commit
355d4c5b #307: move desktop open preparation off the frame thread

Refs #307.

@cyfung1031

Build on @cyfung1031's background-open proposal storytold#1493 at 355d4c5. Split engine preparation from registration and keep CLI/MCP opening synchronous. Explicitly enable the native worker before startup files, carry tool and navigation continuations through password retries, serialize modal completion, and preserve pending session paths on quit. Reprepare stale security-policy snapshots before registration. Keep the bounded worker and UI completion state machine separate. Keep startup view options attached to their original request through retries, failures and cancellation. Add deterministic responsiveness and opening-flow regressions. Seven regressions were observed failing before their fixes. Pin the existing reload test's English labels to an explicit English test locale. Refs storytold#307 Based-on: storytold#1493

echelon · 2026-10-13T04:17:46Z

Thank you, @cyfung1031! This has landed on main: "Desktop: prepare large PDFs off the UI thread". We appreciate the fix and the tests that came with it.

Please join us on Discord if you haven't already: https://discord.gg/artcraft

Brings the fork up to storytold/pdfcraft dfa43be. No conflicts: the fork carries no code, and its tree differs from upstream's only by AGENTS-aniefork.md and ROADMAP-aniefork.md. Committed with the gate failing, on the owner's decision. Upstream's main fails 14 of its own UI tests at this commit, all about opening a document: storytold#1493 (55f76ab) moved opening onto a worker thread, and these tests still look for the open document after a fixed number of frames. Upstream's CI fails the same way on Linux and Windows. In crates/ui-egui/tests: commands (1), document_links (2), editing (2), pickers (3), recovery (5), ui (1). Everything else passes: fmt, clippy, layers, wasm, assets, deny and parity, and 2,964 tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Upstream's main fails 14 of its own UI tests since storytold#1493; the owner decided to merge anyway. Note the state in the sync table and the log, and what to do when a merge fails the gate while the fork carries no code. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Retain a Home-selected tool until PDF preparation or text conversion succeeds, and keep startup documents behind an active pending open. Add barrier regressions for those orderings. Await document and password completion in the affected UI fixtures, preserving consent, cancellation, destination and recovery checks. Check wheel resumption against the configured platform line distance. Validation: 1,091 UI tests, strict UI Clippy, fmt, layers, assets, parity, cargo-deny and 31 Wasm libraries. Refs storytold#1132, storytold#1493

Documents dropped outside the page grid open on a worker. Wait for the second tab before checking the exact tab count and unchanged original document content, rather than assuming preparation finishes in three frames. Validation: all 1,091 UI tests, strict UI Clippy and formatting pass. Refs storytold#1132, storytold#1493

@cyfung1031

…aration Consolidate the reviewed change and its follow-ups on current upstream. Original change notes: M1: Defer document-open scans and share inspection state Integrate deferred marker/link indexes, on-demand signature details and shared inspection parsing to reduce large-document open work. Preserve signed-file rewrite guards independently of signature validation. Keep inspection caches independent, retain stricter stream limits, and fall back to source parsing after object, trailer or encryption edits. Add regression coverage for source isolation, lazy cache invalidation, pre-validation rewrite refusal and signature panel behavior. Based on @cyfung1031's proposals: - storytold#1434 (a4cdc2d) - storytold#1465 (b9cd81f) - storytold#1527 (eaa0c27) Refs storytold#307. Background desktop opening and compact xref storage remain separate; this does not claim to fix cold-cache first rendering. M1: Keep desktop responsive throughout document opening Build on @cyfung1031's background-open proposal storytold#1493 at 355d4c5. Split engine preparation from registration and keep CLI/MCP opening synchronous. Explicitly enable the native worker before startup files, carry tool and navigation continuations through password retries, serialize modal completion, and preserve pending session paths on quit. Reprepare stale security-policy snapshots before registration. Keep the bounded worker and UI completion state machine separate. Keep startup view options attached to their original request through retries, failures and cancellation. Add deterministic responsiveness and opening-flow regressions. Seven regressions were observed failing before their fixes. Pin the existing reload test's English labels to an explicit English test locale. Refs storytold#307 Based-on: storytold#1493 M1: Keep startup bytes queued during password preparation Preserve the local password interaction until background preparation finishes. Add a deterministic barrier regression and exercise deferred tools, wrong passwords, cancellation, and text/image conversion success and failure. Cover the public open-then-tool flow in both synchronous and background hosts. test(ui): wait for interactive recovery password fields (storytold#1634) M1: apply backpressure to native document preparation Keep queued paths unread until the UI can consume their result, so a password or tool dialog does not retain every prepared PDF in a batch. Use the UI pending order for both password retries and stale-policy repreparation instead of independently ordering the worker queue. Add regressions for held results, newly queued files behind a password prompt, and security repreparation ahead of later reads. All three fail on the preceding PR head; 1,130 UI tests pass after the change. Refs storytold#307

@cyfung1031

…aration Consolidate the reviewed change and its follow-ups on current upstream. Original change notes: M1: Defer document-open scans and share inspection state Integrate deferred marker/link indexes, on-demand signature details and shared inspection parsing to reduce large-document open work. Preserve signed-file rewrite guards independently of signature validation. Keep inspection caches independent, retain stricter stream limits, and fall back to source parsing after object, trailer or encryption edits. Add regression coverage for source isolation, lazy cache invalidation, pre-validation rewrite refusal and signature panel behavior. Based on @cyfung1031's proposals: - storytold#1434 (a4cdc2d) - storytold#1465 (b9cd81f) - storytold#1527 (eaa0c27) Refs storytold#307. Background desktop opening and compact xref storage remain separate; this does not claim to fix cold-cache first rendering. M1: Keep desktop responsive throughout document opening Build on @cyfung1031's background-open proposal storytold#1493 at 355d4c5. Split engine preparation from registration and keep CLI/MCP opening synchronous. Explicitly enable the native worker before startup files, carry tool and navigation continuations through password retries, serialize modal completion, and preserve pending session paths on quit. Reprepare stale security-policy snapshots before registration. Keep the bounded worker and UI completion state machine separate. Keep startup view options attached to their original request through retries, failures and cancellation. Add deterministic responsiveness and opening-flow regressions. Seven regressions were observed failing before their fixes. Pin the existing reload test's English labels to an explicit English test locale. Refs storytold#307 Based-on: storytold#1493 M1: Keep startup bytes queued during password preparation Preserve the local password interaction until background preparation finishes. Add a deterministic barrier regression and exercise deferred tools, wrong passwords, cancellation, and text/image conversion success and failure. Cover the public open-then-tool flow in both synchronous and background hosts. test(ui): wait for interactive recovery password fields (storytold#1634) M1: apply backpressure to native document preparation Keep queued paths unread until the UI can consume their result, so a password or tool dialog does not retain every prepared PDF in a batch. Use the UI pending order for both password retries and stale-policy repreparation instead of independently ordering the worker queue. Add regressions for held results, newly queued files behind a password prompt, and security repreparation ahead of later reads. All three fail on the preceding PR head; 1,130 UI tests pass after the change. Refs storytold#307

Reuse the editable COS parse for inspection only while it represents the same unmodified source. Keep independent caches and existing stream limits. Extend native background opening with UI-driven admission: later paths remain unread while a prepared result, password prompt, picker or tool interaction waits. Preserve request-owned startup options, session pages, recovery and tool continuations; reprepare stale security-policy snapshots. Retain upstream's eager signature validation, rendered verdicts, trust refresh and signed snapshots. Keep exact structural rewrite guards for adopted signature fields and standalone document timestamps. Build on cyfung1031's shared-parsing proposal storytold#1527 (eaa0c27), marker/link index proposal storytold#1434 (a4cdc2d), and background preparation storytold#1493 (355d4c5). The latter two are already upstream; preserve their implementations. Refs storytold#307. Cold-cache first rendering, global memory budgets and interrupting an active parse remain outside this change.

cyfung1031 added 2 commits October 13, 2026 00:38

storytold#307: split document open into prepare and commit

76c4262

storytold#307: move desktop open preparation off the frame thread

355d4c5

cyfung1031 mentioned this pull request Oct 12, 2026

Open: share parse state across editing and inspection #1527

Open

LabDennis mentioned this pull request Oct 12, 2026

M1: Share PDF parsing and bound background document opening #1634

Open

echelon merged commit 55f76ab into storytold:main Oct 13, 2026
3 of 5 checks passed

marcoarnulfo mentioned this pull request Oct 13, 2026

fix(ui): a tool chosen with no document runs once the picked file opens #1681

Closed

Erry91 mentioned this pull request Oct 13, 2026

fix(ui): drop tests open the dropped PDF inline, not racing the open worker #1729

Closed

3 tasks done

LabDennis mentioned this pull request Oct 13, 2026

test(ui): extend native opening and continuation regression coverage #1764

Closed

philip-tam mentioned this pull request Oct 13, 2026

M3: make two open-file tests read the file on the frame thread (red CI on main) #1897

Open

7 tasks done

skylartaylor mentioned this pull request Oct 13, 2026

fix(ui): preserve native open actions and password prompts #1914

Merged

This was referenced Oct 13, 2026

feat(ui): a middle click closes a tab (#1853) #1920

Merged

fix(audit): a cover over a TJ displacement hides no text (#1885) #1930

Closed

fosskar Bot mentioned this pull request Oct 15, 2026

pdfcraft-web: 0.7.1 -> 0.8.0 fosskar/nixfiles#1793

Merged

Navigation Menu

Repository navigation

Desktop: prepare large PDFs off the UI thread - #1493

Desktop: prepare large PDFs off the UI thread#1493
echelon merged 2 commits into
storytold:mainfrom
cyfung1031:codex/open-scale-pr-06

cyfung1031 commented Oct 12, 2026

Uh oh!

Uh oh!

echelon commented Oct 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

cyfung1031 commented Oct 12, 2026

Review direction

Summary

Correctness evidence

Scope and limits

Overlapping open PRs

Branch and commits

Uh oh!

Uh oh!

echelon commented Oct 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants