fix(ui): unstick the safety-findings sheet, settle the stopped-answer state, repoint sidebar Documents - #2422
Conversation
Pages whose content already ended still carried a scroll range, so a scrollbar appeared on a page with nothing below the fold and a wheel notch jolted the page into its bottom stop. Measured in Chromium across 39 routes at five viewports: 8px on every standalone page, 38-46px on all 15 shared mode homes, 57px on routes carrying the header nav row, and up to 273px on a tall window. The cause was the same everywhere: page-fill floors written as `calc(100dvh - <chrome estimate>)`. No estimate could be right. `--shell-header-h` (4rem) covers the header's inner bar plus `pb-2` but not the bar's own `pt-[max(0.5rem,var(--safe-area-top))]`; nothing knew about the `header-collapse-addon` nav row on topic routes; nothing knew about `#main-content`'s own `sm:pb-8`; and the dashboard's `11rem` guess had to cover the header block, the wrapper padding, the desktop composer slot and the space-y gap in one number. Surfaces now grow into the box above them instead, which is exact by construction and cannot drift again: - shell `#main-content` grows into `.phone-viewport-frame` (`sm:grow`) - `mobile-composer-reserve-pad` becomes the fill box at sm+ - page shells grow into that pad (`sm:grow`) - the dashboard content wrapper is the fill box for the mode-home canvas, which grows into it (`sm:grow sm:shrink-0`) Phone geometry is untouched: below `sm` the document owns scrolling and there is no bounded box to fill. All 65 phone measurements are byte-identical before and after. Verified in Chromium against 39 routes x 5 viewports: 82 measurements improved, 113 unchanged, 0 increased, and no route gained horizontal overflow. Every page that fits now reports a scroll range of exactly 0; every page with real content reports the same range as before. Adds the "pages that fit the window have no scroll range" guard to tests/ui-chrome-scroll.spec.ts, records the rule as invariant 24 in docs/search-chrome-behaviour.md, and updates the two contract tests that pinned the old floors. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bbm7tQoyg6etyf7NqoA65d
…swer state
Three phone defects reported from device screenshots.
**The safety-findings sheet could not be scrolled; the page behind moved
instead.** Two independent causes, both fixed:
1. The sheet body was given `flex flex-col`, which turned its single child —
the findings card — into a shrinkable flex item. The card was compressed
from its natural height to whatever was left, and because it clips its own
overflow the findings past the fold were cut off rather than scrolled. The
body then had no scroll range at all, so the gesture fell through to the
page. Measured on a 390x400 phone: body scroll range 0px before, 230px
after. The card also now carries `shrink-0` so no flex parent can clip it
again.
2. The shared sheet scroll lock set `overflow: hidden` on `<body>`, which does
nothing here. `globals.css` sets `html { overflow-x: clip }`, and a root
whose overflow is not `visible` stops body's overflow propagating to the
viewport — so every sheet on every phone page left the document freely
scrollable behind it. Measured with the sheet open: 272px of live range
behind it, and one wheel gesture over the sheet ran the page to its bottom.
The lock now also holds `overflow-y` on the root, and the sheet body carries
`overscroll-contain` so a gesture reaching its end cannot chain out.
**"Generation stopped" floated in the middle of the screen.** It is a status
notice about the last action, not a description of the page, but it rendered
inside the mode-home canvas where it was centred as one group with the shared
home hero. It now renders with the other top-of-content notices, directly under
the header. The condition stays on the empty-state chain so a stopped
generation still short-circuits the no-results and error states.
**The stopped state also kept a loading skeleton forever.**
`submittedAnswerSearchActive` stays true after Stop and a cancel is not an
`error`, so the pending branch shimmered on indefinitely underneath the notice
saying the answer had been abandoned. Cancelled now clears pending.
**More room at the bottom of the answer page.** `pb-4` was the smallest tail in
the app and left the last card almost on the bottom edge once the composer dock
scroll-hides and its reserve releases to zero. Phone answer results now use
`pb-10`, matching the `sm:pb-10` every other mode wrapper already uses, and
still far below the old padding that floated a long answer above the dock.
Verified in Chromium on a 390px phone: typecheck and lint clean, and
`test:focused` over the five changed files passed 1526 tests in 129 files.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bbm7tQoyg6etyf7NqoA65dThe scroll work landed on main as 603da96; this branch still carries the three phone fixes that were pushed after that merge. Merging main in makes those the only diff a new pull request will show.
Warning Review limit reachedNext included review available in 41 minutes. View limit detailsLimit details: You’ve used the included review currently available. Your 102 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (9)
Comment |
This pull request has been ignored for the connected project Preview Branches by Supabase. |
CI triageCI failed on this PR. Automated classification of the 2 failed job(s):
Compared with main CI run #14190 (failure). Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
…cuments home
Owner decision, from a device screenshot of the intended destination.
Tapping Documents in the sidebar opened `/documents`, a second landing page
that carries the same subtitle as the shared home under a different title
("Documents" vs "Clinical Documents") plus three rows that only open drawers.
Arriving there from a pinned shortcut read as landing on the wrong screen.
The entry now links at `/?mode=documents`, matching every other consolidated
mode in that list.
Nothing is removed: `/documents` keeps its route, its workspace, and its
inbound nav link from the Tools directory (`tools-catalog.ts`), so route
reachability is unaffected. `appModeDefinition("documents").href` is left
alone deliberately — it drives search routing and `/documents/search`
handoff, which this change has no reason to touch.
Verified in Chromium on a 390px phone: opening the sidebar and tapping
Documents lands on `/?mode=documents` and renders the "Clinical Documents"
home. Full offline unit suite passes (10,814 passed, 3 pre-existing failures
unrelated to this change and identical on an unmodified tree).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bbm7tQoyg6etyf7NqoA65d…olith `Static PR checks` failed on this branch: ClinicalDashboard.tsx reached 4144 lines against its 4140-line no-growth budget. The budget is deliberately pinned at the reclaimed value so it cannot drift back, and its failure message asks for an extraction rather than a higher ceiling — so the notice added in the previous commit moves to its own module instead. `AnswerCancelledNotice` is a self-contained presentational block, so its markup and the rationale for where the dashboard mounts it now live together. Net effect on the monolith: 4123/4140 lines, seven above main's 4116 rather than twenty-eight. The generated design-system adoption manifest is regenerated for the new component, which `tests/design-system-adoption.test.ts` requires. Verified: check:maintainability-budgets passes (4123/4140); typecheck and lint clean; the four other checks in the same CI job (docs index, inventory, script refs, links) and knip all pass locally; design-system-adoption 53 passed; and the browser repro still shows the notice at top: 88px with no leftover skeleton ten seconds after Stop. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bbm7tQoyg6etyf7NqoA65d
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_cca82f56-1bec-4b28-8695-6c0d437fbfb3) |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_fe969c5a-d099-4287-b534-eb75989879db) |
Uh oh!
There was an error while loading. Please reload this page.
CI's Static PR checks job failed because merging main (#2422) shifted component adoption counts without regenerating the tracked snapshot. Re-run npm run design-system:adoption:update to bring docs/design-system/adoption-manifest.json and COMPONENTS.md back in step.
#2434) * issues: queue three follow-ups left open by PR #2422 Three append-only intake requests, plus the regenerated snapshot that check:outstanding-issues requires once the inbox is non-empty. - P2 issue: Lighthouse desktop-root LCP read 961/925/900/852 on the #2422 branch against 748/705 on main, same 786ms baseline. It failed the gate once and cost a re-run. Not root-caused; recorded with the evidence on both sides, the one mechanism still testable (whether extracting answer-cancelled-notice.tsx added a chunk to /), the exact command to test it, and the caveat that the gate's 3-sample majority is taken on a single runner and so cannot see runner-level slowness. - P3 issue: /calculators/search keeps a 2px residual scroll range at 1280x1200, deliberately out of scope for #2419 because it is real content rather than the chrome-estimate floor that PR removed. - P3 task: mode-home-page-skeleton.tsx:32,60 still subtract --shell-header-h from 100dvh - the last page-fill estimates in src/ and the pattern invariant 24 retired. Left because the Suspense parent chain has to carry a definite height first. Requests are immutable and merge independently; reconciliation is a separate serial branch and is deliberately not done here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bbm7tQoyg6etyf7NqoA65d * ledger: record the immutable review for the #2422 branch Owed from the handoff and deliberately not pushed onto the branch while it was green and armed for auto-merge. Records the scope, the merge outcome verified by content rather than by the green tick (git diff c917e0728b1fef is empty, so the squash dropped nothing), the CI evidence on the merged head, and two honest gaps: the Chromium gate could not run in this container under the #255 revision pin, and the desktop-root LCP reading is carried as its own open item rather than reported as cleared. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bbm7tQoyg6etyf7NqoA65d * snapshot: regenerate repo-awareness after the new review record `Static PR checks` failed on this branch: check:repo-awareness-snapshot reported "review_state differs from the repository". The repo-awareness snapshot mirrors the branch-review records, so appending one leaves it behind; I regenerated the outstanding-issues snapshot in the first commit and missed this second one. Regenerated with the command the check itself names. The diff is only the captured revision, the mirrored copy of the new record, and counts 2644 -> 2645 records / 1612 -> 1613 refs. Nothing unrelated moved. Verified: check:repo-awareness-snapshot now reports "in step ... (195 pages, 481 documents, 2645 reviews)"; the failure reproduced locally first. Also re-ran the rest of the failing job's docs and ledger gates plus the whole-tree format check — docs:check-index, docs:check-inventory, docs:check-scripts, docs:check-links, check:outstanding-issues, check:ledger-write-discipline, check:branch-review-ledger, format:check — all pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bbm7tQoyg6etyf7NqoA65d --------- Co-authored-by: Claude <noreply@anthropic.com>
Summary
Four phone defects reported from device screenshots. The first three were pushed to this branch shortly after #2419 squash-merged, so they were never part of it; main is merged in, so the diff below is only this work.
flex flex-col, which turned its single child — the findings card — into a shrinkable flex item. The card was compressed from its natural height to whatever space was left, and because it clips its own overflow the findings past the fold were cut off rather than scrolled. The body then had no scroll range at all, so the gesture fell through to the page. Measured on a 390×400 phone: body scroll range 0px before, 230px after. The card now also carriesshrink-0so no flex parent can clip it again.overflow: hiddenon<body>, which does nothing here.globals.csssetshtml { overflow-x: clip }, and a root whose overflow is notvisiblestops<body>'s overflow propagating to the viewport — so every sheet on every phone page left the document freely scrollable behind it. Measured with the sheet open: 272px of live range behind it, and one wheel gesture over the sheet ran the page to its bottom. The lock now also holdsoverflow-yon the root (which preserves scroll offset, so nothing jumps on close), and the sheet body carriesoverscroll-containso a gesture reaching its end cannot chain out.top: 88px). The condition stays on the empty-state chain so a stopped generation still short-circuits the no-results and error states.submittedAnswerSearchActivestays true after Stop and a cancel is not anerror, so the pending branch shimmered on indefinitely underneath the notice saying the answer had been abandoned. Cancelled now clears pending. Found while verifying the notice placement.pb-4was the smallest tail in the app and left the last card almost on the bottom edge once the composer dock scroll-hides and its reserve releases to zero. Phone answer results now usepb-10, matching thesm:pb-10every other mode wrapper already uses, and still far below the old padding that floated a long answer's last line high above the dock./documents, a second landing page carrying the same subtitle as the shared home under a different title ("Documents" vs "Clinical Documents") plus three rows that only open drawers. The owner confirmed the intended destination by screenshot; the entry now links at/?mode=documents, matching every other consolidated mode in that list. Nothing is removed —/documentskeeps its route, its workspace, and its inbound nav link from the Tools directory, so route reachability is unaffected, andappModeDefinition("documents").hrefis deliberately left alone because it drives search routing and the/documents/searchhandoff.Verification
npm run verify:pr-localRun locally instead, and reported exactly as run:
npm run typecheck— clean.npm run lint— clean,--max-warnings 0.npm run test(full offline unit suite, run because this branch changes test files) — 10,814 passed, 4 skipped, 3 failed. The three failures (clinical-hazard-controls,privacy-readiness-contract,rag-plan-package-parity) are pre-existing and were confirmed earlier in this branch's history to fail identically with all changes stashed;rag-plan-package-parityfails only because the manifest's reconciled base commit is absent from this shallow clone.tests/route-reachability.test.tspasses with the sidebar change in place.answer-cancellednotice attop: 88pxwith zero skeleton elements remaining 10s after Stop; answer wrapper bottom padding 16px→40px; opening the sidebar and tapping Documents lands on/?mode=documentsand renders the "Clinical Documents" home.UI verification not run:
npm run verify:uiand theverify:phone-chromebrowser stages could not execute in this container. The lock pins Playwright chromium-1234; the image ships only chromium-1194, and both remedies are blocked by the environment's proxy (scripts/setup-claude-cloud.sh browsersfails 403 from the apt PPAs;npx playwright install chromiumfails to download). Per the#255guard inscripts/check-playwright-browser-revision.mjs, pointingPLAYWRIGHT_CHROMIUM_EXECUTABLE_PATHat the mismatched shell is forbidden, so browser proof is delegated to CI Production UI. The measurements above were taken with the container's chromium-1194 against the dev server and are evidence of the behaviour change, not the Chromium gate.npm run verify:releasebefore release or handoff confidence claimsRisk and rollout
overscroll-containdefault apply to everySheetin the app, not just this one. That is deliberate — the lock has silently done nothing on phones — but it is the widest part of this diff, and the failure mode to watch for is a surface that legitimately expected the page behind a sheet to move. None is known. The other changes are scoped to the answer surface and one sidebar entry.Notes
docs/search-chrome-behaviour.mdare unaffected: the dock reserve, its zero-when-hidden rule, and the safe-area handling are untouched. The bottom-padding change is ordinary content padding, not a dock-sized or safe-area reserve.favourites-auth-gate.dom.test.tsx, and two sites inui-smoke.spec.ts) and were updated to the new contract, along with the stale rationale comment above one of them.Note
Medium Risk
Root scroll lock and default
overscroll-containapply to everySheetapp-wide; the intended fix for phones but worth watching for surfaces that relied on background scroll. Other changes are scoped to answer layout and one nav entry.Overview
Fixes several phone UX defects around sheets, stopped answer generation, and navigation.
Sheet scrolling (global
Sheetbehavior): The safety-findings sheet could not scroll while the page behind moved. The fix pairs layout changes (dropflex flex-colon the sheet body,shrink-0on the findings card) with rootoverflow-ylock alongside the existing body lock (needed becausehtml { overflow-x: clip }blocks body overflow from locking the viewport), plusoverscroll-containon sheet bodies so gestures do not chain to the page.Stopped answer state:Generation stopped is extracted to
AnswerCancelledNoticeand rendered with top-of-column notices instead of the centred mode-home canvas. Cancelled generation now clears the pending skeleton (showAnswerCancelledNoticegatesshowAnswerPending). Answer-mode mobile bottom padding increases frompb-4topb-10when the composer dock reserve releases.Sidebar: The pinned Documents shortcut now targets
/?mode=documents(shared Clinical Documents home) instead of/documents; the/documentsroute and Tools-directory link remain.Design-system adoption metadata and tests (
favourites-auth-gate,ui-smoke) reflect the new component import and href contract.Reviewed by Cursor Bugbot for commit c917e07. Configure here.