fix(factsheets): restore composer suggestions and fix phone centering - #2101
Conversation
Two follow-up gaps surfaced after PR #2091 landed: `factsheets` was never added to the per-mode search-command-surface config table, so the shared composer silently fell back to a bare input with no "Try this" ticker, autocomplete suggestions, or cross-mode chips (every other hero-placement mode already had an entry). Separately, ModeHomeMain's "center" alignment — used by Factsheets since it has no action/pill content — never actually centered on phone: `<main>`'s flex-1 has no effect because its real DOM parent in GlobalSearchShell is a plain block element, not a flex container, so the box collapsed to its own content height and pinned to the top instead of centering in the viewport. Mirror the existing sm+ min-height rule for phone, scoped to the "center" variant only so startOnPhone/start pages are unaffected. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014aSV4sbw6iFqEA7Y7JeVes
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in:28 minutes Limit details: You’ve used all 1 included review currently available under your plan. You completed 101 included PR reviews in the past 7 days; at that activity level, included reviews refill at 1 review per hour. 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 (5)
Comment |
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014aSV4sbw6iFqEA7Y7JeVes
A frontend-ui-reviewer pass on the prior commit caught two real defects before merge: - The phone min-height only subtracted --shell-header-h, mirroring the sm+ formula verbatim. On phone, main's real parent (mobile-composer-reserve-pad) also carries real top/bottom padding (--phone-overlay-chrome-h, --mobile-composer-reserve) that is zero at sm+ but not on phone, so the under-subtraction pushed the document height ~40px past the viewport on /factsheets and /differentials (live-verified via document.documentElement.scrollHeight vs window.innerHeight — the actual scroll owner on phone per docs/search-chrome-behaviour.md; #main-content's own scrollHeight/ clientHeight is not a meaningful check since it isn't the bounded scrollport here). Subtract both padding terms instead; both are live CSS custom properties, so this composes correctly rather than baking in a snapshot value. - tests/mode-home-main-align.test.ts asserted a literal `center: "justify-center` adjacency, which the longer value broke once Prettier moved it onto its own line. Relaxed the regex to tolerate the whitespace Prettier legitimately introduces, without weakening what it actually checks (the value still starts with justify-center, the ordering the test cares about). test:focused's import-graph selection doesn't cover this test (it reads mode-home-template.tsx via readFileSync, not an import), so it passed focused review while this was broken; caught by running the full suite and the file directly this time. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014aSV4sbw6iFqEA7Y7JeVes
…-regression-8iyvnd' into claude/patient-factsheets-search-regression-8iyvnd
Supersedes the prior record for this branch: that verification missed a broken test and a live phone scroll-overflow regression that a frontend-ui-reviewer pass caught before merge. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014aSV4sbw6iFqEA7Y7JeVes
Uh oh!
There was an error while loading. Please reload this page.
Summary
Follow-up to #2091 (merged). After that PR landed, the user reported the Factsheets composer still didn't match other mode homes' behaviour. Investigating with a live local build (not just a screenshot) turned up two real, separate gaps neither PR #2091 nor its original scope touched:
factsheetswas never added to the per-modesearchCommandSurfaceByModetable insrc/lib/search-command-surface.ts.UniversalSearchCommandSurface(rendered insideMasterSearchHeader's composer chrome for every mode) early-returns to a bare input with no "✨ TRY THIS" suggestion ticker, no autocomplete suggestions, and no cross-mode chips whenever that lookup resolves tonull— every other hero-placement mode (DSM, Services, Forms, Specifiers, Formulation, Tools, Calculators, …) already has an entry; Factsheets silently didn't. Added afactsheetsentry (calculators' shape/pattern: local search,remoteSearchEnabled: false) with examples/suggestions drawn from real, currently-indexed factsheets (sertraline,lithium monitoring,CBT) so "Tap to search" resolves real results.ModeHomeMain'scontentAlign="center"(Factsheets' default, since it has no action/pill content to populatestart/startOnPhonelayouts with) is meant to vertically centre short, empty-ish mode homes in the visible canvas. On phone this silently never worked: its immediate DOM parent inGlobalSearchShell(mobile-composer-reserve-pad) is a plain block element, not a flex container, so itsflex-1/min-h-0classes have no effect there — the box collapsed to its own content height (measured 393px against an 844px viewport) instead of stretching to fill available space, sojustify-centerhad nothing to centre within and content just pinned to the top. The existingsm:min-h-[calc(100dvh-var(--shell-header-h))]rule already solves the identical problem at sm+ using the same calc; mirrored it for phone, scoped only to thecenteralignment variant inMODE_HOME_MAIN_ALIGN_CLASS(notstart/startOnPhone, which don't rely on container height to look correct, and are what every other mode home already uses).Both fixes verified live against a local dev server, side-by-side with
/dsm(composer stack now structurally identical — ticker, input, privacy notice) and/differentials(the only other page usingcontentAlign="center"; confirmed unaffected — its richer content already fills the viewport, and no scroll/overflow regression was introduced:scrollHeight === clientHeightbefore and after on both/factsheetsand/differentials).Verification
npm run verify:pr-local— not yet re-run against this exact combined diff at PR-open time; will run before merge if requested. Already independently verified:npm run test:focused -- --files src/lib/search-command-surface.ts src/components/mode-home-template.tsx(283 tests passed across 32 files),npm run typecheck(clean),eslinton both changed files (clean),prettier --checkon both changed files (clean).npm run verify:uiwhen UI, routing, styling, browser behavior, reduced-motion, or forced-colors behavior changed — not run as the full Chromium gate; instead verified directly with live Playwright checks at 390×844 against the local dev server (npm run ensure): confirmed the "TRY THIS" ticker renders and is tappable (fills the search input with "sertraline") on/factsheets, matches/dsm's composer stack pixel-for-pixel structurally, and the phone-centering fix produces a genuinely centered short home with no overflow/scroll regression on/factsheetsor/differentials(its owncontentAlign="center"no-results state), and no change at all to/dsm(usesstartOnPhone, untouched by this diff).npm run verify:releasebefore release or handoff confidence claims — not run; this is a PR handoff, not a release.This PR does not touch retrieval, ranking, selection, chunking, source/citation rendering, or answer-contract behaviour, so
eval:retrieval:quality/eval:rag/eval:qualityare not applicable.Risk and rollout
src/lib/search-command-surface.tsis a pure per-mode data-table addition (isolated, additive).src/components/mode-home-template.tsxis a shared layout primitive used by 8+ standalone mode homes, but the change is scoped to thecenteralignment variant only, which today is used by exactly two pages (factsheets, anddifferentials' no-results state) — both were checked live for regressions.Clinical Governance Preflight
src/lib/search-command-surface.tsmatches this repo's clinical-risk path pattern (anysrc/lib/**file with "search" in it), which is deliberately broad — but the actual change is a per-mode composer-suggestions data-table entry (search terms shown in a "try this" ticker) plus a layout centering fix; neither touches retrieval, ranking, answer generation, ingestion, or source rendering logic.Clinical KB Database(sjrfecxgysukkwxsowpy)Notes
src/lib/rag/**, clinical-search, retrieval-selection, ranking-config, answer-ranking, the eval harness, or the golden fixture, so noRAG impact:declaration applies.Generated by Claude Code