[juliusmarminge]

The PDF preview browser tests have been flaky since they landed. CI run 41699885540 failed "keeps workspace PDF scroll and zoom after reloads" at getByText('1 / 3 pages') with the viewer still on "Loading file". Locally under load the same test fails in the same way: it does five full pdf.js document loads and more than twenty 1 s poll waits inside its 15 s budget, and a busy machine runs it past the cap (failures at 14.9 s, with clicks left 1–632 ms). Waiting on pdf.js before reading the page count, as https://git.cafe/pingdotgg/t3code/pulls/30328 did, cannot fix this, because the problem is the test's total length, not one wait.

Per-test decisions

Loops are cold vp test run processes with 16 yes > /dev/null processes running on a 16-core machine (load average ~25). Before: 40 runs. After: 100 runs, as two concurrent loops of 50, every PDF test in each run.

Test	Before	Decision	After
keeps workspace PDF scroll and zoom after reloads	5 / 40 failed (all from the 15 s budget running out)	Deleted, replaced	—
clamps workspace PDF scroll when pages shrink or disappear	0 / 40 (also five loads; same budget risk)	Deleted, replaced by a unit test	—
New: keeps a workspace PDF's page, offset and zoom when a new revision reloads it	—	Added	0 / 100
keeps a workspace PDF open across URL renewals… (native viewer, no pdf.js)	0 / 40	Kept	0 / 100
validates PDF fonts/cmaps/wasm assets before decoding	0 / 40	Kept	0 / 100
PdfViewer: fits the width, then finds split and rotated text…	0 / 40	Kept	0 / 100
PdfViewer: restores page, within-page offset and zoom after a remount	0 / 40	Kept	0 / 100

The new test does three loads. Before each check it awaits the document pdf.js opens (task.promise) and every page size, through a small openPdf wrapper in the test file, instead of relying on a 1 s poll. It checks that a revision reload keeps the page, the page-relative offset (the new revision has a taller first page, so only a page-relative restore passes), horizontal scroll and zoom, that no new pdf.js worker starts (https://git.cafe/pingdotgg/t3code/pulls/29626), and that opening another file starts at the top in fit width. Mutation checks: dropping the view carried into the new revision, and starting a worker per document, each make it fail.

Coverage moved or removed

• Clamping when a reloaded revision has shorter or fewer pages (https://git.cafe/pingdotgg/t3code/pulls/25555) moves to a unit test of pdfRestoredScrollTop in pdfLayout.test.ts. The arithmetic moved out of PdfViewer's restoreView unchanged; that test also covers scaling a saved offset to a new width (https://git.cafe/pingdotgg/t3code/pulls/25496).
• Removed from the browser layer: fit-width mode surviving a revision reload, and a zoom set on the first page surviving one. The PdfViewer remount test still covers fit-width restore, and the view state is the same object for every zoom, so I did not keep separate cases.

No component bug found: every local failure was the budget running out while loads were still in progress, never a load that did not finish.

Replaces https://git.cafe/pingdotgg/t3code/pulls/30328.

Model: Claude Opus 5.5 (claude-opus-5-5), Claude Code harness in T3 Code.

🤖 Generated with Claude Code

[macroscopeapp]

Approvability

Verdict: Approved at ce16bbd

Macroscope's review found this PR approvable — The assessed change extracts PDF preview scroll-restoration calculations into a reusable helper while retaining existing positioning behavior and guards. Its limited UI scope presents no obvious material risk requiring human judgment.

^{You can add or adjust custom eligibility rules. Learn more.}

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

The PDF viewer now calculates restored vertical scroll position with a shared helper. Tests cover offset scaling and clamping, preserving the selected view across a PDF revision reload, and resetting the view when the file path changes.

Changes: PDF view restoration

Layer / File(s)	Summary
Calculate restored scroll position apps/web/src/components/files/pdfLayout.ts, apps/web/src/components/files/pdfLayout.test.ts	A new helper scales a saved page offset for the current width and clamps the page and offset to the available layout. Tests cover width changes, page shrinkage, and removed pages.
Restore PDF view during reload apps/web/src/components/files/PdfViewer.tsx, apps/web/src/components/files/AttachmentFilePreview.browser.test.tsx	PdfViewer uses the helper to restore vertical scroll. Browser tests check that a revision reload preserves the selected page, zoom, horizontal scroll, and page-relative vertical offset, while a file path change resets the view.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Other

Possibly related issues

• pingdotgg/t3code#27681 — Reports lost PDF page and scroll position after agent activity. This PR tests preservation across a revision reload but does not change the reload trigger.
• pingdotgg/t3code#23632 — Reports PDF resets after assistant responses. This PR tests view preservation across a document reload but does not cover the response trigger.
• pingdotgg/t3code#24463 — Reports lost PDF page and scroll position after switching conversations. This PR tests preservation across a reload but does not cover conversation switching.
• pingdotgg/t3code#28013 — Reports browser test timeouts. This PR replaces polling with explicit PDF load waits, but the issue concerns a different test.
• pingdotgg/t3code#22324 — Reports load-sensitive browser test failures. This PR uses explicit PDF load waits and reduces repeated loads in the affected test.

Suggested labels: size:M

Suggested reviewers: edemaine, ashx-j, macroscopeapp

Merge Risk: Minimal · up to ce16b

No issue requiring a fix before merge was identified.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title names the main change: the PDF preview browser tests now wait on pdf.js loads, and the change targets the timeouts. The title is a conventional commit in plain language and matches the chang…
Description check		The description follows the template well. It has a clear problem statement with the CI run and local reproduction (15 s budget, load ~25), a change explanation, and a scope section that links the pri…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Title check

Explanation

The title names the main change: the PDF preview browser tests now wait on pdf.js loads, and the change targets the timeouts. The title is a conventional commit in plain language and matches the changeset (new openPdf wrapper awaiting task.promise, replaced scroll/zoom test, and the timeout fix).

Full details: Description check

Explanation

The description follows the template well. It has a clear problem statement with the CI run and local reproduction (15 s budget, load ~25), a change explanation, and a scope section that links the prior PR and gives context for why this is a test-only change to a flaky test. The verification section gives concrete before/after failure counts across 40 and 100 runs, mutation checks, and coverage changes. It lacks an explicit approval link or maintainer-approved issue under Scope and approval, and it does not include a 'Problem' heading exactly as the template labels it, but the sections are present and substantially complete. The model/harness attribution is included as the template requests.

Finishing Touches

Generate docstrings

• Commit to this branch
• Create a new PR

Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Recent review info

Run configuration

• Source mode: Auto
• Repository: pingdotgg/t3code
• Branch: t3/fix-pdf-preview-test-flakes
• File: .coderabbit.config.ts
• Commit: ce16bbd
• Layers: Repository TS config
• Review profile: CHILL
• Plan: Team
• Run ID: 613750f9-a595-4416-a8b4-6b9b900229f1

Commits

Reviewing files that changed from the base of the PR and between 62db662 and ce16bbd.

Files selected for processing: 4

• apps/web/src/components/files/AttachmentFilePreview.browser.test.tsx
• apps/web/src/components/files/PdfViewer.tsx
• apps/web/src/components/files/pdfLayout.test.ts
• apps/web/src/components/files/pdfLayout.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.