Document viewer Phase 3: page virtualization, rail windowing, keyboard reading mode, and the first canvas gate

[BigSimmo]

Summary

Phase 3 of the document viewer redesign (docs/plans/document-viewer-redesign-plan.md), executed from docs/plans/document-viewer-phase3-handover.md. Phases 0–2 merged as #1741. One commit per task so any single item stays independently revertible while this PR is open.

• Task 0 — a browser gate over the viewer canvas (tests/ui-document-canvas.spec.ts). Nothing previously proved a clinical source page actually paints: a blank canvas still reports correct dimensions, a correct aria-label, and a resolved render promise. The gate reads pixels back — ink on page 1, the real page count in the one toolbar readout, and a page flip whose FNV pixel signature differs from page 1's. Closes #279.
• Task 1 — multi-page virtualization (pdf-canvas-viewer.tsx). The reader rasterised exactly one page into one canvas, so every page flip on a long guideline was a cold pdf.js render. It now renders a windowed page column: each slot reserves its page's box whether or not a canvas is in it, and pages outside the window drop their backing store.
• Task 2 — rail windowing (DocumentImageList). #source-images and the audit list mapped every figure into a DocumentImage on mount — a row that parses table markdown, decides structured-table feasibility, computes quality warnings and evidence tags, and mounts a SignedImage. Now six rows, growing on a sentinel, with an explicit control to reveal the rest.
• Task 3 — signed-URL and decode priority. Explicit fetchPriority on SignedImage (high above the fold, low for deferred figures) and a 240px root margin for the rail against the shared 640px default. The batch signed-URL route stays unwired — see below.
• Task 4 — keyboard reading mode. Page Up/Page Down, Home/End, f fit, r rotate, on top of the existing arrows and +/-. Documented in docs/wiring-conventions.md.
• Task 5 — OffscreenCanvas: measured, not implemented. See below.

The two design tensions, resolved explicitly

The canvas raster budget is now document-wide. resolveCanvasRasterPlan bounds one canvas against WebKit's ~2^24 ceiling and says nothing about how many exist, so N individually-legal canvases could still exhaust device memory. resolveLiveCanvasWindow (canvas-raster-budget.ts) caps total retained raster instead. The useful property is the curve rather than the constant: a fit-width phone page costs ~1.3 megapixels and never binds against it, while a page at maximum zoom costs the entire per-canvas ceiling and collapses the window to exactly one — so render-ahead disappears precisely where retaining neighbours would be most dangerous, with no special-casing of zoom anywhere. MAX_CANVAS_PIXELS is unchanged and was not raised. tests/canvas-raster-budget.test.ts asserts the collapse at maximum zoom, the affordance at phone fit-width sizes, and that the window tightens monotonically across the whole supported zoom range.

Render-ahead is reconciled with disableAutoFetch, not traded against it. getDocument({ disableAutoFetch: true, disableStream: true }) exists because a reader looks at one page and pdf.js would otherwise pull a whole guideline over cellular. Pre-rendering neighbours pulls exactly those bytes back, so both flags stay and the policy is bounded on three independent axes: ±1 page only (next before previous, since readers move forward); deferred to requestIdleCallback, so a reader flipping quickly never reaches idle and never pays for the pages they pass; and switched off entirely under Save-Data or a 2g/slow-2g connection, which are the exact conditions the flag was chosen for. Resident pages top out at three; the document is never fetched whole. resolveRenderAheadPages is a pure function with its own tests, and tests/document-viewer-page-virtualization.dom.test.tsx asserts the flags survive and that nothing renders beyond the reader's page until idle fires.

Two races found while building this, both fixed

Neither was hypothetical — both were caught by the new DOM test and reproduced deterministically.

• The route effect re-runs when pdf.js reports its page count, which is always after the reader can have started scrolling. Applied unconditionally it dragged them back to initialPage. It now acts only when the route asks for somewhere the reader is not, while still reconciling an out-of-range deep link either way.
• The in-flight scroll gate was armed inside the requestAnimationFrame that performs the scroll. Intersections landing in the gap between intent and scroll read as reader input and cancelled the jump. It now arms the moment intent is registered.

Deliberately not done

• Crop → page overlay — out of scope per the brief. bbox is already SELECTed at src/lib/document-detail.ts:441 but absent from DocumentDetailImage in src/lib/document-detail-contract.ts, so it is a contract change across src/lib/**document** rather than a viewer change. Recorded as the one remaining Phase 3 capability in the plan, with the shape of the work named.
• The 100-id batch signed-URL route (#283) stays unwired. Beyond keeping a privileged owner-scoped API route out of a component-only diff, the case for it has actually weakened: windowing the rail means a figure-heavy document no longer mounts N rows at once, which was the many-distinct-images scenario the batch was meant to serve. #283 now carries the measurement that should decide it rather than a standing assumption.
• OffscreenCanvas — the plan conditions it on "measured main-thread paint cost", and no such measurement existed. Virtualization already removes the cold-render-per-flip cost that motivated it, and moving the raster off-thread would put the canvas a clinical source is drawn into behind a transfer boundary. The canvas gate now attaches the number (page-flip-raster-cost.json: flip-to-painted, long-task count and total, longest task, backing pixels) on every Production UI run. #290 records how to read it and what result closes the question either way.
• Toolbar density — already shipped in Phase 2; struck from the plan's table.

Verification

• npm run verify:pr-local
• npm run verify:ui — UI verification not run: no browser gate can run in this container. pdfjs-dist@6.2.108 calls Map.prototype.getOrInsertComputed (pdf.mjs:2454, 6889, 6896), which ships in Chromium 151 and not in the 141.0.7390.37 build this container pre-bakes at /opt/pw-browsers/chromium-1194 and pins via PLAYWRIGHT_BROWSERS_PATH. CI's browser (HeadlessChrome/151.0.0.0) and this repo's pinned Playwright build (playwright-core/browsers.json revision 1234 → 151.0.7922.34) both have it, so browser proof is delegated to Production UI. npm run verify:phone-chrome -- --dry-run reports focused ownership/journey coverage is sufficient for this scope and does not select the full UI gate.
• npm run verify:release — not run; not a release handoff, and it is provider-backed.
• npm run eval:retrieval:quality — not applicable; no retrieval, ranking, selection, chunking, or scoring behaviour changed.
• npm run eval:rag -- --limit 15 — not applicable; answer generation, the synthesis prompt, and answer post-processing are untouched.
• npm run check:production-readiness — not run; no clinical workflow, privacy, environment, Supabase, source-governance, or deployment behaviour changed. This diff is confined to viewer components, their tests, the Playwright spec registry, and docs.
• npm run check:deployment-readiness — not applicable; no deployment startup, hosting, or rollout behaviour changed.

verify:pr-local selected the full heavy route for this scope and every step passed except one pre-existing, environment-only failure. Decisive lines:

- completed: check:runtime, check:installed-lock-parity, format:changed, sitemap:check,
             docs:check-index, docs:check-inventory, docs:check-scripts, docs:check-links,
             check:branch-review-ledger, check:outstanding-issues, lint, typecheck
  Test Files  1 failed | 543 passed (544)
       Tests  1 failed | 5839 passed | 4 skipped (5844)

npm run build              ✓ Compiled successfully in 59s
                           Client bundle secret surface check passed.
npm run eval:rag:offline   Offline RAG fixture and manifest validation passed (36 golden cases, 23 suites).
                           Test Files 23 passed (23) · Tests 574 passed (574)
npm run check:bundle-budget  306 files, 1538.4 KiB gzip; baseline 1406.4 KiB; within tolerance.

The one failing test is pre-existing and not from this branch.
tests/pr-handoff-stop.test.ts > emits handoff context only when the marker file exists sets a fake git dir to 0o555 to force a write failure, then asserts the marker was not written. This container runs as uid 0, and root ignores directory write bits, so the write succeeds. I reproduced it on a clean detached worktree at origin/main (883e725) with no local diff, and confirmed by probe that a touch succeeds inside a 0555 directory here. CI runs non-root, so it stays green there. Captured as #291 with a root-proof fix (inject the failure via a path whose parent is a regular file, rather than via permissions).

build and eval:rag:offline show as "not reached" in the gate's own summary because it stops at the first failure; both were then run directly and are quoted above. The build additionally needed this session's own dev server stopped first — guard-next-build.mjs refuses to run beside it (exit 76) — which is the guard working, not a defect.

An earlier run of the same gate had a second failure that was mine and is fixed in f116e00: the new spec imported blockExternalRequests from tests/helpers/phone-scroll, which silently enrolled it in the phone-chrome consumer list that tests/verify-phone-chrome.test.ts pins. It now keeps a local copy, since a desktop raster gate has no business in phone-chrome selection.

The new canvas gate is registered in all three hand-maintained lists that must agree — playwright.config.ts (testMatch and productionSpecPattern) and scripts/playwright-pr-shards.mjs (productionSpecFilePattern and shard group 3) — and tests/playwright-project-isolation.test.ts gained a fail-closed assertion for it, because "did not run" and "ran and skipped" are indistinguishable in a log otherwise. Its skip guard is asymmetric on purpose: without CI it skips with a reason naming the browser version; with CI set a missing engine feature fails. Both directions were verified locally (3 skipped without CI; 1 failed at the probe with CI=1).

check:bundle-budget passes at +9.4% against a 10% tolerance — about 8 KiB of gzip headroom. That drift is pre-existing (baseline captured 2026-08-04; this diff adds no dependency and its code delta is small), but it is thin enough to be worth saying out loud: the next feature-sized PR of any kind will trip that gate whatever it touches. Measured onto #252, which already tracks whether counting mockup chunks makes it a real signal. I did not refresh the baseline — doing that as an incidental step inside an unrelated PR is exactly how a budget stops meaning anything.

Expect the advisory document-viewer visual baseline to move if it moves at all. Its target is the single-page lithium document, whose holder geometry is deliberately unchanged — only multi-page documents get the bounded reading pane — but the fit scale now derives from the holder's content box rather than clientWidth minus a fixed 16px, which was 16px short at sm:p-4. That job is advisory and cannot block the merge.

Risk and rollout

• Risk: medium — the PDF reader is how a clinician reads a clinical source, so the failure mode that matters is showing the wrong page, or no page. Virtualization is the substantial rewrite here; the raster path itself (viewport, output scale, canvas sizing, render task) is carried over unchanged, and the per-run pageToCleanup isolation (Sentry 15801413), canvas zeroing, renderZoom debounce with its interim transform, and the isLikelyExpiredUrl → reportUrlExpired recovery are all preserved. Expiry recovery now also fires for a neighbour's range 403 while refusing to blank the reader's good page over a failed prefetch. Residual risk sits in scroll-derived page tracking on real layout, which is what the new browser gate exists to cover and which this container cannot exercise.
• Rollback: revert the offending commit — one commit per task, so any single item reverts alone while this PR is open. After a squash merge that guarantee ends — reverting a single item then means reverting hunks of the squash commit by hand.
• Provider or production effects: None. No provider-backed command was run; no Supabase, OpenAI, or Railway surface is touched.

Clinical Governance Preflight

scripts/pr-policy.mjs classifies this diff as clinicalRisk: false, because clinicalRiskPatterns does not match src/components/** unless the path also mentions auth/permission/privacy/security/upload/download/patient. That is the classifier under-approximating, not policy granting a pass — the comment at pr-policy.mjs:62 records PR #1489 shipping 205 therapy records past exactly this gap. AGENTS.md keys the preflight to behaviour, and this change alters source rendering (virtualization changes how a clinical source page is displayed) and document access (the signed-URL and decode-priority work), so it is completed here regardless of what the gate demands.

• Source-backed claims still require linked source verification before clinical use
• No patient-identifiable document workflow was introduced or expanded without explicit governance approval
• Supabase target remains Clinical KB Database (sjrfecxgysukkwxsowpy)
• Service-role keys and private document access remain server-only
• Demo/synthetic content remains clearly separated from real clinical sources
• Source metadata, review status, and outdated/unknown-source behavior remain conservative
• Deployment classification/TGA SaMD impact was checked when clinical decision-support behavior changed

Evidence for each, in the same order:

1. Nothing about citation, grounding, or answer rendering changed. The viewer still displays the stored source and links out to it; no claim is presented without its source.
2. No new document workflow. The rail shows the same indexed figures, from the same owner-scoped endpoints, to the same readers.
3. No Supabase configuration, migration, RPC, or env value is touched by this diff.
4. use-signed-image-url.ts is unmodified. Its identity-in-the-dedupe-key and cache-write-outside-the-shared-promise fixes were confirmed green before and after (tests/auth-signed-url-cache.dom.test.tsx, 6 passed). Signed URLs are still minted server-side per image through /api/images/{id}/signed-url; no client gained a broader minting path, and the batch route stays unwired.
5. Demo behaviour is unchanged. The new browser gate reads the synthetic clozapine demo document, which is the same corpus the rest of the Playwright suite uses and is separated from real sources by isDemoMode() exactly as before.
6. Source metadata and review status are untouched. The one behavioural change in this area is conservative in the right direction: a neighbour page that fails to render marks only its own slot and never blanks the page the reader is looking at, and an expiry signal from any page still triggers the existing bounded refresh.
7. Checked and unchanged. This is a rendering and performance change to how an existing source is displayed; it introduces no new clinical decision-support behaviour, no new claim, and no new inference, so the deployment classification is unaffected.

No RAG impact: line is required. No protected ranking surface is in scope: nothing under src/lib/rag/**, clinical-search, retrieval-selection, ranking-config, answer-ranking, the eval harness, the golden fixture, or the retrieval RPCs is touched. verify:pr-local still ran eval:rag:offline as part of the heavy route and it passed.

Notes

• Ledger #279's two remedies were refuted, and neither was actioned. Its measurements were re-derived after install and match exactly: browsers.json chromium revision 1234 = 151.0.7922.34, container = 141.0.7390.37, pdfjs-dist 6.2.108 calling getOrInsertComputed. One new datapoint for the record: Node 24.13.0 also lacks Map.prototype.getOrInsertComputed, so pdf.js 6 cannot be driven from this runtime headlessly either — the gap is not Playwright-specific.
• One correction to the Phase 3 brief. It states that collapsed audit rows "still mint signed URLs". SignedImage already defers behind an IntersectionObserver and a closed <details> is display: none, so that is at least doubtful — but it is a claim about real browser layout, and jsdom performs none, so nothing available here settles it. The windowing is justified on the mounting cost instead, which is real whether the section is open or shut. The test says so explicitly rather than implying the fetch claim was verified.
• Running this environment's npm ci needed --engine-strict=false as a one-off CLI flag: jsdom@30.0.1 requires Node ^24.15.0 and this container ships 24.13.0. .npmrc is unchanged, and the suite runs clean on 24.13.0.
• The demo fixture the canvas gate reads is genuinely two visibly different pages — page 1 is the monitoring protocol text, page 2 the embedded-image evidence page — verified by inflating the PDF's content streams rather than assumed, so "the flip changed the raster" is a real assertion and not a tautology.

---

Generated by Claude Code

Summary by CodeRabbit

• New Features

  • Improved PDF viewing with multi-page scrolling, faster page loading, zoom, rotation, keyboard navigation, and route synchronization.
  • Added progressive loading for document image rails, including a “Show more” option.
  • Prioritized visible images and deferred lower-priority loading to improve responsiveness.
  • Added browser validation for PDF canvas rendering and page navigation.

• Documentation

  • Documented PDF keyboard controls, viewer behavior, and completed document-viewer improvements.

• Tests

  • Expanded coverage for PDF virtualization, keyboard controls, image loading, canvas budgets, and browser rendering.

[supabase]

This pull request has been ignored for the connected project sjrfecxgysukkwxsowpy because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.

---

Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

[coderabbitai]

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 56 minutes

Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 21653674-eac7-44a6-9f15-766da11a3cac

📥 Commits

Reviewing files that changed from the base of the PR and between 8397aeb and c13ccbc.

📒 Files selected for processing (9)

• docs/branch-review-ledger.md
• docs/outstanding-issues.md
• docs/plans/document-viewer-redesign-plan.md
• docs/wiring-conventions.md
• src/components/document-viewer/document-rail-panels.tsx
• src/components/document-viewer/pdf-canvas-viewer.tsx
• src/components/document-viewer/source-panels.tsx
• tests/document-rail-image-window.dom.test.tsx
• tests/document-viewer-keyboard.dom.test.tsx

📝 Walkthrough

Walkthrough

The document viewer now supports virtualized PDF pages, bounded canvas retention, windowed image rails, prioritized image loading, keyboard reading controls, shared rotation state, and a Chromium canvas browser gate. Related plans, issue records, and Playwright collection rules were updated.

Changes

Document viewer rendering

Layer / File(s)	Summary
PDF raster budgeting and page virtualization src/components/document-viewer/canvas-raster-budget.ts, src/components/document-viewer/pdf-canvas-viewer.tsx, tests/document-viewer-page-virtualization.dom.test.tsx, tests/canvas-raster-budget.test.ts, tests/client-performance-boundaries.test.ts	PDF documents use page slots, bounded render-ahead, document-wide canvas budgets, cleanup, geometry measurement, and route synchronization.
Rail image windowing and request prioritization src/components/document-viewer/source-panels.tsx, src/components/document-viewer/document-rail-panels.tsx, src/components/clinical-dashboard/signed-image.tsx, tests/document-rail-image-window.dom.test.tsx, tests/signed-image.dom.test.tsx	Clinical and audit image rails render in batches. Signed images use rail-specific lookahead and conditional fetch priority.
Keyboard reader controls src/components/DocumentViewer.tsx, src/components/document-viewer/pdf-canvas-viewer.tsx, docs/wiring-conventions.md, tests/document-viewer-keyboard.dom.test.tsx	The viewer supports page navigation, fit-width, zoom, optional rotation, modified-key filtering, child-control isolation, and accessible keyboard documentation.

Canvas browser gate

Layer / File(s)	Summary
Canvas browser gate and Playwright wiring tests/ui-document-canvas.spec.ts, playwright.config.ts, scripts/playwright-pr-shards.mjs, tests/playwright-project-isolation.test.ts	The Chromium gate verifies painted canvas output, page count, page changes, route synchronization, and raster metrics. Production matching, shard assignment, and collection tests include the new spec.

Project records

Layer / File(s)	Summary
Phase and issue records docs/branch-review-ledger.md, docs/outstanding-issues.md, docs/plans/document-viewer-redesign-plan.md	The records mark Phases 2–3 as landed, document deferred crop-overlay and OffscreenCanvas work, archive the resolved canvas issue, and add updated follow-up items.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Reader
  participant DocumentViewer
  participant PdfCanvasViewer
  participant pdfjs
  Reader->>DocumentViewer: open document or change page
  DocumentViewer->>PdfCanvasViewer: provide document and route state
  PdfCanvasViewer->>pdfjs: fetch and render selected page slots
  pdfjs->>PdfCanvasViewer: return page geometry and raster state
  PdfCanvasViewer->>DocumentViewer: report page changes and route synchronization

Loading

Possibly related PRs

• BigSimmo/Database#1765: Defines the related Document Viewer Phase 3 handover.
• BigSimmo/Database#1687: Introduces shared document-viewer zoom and control integration.
• BigSimmo/Database#1686: Updates the Playwright production-spec sharding system.

Suggested labels: codex

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 68.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description follows the required template and clearly documents scope, verification results, risks, rollout, governance, and remaining work.
Title check	✅ Passed	The title clearly and concisely summarizes the main Phase 3 document viewer changes, including virtualization, rail windowing, keyboard controls, and canvas testing.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch claude/document-viewer-phase-3-bj5k5v

---

_{Comment @coderabbitai help to get the list of available commands.}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0436c3b495

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[BigSimmo]

@cursoragent Work the current open PR end-to-end. Confirm the PR number and GitHub head first from context. If more than one open PR could apply, stop and say which one you would use and why.

Fetch and start from the remote tip that matches that GitHub head. If the named branch ref is missing or stale, use the PR head ref. Preserve unrelated local WIP, including any local-only ledger commits; do not discard dirty work, and do not treat a local-ahead commit as the reviewed tip. Do not merge the PR, force-push, rebase, or arm auto-merge unless I explicitly ask. No provider-backed gates without separate approval. If you cannot push or resolve threads, diagnose and comment only; if inline replies fail, resolve when possible and put dispositions in the summary comment. If auto-merge is already armed, push only for a real blocker, and avoid pushes that would cancel in-flight required CI unless the push itself clears that blocker.

If the PR is already merged or closed: confirm the head and merge commit, note required-CI outcome, post one summary, and stop.

Goal: deep review plus Bugbot, fix actionable issues with the smallest correct changes, clear merge / required-CI / thread blockers, run strong local offline verification, push fixes, append the review ledger, and post one PR summary. Prefer thoroughness over speed. Regenerate large assets only when a fix requires it; then run the asset check and keep compatibility aliases byte-identical where the repo uses them.

Snapshot the GitHub head SHA: tip, base, behind/ahead, mergeable state, merge-tree versus origin/main (real conflict versus behind-but-clean), required checks on that tip including Production UI when selected, advisory separately, unresolved actionable threads. Missing checks while dirty are not green. If the tip moves mid-work, re-snapshot and continue from the new head.

Ledger-lookup against that GitHub head under the heavy review-and-fix scope for this PR. Already reviewed at this head with clean merge-tree, green required checks, and no new actionable threads → summarize, comment, stop unless I asked for a fresh superseding pass. Follow the repo review protocol.

Unblock once: real conflict → merge origin/main (prefer main’s shared queues; keep this PR’s notes); behind-but-clean → one sync, late if required CI is in flight; clean → leave. Labels like skip-branch-sync do not block a manual sync needed to clear a real blocker. No sync thrash. After any sync or push, re-snapshot tip, merge-tree, and required checks before declaring done. Dedupe the ledger if a merge touched it.

Review high-confidence delta risks only. Separate PR-introduced defects from pre-existing re-emitted debt. For generated assets, review contracts, aliases, cache, and manifests—not every generated line. If protected RAG or ranking surfaces are touched, say so before editing; fix PR-body policy text only when wrong or missing. Ignore bot noise. No nit spam or broad rewrites. Escalate verification by touched risk: clinical / RAG / privacy / migrations / auth → domain check plus production-readiness when warranted; UI / phone-chrome / routing / styling → ensure plus phone-chrome or UI gates when warranted; generated assets or docs inventory/links → asset or docs checks when warranted.

Fix P0/P1 always; clear scoped P2 when locally provable; else disposition and resolve. Required-check failures on this tip only; ignore advisory. Prefer reverting a bad autofix commit. After fixes: format and commit before push; smallest targeted proof; repo cheap gate; repo PR-local gate. No release, lighthouse, live eval, or live provider gates without approval.

Push only this PR’s fix commits. Append ledger for the final GitHub head under the heavy scope. Never push a ledger-only tip—include ledger in a real fix push, or leave a no-change ledger append local and unpushed.

One PR comment: tip, sync/merge-tree, fixed versus dispositioned, threads, required CI, decisive local gate lines, residual risks; merge left to me. Inline only for remaining human-needed P0/P1.

Stop when merge-tree is clean on the current GitHub tip, actionable threads are clear, required checks are green or clearly in progress, heavy gates for scope passed, summary posted, merge left to me.

[cursor]

You need to increase your spend limit or enable usage-based billing to run background agents. Go to Cursor

[Copilot]

Pull request overview

Adds scalable document viewing through PDF page virtualization, rail windowing, keyboard controls, and canvas browser coverage.

Changes:

• Adds budgeted PDF canvas virtualization and render-ahead.
• Windows image rails and prioritizes image loading.
• Adds keyboard navigation and focused browser/DOM tests.

Reviewed changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/components/DocumentViewer.tsx	Connects keyboard rotation.
src/components/document-viewer/pdf-canvas-viewer.tsx	Implements virtualized multi-page reading.
src/components/document-viewer/canvas-raster-budget.ts	Adds document-wide raster budgeting.
src/components/document-viewer/source-panels.tsx	Adds windowed image lists.
src/components/document-viewer/document-rail-panels.tsx	Uses windowed rail lists.
src/components/clinical-dashboard/signed-image.tsx	Adds explicit fetch priority.
tests/ui-document-canvas.spec.ts	Verifies painted PDF canvases.
tests/document-viewer-page-virtualization.dom.test.tsx	Tests virtualization behavior.
tests/document-viewer-keyboard.dom.test.tsx	Tests keyboard controls.
tests/document-rail-image-window.dom.test.tsx	Tests rail windowing.
tests/canvas-raster-budget.test.ts	Tests raster budgets.
tests/signed-image.dom.test.tsx	Tests image priority behavior.
tests/client-performance-boundaries.test.ts	Guards virtualization boundaries.
tests/playwright-project-isolation.test.ts	Guards canvas-spec collection.
playwright.config.ts	Registers the canvas spec.
scripts/playwright-pr-shards.mjs	Assigns the spec to CI shards.
docs/wiring-conventions.md	Documents keyboard bindings.
docs/plans/document-viewer-redesign-plan.md	Records Phase 3 status.
docs/outstanding-issues.md	Updates viewer follow-ups.
docs/branch-review-ledger.md	Records the branch review.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[coderabbitai]

Actionable comments posted: 6

🧹 Nitpick comments (3)

tests/ui-document-canvas.spec.ts (1)

89-129: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Bound the canvas readback before polling.

Line 105 copies the complete backing store on every poll. The loops only inspect a bounded sample grid. A large zoomed canvas can therefore allocate and scan far more pixels than the gate needs.

Draw into a bounded scratch canvas before calling getImageData.

Proposed refactor

-    const context = node.getContext("2d", { willReadFrequently: true });
-    if (!context) throw new Error("could not acquire a 2d context to read the raster back");
+    const sourceContext = node.getContext("2d");
+    if (!sourceContext) throw new Error("could not acquire a 2d context to read the raster back");
 
     const width = node.width;
     const height = node.height;
     if (width === 0 || height === 0) {
       return { width, height, sampled: 0, inkPixels: 0, signature: 0 };
     }
 
-    const stepX = Math.max(1, Math.floor(width / 240));
-    const stepY = Math.max(1, Math.floor(height / 240));
-    const { data } = context.getImageData(0, 0, width, height);
+    const sampleWidth = Math.min(width, 240);
+    const sampleHeight = Math.min(height, 240);
+    const sampleCanvas = document.createElement("canvas");
+    sampleCanvas.width = sampleWidth;
+    sampleCanvas.height = sampleHeight;
+    const sampleContext = sampleCanvas.getContext("2d", { willReadFrequently: true });
+    if (!sampleContext) throw new Error("could not create the canvas sample context");
+    sampleContext.drawImage(node, 0, 0, sampleWidth, sampleHeight);
+    const { data } = sampleContext.getImageData(0, 0, sampleWidth, sampleHeight);
 
     let inkPixels = 0;
     let sampled = 0;
     // FNV-1a over the sampled channel bytes. `>>> 0` after each step keeps it in
     // uint32 so the value is stable and comparable across runs.
     let signature = 0x811c9dc5;
-    for (let y = 0; y < height; y += stepY) {
-      for (let x = 0; x < width; x += stepX) {
-        const offset = (y * width + x) * 4;
+    for (let y = 0; y < sampleHeight; y += 1) {
+      for (let x = 0; x < sampleWidth; x += 1) {
+        const offset = (y * sampleWidth + x) * 4;

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/ui-document-canvas.spec.ts` around lines 89 - 129, Update readCanvas to
draw the source canvas into a bounded scratch canvas before calling
getImageData, using dimensions capped to the sample-grid limit while preserving
the returned dimensions and sampling/signature behavior. Ensure each poll reads
only the bounded scratch raster instead of copying and scanning the full backing
store.

tests/document-viewer-page-virtualization.dom.test.tsx (1)

149-153: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Make cancelIdleCallback remove the captured callback.

The stub returns a handle but never removes anything, so idleCallbacks keeps callbacks the viewer already cancelled. flushIdle then runs them. Today the effect only ever sets renderAheadReady to true, so the outcome is the same. If the render-ahead gate later becomes conditional, this stub would let a cancelled schedule still open it, and the "only once idle" assertion would stop meaning what it says.

♻️ Proposed change

-  vi.stubGlobal("requestIdleCallback", (callback: () => void) => {
-    idleCallbacks.push(callback);
-    return idleCallbacks.length;
-  });
-  vi.stubGlobal("cancelIdleCallback", () => {});
+  vi.stubGlobal("requestIdleCallback", (callback: () => void) => {
+    idleHandles.set(nextIdleHandle, callback);
+    return nextIdleHandle++;
+  });
+  vi.stubGlobal("cancelIdleCallback", (handle: number) => {
+    idleHandles.delete(handle);
+  });

flushIdle then drains idleHandles.values() and clears the map.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/document-viewer-page-virtualization.dom.test.tsx` around lines 149 -
153, Update the requestIdleCallback/cancelIdleCallback stubs to track callbacks
by their returned handle, and have cancelIdleCallback remove the corresponding
entry. Adjust flushIdle to drain the tracked callback values and clear the
collection so cancelled callbacks are never executed.

src/components/document-viewer/pdf-canvas-viewer.tsx (1)

773-783: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Use the render-time rotation reset.

useEffect commits one render with the pre-rotation referenceGeometry, then schedules another render to clear it. Align this reset with DocumentViewer’s existing render-time adjustment.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/document-viewer/pdf-canvas-viewer.tsx` around lines 773 - 783,
Replace the useEffect-based rotation reset around prevRotationRef with a
render-time adjustment, matching DocumentViewer’s existing pattern. When
rotation changes, update prevRotationRef and clear referenceGeometry during
render so no frame uses the stale pre-rotation geometry.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/branch-review-ledger.md`:
- Line 829: Append a superseding record for the existing 2026-08-09 ledger entry
using npm run ledger:append with --supersede, without editing the original row.
Re-run the reported gates and include each exact output line showing pass,
skip-with-reason, or failure results instead of summary phrases such as “build
OK” or “within tolerance.”

In `@docs/outstanding-issues.md`:
- Line 335: Remove the duplicate issue `#291` row from docs/outstanding-issues.md,
preserving the existing open issue `#284` row as the single ledger entry for this
root-only pr-handoff-stop.test.ts failure. Do not alter or delete the retained
issue’s details.
- Line 334: Update the `#290` decision criteria in the outstanding-issues
documentation to require explicit CI thresholds for flipToPaintedMs and
longTaskTotalMs in addition to longestTaskMs. Specify that the decisive CI log
lines and aggregate measurements must be recorded before closing the issue or
removing the Phase 3 plan row, while preserving the existing OffscreenCanvas
scope.
- Line 300: Remove the unsupported “pre-existing and not from this branch”
attribution and its claim that the Phase 3 diff has no bundle impact from issue
`#252`. Retain the measured headroom and urgency only if supported by the current
documented artifact, or replace the comparison with decisive bundle-budget
outputs for both revisions.

In `@src/components/document-viewer/pdf-canvas-viewer.tsx`:
- Around line 908-925: Update the ArrowLeft/PageUp and ArrowRight/PageDown
branches in the key handler to compute relative destinations from
pageRef.current instead of the stale page value, while preserving the existing
absolute Home and End behavior.

In `@src/components/document-viewer/source-panels.tsx`:
- Around line 486-493: Update DocumentViewerRail and its requestedCount state so
the image window resets to RAIL_IMAGE_WINDOW whenever a document-specific
collection key changes, while preserving the existing clamped visibleCount
behavior. Pass the key into the relevant state-reset mechanism and add a
regression test covering an expanded long list followed by a different long
list.

---

Nitpick comments:
In `@src/components/document-viewer/pdf-canvas-viewer.tsx`:
- Around line 773-783: Replace the useEffect-based rotation reset around
prevRotationRef with a render-time adjustment, matching DocumentViewer’s
existing pattern. When rotation changes, update prevRotationRef and clear
referenceGeometry during render so no frame uses the stale pre-rotation
geometry.

In `@tests/document-viewer-page-virtualization.dom.test.tsx`:
- Around line 149-153: Update the requestIdleCallback/cancelIdleCallback stubs
to track callbacks by their returned handle, and have cancelIdleCallback remove
the corresponding entry. Adjust flushIdle to drain the tracked callback values
and clear the collection so cancelled callbacks are never executed.

In `@tests/ui-document-canvas.spec.ts`:
- Around line 89-129: Update readCanvas to draw the source canvas into a bounded
scratch canvas before calling getImageData, using dimensions capped to the
sample-grid limit while preserving the returned dimensions and
sampling/signature behavior. Ensure each poll reads only the bounded scratch
raster instead of copying and scanning the full backing store.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 2152490a-82d5-455b-8022-1ab8a3240c4d

📥 Commits

Reviewing files that changed from the base of the PR and between 73d0966 and 8397aeb.

📒 Files selected for processing (20)

• docs/branch-review-ledger.md
• docs/outstanding-issues.md
• docs/plans/document-viewer-redesign-plan.md
• docs/wiring-conventions.md
• playwright.config.ts
• scripts/playwright-pr-shards.mjs
• src/components/DocumentViewer.tsx
• src/components/clinical-dashboard/signed-image.tsx
• src/components/document-viewer/canvas-raster-budget.ts
• src/components/document-viewer/document-rail-panels.tsx
• src/components/document-viewer/pdf-canvas-viewer.tsx
• src/components/document-viewer/source-panels.tsx
• tests/canvas-raster-budget.test.ts
• tests/client-performance-boundaries.test.ts
• tests/document-rail-image-window.dom.test.tsx
• tests/document-viewer-keyboard.dom.test.tsx
• tests/document-viewer-page-virtualization.dom.test.tsx
• tests/playwright-project-isolation.test.ts
• tests/signed-image.dom.test.tsx
• tests/ui-document-canvas.spec.ts