[juliusmarminge]

visiblePdfPages in apps/web/src/components/files/pdfLayout.ts measured its lower bound as scrollTop + 2viewportHeight, which is the viewport top plus two heights, so only one viewport below the visible bottom stayed mounted, contrary to its own doc comment and test name. Pages in the missing band had no canvas until the reader scrolled nearer. The fix measures the bound from the visible bottom (scrollTop + 3viewportHeight). The upper bound (one viewport above) was already correct. PdfViewer.tsx is the only caller. No new regex or parser. No other visible change: pages in the missing band now get their canvas earlier, which is the bug fix.

Fixes #35650

Reproduction on main

On origin/main a3cde98 (worktree, vp i). (1) Added expect(indexes(700, 100)).toEqual([1, 2]) to the existing 'mounts pages within a viewport above and two below' case in apps/web/src/components/files/pdfLayout.test.ts; vp test run src/components/files/pdfLayout.test.ts fails with expected [ 1 ] to deeply equal [ 1, 2 ] (offsets [16,172,980,1302]; page 2 top is 180px = 1.8 viewports below the visible bottom of 800). (2) Real Chromium run of the PdfViewer component (temporary browser test, not committed): a 2-page PDF (612x2448 then 612x792) in a 320x300 container; the scroll region was 223px tall and page 2's top was at 1160. Scrolling to 603 put page 2 1.50 viewports below the visible bottom (826). Only canvas 'Page 1' was mounted; 'Page 2' had no canvas. After clicking 'Next page', the Page 2 canvas appeared. The failing unit assertion is pushed on factory/repro-35650 (213628b).

Evidence

Open the same synthetic two-page PDF in the real web app, constrain its viewer to 320×300, and scroll to 588px. With a 229px scroll viewport and Page 2 starting at 1160px, Page 2 is 1.50 screens below the visible bottom. The read-only inspection panel shows Page 2 missing on base and its rendered canvas on head.

Before	After
a3cde98425	5656b9a98a
Before recording	After recording

Scenario and synthetic PDF generator

AI-assisted; verified with the tests and tools listed above.

Tests

vp test run src/components/files/pdfLayout.test.ts (apps/web): without the fix 1 failed / 17 passed (the repro assertion expected [1] to equal [1, 2]); with the fix 18 passed. vp run check:changed: format, lint, knip, typecheck (2 packages), test (1 file) and react compiler all passed.

AI-assisted; verified with the tests and tools listed above.

[macroscopeapp]

Approvability

Verdict: Approved at 5656b9a

Macroscope's review found this PR approvable — PDF previews now prerender pages two viewports below the visible area, matching the existing documented behavior. The additional work is confined to page rendering, with existing canvas limits and resource cleanup unchanged.

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

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

The PDF page visibility helper now includes pages farther below the viewport. A test checks that pages 1 and 2 are returned at scroll position 700 with a 100-pixel viewport.

Changes: PDF page prerender window

Layer / File(s)	Summary
Extend and verify the visibility window apps/web/src/components/files/pdfLayout.ts, apps/web/src/components/files/pdfLayout.test.ts	The helper extends its lower bound from two to three viewport heights below the scroll position. The test checks that pages 1 and 2 are returned at the specified scroll position and viewport height.

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

Change: Bug fix · Severity of issue fixed: Low

Possibly related PRs

• pingdotgg/t3code#25699: Adds visiblePdfPages and its tests, which this change updates.

Suggested labels: bugfix

Suggested reviewers: macroscopeapp

Merge Risk: Minimal · up to 5656b

The PDF viewer now keeps pages up to two viewports below the visible area mounted, as documented. No merge-blocking risk was found.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title names the change: the PDF preview's prerender bound is fixed to cover pages two viewports below the visible area. The raw summary shows the lower bound in visiblePdfPages moved from two to t…
Linked Issues check		[#35650] requires pages up to two viewport heights below the visible bottom to stay mounted, with the upper bound (one viewport above) unchanged. At the reviewed head, pdfLayout.ts line 43 uses `offse…
Out of Scope Changes check		The whole-PR diff against the merge base a3cde98 changes two files. pdfLayout.ts changes only the lower bound multiplier from 2 to 3. pdfLayout.test.ts adds a comment and one assertion for the same…
Description check		The description states the bug, the one-line fix, and the reproduction on main with a failing unit assertion. It includes before/after evidence with screenshots and recordings, and it lists the test a…

Full details: Title check

Explanation

The title names the change: the PDF preview's prerender bound is fixed to cover pages two viewports below the visible area. The raw summary shows the lower bound in visiblePdfPages moved from two to three viewport heights, which matches the title's conventional fix(web) form. The title is specific enough for a teammate to identify the primary change. One wording issue: the code change makes the mounted band reach three viewport heights below the scroll position, while the title says pages two viewports below the visible area are prerendered. Those descriptions match only if measured from the visible bottom, which the PR description confirms.

Full details: Linked Issues check

Explanation

[#35650] requires pages up to two viewport heights below the visible bottom to stay mounted, with the upper bound (one viewport above) unchanged. At the reviewed head, pdfLayout.ts line 43 uses offsets[index]! <= scrollTop + viewportHeight * 3. The visible bottom is scrollTop + viewportHeight, so the lower bound now covers two viewport heights below it. The upper bound offsets[index + 1]! >= scrollTop - viewportHeight is unchanged. The added assertion indexes(700, 100) expects [1, 2]. Page 2 starts at 980, which is 180 px below the visible bottom of 800 and within the 1000 px bound, so the assertion covers the reported band. The reported offending case is therefore fixed in code and covered by a unit test. The remaining PR evidence section (real-app before/after capture) is verification, not a coding requirement. The browser-level behavior was not re-run in this review, but the unit-level requirement is met.

Full details: Out of Scope Changes check

Explanation

The whole-PR diff against the merge base a3cde98 changes two files. pdfLayout.ts changes only the lower bound multiplier from 2 to 3. pdfLayout.test.ts adds a comment and one assertion for the same bound. Both changes directly implement [#35650]. No unrelated files, refactors, or behavior changes appear in the diff. The PR description states that PdfViewer.tsx is the only caller and was not changed. The description has no new regex or parser code, and the diff confirms none.

Full details: Description check

Explanation

The description states the bug, the one-line fix, and the reproduction on main with a failing unit assertion. It includes before/after evidence with screenshots and recordings, and it lists the test and check commands that passed. The Problem, Change, Scope, and Verification sections of the template are all covered. The Scope section links issue #35650 and says the change is a bug fix, not a feature. The title line is absent from the supplied description, and the Verification section repeats the AI-assisted line, but neither gap is critical. Evidence links point to an external file drop, which the template allows.

Finishing Touches

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: factory/issue-35650
• File: .coderabbit.config.ts
• Commit: 5656b9a
• Layers: Repository TS config
• Review profile: CHILL
• Plan: Team
• Run ID: 164cc74f-5746-4c84-9dce-396ed5304b74

Commits

Reviewing files that changed from the base of the PR and between b1c1e07 and 5656b9a.

Files selected for processing: 2

• apps/web/src/components/files/pdfLayout.test.ts
• apps/web/src/components/files/pdfLayout.ts

Limit details: You’ve used all 10 included reviews currently available.