fix(chrome): overlay the phone header so hiding it never moves content - #1396
Conversation
Reported symptom: the header bar is choppy when it hides, and content gets pushed up and down with it. Cause: phones used the collapse mechanism, which is a 1fr -> 0fr grid on the header row PLUS a height transition on chrome-safe-area-top PLUS the reserve-pad padding-bottom transition. Every hide animated three heights and handed that layout back to the scroller, so content slid under the animation. No timing tuning fixes that -- the layout genuinely moves. Overlay motion already existed and was already proven, but was gated to isDocumentViewerOwnedRoute(pathname). It translates the whole stack instead and charges zero released top geometry in readChromeCollapseMetrics, so content geometry never changes. Phones now use it on every route. Overlay takes the stack out of flow (.phone-overlay-header is fixed in browser tabs, absolute in standalone), so content needs clearance or the first row sits under the chrome at scroll top. usePhoneOverlayChromeReserve publishes the measured stack height as --phone-overlay-chrome-h and the shell reserve pad reads it. Two properties of that reserve are load-bearing: - measured, not tokenised, because the collapse row grows with page navigation portaled into header-collapse-addon and a fixed 4rem would clip those routes; - never varies with hide state. offsetHeight ignores the hide transform, so one value is correct in both states; a reserve that animated or zeroed on hide would reintroduce the exact shift this removes. It is only visible near scroll top, where the header is always revealed. Verified: typecheck clean; phone-chrome contract tests 67 passed; the new guard was checked against the reverted route-conditional shape and fails there (3 failures), so it is not vacuous. The browser gate (verify:phone-chrome, which escalates to verify:ui) is still running at commit time and its result is not yet included. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01361jh3eYVjJCzXWjAhdZiF
Follow-up to the overlay switch in the parent commit, which regressed the shell's phone bottom search dock. The overlay hide translates the header stack, and a non-none transform -- including translateY(0) at rest -- makes an element a containing block for position: fixed descendants. searchComposer rendered inside that stack, so on non-document routes the dock's `bottom: 0` resolved against the ~72px header instead of the viewport and landed near the top of the screen. `bottom` still computed to 0px, which is what makes this read as a positioning puzzle rather than a CSS error. Measured as formBottom 772px from the viewport bottom at 390x844 (844 - 72). Document routes never hit it because their composer is DocumentViewer-owned and already portals to the frame footer host. Fix: wrap the composer in PhoneFooterLayerPortal when phoneOverlayMotion && usesPhoneBottomDock. Phones host it on the footer layer, sm+ keeps it inline in the sticky [top bar | search] stack -- tablet may not double-sticky the composer, and the portal is breakpoint-aware, which is the one mechanism that satisfies both. Same approach invariant 21 already requires of the DocumentViewer, calculator and differential footers. Rejected: rendering the composer twice per breakpoint (duplicate page-root data-testids are their own failure mode, invariant 17), and animating `top` instead of transform (would change the already-proven document-route path for no gain). Verified with real Chromium: the 13 focused phone-chrome journeys all pass, including both `document detail header overlay and footer follow` tests that were failing in CI, and the dock geometry test that caught this regression. verify:cheap: Test Files 432 passed, Tests 4452 passed | 4 skipped. New guard checked against the un-portalled shape and fails there. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01361jh3eYVjJCzXWjAhdZiF
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 10 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 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 configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (11)
📝 WalkthroughWalkthroughPhone search headers now use overlay motion on all phone routes, reserve measured top clearance, and portal bottom-docked composers outside transformed layers. Documentation, contract tests, issue records, and review ledger entries were updated accordingly. ChangesPhone search chrome
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant GlobalStandaloneSearchShellBody
participant usePhoneOverlayChromeReserve
participant MasterSearchHeader
participant PhoneFooterLayerPortal
GlobalStandaloneSearchShellBody->>usePhoneOverlayChromeReserve: measure and publish phone overlay clearance
GlobalStandaloneSearchShellBody->>MasterSearchHeader: configure phoneMotion as overlay
MasterSearchHeader->>PhoneFooterLayerPortal: portal composer when overlay and bottom dock are active
PhoneFooterLayerPortal-->>MasterSearchHeader: render composer outside transformed overlay layer
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Comment |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
…gaps #112 (P2): verify:phone-chrome exited 0 while its own output ended "13 failed". All 13 failed at browser launch, so zero assertions ran and the gate still returned success -- the green-when-broken case AGENTS.md warns about, in the gate that is meant to be the proof. #113 (P3): the container's Playwright browsers are build 1194 while the repo's client resolves 1234, so browser gates cannot run in a remote session without hand-patching. Worked around in-session with symlinks under /opt/pw-browsers; nothing committed, and it vanishes with the session. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01361jh3eYVjJCzXWjAhdZiF
|
@cursoragent Inspect the PR and repository state, identify the root cause of every failing CI check, unresolved review comment, merge conflict, and related defect. Apply the smallest robust fix using existing project patterns and best practices, without unrelated changes. Run Bugbot and all relevant lint, type-check, test, build, and CI-equivalent commands, fix any issues found, then resolve only the review comments that are fully addressed. Summarise the root causes, changes made, validation results, and any remaining blockers. |
|
PR #1396 is MERGEABLE again ( Root causes
Changes made
Validation
Remaining blockers
|
|
Two environment traps on this branch, from having just hit both. Flagging them because they will mislead a verification sweep rather than merely slow it down. 1. 2. The container's Playwright browsers do not match the pinned client. The repo's client resolves headless-shell build Do not run SRC=/opt/pw-browsers/chromium_headless_shell-1194/chrome-linux
DST=/opt/pw-browsers/chromium_headless_shell-1234/chrome-headless-shell-linux64
mkdir -p "$DST"
for f in "$SRC"/*; do ln -sfn "$f" "$DST/$(basename "$f")"; done
ln -sfn "$SRC/headless_shell" "$DST/chrome-headless-shell"
touch /opt/pw-browsers/chromium_headless_shell-1234/{INSTALLATION_COMPLETE,DEPENDENCIES_VALIDATED}That is container-local, commits nothing, and does not survive the session. Filed as Verification already completed on
What is genuinely outstanding is the full Generated by Claude Code |
|
@cursoragent Inspect the PR and repository state, identify the root cause of every failing CI check, unresolved review comment, merge conflict, and related defect. Apply the smallest robust fix using existing project patterns and best practices, without unrelated changes. Run Bugbot and all relevant lint, type-check, test, build, and CI-equivalent commands, fix any issues found, then resolve only the review comments that are fully addressed. Summarise the root causes, changes made, validation results, and any remaining blockers. |
|
PR #1396 tip Root causes
Changes made
Validation
Remaining blockers
|
|
@codex review PR |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 581dd31eb4
ℹ️ 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".
|
@cursoragent please resolve the comments and apply fixes from the comments and fix merge conflicts also |
|
Resolved the open review work and the Production UI / merge blockers on #1396. Tip is Root causes
Changes
Validation
|
|
@CodeRabbit review PR |
|
✅ Action performedReview finished.
|
|
@codex resolve actionable Codex review findings for this pull request and current head using the repository instructions. This is the pull request's single automatic repair pass: do not perform a fresh review, create new standalone findings, or request another review. Work only the existing unresolved Codex threads on the current head. Always fix P0 and P1 findings. For P2 and lower findings, fix only clear, scoped, low-risk issues; otherwise disposition them with a concise reason. After fixing or dispositioning a thread, reply in that thread with as the first line, followed by a concise summary; that marker authorizes the workflow to close that exact thread. If human input or new authorization is required, do not use the marker and leave the thread open with the blocker. Finish only after every actionable thread is fixed or dispositioned and closed, or explicitly left open for a human decision. Do not update the branch from main, address unrelated reviews, broaden scope, or create more than one scoped fix commit. Do not use external APIs, paid services, credentials, dependency changes, or broad refactors unless explicitly authorized. Add targeted tests where behavior changes and run the narrowest relevant validation. |
Fixes both Codex P1 findings. They share one root cause: the transform and the reserve were applied at render time while their compensations only arrived after hydration, so a cold load painted wrong and then corrected itself -- a layout jump introduced by a change whose purpose is removing layout jumps. P1 (reserve): --phone-overlay-chrome-h did not exist until a passive useEffect ran, and the utility fell back to 0px, so first paint put content underneath the out-of-flow header and hydration pushed it down by the measured stack height. Now seeded in globals.css as calc(var(--safe-area-top) + var(--shell-header-h)) so it is never absent, refined in useLayoutEffect so the measurement commits before paint, and the 0px fallback is gone. SSR cannot measure, so the seed is exact only for routes without a header-collapse-addon row; addon routes refine by that row's height, which is far smaller than the previous full-header jump but not zero. P1 (dock): PhoneFooterLayerPortal starts with isPhone=false, so on SSR and first client render the composer rendered inline -- inside the stack that carried max-sm:translate-y-0. The containing block existed from first paint but the portal escaping it did not, leaving the dock header-anchored until hydration. Fixed at the source by dropping the resting transform: the revealed state is now transform-free, so no containing block exists at rest and the dock is viewport-anchored in the server markup. The hidden state keeps -translate-y-full, and a transition from none interpolates from the identity transform, so the hide still animates. The portal stays as defence for the during-hide window. Also records why the earlier browser proof missed this: the 13 focused journeys all assert after hydration, so a pre-paint shift is invisible to them. Two new guards close that gap statically -- the CSS seed plus useLayoutEffect, and no resting transform. Verified: 13 focused Chromium phone-chrome journeys still pass (the hide animation survives losing the resting transform); verify:cheap Test Files 432 passed, Tests 4461 passed | 4 skipped; typecheck, lint, prettier clean; contract suite 28 passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01361jh3eYVjJCzXWjAhdZiF
The contract still described PhoneFooterLayerPortal as the fix for the containing-block trap. After 8e003c8 that is no longer the primary fix and the doc would have taught the wrong thing: the portal starts with isPhone === false, so it does nothing until hydration and cannot protect first paint on its own. Resting-state-clean is the actual rule; the portal is defence in depth for the during-hide window. Also records the third load-bearing property of the reserve, which was missing entirely: it must be correct before the first paint, hence the CSS seed plus useLayoutEffect and no `,0px` fallback. Notes that SSR cannot measure, so the seed is exact only for routes without an addon row. Caught while reviewing my own change rather than by a reviewer, which is the sweep I should have run after the previous three retractions in this branch's history. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01361jh3eYVjJCzXWjAhdZiF
|
@Cursorsgent fix merge conflict |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4febc11bb7
ℹ️ 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".
Overlay/reserve-only hide activates after 8px, so the old 40px pre-scroll hid the portaled dock before the click and timed out as outside-viewport.
Keep GitHub mergeability honest after main advanced; merge-tree was already clean.
Exact duplicate of the white-element-positioning pr-babysit record reappeared via merge=union when syncing main; remove the later twin only.
The overlay-aware asserts in f2e7d3e/45f9aeb9 guarded with `Boolean(collapse) && collapse.getAttribute(...)`. That reads as a null guard but TypeScript does not narrow through a `Boolean()` call, so all seven snapshot sites went red under TS 6.0.3: tests/ui-phone-scroll.spec.ts(997,42): error TS18047: 'collapse' is possibly 'null'. ... same at 1493, 1531, 1556, 1816, 1845, 1874 `collapse !== null && ...` narrows and is runtime-identical for `HTMLElement | null`, so the overlay/collapse semantics those commits established are unchanged: a missing collapse track (Answer mode's strategy:"overlay") still reports usesCollapse false. typecheck: exit 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01361jh3eYVjJCzXWjAhdZiF
…01v' into claude/latency-findings-impl-s8g01v
…able Every open PR sampled carries the same ci/circleci: verify failure, including #1400 which is a docs-only AGENTS.md change, so it is not branch-specific. The job's whole contents were mirrored locally on #1396's tip and all of it is green, including the PyMuPDF-gated PDF tests under a venv built exactly as .circleci/config.yml builds it. That puts the fault in the job environment. Recorded with the quota hypothesis marked explicitly unverified: the CircleCI project is private, no session has a CircleCI token, and the unauthenticated API returns "Build not found". Reading the failing step needs an operator. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01361jh3eYVjJCzXWjAhdZiF
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f79c91856c
ℹ️ 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".
Codex P1 (second round). My seed was calc(var(--safe-area-top) + var(--shell-header-h)), but the overlay header pads its own top with pt-[max(0.5rem,var(--safe-area-top))] while --shell-header-h (4rem) covers only the inner min-h-14 bar plus its pb-2. The two classes sit on different elements, so the padding genuinely adds. Verified arithmetic rather than taking the report's figure: rendered height is max(0.5rem, inset) + 3.5rem + 0.5rem, so the seed under-reserved by exactly max(0, 0.5rem - inset). That is nothing on a notched iPhone, where the inset already exceeds 0.5rem, and a full 8px on any phone reporting no top inset -- Android, and Playwright's default emulation. The layout effect then corrected it after paint, which is the same cold-load shift this change exists to remove, just smaller than the one the first round fixed. Guards now pin both halves so they cannot drift apart: the CSS seed's max() form, and the header's pt-[max(0.5rem,...)] it has to mirror. verify:cheap: exit 0, Test Files 432 passed (432), Tests 4473 passed | 4 skipped. Contract suite: 37 passed (37). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01361jh3eYVjJCzXWjAhdZiF
…01v' into claude/latency-findings-impl-s8g01v
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
…01v' into claude/latency-findings-impl-s8g01v
The 9df21db main sync re-duplicated the claude/white-element-positioning-t607pk pr-babysit row (lines 1329/1335, byte-for-byte identical), failing check:branch-review-ledger and with it Static PR checks and the PR required aggregate. This is ledger #88 recurring on every main sync: merge=union keeps both sides of a row that both branches appended. Removing an exact duplicate is the one mutation the append-only contract permits. Verified identical with diff before deleting the later copy. check:branch-review-ledger: 1296 table records, no duplicates. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01361jh3eYVjJCzXWjAhdZiF
Follow-up to the #1396 review. The overlay branch of this test was moved until it passed, and nothing checked that its scroll position still exercised a refusal, so it could have stopped guarding anything silently. Derives both target offsets from computeScrollHideUpdate's own clauses instead of chosen numbers, because the two motions refuse for different reasons: in-flow runwayAfterCollapse <= revealIntentDistance + collapseRunwaySlack reserve-only offset > postCollapseMaxOffset + bottomClampTolerance Adds non-vacuity assertions ahead of the behavioural check. "Chrome stayed visible" only evidences a refusal if the offset sat inside the refusal band and the other clauses were satisfied. The load-bearing one is postCollapseMaxOffset >= topRevealOffset + hideIntentDistance: without it, a short post-collapse range refuses the hide on its own and the near-bottom clause is never reached, so the test passes while proving nothing. Renames the test. "96px of near-bottom runway" named collapseRunwaySlack, which exists only on the in-flow branch; reserve-only has no slack term. One test covers both owners, so the title now names the behaviour. Nothing outside the definition referenced the old name, so no flake-ledger identity is broken. No production hide policy changed — only what the test asserts about it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01361jh3eYVjJCzXWjAhdZiF
PR #1396 repeatedly declared physical-device Safari/PWA acceptance (docs/phone-chrome-physical-acceptance.md) as required before merge because headless Chromium cannot certify Safari chrome minimisation or cold-launch PWA paint, then merged with the checklist still blank. Also notes a related missing pre-paint/cold-load hydration test the same PR's review flagged but never filed.
… PR babysitting (#1421) * issues: capture two CI/merge operational findings from PR babysitting #117: this repo's CI (on: pull_request with default types) doesn't retrigger on the draft-to-ready transition, only on opened/synchronize/reopened — a marked-ready PR can sit with a minimal check set until an actual new commit lands. #118: GitHub's update-branch API doesn't honor the merge=union .gitattributes driver on docs/branch-review-ledger.md, so it can 422 with a false conflict that a local git merge resolves cleanly. Both observed today on PR #1406. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Re9ERtQwJ82ErbAnahAhsa * Address Codex review: clarify ledger rows #125/#126 wording #125: spell out that ready_for_review must be added alongside the existing pull_request.types defaults, not replace them. #126: qualify the update-branch fallback push with the same explicit user confirmation the provider boundary already requires. * docs(issues): capture #1396's unaddressed physical-device chrome gate PR #1396 repeatedly declared physical-device Safari/PWA acceptance (docs/phone-chrome-physical-acceptance.md) as required before merge because headless Chromium cannot certify Safari chrome minimisation or cold-launch PWA paint, then merged with the checklist still blank. Also notes a related missing pre-paint/cold-load hydration test the same PR's review flagged but never filed. * style: fix table padding drift from the main merge npx prettier --write after merging main (9e2fe44) — a table cell width shifted during the merge and format:check would have caught it. --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Main's side carried #128-#130, archived #116 and reordered #117-#119, so it is taken whole; only the #127 row is grafted from this branch, since main still holds the withdrawn "sharedChromePinned is stuck" text. Marker stays at main's 131. The file now reports `union merge active`, so this class of collision should stop. Also cross-links #130 into #127. PR #1396 ("overlay the phone header so hiding it never moves content") merged at 06:49:55, about twenty minutes before the first failure, and rewrote the exact overlay path this defect sits in — phoneOverlayMotion, the overlay stack transition, and readChromeCollapseMetrics charging zero released top geometry. Its merge commit 90b3e34 is the single main run that passed, which fits the passed-once-then-failed-twice-escalating shape better than the focus-latch reading. #130 also records that #1396's own declared physical-device prerequisite was never run, and that headless Chromium cannot certify what it covers. Verified after the merge: check:outstanding-issues 128 rows / unique ids / next-id=131 / union merge active; check:branch-review-ledger 97 live + 1206 archived; prettier clean on all three changed files. Not re-run after the merge: verify:cheap and the phone-scroll spec, which both passed on this branch's own commit before it (434 test files / 4562 tests, and 56 passed). The merge adds only main's already-verified tree plus one docs cell, so the earlier evidence still covers this diff — but it is the earlier commit's evidence, not this one's. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XrPbbfU9yWuEjEVypCr4ZQ
…ottom refusal guard non-vacuous (#1423) * docs(operator): correct the mislabelled ordering-safe documents index My error, live in a merged runbook. The 2026-07-29 correction identified the unordered LIMIT 12 in rag-candidate-sources.ts:482 as the RAG hazard, then attributed it only to the trigram index and listed documents_status_id_idx as ordering-safe. Re-reading the statement it describes: query = query.or(filters).eq("status", "indexed").limit(12); One statement carries both the status equality and the unordered LIMIT 12, and (status, id) is exactly the index serving that equality. So the mechanism the doc already documents applies to it verbatim: a new plan can return a different twelve title-alias rows into candidate assembly. It is canary-gated. The generalisation that produced the error is worth naming, because it is the trap: (status, id) has two consumers. search-scope.ts:271-277 pages with an explicit .order("id") and is genuinely safe; rag-candidate-sources.ts:482 is unordered and is not. Safety of one consumer is not transitive to the other. Also marks the SQL block itself, which is what an operator actually copies. The prose gated the indexes while the block still listed all three together with no distinction, so a copy-paste run would apply the canary-gated ones regardless. Gates: prettier clean; docs:check-links 1381 references resolve; docs:check-scripts 395 npm-run references resolve. No code or SQL applied. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01361jh3eYVjJCzXWjAhdZiF * test(chrome): make the calculator near-bottom refusal non-vacuous Follow-up to the #1396 review. The overlay branch of this test was moved until it passed, and nothing checked that its scroll position still exercised a refusal, so it could have stopped guarding anything silently. Derives both target offsets from computeScrollHideUpdate's own clauses instead of chosen numbers, because the two motions refuse for different reasons: in-flow runwayAfterCollapse <= revealIntentDistance + collapseRunwaySlack reserve-only offset > postCollapseMaxOffset + bottomClampTolerance Adds non-vacuity assertions ahead of the behavioural check. "Chrome stayed visible" only evidences a refusal if the offset sat inside the refusal band and the other clauses were satisfied. The load-bearing one is postCollapseMaxOffset >= topRevealOffset + hideIntentDistance: without it, a short post-collapse range refuses the hide on its own and the near-bottom clause is never reached, so the test passes while proving nothing. Renames the test. "96px of near-bottom runway" named collapseRunwaySlack, which exists only on the in-flow branch; reserve-only has no slack term. One test covers both owners, so the title now names the behaviour. Nothing outside the definition referenced the old name, so no flake-ledger identity is broken. No production hide policy changed — only what the test asserts about it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01361jh3eYVjJCzXWjAhdZiF * docs(issues): correct #102's stale ordering-safe claim for (status,id) Codex review on this PR caught that docs/outstanding-issues.md #102 still called documents_status_id_idx ordering-safe after this PR's own runbook correction reclassified it as canary-gated (its rag-candidate-sources.ts consumer shares the RAG-hazard mechanism with the trigram index; only the search-scope.ts ordered consumer stays safe). Mirrors the runbook's exact corrected wording so an operator following the ledger entry alone doesn't retain an unguarded index. * docs: record PR 1423 review --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>


Summary
Reported symptom: the phone header bar is choppy when it hides, and content is pushed up and down with it.
The cause was the hide mechanism, not its timing. Phones used
collapse, which is a1fr → 0frgrid on the header row plus aheighttransition onchrome-safe-area-topplus the reserve-padpadding-bottomtransition. Every hide animated three heights and handed that layout back to the scroller, so content slid up underneath the animation. No amount of easing fixes that — the layout genuinely moves.Overlay motion already existed, was already proven, and was gated to
isDocumentViewerOwnedRoute(pathname). It translates the whole stack instead and charges zero released top geometry inreadChromeCollapseMetrics, so content geometry never changes. Phones now use it on every route, which is why document detail already felt right and everything else did not.global-search-shell.tsx—phoneMotion: "overlay"unconditionally.use-phone-overlay-chrome-reserve.ts(new) — overlay takes the stack out of flow (.phone-overlay-headerisfixedin browser tabs,absolutein standalone), so content needs clearance or the first row sits under the chrome at scroll top. Publishes the measured stack height as--phone-overlay-chrome-h; the shell reserve pad reads it. It is measured, not tokenised, because the collapse row grows with page navigation portaled intoheader-collapse-addonand a fixed4remwould clip those routes. It never varies with hide state —offsetHeightignores the hide transform, so one value is correct in both states, and a reserve that animated would reintroduce the exact shift this removes. It is only visible near scroll top, where the header is always revealed, so it costs no usable height.master-search-header.tsx— portals the phone bottom dock out of the transformed layer. See below; this fixes a regression the overlay switch introduced.docs/search-chrome-behaviour.md— the contract, since this is a governed surface.header-scroll-hide-contract.test.ts— re-pointed at the new invariant, plus two new guards.Correction — the overlay switch regressed the phone bottom dock
The first commit broke
phone bottom search dock stays edge-to-edge with safe-area padding inside the form. This is worth recording because the failure mode is genuinely counter-intuitive.The overlay hide translates the header stack, and a non-
nonetransform— includingtranslateY(0)at rest — makes an element a containing block forposition: fixeddescendants.searchComposerrenders inside that stack, so on non-document routes the dock'sbottom: 0resolved against the ~72px header rather than the viewport and landed near the top of the screen.bottomstill computed to0px, which is what makes this read as a positioning puzzle rather than a CSS error. Measured as a form bottom 772px from the viewport bottom at 390×844 — exactly 844 − 72.Document routes never hit it because their composer is DocumentViewer-owned and already portals to the frame footer host.
Fix: wrap the composer in
PhoneFooterLayerPortalwhenphoneOverlayMotion && usesPhoneBottomDock. Phones host it on the footer layer;sm+keeps it inline in the sticky [top bar | search] stack. That split matters — tablet may not double-sticky the composer, and a breakpoint-aware portal is the one mechanism satisfying both constraints. It is also the mechanism invariant 21 already requires of the DocumentViewer, calculator and differential footers, so this aligns the shell dock with the three footers that already do it.Two alternatives were considered and rejected. Rendering the composer twice per breakpoint would satisfy the CSS but duplicate page-root
data-testids, which is its own failure mode (invariant 17, and the live#093). Animatingtopinstead oftransformavoids the containing block, but changes the already-proven document-route path for no gain.Verification
npm run verify:cheap—Test Files 432 passed (432),Tests 4452 passed | 4 skipped (4456).verify:phone-chromeselects for these files. Includes the dock-geometry test that caught the regression above, and bothdocument detail header overlay and footer followtests (browser + standalone PWA) that were failing in CI on Implement the free and flag-gated latency audit findings #1377.typecheck,lint,prettier --checkclean.UI verification not run:
npm run verify:uicould not execute in this container.npm run verify:phone-chromeescalates to it, and the container's Playwright browsers are build1194while the repo's client expects1234, so every browser test failed at launch withExecutable doesn't exist … chrome-headless-shell. I bridged that locally with symlinks under/opt/pw-browsersto obtain the focused proof above; nothing in the repo changed. The fullverify:uisweep and physical-device acceptance (docs/phone-chrome-physical-acceptance.md) are still outstanding — local Chromium cannot certify Safari or cold-launch PWA paint, and this change alters shared safe-area/ownership behaviour, so that checklist genuinely applies before merge.Separately, and worth fixing independently of this PR:
verify:phone-chromeexited 0 while reporting 13 failed tests, because every failure was a browser-launch error. A gate that returns success when no test ran is a green-when-broken gate on the phone-chrome path.eval:retrieval:quality,eval:rag,eval:quality,check:production-readinessandcheck:supabase-projectwere not run: no retrieval, ranking, selection, chunking, scoring, answer-generation, privacy, Supabase or deployment behaviour changed.check:driftwas not run becausesupabase/**is untouched.Risk and rollout
Clinical Governance Preflight
Clinical KB Database(sjrfecxgysukkwxsowpy)Notes on the above: this change moves chrome, not content. No answer generation, citation, verification, source-governance or document-access logic is touched, so source-backed claims and conservative source-metadata behaviour are unchanged by construction. No Supabase configuration is touched, so the pinned target is unaffected, and no credential enters the client graph — the new hook reads only element geometry and writes one CSS custom property. Nothing alters clinical decision-support behaviour, so the SaMD classification is unaffected. The one clinically-relevant property is legibility of the document surface, which improves: the reading position no longer shifts when chrome hides.
Notes
phoneMediaQueryis now exported fromuse-hide-on-scroll.tsrather than duplicated in the new hook, so the phone breakpoint keeps a single definition.mobile-composer-reserve-padrather than#main-content, matching the existing comment there: padding on the scrollport itself is omitted fromscrollHeightin some flex/overflow combinations, while the inner block box includes it.🤖 Generated with Claude Code
https://claude.ai/code/session_01361jh3eYVjJCzXWjAhdZiF
Generated by Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests