fix(navigation): keep focus and search context when seeding the home mode - #1760
Conversation
…mode The cold-`/` seed rewrote the URL with `appModeSelectionHref(lastAppMode)` and nothing else, so a visit carrying `focus=1` or scope/queryMode context lost it. Seeding only runs when the URL names no mode or query, but those params can still be present, and dropping them silently discarded a requested composer focus and any scoped-search context. Carry the parsed navigation context through, reusing readSearchNavigationContext the same way the root and /medications routes already do. Raised by CodeRabbit on PR #1744. The two sibling sites it flagged (`(search-app)/page.tsx`, `medications/page.tsx`) already preserve the context; this was the only one left. The contract test pinned the old single-line replaceState call, so it now pins the behaviour instead: replaceState rather than push, plus the context being carried. Its fixed 1800-character slice window is gone too — the window had already stopped covering the effect once the doc comment grew, which is exactly how a source-contract test goes quietly blind. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wa7MTVhAJMztFMMhzJf2J4
…op false regression (#286), and a third #255 reproduction Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wa7MTVhAJMztFMMhzJf2J4
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in:53 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 (4)
Comment |
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wa7MTVhAJMztFMMhzJf2J4
…ve outstanding-issues) Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Uh oh!
There was an error while loading. Please reload this page.
Summary
Follow-up to PR #1744 (merged as
a503c22), which made/the single mode-routed home page. Three changes, each its own commit:Keep
focus=1and the scope/queryMode context when seeding the home mode.useHomeModeSeedrewrote a cold/visit withappModeSelectionHref(lastAppMode)and nothing else, so a visit to/?focus=1— or to/carrying scoped-search context — silently lost the requested composer focus and the scope on thereplaceState. It now parses the incoming params withreadSearchNavigationContextand carriesfocus,queryMode,scopeFiltersandscopeRefthrough the rewrite, matching what(search-app)/page.tsxandmedications/page.tsxalready do. This was the one genuinely outstanding CodeRabbit finding from feat(navigation): one shared home page, mode-routed search #1744; the other three it raised were already fixed onmain, verified rather than assumed (sitemap:checkpasses, and the drift manifest reportssanctionedPatternsPresent: []for the medications route).Repoint that hook's contract test at behaviour instead of an exact string.
tests/search-route-ownership.test.tspinned the old single-linereplaceStatecall verbatim. It now pins the two things that actually matter —replaceStaterather thanpushState(no history entry per mode seed), and the carried navigation context. Its fixed 1800-character slice window is also gone in favour of slicing to end of file: that window had already silently stopped covering the effect once the hook's doc comment grew, so the assertion was passing against a region that no longer contained the code under test.Three
docs/outstanding-issues.mdrows, deduped against the existing 140 open items. New#285records that this web container runs Node 24.13.0 whilejsdom@30.0.1requires^24.15.0, sonpm cicannot complete underengine-strict. New#286records thattests/pr-handoff-stop.test.tsfails on a clean checkout ofmainin this container, which reads as a false regression to any session that runs the full unit suite here. The third is an update to existing#255, described below.The
#255update is a self-correction#255already carried a stop rule: "do not pointexecutablePathat the mismatched 1194 build to force a green browser run — a browser gate run against the wrong revision is not evidence." The session that prepared PR #1744 did exactly that, and quoted the resulting406 passed, 2 failedinto that PR's body as browser evidence. CI's Production UI jobs did pass on correct binaries, so the change merged in #1744 is genuinely proven — but by CI, not by that local run. The ledger row now records this as a third reproduction, along with the two contributing causes: the tooling's own preflight actively recommends the override, and the prohibition lives only in the ledger rather than indocs/testing.mdwhere a session would look for it.Verification
Run on this exact tree, before
npm run formatwas committed:npm run lint— clean, 0 problemsnpx tsc --noEmit— cleanTests 1 failed | 5710 passed | 4 skipped (5715). The single failure istests/pr-handoff-stop.test.ts, which is the pre-existing container-specific failure this PR records as#286; it fails identically on a clean checkout ofmain.npm run check:maintainability-budgets—Maintainability hotspot budgets passednpm run check:outstanding-issues— passed,284 rows … unique idsnpm run format— run, and the result committedUI verification not run: this container's Playwright/Chromium revisions are mismatched (
#255), and forcing a green run by pointingexecutablePathat the wrong build is the exact anti-pattern this PR documents — a browser gate run against the wrong revision is not evidence. Browser proof is delegated to CI's Production UI jobs, which run against correct binaries. That is what#255prescribes.npm run verify:pr-localwas not run as a single command: it fails closed on the samenpm cifloor recorded in#285. The gates it would have selected for this scope — lint, typecheck, the full unit suite, format — were each run directly and are listed above.Risk and rollout
/seeding path. The changed hook only runs when the URL names no mode and no query, and the change strictly adds parameters to a URL it was already rewriting; it cannot make the rewrite fire in a case where it did not fire before. The remaining two commits are a test and documentation.git revert 51fc832restores the previous single-argumentreplaceState, and reverting the test commit alongside it restores the old assertion. The ledger rows are append-only documentation and need no rollback.Notes
RAG impact: no retrieval behaviour change — the diff touches only a client-side URL-seeding hook, its contract test, and a documentation ledger. No file under
src/lib/rag/**, clinical-search, retrieval-selection, ranking-config, answer-ranking, the eval harness, or the golden fixture is modified. This line is stated for the record;classifyPullRequestFilesreportsragRanking: falsefor this file set, so the declaration is not gate-required here.## Clinical Governance Preflightis omitted deliberately:classifyPullRequestFilesreportsclinicalRisk: falsefor these three paths.use-home-mode-seed.tssits undersrc/components/clinical-dashboard/but matches none of the access-control, privacy, patient-data, or upload/download patterns that make a presentation surface clinical-risk — the classifier's own comment calls out that living in the clinical-dashboard tree is not itself a trigger. Verified by runningevaluatePullRequestPolicyagainst this body and file list rather than inferred.Generated by Claude Code