diff --git a/docs/branch-review-ledger.md b/docs/branch-review-ledger.md index 9a717c5250..bd68740a7b 100644 --- a/docs/branch-review-ledger.md +++ b/docs/branch-review-ledger.md @@ -752,6 +752,8 @@ Records before 2026-07-28 were written by hand and had drifted: 146 lines carrie | 2026-08-08 | claude/ds-tap-and-linkaction | 6916c80526603514d91bd29d224959dd420af59c | M5 LinkAction tone refusal plus re-measured corrections to outstanding-issues #270, #118 and #269 — final reviewed head, adds the tone?: never fix, its type-contract test and both regenerated manifests | PR #1720, superseding the 824c1b74a record. Codex found the Omit form still accepted tone through a spread; verified with a focused tsc probe before changing anything (Omit accepted the spread with no diagnostic, tone?: never rejected it with TS2345), because excess-property checking only fires on object literals. Fixed with tone?: never plus a type-level contract test that stops compiling if the prop widens back. CodeRabbit's future-dated finding fixed in ff307cc5b. CodeRabbit's ledger-scope finding does not apply: that row records a different ref and head and was accurate as written, but a superseding row for the final #1719 head was appended anyway since its scope grew after the review pass | tsc -p tsconfig.typecheck.json --noEmit exit 0 zero diagnostics; lint exit 0; check:design-system-contract exit 0 (676 production files, legacy shadow aliases 228 confirming the #262 re-measure, adoption 53 components 55 roots, design-sync 53 components and 7 guidelines); check-icon-scale.mjs --strict exit 0; vitest threads pool 3 files 164 tests passed; check:outstanding-issues pass; check:branch-review-ledger pass; prettier --check . pass whole-tree; main merged in with merge-tree proven clean first and an id-set proof over both merge parents showing 274 ids each side, none lost, none invented | | 2026-08-08 | claude/ds-close-276 (PR #1724) | 75c89993f3ea23b70a250f605b21437b4ea9aac8 | PR #1724 review-and-fix | fixed Codex P2 wrong #118 Lighthouse cause (150 overwrite vs 151 pin); dispositioned CodeRabbit #276 archive claim as false (issues:done move); merge-tree clean; required CI was green on prior tip 8ae8c48f; no Bugbot findings | check:outstanding-issues pass; prettier --check docs/outstanding-issues.md pass; no provider-backed checks | | 2026-08-08 | claude/ds-close-276 (PR #1724) | 4baa9a1b42fa05731a6f983b3e0d0ebbd37f5271 | PR #1724 review-and-fix | synced origin/main (#1725 conflict on outstanding-issues resolved by preferring main queue then re-applying #276 done + corrected #118 diagnosis); Codex P2 fixed; CodeRabbit #276 archive claim dispositioned false; merge-tree clean after sync | check:outstanding-issues pass; prettier --check docs/outstanding-issues.md pass; merge-tree clean vs origin/main; no provider-backed checks | -| 2026-08-08 | PR #1740 / claude/inpage-nav-info-pages-v8rhnd | b67f33f65e00529eb0dd1682d6925e708243ee93 | Extract InPageNavHeader (default in-page nav template) + convert differentials detail; PR 1 of 3 | HANDOFF. Template extracted from the duplicated DocumentViewer/differential-detail markup into src/components/in-page-nav/ (InPageNavHeader, PageSection/toDocumentSections, usePageSectionWeights); differential-detail-page converted (-207 lines), behaviour-neutral. section-index.ts untouched so document tests unaffected. DocumentViewer deliberately NOT converged (owns h1, edge-glass-header, visual baselines) - follow-up. Anchor-offset hook generalisation deferred to PR 2 where it is consumed. 3 source-scanning contracts + addon-slot guard updated to follow the markup and additionally assert adoption; addon-slot scan widened to InPageNavHeader or it would go silent for every future adopter. Single failing test (pr-handoff-stop) is a root-uid artifact: chmod 0555 does not block root, reproduced with work stashed on clean tree. | verify:cheap 5618 passed/1 failed (root artifact); verify:pr-local same, short-circuits at test so build not reached; build run separately - Compiled successfully in 53s + client bundle secret check passed; verify:phone-chrome EXIT=0 (stage1 119 passed, stage2 7 passed 23.5s, full UI policy auto not selected); lint/typecheck/prettier --check . clean. No provider-backed gates. Deps installed with engine check relaxed (user-approved; Node 24.13.0 vs jsdom floor 24.15) - lockfile untouched. | +| 2026-08-08 | claude/document-viewer-optimization-tu8tnj | 98b799a372b1e341c86e8807d5cf37e987413e49 | document viewer phone/PWA rework: CSP-blocked native reader removed, one toolbar, fit-mode pinch, canvas pixel budget, source-first phone order, in-window detail-refetch guard, pdf.js on-demand fetch + teardown, image/signed-URL wins | ship: PR #1741 | lint, typecheck, test 5625 pass (1 pre-existing root-container failure), build, check:rag:fixtures, check:bundle-budget 1499.8 KiB vs base 1500.0 KiB, check:runtime, check:installed-lock-parity, format:changed; verify:ui not run (container Chromium 141 cannot raster pdfjs 6, see #278) | +| 2026-08-08 | claude/document-viewer-optimization-tu8tnj | 2359e158cb7bca5954e9c5ee84ca0766964ad901 | PR #1741 document-viewer phone/PWA review-and-fix | supersede: fixed Production UI phone Zoom/section-trigger; handlePdfLoadSuccess clamp; prior P1/P2 fixes retained; merge-tree clean | prior verify:cheap+pr-local green; ui-smoke selectors fixed for overflow Zoom + revealPhoneHeaderControl; no provider gates | | 2026-08-08 | claude/document-image-mobile-view-30xzw8 | 2394d903a6ca1ba7a84e380c9ed5cada038fa5c0 | document-viewer phone image layout + lightbox geometry (PR #1737) | implemented: capped rail/body grid tracks, removed aspect-ratio min-height transfer, rebuilt phone image viewer (legible open scale, rotation re-fit, clamped pan, double-tap, footer controls) | lint, typecheck, test (5647 pass / 1 pre-existing fail), build, eval:rag:offline, check:bundle-budget, all verify:pr-local static steps by hand; browser gates blocked by #255 | | 2026-08-08 | claude/document-image-mobile-view-30xzw8 | d257df7e11913db1d367535171fac726f47e7f1c | PR #1737 document-viewer phone image review-and-fix | fixed P1 expand fixture/threshold + P2 double-tap stage coords/pointer-up + resize re-clamp; Production UI timeout root cause cleared; merge-tree clean | verify:pr-local PASS (525 files/5653 tests); lint; typecheck; focused vitest 64/64; Production UI delegated to CI | +| 2026-08-08 | PR #1740 / claude/inpage-nav-info-pages-v8rhnd | b67f33f65e00529eb0dd1682d6925e708243ee93 | Extract InPageNavHeader (default in-page nav template) + convert differentials detail; PR 1 of 3 | HANDOFF. Template extracted from the duplicated DocumentViewer/differential-detail markup into src/components/in-page-nav/ (InPageNavHeader, PageSection/toDocumentSections, usePageSectionWeights); differential-detail-page converted (-207 lines), behaviour-neutral. section-index.ts untouched so document tests unaffected. DocumentViewer deliberately NOT converged (owns h1, edge-glass-header, visual baselines) - follow-up. Anchor-offset hook generalisation deferred to PR 2 where it is consumed. 3 source-scanning contracts + addon-slot guard updated to follow the markup and additionally assert adoption; addon-slot scan widened to InPageNavHeader or it would go silent for every future adopter. Single failing test (pr-handoff-stop) is a root-uid artifact: chmod 0555 does not block root, reproduced with work stashed on clean tree. | verify:cheap 5618 passed/1 failed (root artifact); verify:pr-local same, short-circuits at test so build not reached; build run separately - Compiled successfully in 53s + client bundle secret check passed; verify:phone-chrome EXIT=0 (stage1 119 passed, stage2 7 passed 23.5s, full UI policy auto not selected); lint/typecheck/prettier --check . clean. No provider-backed gates. Deps installed with engine check relaxed (user-approved; Node 24.13.0 vs jsdom floor 24.15) - lockfile untouched. | diff --git a/docs/design-system/ADOPTION.md b/docs/design-system/ADOPTION.md index e6b82e066c..1bdf76aa66 100644 --- a/docs/design-system/ADOPTION.md +++ b/docs/design-system/ADOPTION.md @@ -133,9 +133,11 @@ src/components/DocumentViewer.tsx ``` `DocumentFrame` is **built** (`src/components/ui/document-frame.tsx`) and used by -`DocumentViewer` as a shell-only surround (no `controls` toolbar — PDF chrome stays on -`PdfCanvasViewer`). It is not yet design-sync registered among the 53 published visual -exports. Do not invent a second frame or add inversion/filters; route document renders through +`DocumentViewer` as the single owner of viewing chrome: the `controls` toolbar carries page +navigation, zoom, fit, rotation, the viewing aid and fullscreen, and `PdfCanvasViewer` renders +source pixels only. There is exactly one toolbar and one page readout in the viewer, and the +contract test `tests/document-frame-contract.test.ts` holds that. It is not yet design-sync +registered among the 53 published visual exports. Do not invent a second frame or add inversion/filters; route document renders through the existing viewer + frame. Keep every `role="alert"` semantic; route announcements through the announcer policy rather than deleting roles, because many `role="status"` sites are implicit polite live regions with no `aria-live` attribute and removing the role without an diff --git a/docs/outstanding-issues.md b/docs/outstanding-issues.md index dc0e3e09b9..4b8f7dab37 100644 --- a/docs/outstanding-issues.md +++ b/docs/outstanding-issues.md @@ -165,7 +165,7 @@ removed after current-main verification; it is not missing recommended work. | 112 | `#257` | Optional | High — formulation/specifiers flake | Standing until second reproduction | 15–30 min | Single unreproduced ui-formulation flake when run with ui-specifiers — record a second sighting only; do not quarantine until three on the same SHA. **Stop:** do not weaken assertions. | - + ## Open items > **Merged-main canary update (2026-07-23, run `30018289898`):** the new structured report correctly recorded evaluated tree `c24f2e8f2d30d0c59fc1eba025d3dcd63478137e`, run/attempt identity and `cross-region-runner` latency context. Golden retrieval remained 36/36 with document/content recall 1.0 and no failed cases. The 44-case answer gate had grounded-supported and unsupported-correct rates of 1.0, but failed because `neuroleptic-side-effect-escalation` again returned one citation where two are required (citation-failure rate 0.0227). `admission-discharge-comparison` again omitted the specific AKG admission document after `comparison_source_extractive_fallback`; `admission-discharge-coverage-paraphrase` was advisory-only at 24,870 ms. Answer cost was reported as `$0.234736`. Do not retry immediately: retain this as the first structured datapoint, compare it with the scheduled 2026-07-26 report, and keep retrieval/ranking unchanged. @@ -315,6 +315,12 @@ removed after current-main verification; it is not missing recommended work. | #275 | P2 | task | The shared filter trigger carries arbitrary spacing values inherited from DocumentFilterTrigger | **Outcome:** the phone filter trigger expresses its measurements as named tokens rather than bracketed values. **Detail:** `result-filter-control.tsx`'s `ResultFilterTrigger` uses `pr-[0.6875rem]`, `h-[1.0625rem]`/`min-w-[1.0625rem]` for the badge, and the raw breakpoint window `min-[414px]:max-[429px]` for the label. CodeRabbit flagged these against the design-token rule in PR #1706. Every one of them is copied *verbatim* from `DocumentFilterTrigger`, which shipped on main earlier and is the component this one was deliberately lifted from so the two cannot drift — so the finding is real but its scope is both call sites, not the new one. Changing only the copy would reintroduce exactly the drift the extraction removed, and each value carries a measured justification in its own comment (the asymmetric padding answers a stroked glyph against a filled pill; the breakpoint window is the one band that is single-line and short of width). **Next:** tokenise in `@theme` once, then update the trigger — there is now only one implementation, so it is a single edit. Confirm the badge and padding render identically at 393/402/414/430px before and after. **Stop:** do not tokenise the trigger without also retiring the values from the documents original, and do not treat this as licence to change the measurements themselves. | CodeRabbit review on PR #1706; DocumentFilterTrigger on main | 2026-08-07 | | #277 | P2 | issue | docs/design-system/HANDOVER-2026-08-07.md is cited as provenance by nine ledger rows but is measurably wrong | Outcome: no session scopes design-system work from a document whose figures have already been disproved. Evidence: rows #261, #262, #264, #265, #266, #267, #268, #269 and #270 all carry 'session 2026-08-07 — design-system HANDOVER-2026-08-07 Track A1 handoff (PR #1678)' as their Source, and docs/design-system/README.md links it as 'measured state, the ordered plan'. Measured wrong so far, all corrected into the rows themselves rather than the document: its '229 --shadow-tight aliases' is the seven-token legacyShadowAliases total mislabelled as one token (real figure 100 sites across 55 files, total 228); its adoption figure was 24 unadopted against a measured 23; its claim that visual baselines cannot be generated on Windows is half true and led to the wrong conclusion, since the ubuntu CI job already produces the ones that count; and #270's '22 call sites pair a tap token with a dead numeric height' does not survive re-measurement at all (zero same-variant pairs, 84 cross-variant responsive step-downs that are not dead). The document itself still asserts the originals. Next: cheapest fix is a superseded banner at the top naming the ledger rows as the current source of truth, plus the same in docs/design-system/README.md's link text — not a rewrite, because the corrections already live in the rows and duplicating them re-creates the drift. If the live path should leave docs/design-system/, move the file to docs/archive/ (or the design-system archive) and update inbound links per docs/README.md; do not delete it, because the nine Source citations, the PR/commit record, and the handover's verification/gotcha sections are provenance the ledger is meant to preserve. Stop: do not re-copy its figures into any new plan or handover, do not delete the evidence, and do not silently correct it in place, which would leave the nine Source citations pointing at a document that no longer says what those rows were derived from. | session 2026-08-08 — measured while closing #263 follow-ups across PRs #1719 and #1720 | 2026-08-08 | | #278 | P3 | issue | The document-viewer visual baseline bakes in viewport-pinned chrome that overlaps content | Measured 2026-08-08 while adopting the baselines (#118 / PR #1729). The document-viewer target clips #main-content, which is 1196x2903 against a 900px viewport, and contains viewport-pinned chrome: the sm:sticky sm:top-0 document header (DocumentViewer.tsx:1028) and the sm:fixed search composer (DocumentViewer.tsx:1511). Playwright stitches an oversized element clip, so both composite partway down the image and OVERLAP the content behind them — the cited-excerpt card and a source passage are partly covered in the committed golden. Position tracks total content height, so any content-height change above them moves the pinned chrome and inflates the diff well beyond what actually changed. NOT a product bug and NOT a #1705 regression: the pre-#1705 candidate from run 31249978408 shows the same overlap, so it is inherent to the target's design. The capture is deterministic, so the comparison still means something — five of six candidates were byte-identical by SHA-256 across two independent CI runs. Next: narrow that target's clip to a smaller locator, or add the pinned chrome to the target's mask array (the spec already supports mask, with a comment warning a mask is a hole in the gate). Stop: do not fix this by capturing fullPage — the spec bans it because ledger #093 leaves a hidden duplicate page root under CI load. | session 2026-08-08 — visual baseline adoption, #118 | 2026-08-08 | +| #279 | P2 | issue | pdf.js 6 cannot raster in this container's Chromium, so no browser gate covers the viewer canvas | **Outcome:** viewer canvas behaviour is provable by a gate rather than only by unit test and device. **Detail:** the document route renders 'this[#methodPromises].getOrInsertComputed is not a function' instead of a page in Chromium 141.0.7390.37 at /opt/pw-browsers — pdfjs-dist 6.2.108 calls a Map builtin that shipped after 141. Reproduced identically on bc33d41 and on the viewer-optimization branch, so it is the environment, not a regression. Consequence: every Playwright assertion that depends on a painted canvas is unprovable here, which covers the canvas pixel budget and the fit-mode pinch gesture landed in this branch, and any future 'verify:ui' viewer journey run in this container or a like-configured cloud session. **Next:** confirm whether CI's Chromium is newer than 141 (if so this is container-only and should be recorded as such); otherwise either bump the pinned Playwright browser build or pin pdfjs-dist to a release whose baseline the gate's browser meets. **Stop:** do not weaken a viewer assertion to make it pass in this container. | session 2026-08-08 document-viewer optimisation; /opt/pw-browsers/chromium-1194 = Chromium 141.0.7390.37 | 2026-08-08 | +| #280 | P2 | task | Physical iPhone acceptance is owed for the viewer pinch gesture and the canvas pixel budget | **Outcome:** the two phone-only viewer fixes are confirmed on the device class they were written for. **Detail:** the viewer-optimisation branch revives pinch-to-zoom in fit mode (it was gated off in the default state, so a pinch reached neither the viewer nor the browser) and adds a canvas pixel budget so WebKit stops blanking the page above roughly 2.3x zoom on a dpr-3 display. Neither is verifiable in this container (see the Chromium/pdf.js row) and neither is a Chromium behaviour anyway — the canvas ceiling is a WebKit limit and the touch-action contention is a Safari gesture question. **Next:** on a real iPhone, in Safari and in the installed PWA: pinch a freshly opened document and confirm it zooms without first tapping a control; zoom to maximum and confirm the page stays painted rather than going blank; confirm a pinch that drifts vertically is not cancelled mid-gesture by the holder's 'touch-action: pan-y' (the mitigation if it is, is switching touch-action to none while two pointers are down — the gesture hook already tracks pointer count and exposes 'pinching'). Record the result against docs/phone-chrome-physical-acceptance.md. **Stop:** do not re-gate pinch on '!fitWidth' to resolve a gesture-contention finding — that restores the original defect. | session 2026-08-08 document-viewer optimisation; docs/design-system/COMPONENTS.md phone clause | 2026-08-08 | +| #281 | P2 | rec | The phone document route renders two clinical-summary surfaces and neither is canonical | **Outcome:** one clinical summary on the document route, chosen deliberately. **Detail:** a phone reader gets the gradient 'High-yield clinical summary' card (DocumentClinicalSummary, built by buildDocumentClinicalSummaryModel) and, further down, the rail's '#source-summary' / 'high-yield-summary' disclosure (DocumentSectionSummary + FormattedHighYieldSummary + BadgeCluster). They render the same document.summary row two different ways. The rail is not hidden on phones — only its DocumentSectionIndexCard is lg:block — so both appear. Only the rail panel carries the section anchor, so the more prominent card is the unnavigable one. Note the two disagree about emptiness as well: the card now renders nothing when the model yields no usable text, while the rail panel still renders for its label badges, which is why 'hasStoredSummary' was deliberately left keyed to the stored row rather than to card content. **Next:** decide which rendering is canonical — this is a clinical-content judgement about how a summary should read, not a layout fix — then delete the other and give the survivor the 'source-summary' anchor. If the rail's badges are the part worth keeping, they can move without the second summary body. **Stop:** do not merge the two renderings mechanically; they format clinical text differently and the difference is the decision. | session 2026-08-08 document-viewer optimisation; document-rail-panels.tsx; document-clinical-summary.tsx | 2026-08-08 | +| #282 | P3 | task | Probe the corpus for JBIG2/JPX before deciding whether pdf.js needs its decoder assets shipped | **Outcome:** a measured decision about pdf.js's cMap/standard-font/WASM assets rather than an assumption either way. **Detail:** getDocument is configured with url plus the on-demand fetch flags and nothing else, so 'wasmUrl', 'standardFontDataUrl', 'cMapUrl' and 'iccUrl' are all unset. pdfjs-dist ships those assets (wasm 1.5 MB, standard_fonts 804 KB, cmaps 1.7 MB) and nothing copies them into public/. With wasmUrl null, 'useWorkerFetch' resolves false and the WASM image decoders cannot load, so JBIG2 and JPEG2000 images fall back to the JS decoders or fail; those are exactly the encodings a scanned guideline uses, and this repo runs an OCR pipeline, which implies scanned sources exist. Non-embedded standard-14 fonts fall back to system fonts, which is a fidelity risk on a clinical document rather than a failure. **Next:** sample the real corpus for JBIG2/JPX-encoded images and for PDFs relying on the standard 14 before shipping ~2 MB of static assets; if the corpus does use them, copy into public/pdfjs, set the URLs, and add immutable cache headers in next.config.ts (public/ is not counted by check:bundle-budget, so there is no budget risk — the cost is bytes over the wire on first use). **Stop:** do not ship the assets on the assumption alone. | session 2026-08-08 document-viewer optimisation; node_modules/pdfjs-dist/types/src/display/api.d.ts | 2026-08-08 | +| #283 | P3 | rec | The 100-id batch signed-URL route still has no caller | **Outcome:** either the batch minter is used or it is retired, rather than sitting as an untested, unreachable privileged surface. **Detail:** src/app/api/images/signed-urls/route.ts POSTs up to 100 image ids and returns their signed URLs, with its own rate limit, owner scoping and committed-generation filter. Nothing in src/ calls it — only tests/private-access-routes.test.ts imports it. The viewer resolves images one at a time through use-signed-image-url.ts. The 2026-08-08 pass added in-flight deduplication there, which removes the duplicate-consumer case (a figure and its lightbox racing for the same asset) but not the many-distinct-images case: a page of N figures is still N round trips where one batch call would do. **Next:** decide deliberately — wire the rail/filmstrip to the batch route when a page mounts several distinct images at once, or delete the route and its tests. The cost of leaving it is a privileged endpoint no product code exercises. **Stop:** if wiring it, keep the per-image endpoint for the lightbox's retry path; do not make the batch the only way to mint a URL. | session 2026-08-08 document-viewer optimisation; src/app/api/images/signed-urls/route.ts | 2026-08-08 | +| #284 | P3 | issue | tests/pr-handoff-stop.test.ts fails whenever the suite runs as root | **Outcome:** 'npm run test' is green in a root container, so a real failure is not hidden behind a known one. **Detail:** 'pr-handoff-stop hook > emits handoff context only when the marker file exists' expects markerExists('sess-readonly') to be false — it makes the marker directory read-only and asserts the hook could not write there. Root ignores the permission bits, so the write succeeds and the assertion fails. Reproduced on an unmodified bc33d41 checkout as well as on the viewer-optimisation branch, so it is environment-dependent, not a regression. Cost is that every full-suite run in a root container reports '1 failed', which trains readers to skim past the failure count. **Next:** skip the case when 'process.getuid?.() === 0' with an explicit reason, or drop privileges for that assertion. **Stop:** do not delete the coverage — the read-only case is the point of the test on a normal user account. | session 2026-08-08 full-suite runs; reproduced on bc33d41 | 2026-08-08 | ## Resolved / archive diff --git a/docs/plans/document-viewer-phase2-unified-chrome.md b/docs/plans/document-viewer-phase2-unified-chrome.md index 1fc2ebb273..fcecd54fb1 100644 --- a/docs/plans/document-viewer-phase2-unified-chrome.md +++ b/docs/plans/document-viewer-phase2-unified-chrome.md @@ -52,8 +52,8 @@ Independent slices keep each failure class attributable and revertible. ``` DocumentViewer - └─ DocumentFrame (state + optional controls) - ├─ PdfCanvasViewer | NativePdfEmbed // PDF pixels + page/rotate/fullscreen owner + └─ DocumentFrame (state + the viewer's one toolbar + fullscreen) + ├─ PdfCanvasViewer // PDF pixels only; no chrome └─ NonPdfSourcePreview // image/text/download └─ ImageLightbox (URL or endpoint) diff --git a/lighthouse-budget.json b/lighthouse-budget.json index f2781e9272..ebd753d665 100644 --- a/lighthouse-budget.json +++ b/lighthouse-budget.json @@ -20,75 +20,75 @@ }, "baseline": { "desktop-documents-search": { - "lcpMs": 790.8669, + "lcpMs": 816.54435, "cls": 0.1192626548872241, - "tbtMs": 0, - "fcpMs": 390.7482, - "chromeVersion": "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) HeadlessChrome/150.0.0.0 Safari/537.36" + "tbtMs": 2.499999999999943, + "fcpMs": 381.4543, + "chromeVersion": "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) HeadlessChrome/151.0.0.0 Safari/537.36" }, "desktop-dsm": { - "lcpMs": 836.0241999999998, + "lcpMs": 843.7154499999999, "cls": 0.01342963896133897, - "tbtMs": 14.999999999999886, - "fcpMs": 381.7212, - "chromeVersion": "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) HeadlessChrome/150.0.0.0 Safari/537.36" + "tbtMs": 0, + "fcpMs": 383.9187, + "chromeVersion": "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) HeadlessChrome/151.0.0.0 Safari/537.36" }, "desktop-forms": { - "lcpMs": 870.6212000000005, + "lcpMs": 821.8480500000005, "cls": 0.05606642664873546, "tbtMs": 0, - "fcpMs": 383.5404, - "chromeVersion": "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) HeadlessChrome/150.0.0.0 Safari/537.36" + "fcpMs": 382.6329, + "chromeVersion": "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) HeadlessChrome/151.0.0.0 Safari/537.36" }, "desktop-root": { - "lcpMs": 812.1882499999992, + "lcpMs": 819.4945, "cls": 0.006778250591016549, - "tbtMs": 16, - "fcpMs": 340.8753, - "chromeVersion": "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) HeadlessChrome/150.0.0.0 Safari/537.36" + "tbtMs": 14.5, + "fcpMs": 339.8315, + "chromeVersion": "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) HeadlessChrome/151.0.0.0 Safari/537.36" }, "desktop-therapy-compass": { - "lcpMs": 895.1419000000001, + "lcpMs": 826.6481999999996, "cls": 0, "tbtMs": 0, - "fcpMs": 389.1834, - "chromeVersion": "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) HeadlessChrome/150.0.0.0 Safari/537.36" + "fcpMs": 383.6996, + "chromeVersion": "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) HeadlessChrome/151.0.0.0 Safari/537.36" }, "mobile-documents-search": { - "lcpMs": 2314.371, + "lcpMs": 2307.8, "cls": 0, - "tbtMs": 383.43999999999915, - "fcpMs": 2314.371, - "chromeVersion": "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) HeadlessChrome/150.0.0.0 Safari/537.36" + "tbtMs": 343.34200000000055, + "fcpMs": 2307.8, + "chromeVersion": "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) HeadlessChrome/151.0.0.0 Safari/537.36" }, "mobile-dsm": { - "lcpMs": 2339.109, + "lcpMs": 2336.558, "cls": 0, - "tbtMs": 352.6870000000008, - "fcpMs": 2339.109, - "chromeVersion": "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) HeadlessChrome/150.0.0.0 Safari/537.36" + "tbtMs": 318.4069999999979, + "fcpMs": 2336.558, + "chromeVersion": "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) HeadlessChrome/151.0.0.0 Safari/537.36" }, "mobile-forms": { - "lcpMs": 2280.29, + "lcpMs": 2306.606, "cls": 0.08045469317717091, - "tbtMs": 326.0409999999997, - "fcpMs": 2280.29, - "chromeVersion": "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) HeadlessChrome/150.0.0.0 Safari/537.36" + "tbtMs": 295.9940000000006, + "fcpMs": 2306.606, + "chromeVersion": "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) HeadlessChrome/151.0.0.0 Safari/537.36" }, "mobile-root": { - "lcpMs": 3929.966, + "lcpMs": 2308.077, "cls": 0, - "tbtMs": 785.6220000000021, - "fcpMs": 3929.966, - "chromeVersion": "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) HeadlessChrome/150.0.0.0 Safari/537.36" + "tbtMs": 292.7889999999693, + "fcpMs": 2308.077, + "chromeVersion": "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) HeadlessChrome/151.0.0.0 Safari/537.36" }, "mobile-therapy-compass": { - "lcpMs": 2336.784, + "lcpMs": 2667.986, "cls": 0, - "tbtMs": 333.18399999999883, - "fcpMs": 2336.784, - "chromeVersion": "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) HeadlessChrome/150.0.0.0 Safari/537.36" + "tbtMs": 279.28999999999996, + "fcpMs": 2335.23, + "chromeVersion": "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) HeadlessChrome/151.0.0.0 Safari/537.36" } }, - "updatedAt": "2026-08-08T05:38:39.988Z" + "updatedAt": "2026-08-08T16:48:18.630Z" } diff --git a/src/app/globals.css b/src/app/globals.css index 17ee21e81e..6f2f8cf0a6 100644 --- a/src/app/globals.css +++ b/src/app/globals.css @@ -3365,11 +3365,12 @@ html[data-motion="reduced"] .source-capsule-hit[aria-expanded="true"]:hover .sou display: none !important; } - /* Transitional element-name chrome hide — do not extend this list. */ + /* Transitional element-name chrome hide — do not extend this list. The viewer + toolbar left it when DocumentFrame took ownership: that band carries + `data-print-hide`, so the attribute rule above already covers it. */ header, nav, - button, - [data-testid="pdf-toolbar"] { + button { display: none !important; } diff --git a/src/components/DocumentViewer.tsx b/src/components/DocumentViewer.tsx index 8f5ba4d705..57c2989cc2 100644 --- a/src/components/DocumentViewer.tsx +++ b/src/components/DocumentViewer.tsx @@ -34,7 +34,7 @@ import { textMuted, } from "@/components/ui-primitives"; import { NonPdfSourcePreview } from "@/components/document-viewer/non-pdf-source-preview"; -import { NativePdfEmbed, PdfCanvasViewer } from "@/components/document-viewer/pdf-readers-lazy"; +import { PdfCanvasViewer } from "@/components/document-viewer/pdf-readers-lazy"; import { requestSignedUrlPayload, rowsById, @@ -42,7 +42,6 @@ import { } from "@/components/document-viewer/signed-url-request"; import { useDocumentSummarize } from "@/components/document-viewer/use-document-summarize"; import { useDocumentViewerRoute } from "@/components/document-viewer/use-document-viewer-route"; -import { usePdfViewerPreference } from "@/components/document-viewer/use-pdf-viewer-preference"; import { DocumentFrame, type DocumentFrameControls, type DocumentFrameSource } from "@/components/ui/document-frame"; import { VIEWER_DEFAULT_ZOOM, @@ -54,9 +53,11 @@ import { clearCachedSignedUrl, getCachedSignedUrl, setCachedSignedUrl } from "@/ import { resolveScrollBehavior } from "@/lib/scroll-behavior"; import { readLocalProjectIdentity, unsafeLocalProjectMessage } from "@/lib/local-project-identity"; import { + canSkipDetailRequest, documentLoadKey, documentPageHref, isFullDocumentReload, + type LoadedDetailWindow, nextLoadedDocumentKey, } from "@/lib/document-viewer-navigation"; import { partitionViewerImages } from "@/lib/image-filtering"; @@ -84,6 +85,7 @@ import type { import { IndexedTextPanel, PinnedSourceEvidence } from "@/components/document-viewer/source-panels"; import { DocumentViewerRail } from "@/components/document-viewer/document-rail-panels"; import { DocumentOverviewLanding } from "@/components/document-viewer/document-overview-landing"; +import { DocumentClinicalSummary } from "@/components/document-viewer/document-clinical-summary"; import { buildDocumentSectionIndex, documentOverviewSectionId } from "@/components/document-viewer/section-index"; import { DocumentSectionSheet, @@ -210,19 +212,27 @@ export function DocumentViewer({ composerChromeFocused, ); const activeScrollOwner = useActiveScrollOwner(shellScrollContainer, documentId); - const { useNativePdfViewer, togglePdfViewerMode } = usePdfViewerPreference(); - // Phase 2a: DocumentFrame owns zoom/fit/viewing-aid chrome for canvas PDF. - // Reset viewing chrome when the document identity changes (render-time adjust, - // not an effect — avoids react-hooks/set-state-in-effect). + // DocumentFrame owns every viewing control for the canvas PDF — there is one + // reader, so there is one toolbar. Reset viewing chrome when the document + // identity changes (render-time adjust, not an effect — avoids + // react-hooks/set-state-in-effect). const [pdfViewingDocumentId, setPdfViewingDocumentId] = useState(documentId); const [pdfFitWidth, setPdfFitWidth] = useState(true); const [pdfZoom, setPdfZoom] = useState(VIEWER_DEFAULT_ZOOM); const [pdfViewingAid, setPdfViewingAid] = useState(false); + const [pdfRotation, setPdfRotation] = useState(0); + const [pdfFullscreen, setPdfFullscreen] = useState(false); + // pdf.js is authoritative for the page count: `document.page_count` can be + // absent or stale for a document whose indexing has not caught up. + const [pdfPageCount, setPdfPageCount] = useState(null); if (pdfViewingDocumentId !== documentId) { setPdfViewingDocumentId(documentId); setPdfFitWidth(true); setPdfZoom(VIEWER_DEFAULT_ZOOM); setPdfViewingAid(false); + setPdfRotation(0); + setPdfFullscreen(false); + setPdfPageCount(null); } const { status: authStatus, @@ -491,6 +501,28 @@ export function DocumentViewer({ const localProjectIdentityPromiseRef = useRef | null>(null); const initialRouteRef = useRef({ documentId, initialPage, chunkId }); const navigatedFromInitialRouteRef = useRef(false); + // The page window already in hand, and the request identity it was loaded + // under. A page flip inside this window needs no network at all: the server + // returns a window of pages centred on the requested one, so the neighbours + // arrived with it. + const loadedWindowRef = useRef(null); + + // Everything the detail request depends on except the page. Callback identity + // is deliberately excluded — a new function reference does not change what + // would be fetched, and letting it force a refetch is what made page flips + // look like network work. + const detailRequestSignature = [ + documentId, + previewAttempt, + activeChunkId ?? "", + authStatus, + String(clientDemoMode), + String(canUsePrivateApis), + String(isConfigured), + String(initialDetailIdentityStale), + // AuthProvider emits lowercase `authorization`; do not read Authorization. + authorizationHeader.authorization ?? authorizationHeader.Authorization ?? "", + ].join("|"); useEffect(() => { if (!canViewSourceDocuments && authStatus === "loading") { @@ -500,6 +532,12 @@ export function DocumentViewer({ return () => undefined; } + // Skip the round trip when the only thing that moved is the page and the new + // page is already inside the loaded window. + if (canSkipDetailRequest(loadedWindowRef.current, detailRequestSignature, activePage)) { + return () => undefined; + } + const matchesInitialRoute = initialRouteRef.current.documentId === documentId && initialRouteRef.current.initialPage === activePage && @@ -613,6 +651,13 @@ export function DocumentViewer({ if (detailLoaded) { const detail = detailResult.value; + // Remember the window this payload covers so a flip inside it can skip + // the network entirely. A chunk route is excluded: its window is centred + // on the selected chunk, so page arithmetic does not describe it. + loadedWindowRef.current = + !activeChunkId && detail.pageWindow + ? { signature: detailRequestSignature, from: detail.pageWindow.from, to: detail.pageWindow.to } + : null; setDocument(detail.document ?? null); // Keep the previous window visible while loading, then atomically // replace it so client memory and mounted DOM stay bounded. @@ -626,6 +671,7 @@ export function DocumentViewer({ } else { // Never retain evidence from the previous page under a newly selected // route. A navigation failure becomes an explicit retryable error. + loadedWindowRef.current = null; setDocument(null); setPages([]); setImages([]); @@ -657,6 +703,7 @@ export function DocumentViewer({ !isAuthEpochCurrent(authRequest.epoch) ) return; + loadedWindowRef.current = null; setDocument(null); setPages([]); setImages([]); @@ -684,6 +731,9 @@ export function DocumentViewer({ canUsePrivateApis, canViewSourceDocuments, clientDemoMode, + // Every primitive input above is already a dependency; this is their joined + // form, used to decide whether an in-window page flip needs the network. + detailRequestSignature, documentId, activeChunkId, activePage, @@ -804,7 +854,6 @@ export function DocumentViewer({ const canvasPdfReady = Boolean(signedUrl) && document?.file_type === "application/pdf" && - !useNativePdfViewer && !effectiveLoadingDocument && !effectiveViewerError && !previewError; @@ -818,6 +867,10 @@ export function DocumentViewer({ const handlePdfFitWidthChange = useCallback((nextFitWidth: boolean) => { setPdfFitWidth(nextFitWidth); }, []); + const handlePdfRotate = useCallback(() => { + setPdfRotation((current) => (current + 90) % 360); + }, []); + const effectivePdfPageCount = pdfPageCount ?? document?.page_count ?? undefined; const pdfFrameControls: DocumentFrameControls | undefined = canvasPdfReady ? { fitWidth: pdfFitWidth, @@ -829,6 +882,13 @@ export function DocumentViewer({ minZoom: VIEWER_MIN_ZOOM, maxZoom: VIEWER_MAX_ZOOM, zoomStep: VIEWER_ZOOM_STEP, + page: activePage, + pageCount: effectivePdfPageCount, + onPageChange: navigateToPage, + rotation: pdfRotation, + onRotate: handlePdfRotate, + fullscreen: pdfFullscreen, + onFullscreenChange: setPdfFullscreen, } : undefined; const headerTitle = readyDocument @@ -956,9 +1016,21 @@ export function DocumentViewer({ // A successful reload means the refreshed URL was accepted, so the recovery // worked — restore the budget for the next (unrelated) TTL expiry. A broken // URL never loads, so it never resets, and the cap still stops its loop. - const handlePdfLoadSuccess = useCallback(() => { - signedUrlRefreshCountRef.current = 0; - }, []); + const handlePdfLoadSuccess = useCallback( + (pageCount: number) => { + signedUrlRefreshCountRef.current = 0; + // pdf.js has opened the file, so its page count now outranks the indexed + // metadata the toolbar fell back to. + setPdfPageCount(pageCount > 0 ? pageCount : null); + // Deep links / stale page_count can leave the route past the real end. + // PdfCanvasViewer also reconciles via onPageChange; clamp here so the + // authoritative count cannot leave the toolbar on a non-existent page. + if (pageCount > 0 && activePage > pageCount) { + navigateToPage(pageCount); + } + }, + [activePage, navigateToPage], + ); const handleDocumentRenamed = (updatedDocument: ClinicalDocument) => { setDocument((current) => (current?.id === updatedDocument.id ? { ...current, ...updatedDocument } : current)); }; @@ -1308,26 +1380,39 @@ export function DocumentViewer({ {readyDocument ? (
void summarize()} onAddToScope={() => router.push(scopedDocumentHref)} onDownload={() => void openSourceDownload()} downloading={downloadingSource} canSummarizeDocument={canSummarizeDocument} + /> +
+ ) : null} + + {/* Phone order is source-first: the title strip, then the PDF, then this. + `buildDocumentSectionIndex` has always described the summary as coming + after the source — the DOM was what disagreed, by rendering this card + inside the overview landing above the PDF. Desktop keeps its position + directly under the title card, so only the phone order changes. */} + {readyDocument ? ( +
+
) : null} {!readyDocument && viewerState !== "loading" ? ( -
+
) : null} -
+
{signedUrl && document?.file_type === "application/pdf" ? ( - <> -
- -
- {useNativePdfViewer ? ( - - ) : ( - - )} - + ) : ( >(); + +function authorizationIdentity(headers: Record) { + // AuthProvider emits lowercase `authorization` (Fetch/Headers convention). + // Accept either casing so a future uppercase writer cannot collapse every + // identity onto the empty key and share one in-flight promise across users. + return headers.authorization ?? headers.Authorization ?? ""; +} + +function signedUrlRequestKey(endpoint: string, headers: Record) { + return `${endpoint}\u0000${authorizationIdentity(headers)}`; +} + +function beginSignedUrlRequest(key: string, endpoint: string, headers: Record) { + // Do not write the module LRU here. Cache keys are endpoint-only, while this + // map is keyed by endpoint+identity: a superseded response that landed after + // an account switch would repopulate the cleared cache with the prior user's + // signed URL. Active consumers write the cache after their own identity check. + const request = fetch(endpoint, { headers }) + .then(async (response): Promise => { + const data = response.ok ? await response.json() : null; + return { status: response.status, data }; + }) + .finally(() => { + inFlightSignedUrlRequests.delete(key); + }); + inFlightSignedUrlRequests.set(key, request); + return request; +} + +/** Drop any shared request for this endpoint so a retry genuinely refetches. */ +function dropInFlightSignedUrlRequests(endpoint: string) { + for (const key of inFlightSignedUrlRequests.keys()) { + if (key.startsWith(`${endpoint}\u0000`)) inFlightSignedUrlRequests.delete(key); + } +} + /** * Resolve a private image's signed URL through its `/signed-url` endpoint, with * the client LRU cache in front and the auth-session authorization header. @@ -52,17 +101,24 @@ export function useSignedImageUrl(endpoint: string, enabled: boolean) { } let active = true; - fetch(endpoint, { headers: authorizationHeader }) - .then((response) => { - if (response.status === 401) markSessionExpired(); - return response.ok ? response.json() : null; - }) - .then((data) => { - if (active && data?.url) { - setCachedSignedUrl(endpoint, data); + // Share one request per endpoint. A document page can mount several + // consumers of the same image at once — a figure and its lightbox, a rail + // panel and a filmstrip — and on a cold cache each was minting its own + // signed URL, so N components meant N round trips for one asset. + const key = signedUrlRequestKey(endpoint, authorizationHeader); + const request = inFlightSignedUrlRequests.get(key) ?? beginSignedUrlRequest(key, endpoint, authorizationHeader); + request + .then(({ status, data }) => { + if (status === 401) markSessionExpired(); + if (!active) return; + if (data?.url) { + // Only an active consumer for this identity may populate the shared + // endpoint-keyed cache — otherwise a late response after sign-out / + // account switch can hand the next user the prior bearer URL. + setCachedSignedUrl(endpoint, { ...data, url: data.url }); setUrl(data.url); setFailed(false); - } else if (active) { + } else { setFailed(true); } }) @@ -77,6 +133,7 @@ export function useSignedImageUrl(endpoint: string, enabled: boolean) { // Drop the cached URL and refetch (e.g. after a 403 on an expired URL). const retry = useCallback(() => { clearCachedSignedUrl(endpoint); + dropInFlightSignedUrlRequests(endpoint); setUrl(null); setFailed(false); setAttempt((current) => current + 1); @@ -85,6 +142,7 @@ export function useSignedImageUrl(endpoint: string, enabled: boolean) { // Mark the current URL dead (e.g. onError) so the frame shows its failure state. const markFailed = useCallback(() => { clearCachedSignedUrl(endpoint); + dropInFlightSignedUrlRequests(endpoint); setUrl(null); setFailed(true); }, [endpoint]); diff --git a/src/components/document-viewer-lazy.tsx b/src/components/document-viewer-lazy.tsx index 577c5e8faf..68391a8034 100644 --- a/src/components/document-viewer-lazy.tsx +++ b/src/components/document-viewer-lazy.tsx @@ -5,7 +5,7 @@ * * This is NOT a code-splitting boundary — Next still includes DocumentViewer in * the document client graph. The real PDF reader split lives in - * `pdf-readers-lazy.tsx` (`next/dynamic` of PdfCanvasViewer / NativePdfEmbed). + * `pdf-readers-lazy.tsx` (`next/dynamic` of PdfCanvasViewer). * Kept so the Server Component can pass `initialDetail` into the client shell * without renaming every import site. */ diff --git a/src/components/document-viewer/canvas-raster-budget.ts b/src/components/document-viewer/canvas-raster-budget.ts new file mode 100644 index 0000000000..2d604a5ada --- /dev/null +++ b/src/components/document-viewer/canvas-raster-budget.ts @@ -0,0 +1,77 @@ +/** + * Canvas raster budget for the PDF page surface. + * + * WebKit refuses to back a canvas larger than roughly 2^24 device pixels: the + * element keeps its layout box but paints nothing, so the reader sees a blank + * page rather than a crisp one. There is no exception to catch and no event to + * observe — the only defence is to never ask for a canvas that large. + * + * The viewer's own numbers reach that ceiling easily. An iPhone reports + * `devicePixelRatio` 3, the raster used a flat `min(2.5, dpr)` output scale, and + * `VIEWER_MAX_ZOOM` is 4, so an A4 page at maximum zoom asked for + * 595·4·2.5 x 842·4·2.5 = about 50 megapixels — three times over. Zooming past + * roughly 2.3x blanked the page on iOS. + * + * Degrade the output scale (raster density) rather than the viewport scale + * (layout size), so a page that cannot be rendered at full device density is + * merely softer, never blank and never a different size than the reader asked + * for. The output scale is allowed below 1 in the extreme — a very large page + * sheet at maximum zoom — because a soft page still reads and a blank one does + * not. + */ + +/** WebKit's per-canvas ceiling, in device pixels. */ +export const MAX_CANVAS_PIXELS = 16_777_216; + +/** Never raster finer than this multiple of CSS pixels, whatever the display reports. */ +export const MAX_RENDER_SCALE = 2.5; + +/** Never collapse the backing store past this, however large the page. */ +const MIN_OUTPUT_SCALE = 0.1; + +export type CanvasRasterPlan = { + /** Device-pixel multiplier to pass to `getViewport({ scale: viewportScale * outputScale })`. */ + outputScale: number; + /** Backing-store dimensions, in device pixels. */ + width: number; + height: number; + /** True when the display's own density had to be given up to stay inside the budget. */ + budgetLimited: boolean; +}; + +/** + * Resolve the raster density for one page render. + * + * `baseWidth`/`baseHeight` are the unscaled pdf.js viewport dimensions, and + * `viewportScale` is the fit-or-zoom factor already chosen for layout. + */ +export function resolveCanvasRasterPlan({ + baseWidth, + baseHeight, + viewportScale, + devicePixelRatio, + maxRenderScale = MAX_RENDER_SCALE, + maxCanvasPixels = MAX_CANVAS_PIXELS, +}: { + baseWidth: number; + baseHeight: number; + viewportScale: number; + devicePixelRatio: number; + maxRenderScale?: number; + maxCanvasPixels?: number; +}): CanvasRasterPlan { + const cssWidth = Math.max(1, baseWidth * viewportScale); + const cssHeight = Math.max(1, baseHeight * viewportScale); + const cssArea = cssWidth * cssHeight; + + const preferred = Math.min(maxRenderScale, Math.max(1, devicePixelRatio || 1)); + const affordable = Math.sqrt(maxCanvasPixels / cssArea); + const outputScale = Math.max(MIN_OUTPUT_SCALE, Math.min(preferred, affordable)); + + return { + outputScale, + width: Math.floor(cssWidth * outputScale), + height: Math.floor(cssHeight * outputScale), + budgetLimited: affordable < preferred, + }; +} diff --git a/src/components/document-viewer/document-clinical-summary.tsx b/src/components/document-viewer/document-clinical-summary.tsx index 99bee1d113..05b6eb53d6 100644 --- a/src/components/document-viewer/document-clinical-summary.tsx +++ b/src/components/document-viewer/document-clinical-summary.tsx @@ -208,6 +208,22 @@ export function buildDocumentClinicalSummaryModel(document: ClinicalDocument): D }; } +/** + * Whether a built model has anything worth a card. + * + * A `document_summaries` row is not the same thing as usable content: the model + * discards placeholder text (see `usefulSummaryText`), so a stored row can still + * yield nothing to render. + * + * This deliberately does not feed `buildDocumentSectionIndex`. The section index's + * `source-summary` entry anchors the rail's document-profile panel, which renders + * its label badges whether or not a summary was indexed — so gating the nav entry + * on summary text would hide a section that is genuinely on the page. + */ +export function documentClinicalSummaryHasContent(model: DocumentClinicalSummaryModel): boolean { + return Boolean(model.summary) || model.priorities.length > 0; +} + function supportLabel(support: DocumentSummarySupportLevel | null) { if (support === "direct") return "Direct support"; if (support === "partial") return "Partial support"; @@ -354,6 +370,7 @@ export function DocumentClinicalSummary({ compact?: boolean; }) { const model = buildDocumentClinicalSummaryModel(document); + const hasContent = documentClinicalSummaryHasContent(model); const [summaryExpanded, setSummaryExpanded] = useState(false); const [prioritiesExpanded, setPrioritiesExpanded] = useState(!compact); const [prevCompact, setPrevCompact] = useState(compact); @@ -374,6 +391,11 @@ export function DocumentClinicalSummary({ onPageChange(page); } + // Render nothing rather than a gradient header, an icon and a heading wrapped + // around "not been indexed for this document yet". On a phone that shell cost + // most of the first viewport to say the document has no summary. + if (!hasContent) return null; + return ( <>
diff --git a/src/components/document-viewer/document-overview-landing.tsx b/src/components/document-viewer/document-overview-landing.tsx index faa2b5ac3a..f9f6ad5e58 100644 --- a/src/components/document-viewer/document-overview-landing.tsx +++ b/src/components/document-viewer/document-overview-landing.tsx @@ -1,9 +1,12 @@ -// Overview landing for the document viewer: the compact header, quick actions, -// and high-yield clinical summary. Extracted from DocumentViewer.tsx (maturity -// X3) as a pure move. +// Overview landing for the document viewer: the compact header and quick +// actions. Extracted from DocumentViewer.tsx (maturity X3) as a pure move. +// +// The high-yield clinical summary used to live here too, which put it above the +// PDF on phones — most of the first viewport spent before any of the source. +// DocumentViewer now places it after the source, matching the order +// `buildDocumentSectionIndex` has always described. import { Download, Loader2, MoreHorizontal, Sparkles, Target } from "lucide-react"; import { documentDisplayTitle, documentOrganizationProfile } from "@/components/DocumentOrganizationBadges"; -import { DocumentClinicalSummary } from "@/components/document-viewer/document-clinical-summary"; import { formatDocumentLabelDisplay } from "@/lib/document-tags"; import { DocumentActionAnchor, @@ -44,32 +47,26 @@ export function DocumentOverviewLanding({ document, signedUrl, pages, - pageHref, - onPageChange, onAskFromDocument, onAddToScope, onDownload, downloading, canSummarizeDocument, - compact, }: { document: ClinicalDocument; signedUrl: string | null; pages: PageRow[]; - pageHref: (page: number) => string; - onPageChange: (page: number) => void; onAskFromDocument: () => void; onAddToScope: () => void; onDownload: () => void; downloading: boolean; canSummarizeDocument: boolean; - compact: boolean; }) { const documentType = compactDocumentType(document); return ( -
-
+
+
-
); } diff --git a/src/components/document-viewer/document-rail-panels.tsx b/src/components/document-viewer/document-rail-panels.tsx index 766a30c96e..8541bc6822 100644 --- a/src/components/document-viewer/document-rail-panels.tsx +++ b/src/components/document-viewer/document-rail-panels.tsx @@ -34,6 +34,7 @@ import type { FormattedDocumentSummary } from "@/lib/document-summary-formatting import type { DocumentSummaryBadge } from "@/lib/document-summary-badges"; export function DocumentViewerRail({ + className, headerHidden, documentSections, activeSectionId, @@ -60,6 +61,7 @@ export function DocumentViewerRail({ activePage, onSelectPage, }: { + className?: string; headerHidden: boolean; documentSections: DocumentSection[]; activeSectionId: string | null; @@ -98,12 +100,15 @@ export function DocumentViewerRail({ // remainder instead of making it scrollable. `md:`/`lg:` never showed it // because Tailwind emits `repeat(n, minmax(0,1fr))` for those. "min-w-0 grid grid-cols-1 content-start gap-4 sm:gap-5 md:grid-cols-2 md:items-start lg:sticky lg:grid-cols-1 lg:self-start lg:pr-1", + className, // The rail clears the top bar while it is there, and reclaims that // space the moment it hides — otherwise a dead band the height of the // bar sits above the rail for as long as chrome stays away. When the // universal bar is hidden, the page-owned sticky document header still // owns the top edge on sm+. - headerHidden ? "lg:top-[var(--document-sticky-header-height,0px)]" : "lg:top-[69px]", + headerHidden + ? "lg:top-[var(--document-sticky-header-height,0px)]" + : "lg:top-[var(--document-collapse-height,69px)]", )} >
+ {/* Eager, not lazy: for an image-source document this *is* the document, + and it sits above the fold. Lazy-loading it puts the largest element + on the page behind the browser's own lazy threshold and delays LCP + (performance-image-cwv-audit-2026-08-02, LCP-1/LL-1). `fetchPriority` + says the same thing to the preload scanner. */} {title} setFailed(true)} className="mx-auto max-h-[min(70vh,36rem)] w-full object-contain" /> diff --git a/src/components/document-viewer/pdf-canvas-viewer.tsx b/src/components/document-viewer/pdf-canvas-viewer.tsx index a394b52d15..5ffea2c93f 100644 --- a/src/components/document-viewer/pdf-canvas-viewer.tsx +++ b/src/components/document-viewer/pdf-canvas-viewer.tsx @@ -9,22 +9,11 @@ import { useRef, useState, } from "react"; -import { - ChevronLeft, - ChevronRight, - ExternalLink, - FileText, - Loader2, - Maximize2, - Minimize2, - Minus, - Plus, - RefreshCw, - RotateCw, -} from "lucide-react"; -import type { PDFDocumentLoadingTask, PDFDocumentProxy, RenderTask } from "pdfjs-dist"; +import { ExternalLink, FileText, Loader2, RefreshCw } from "lucide-react"; +import type { PDFDocumentLoadingTask, PDFDocumentProxy, PDFPageProxy, RenderTask } from "pdfjs-dist"; -import { cn, floatingControl, toolbarButton } from "@/components/ui-primitives"; +import { cn, floatingControl } from "@/components/ui-primitives"; +import { resolveCanvasRasterPlan } from "@/components/document-viewer/canvas-raster-budget"; import { announce } from "@/components/ui/live-announcer"; import { useViewerGestures } from "@/components/document-viewer/use-viewer-gestures"; import { @@ -35,11 +24,9 @@ import { VIEWER_ZOOM_STEP, } from "@/components/document-viewer/viewer-zoom"; -const iconButton = toolbarButton; const secondaryButton = floatingControl; const MAX_FIT_SCALE = 2.8; -const MAX_RENDER_SCALE = 2.5; // A signed URL that has passed its (10-min) TTL fails pdf.js with an auth/HTTP // error rather than a parse error. Detect those so the parent can re-issue a @@ -59,6 +46,10 @@ function isLikelyExpiredUrl(error: unknown): boolean { // pdf.js document and re-rasters the canvas). With stable props from the parent // it skips re-render when unrelated parent state (search, composer, connectivity) // changes, so a keystroke elsewhere never re-rasterises the page. +// +// This component renders source pixels and nothing else. Every viewing control — +// page navigation, zoom, fit, rotation, viewing aid, fullscreen — belongs to +// DocumentFrame, so the viewer has exactly one toolbar and one page readout. export const PdfCanvasViewer = memo(function PdfCanvasViewer({ url, title, @@ -66,8 +57,10 @@ export const PdfCanvasViewer = memo(function PdfCanvasViewer({ onUrlExpired, onLoadSuccess, onPageChange, - fitWidth: fitWidthProp, - zoom: zoomProp, + fitWidth, + zoom, + rotation = 0, + fullscreen = false, onFitWidthChange, onZoomChange, }: { @@ -76,39 +69,31 @@ export const PdfCanvasViewer = memo(function PdfCanvasViewer({ initialPage: number; /** Called when a load/render fails in a way consistent with an expired signed URL. */ onUrlExpired?: () => void; - /** Called when the PDF document loads successfully (a genuine URL is valid). */ - onLoadSuccess?: () => void; + /** Called with the document's page count when pdf.js opens it successfully. */ + onLoadSuccess?: (pageCount: number) => void; /** Keeps the document route in sync when the reader changes pages. */ onPageChange?: (page: number) => void; - /** - * Controlled fit/zoom from DocumentFrame (Phase 2a). When both change handlers - * are provided, Frame owns the chrome and this toolbar keeps page/rotate/fullscreen. - */ - fitWidth?: boolean; - zoom?: number; - onFitWidthChange?: (fitWidth: boolean) => void; - onZoomChange?: (zoom: number) => void; + /** Controlled viewing state owned by DocumentFrame via DocumentViewer. */ + fitWidth: boolean; + zoom: number; + rotation?: number; + fullscreen?: boolean; + onFitWidthChange: (fitWidth: boolean) => void; + onZoomChange: (zoom: number) => void; }) { - const fullscreenRootRef = useRef(null); const holderRef = useRef(null); const canvasRef = useRef(null); const [pdf, setPdf] = useState(null); const [page, setPage] = useState(initialPage); - const [pageInput, setPageInput] = useState(String(initialPage)); const [totalPages, setTotalPages] = useState(0); - const [internalZoom, setInternalZoom] = useState(VIEWER_DEFAULT_ZOOM); // Debounced mirror of `zoom`. Zoom steps update `zoom` immediately (an interim // CSS transform gives instant visual feedback) but only `renderZoom` drives the // pdf.js raster, so rapid +/-, wheel, and pinch input re-rasterise once on // settle instead of queueing a RenderTask per delta. const [renderZoom, setRenderZoom] = useState(VIEWER_DEFAULT_ZOOM); - const [rotation, setRotation] = useState(0); - const [internalFitWidth, setInternalFitWidth] = useState(true); - const frameOwnsZoomChrome = typeof onFitWidthChange === "function" && typeof onZoomChange === "function"; - const fitWidth = frameOwnsZoomChrome ? Boolean(fitWidthProp) : internalFitWidth; - const zoom = frameOwnsZoomChrome ? (typeof zoomProp === "number" ? zoomProp : VIEWER_DEFAULT_ZOOM) : internalZoom; // Eager refs so rapid functional updates (wheel/pinch) compose before React - // re-renders — especially on the Frame-owned path where zoom lives in a parent. + // re-renders — the zoom lives in a parent, so a closed-over prop would drop + // every delta but the last (Sentry 15778840). const fitWidthRef = useRef(fitWidth); const zoomRef = useRef(zoom); useLayoutEffect(() => { @@ -116,39 +101,29 @@ export const PdfCanvasViewer = memo(function PdfCanvasViewer({ zoomRef.current = zoom; }, [fitWidth, zoom]); const setFitWidth = useCallback( - (next: boolean | ((current: boolean) => boolean)) => { - if (frameOwnsZoomChrome) { - const resolved = typeof next === "function" ? next(fitWidthRef.current) : next; - fitWidthRef.current = resolved; - onFitWidthChange?.(resolved); - return; - } - setInternalFitWidth((current) => (typeof next === "function" ? next(current) : next)); + (next: boolean) => { + fitWidthRef.current = next; + onFitWidthChange(next); }, - [frameOwnsZoomChrome, onFitWidthChange], + [onFitWidthChange], ); const setZoom = useCallback( (next: number | ((current: number) => number)) => { - if (frameOwnsZoomChrome) { - const clamped = resolveViewerZoomUpdate(zoomRef.current, next); - zoomRef.current = clamped; - onZoomChange?.(clamped); - return; - } - setInternalZoom((current) => resolveViewerZoomUpdate(current, next)); + const clamped = resolveViewerZoomUpdate(zoomRef.current, next); + zoomRef.current = clamped; + onZoomChange(clamped); }, - [frameOwnsZoomChrome, onZoomChange], + [onZoomChange], ); const [holderWidth, setHolderWidth] = useState(0); const [loading, setLoading] = useState(true); const [rendering, setRendering] = useState(false); const [error, setError] = useState(null); const [loadAttempt, setLoadAttempt] = useState(0); - const [isFullscreen, setIsFullscreen] = useState(false); - const [fullscreenFallback, setFullscreenFallback] = useState(false); const onUrlExpiredRef = useRef(onUrlExpired); const onLoadSuccessRef = useRef(onLoadSuccess); + const onPageChangeRef = useRef(onPageChange); const urlRef = useRef(url); const reportedExpiredUrlRef = useRef(null); useEffect(() => { @@ -157,6 +132,9 @@ export const PdfCanvasViewer = memo(function PdfCanvasViewer({ useEffect(() => { onLoadSuccessRef.current = onLoadSuccess; }, [onLoadSuccess]); + useEffect(() => { + onPageChangeRef.current = onPageChange; + }, [onPageChange]); useEffect(() => { urlRef.current = url; }, [url]); @@ -196,7 +174,19 @@ export const PdfCanvasViewer = memo(function PdfCanvasViewer({ "pdfjs-dist/build/pdf.worker.min.mjs", import.meta.url, ).toString(); - loadTask = pdfjs.getDocument({ url }); + // Range-fetch on demand instead of pulling the whole file down. pdf.js + // otherwise keeps fetching the rest of the document in the background + // even when the reader only ever looks at one page — the wrong default + // for a long guideline opened on a phone, on cellular. `disableAutoFetch` + // does not take effect without `disableStream`; the installed types say + // so explicitly. + // + // The trade is that later bytes are requested later, so a signed URL can + // expire mid-read. That path already exists and recovers: a range failure + // is an auth/HTTP error, `isLikelyExpiredUrl` catches it, and the parent + // re-issues the URL. Its refresh budget resets on every successful load, + // so a long reading session is not capped at two recoveries. + loadTask = pdfjs.getDocument({ url, disableAutoFetch: true, disableStream: true }); const loadedPdf = await loadTask.promise; if (!active) return; setPdf(loadedPdf); @@ -204,7 +194,7 @@ export const PdfCanvasViewer = memo(function PdfCanvasViewer({ setPage((current) => Math.min(Math.max(current, 1), loadedPdf?.numPages ?? current)); // A valid load means any prior expiry was genuinely recovered — let the // parent restore the refresh budget so a long session isn't dead-ended. - onLoadSuccessRef.current?.(); + onLoadSuccessRef.current?.(loadedPdf.numPages); } catch (loadError) { if (active) { if (isLikelyExpiredUrl(loadError)) reportUrlExpired(); @@ -228,7 +218,12 @@ export const PdfCanvasViewer = memo(function PdfCanvasViewer({ const boundedPage = totalPages > 0 ? Math.min(nextPage, totalPages) : nextPage; const frame = window.requestAnimationFrame(() => { setPage((current) => (current === boundedPage ? current : boundedPage)); - setPageInput(String(boundedPage)); + // pdf.js is authoritative for page count. A deep link or stale indexed + // page_count can leave the frame toolbar on an out-of-range route value + // while the canvas shows the clamped page — reconcile the parent route. + if (totalPages > 0 && boundedPage !== nextPage) { + onPageChangeRef.current?.(boundedPage); + } }); return () => window.cancelAnimationFrame(frame); }, [initialPage, totalPages]); @@ -249,35 +244,6 @@ export const PdfCanvasViewer = memo(function PdfCanvasViewer({ }; }, []); - useEffect(() => { - function updateFullscreenState() { - const active = document.fullscreenElement === fullscreenRootRef.current; - setIsFullscreen(active); - if (active) setFullscreenFallback(false); - } - - document.addEventListener("fullscreenchange", updateFullscreenState); - return () => document.removeEventListener("fullscreenchange", updateFullscreenState); - }, []); - - // Escape exits whichever fullscreen mode is active. Native fullscreen usually - // exits via the browser, but handling it here too keeps the in-app state in - // sync (and covers the in-app fallback overlay, which the browser doesn't own). - useEffect(() => { - if (!isFullscreen && !fullscreenFallback) return; - - function exitOnEscape(event: KeyboardEvent) { - if (event.key !== "Escape") return; - setFullscreenFallback(false); - if (document.fullscreenElement === fullscreenRootRef.current && document.exitFullscreen) { - void document.exitFullscreen(); - } - } - - window.addEventListener("keydown", exitOnEscape); - return () => window.removeEventListener("keydown", exitOnEscape); - }, [isFullscreen, fullscreenFallback]); - // Settle rapid zoom deltas into a single raster. The interim CSS transform on // the canvas keeps the view visually correct during this window. useEffect(() => { @@ -291,12 +257,22 @@ export const PdfCanvasViewer = memo(function PdfCanvasViewer({ const activePdf = pdf; let cancelled = false; let renderTask: RenderTask | null = null; + // Local to this effect run so a rapid page change cannot clean up the next + // page via a shared ref (Sentry 15801413). + let pageToCleanup: PDFPageProxy | null = null; async function renderPage() { setRendering(true); try { const pdfPage = await activePdf.getPage(page); - if (cancelled || !canvasRef.current || !holderRef.current) return; + if (cancelled) { + // getPage resolved after we left this page — release it here so the + // next effect's page is never touched by this run's cleanup. + pdfPage.cleanup(); + return; + } + pageToCleanup = pdfPage; + if (!canvasRef.current || !holderRef.current) return; // Rotation is applied in the viewport so width/height already reflect the // 90°/270° swap — the fit calculation and canvas sizing follow for free. const baseViewport = pdfPage.getViewport({ scale: 1, rotation }); @@ -305,7 +281,15 @@ export const PdfCanvasViewer = memo(function PdfCanvasViewer({ ? Math.min(MAX_FIT_SCALE, Math.max(VIEWER_MIN_ZOOM, availableWidth / baseViewport.width)) : renderZoom; const viewportScale = Math.min(VIEWER_MAX_ZOOM, Math.max(VIEWER_MIN_ZOOM, requestedScale)); - const outputScale = Math.min(MAX_RENDER_SCALE, window.devicePixelRatio || 1); + // WebKit paints nothing at all above ~2^24 canvas pixels, and this page + // at full device density can ask for three times that. Give up raster + // density before layout size — a soft page reads, a blank one does not. + const { outputScale } = resolveCanvasRasterPlan({ + baseWidth: baseViewport.width, + baseHeight: baseViewport.height, + viewportScale, + devicePixelRatio: window.devicePixelRatio, + }); const viewport = pdfPage.getViewport({ scale: viewportScale * outputScale, rotation }); const canvas = canvasRef.current; const context = canvas.getContext("2d"); @@ -341,54 +325,44 @@ export const PdfCanvasViewer = memo(function PdfCanvasViewer({ return () => { cancelled = true; renderTask?.cancel(); + // Release this run's page only. pdf.js declines while a render is still + // live, so cancel above cannot be cut short by cleanup. + pageToCleanup?.cleanup(); }; }, [fitWidth, holderWidth, page, pdf, renderZoom, reportUrlExpired, rotation]); - function jumpToPage(nextPage: number) { - const bounded = Math.min(Math.max(nextPage, 1), totalPages || nextPage); - setPage(bounded); - setPageInput(String(bounded)); - if (bounded !== page) onPageChange?.(bounded); - } - - function zoomBy(delta: number) { - setFitWidth(false); - setZoom((current) => Number((current + delta).toFixed(2))); - } - - async function enterFullscreenFitView() { - setFitWidth(true); - const element = fullscreenRootRef.current; - if (!element) return; - - try { - if (document.fullscreenElement === element) { - setIsFullscreen(true); - return; - } - if (element.requestFullscreen) { - await element.requestFullscreen(); - setIsFullscreen(true); - return; - } - } catch { - // Fall back to a fixed in-app fullscreen surface when native fullscreen is unavailable. - } + // A canvas keeps its backing store until the element is collected, which on a + // phone is memory held for a document the reader has already left. Zeroing the + // dimensions releases it at unmount instead. + useEffect( + () => () => { + const canvas = canvasRef.current; + if (!canvas) return; + canvas.width = 0; + canvas.height = 0; + }, + [], + ); - setFullscreenFallback(true); - setIsFullscreen(true); - } + const jumpToPage = useCallback( + (nextPage: number) => { + const bounded = Math.min(Math.max(nextPage, 1), totalPages || nextPage); + if (bounded === page) return; + setPage(bounded); + onPageChange?.(bounded); + }, + [onPageChange, page, totalPages], + ); - async function exitFullscreenView() { - if (document.fullscreenElement === fullscreenRootRef.current && document.exitFullscreen) { - await document.exitFullscreen(); - } - setFullscreenFallback(false); - setIsFullscreen(false); - } + const zoomBy = useCallback( + (delta: number) => { + setFitWidth(false); + setZoom((current) => Number((current + delta).toFixed(2))); + }, + [setFitWidth, setZoom], + ); const pagesReady = Boolean(pdf && totalPages > 0 && !loading); - const fullscreenActive = isFullscreen || fullscreenFallback; // While a zoom step waits for its debounced raster, scale the last raster with // a CSS transform so the view tracks the target zoom instantly. It resets to 1 // the moment `renderZoom` catches up and the crisp raster paints. Fit mode is @@ -410,12 +384,23 @@ export const PdfCanvasViewer = memo(function PdfCanvasViewer({ holder.scrollTop -= dy; }, []); - // Wheel/pinch zoom and drag-to-pan. Pointer gestures (pinch + drag) only take - // over when zoomed; in fit mode the holder keeps native momentum scrolling. + // Pinch is live in fit mode too, which is the viewer's default state. It used + // to be gated on `!fitWidth`, so a two-finger pinch on a freshly opened + // document did nothing at all: the holder's `touch-action: pan-y` suppressed + // the browser's own pinch-zoom, and this hook declined to handle it. That left + // no way to magnify a page by gesture without first finding a zoom button, and + // it contradicted the blocking phone clause in docs/design-system/COMPONENTS.md. + // + // The first pinch delta drops fit mode (see `handleZoomByFactor`), which + // switches the holder to `touch-action: none` — so the browser can only + // contend for the opening moment of the gesture, never the rest of it. + // + // Drag-to-pan stays gated on `!fitWidth`: in fit mode the holder is a scroll + // container and must keep native momentum scrolling. const { handlers: gestureHandlers } = useViewerGestures({ targetRef: holderRef, wheelZoom: pagesReady, - pinchZoom: pagesReady && !fitWidth, + pinchZoom: pagesReady, pan: pagesReady && !fitWidth, touchPan: true, onZoomBy: handleZoomByFactor, @@ -457,131 +442,9 @@ export const PdfCanvasViewer = memo(function PdfCanvasViewer({ return (
-
- - {pagesReady ? ( - - ) : ( -
-
- )} - -
- {frameOwnsZoomChrome ? null : ( - <> - - - - - )} - - {fullscreenActive ? ( - - ) : ( - - )} -
-
-
); }); - -function nativePdfEmbedUrl(url: string, initialPage: number) { - const page = Math.max(1, Math.trunc(initialPage || 1)); - return `${url.split("#")[0]}#page=${page}`; -} - -export const NativePdfEmbed = memo(function NativePdfEmbed({ - url, - title, - initialPage, -}: { - url: string; - title: string; - initialPage: number; -}) { - return ( -
-