fix(mobile): keep mode-home composer hero-centred on phones and never let it vanish#470
Conversation
… let it vanish Follow-up to #456, which fixed the phone-width composer vanish by raising the mode-home hero query to 640px — moving the phone composer into the bottom dock and regressing the hero-centred landing design that four advisory UI tests encode (confirmed A/B against pre-merge main). - Restore modeHomeComposerMediaQuery to (min-width: 0px) so mode-home shells portal the composer into the hero at every width. - Key the inline fallback on portal outcome instead of the media match: while a slot id is present the inline composer stays suppressed only until the portal mounts or the slot-retry budget exhausts, so the search can never vanish at any width even when the slot never appears. - Add a @critical composer-presence test (phone + desktop, dashboard and standalone shells) so this bug class blocks merges via the required Critical UI smoke gate instead of only failing the advisory suite. - Record the PR #456 review in docs/branch-review-ledger.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
Warning Review limit reached
Next review available in: 7 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. 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: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe mode-home search composer now matches all viewport widths, retries hero portal attachment, and enables an inline fallback after portal unavailability. A critical Playwright test covers phone and desktop routes, and the review ledger gains a new record. ChangesMode-home composer availability
Review ledger update
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant MasterSearchHeader
participant ModeHomeHeroSlot
participant InlineSearchComposer
MasterSearchHeader->>ModeHomeHeroSlot: synchronize portal target
ModeHomeHeroSlot-->>MasterSearchHeader: provide slot or remain unavailable
MasterSearchHeader->>MasterSearchHeader: retry target attachment
MasterSearchHeader->>InlineSearchComposer: suppress while portal is active
MasterSearchHeader->>InlineSearchComposer: enable after fallback eligibility
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error)
✅ Passed checks (10 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
…oser-hero-fix # Conflicts: # docs/branch-review-ledger.md
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/ui-tools.spec.ts (1)
479-483: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffFallback branch is not positively exercised by this gate.
The count check (Line 480) confirms one input, but the hero-containment assert (Line 483) only passes when the portal path succeeds. If the inline
desktopHomeComposerFallbackpath ever engages (slot never mounts within the retry budget), this test fails rather than validating that the safety net keeps a composer available. Consider adding a companion assertion/case that exercises the fallback so the new core logic has direct coverage.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/ui-tools.spec.ts` around lines 479 - 483, Extend the test around visibleGlobalSearchInput and the mode-home hero assertion to explicitly simulate the portal slot failing to mount or timing out, then verify the desktopHomeComposerFallback renders exactly one visible search input in its intended fallback location. Keep the existing portal-path assertions, and add a companion case that positively confirms the fallback composer remains available without requiring hero containment.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/ui-tools.spec.ts`:
- Around line 479-483: Extend the test around visibleGlobalSearchInput and the
mode-home hero assertion to explicitly simulate the portal slot failing to mount
or timing out, then verify the desktopHomeComposerFallback renders exactly one
visible search input in its intended fallback location. Keep the existing
portal-path assertions, and add a companion case that positively confirms the
fallback composer remains available without requiring hero containment.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 0f1088c5-0879-4e28-afb2-f9ec1405f8a1
📒 Files selected for processing (3)
docs/branch-review-ledger.mdsrc/components/clinical-dashboard/master-search-header.tsxtests/ui-tools.spec.ts
…oser-hero-fix # Conflicts: # docs/branch-review-ledger.md
…oser-hero-fix # Conflicts: # docs/branch-review-ledger.md
…anary discharge fix) (#529) * feat: harden public API limits, CI scoping, and server-only env boundaries - Fail closed on paid anonymous answer rate limits: drop caller-controlled User-Agent from quota keys, add a global durable anonymous ceiling, and refuse in-memory limiter fallback for the answer bucket outside local dev - Enforce server-only env.ts with a client-safe public-env module, tsx/vitest server-only stubs, and a post-build client-bundle secret scan - Replace monolithic CI with risk-scoped lanes (changes/static-pr/safety/ coverage/build/ui-critical/db-replay) behind a single PR required aggregate; full-run sentinel exercises every lane and force-push diffs fail open - Make registry corpus sync best-effort with hash-gated re-embedding that still refreshes rows on derived-metadata drift without new OpenAI calls - Redact structured error logs via safeErrorLogDetails across routes/seeds, return typed error codes from jsonError, and skip public scope enumeration in favour of the retrieval owner sentinel - Polish mode-home/medication UI (tap targets, scrollable pill rows, portal composer breakpoints) and tag @critical Chromium smokes for the CI lane Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test: align tsx runner assertions with reconciled offline eval and vitest config Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test: align documents-mode phone layout assertion with landed hero composer UX Main's #470 places the composer above the Start here region on phone mode homes; this branch's pre-merge assertion encoded the superseded layout. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: prettier-normalize merged branch review ledger table Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(eval): accept source-only answers for the diffuse discharge-documentation case discharge-documentation (and its P3 sibling quality-discharge-documentation) is the swing case in the Eval Canary's rag-only gate. The question has no single authoritative "discharge documentation contents" source, so the pipeline correctly returns a source-only answer citing the real discharge SOPs (Admission-to-Discharge / MHHITH). Whether it labels that answer grounded is environment-sensitive — a fragile source-backed recovery past missing_query_overlap fires locally but not in CI/prod — which flapped the nightly canary red on this one case even though retrieval always passes. Add an acceptSourceOnly flag on RagEvalCase and set it on both discharge cases. The eval now accepts a grounded answer OR a source-only answer, but only when the expected discharge documents are still cited: validateRagAnswer skips the grounded requirement, and grounded_supported_rate counts a source-only answer as satisfied only when expectedHit && citations>0 — so a genuine retrieval regression that stops surfacing the discharge docs still hard-fails. scoreAnswerQualityEvalCase honors the flag for the P3 metric. Tests cover the source-only-accepted, regression, and scorer paths. The underlying recovery-gate false-positive (shouldPreserveSourceBackedGeneratedAnswer preserving an off-topic extract as grounded) is a separate RAG-quality bug tracked out of band; this change only aligns the golden set with the correct source-only behavior. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Summary
01f2cee0d) confirmed all four Advisory UI regression failures on the fix(mobile): restore search composer on phone-width mode-home pages #456 run are caused by that change (tools launcher is usable at mobile,mode home routes center the shared search on mobile,document search mode lists matching documents and scope actions,phone bottom search dock hides while scrolling down on search results).modeHomeComposerMediaQueryto(min-width: 0px)so mode-home shells (dashboard and standalone) portal the composer into the hero at every width, phones included.@criticalcomposer-presence test (390px and 1280px,/?mode=answerand/services) so composer-vanish regressions block merges via the requiredCritical UI smokegate; the full 5-route design spec stays in the advisory suite.docs/branch-review-ledger.md.Verification
npm run verify:pr-localDuring development, use
npm run verify:cheapas the faster iteration gate before the final PR-local preflight.npm run verify:uiwhen UI, routing, styling, browser behavior, reduced-motion, or forced-colors behavior changednpm run verify:releasebefore release or handoff confidence claimsFor retrieval, ranking, selection, chunking, source/citation rendering, or answer-contract changes,
verify:pr-localrunseval:rag:offlineautomatically. Run the offline command directly during iteration before spending a live eval.npm run eval:retrieval:quality(must stay 36/36) when retrieval, ranking, selection, chunking, or scoring behavior changed — not applicable: no retrieval/ranking/scoring changenpm run eval:rag -- --limit 15+npm run eval:quality -- --rag-only— not applicable: no answer-generation changenpm run check:production-readiness— not applicable: no clinical workflow, privacy, env, Supabase, governance, or deployment changenpm run check:deployment-readiness— not applicable: no deployment startup changeRan locally (all green):
@criticaltest: 6/6 pass (tests/ui-tools.spec.ts,tests/ui-smoke.spec.tsgrep run against the repo dev server)npm run typecheck,npm run lint, focusedprettier --checkon changed filesnpm run test(vitest 1441: all pass;tests/architecture-boundaries.test.tsneeds--testTimeoutabove 15s on the slow local machine — passes 4/4 at 60s, matching prior ledger runs)npm run check:runtime,npm run check:github-actions,npm run sitemap:checkNot run locally: full
verify:uichromium matrix (multi-hour on this machine under load). Equivalent coverage runs in CI on this PR: requiredCritical UI smokeplus the fullAdvisory UI regressionsuite, which this PR is expected to return to green.Clinical Governance Preflight
Not applicable — this change touches the search composer placement UI and Playwright tests only; no ingestion, answer generation, search/ranking, source rendering, document access, privacy, production env, or clinical output behavior changed.
Notes
maintoday (merged with fix(mobile): restore search composer on phone-width mode-home pages #456 while its advisory check was red — the check is non-required by design).mainCI fails on every push because theRAG_QUERY_HASH_SECRETrepo secret is missing; the deployment boot smoke exits 1 andrelease-browser-matrixnever runs. Owner is adding the secret.🤖 Generated with Claude Code