fix(dashboard): clearing the composer brought back two retired mode homes - #2730
fix(dashboard): clearing the composer brought back two retired mode homes#2730BigSimmo wants to merge 7 commits into
Conversation
…retired mode homes came back
Clearing the search box emptied the React query but left `q` and `run=1` in the
URL. `showSharedHome` reads `run=1` straight off the URL, so it stayed suppressed
while the mode branch below it, now seeing no query, fell back to the home its
route had been consolidated away from. Two modes were affected and both were
reproduced in a browser against a running build before the fix:
- `/?mode=prescribing&q=…&run=1`, clear -> `medication-home` (the Dose/Safety/
Monitoring tiles). `tests/ui-tools.spec.ts` asserts in a comment that this view
is "retired and no longer reachable from any route". It was reachable, by that
one click.
- `/documents/search?q=…&run=1`, clear -> `document-search-empty-state`, the
"Start here" home that `documents/page.tsx` records as deliberately retired.
This is the same defect a clinician reported for Sources, in two more modes: the
URL and the composer disagreed, and the mode branch resolved the disagreement by
rendering a page that no longer exists as a destination.
The fix is one handler. `onClearQuery` now also drops the submitted search from
the URL, so the shared home renders. `appModeSelectionHref` rather than
`appModeHomeHref`: every mode reaching this handler is dashboard-owned, so its
home is the shared home at `/`, and `appModeHomeHref("prescribing")` still answers
`/medications`, which only 307s back. `/documents/search` has to leave its own
route entirely, which the selection href also does.
The pre-existing `answer` guard is kept and is load-bearing. A generated answer is
durable state the URL does not own, so the clear button must not discard one; a
browser check confirms answer mode is untouched.
Both paths are pinned by new browser cases, which is where the defect was proved
rather than inferred.
Also fixes the third governance suite that fails on a shallow clone. `#1M0J6D`
added guards to the clinical-hazard and rag-plan suites; `privacy-readiness` was
left without one and still reported `reviewedCommit does not exist: <sha>`, which
reads as a governance breach rather than a missing object. It now mirrors the two
siblings exactly. The gate is not weakened: on a full clone the commit resolves,
`checkGit` stays true, and the real ancestry check runs.
Queues a correction to ledger row `#ZKR5YK`, whose Sources, Documents and
Medication examples are gone now that all three bare paths redirect. Tools is the
only live case left, and it stays an owner ruling rather than a fix.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QaizDqAQZWHgfGLitWU8xf
…queued for it Another session had a `done` pending on the same row (4036f064), backed by an implementation of the mode-pill change the row asks for. Two pending mutations on one row block reconciliation for everyone, which is the race `#686WHW` records, and a re-scope is pointless against a close that supersedes the whole row. The findings that prompted the re-scope are verified and worth keeping, so they are recorded in the cancel reason and in this branch's PR rather than lost: the row's Sources, Documents and Medication examples are all gone now that those three bare paths redirect, Favourites is settled by owner decision `#V0EDR4`, and Tools was the only live case left at the time of the check. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QaizDqAQZWHgfGLitWU8xf
…udget The explanatory comment pushed ClinicalDashboard.tsx to 4141 lines, one over its 4140-line no-growth budget. The gate's advice is to extract a module rather than grow the monolith; extracting this file is a refactor of its own, so the proportionate answer to a one-line overrun is to say the same thing in fewer lines. The full reasoning, the measured reproduction and the mutation check all live in the commit that carried the fix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QaizDqAQZWHgfGLitWU8xf
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
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_009cdec3-35ec-4549-a5c5-8d07f370c72f) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d8367c6a71
ℹ️ 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 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. The only repository destination is BigSimmo/Database, and the only branch destination is the pull request head branch claude/mode-home-clear-fix at starting commit d8367c6; never publish fixes to a detached or synthetic work branch and never create a stacked pull request. Use the authenticated GitHub connector to commit each approved fix to BigSimmo/Database:claude/mode-home-clear-fix, then verify that the pull request head contains the published commit before reporting success. 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. For a fixed thread, reply with as the first line and as the second line. For a no-code disposition, use followed by . These result markers authorize the workflow to close that exact thread only after it verifies a fixed commit is the pull request head; a local-only commit is not a fix. If publication or verification fails, do not use either result marker, do not claim success, and leave the thread open with the blocker. If human input or new authorization is required, do the same. 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. |
Summary
Testing
|
…ct its alias Tools had two homes. `/tools` is the canonical directory (filter ribbon, full catalogue, detail panel). `/?mode=tools` rendered a separate hub-shaped launcher, because `shouldShowSharedHome` excludes `tools` and so the dashboard had nothing else to fall back to. Which surface a clinician saw depended only on how they arrived: the mode pill gave them the hub, every other link gave them the directory. The hub's one unique feature was its verb shortcut row (Ask, Compare, Prescribe, Safety, Docs, Refer, Forms, Saved), which answers "what am I trying to do" rather than naming a mode. That row moves to `src/components/tools/tool-quick-actions.tsx` and renders above the directory results, sourced from `accessibleTools` so a category filter cannot empty it, and hidden once a query is running. With nothing of its own left to show, `/?mode=tools` now redirects to `/tools` unconditionally, carrying its query string through. `localSmartExcludedToolIds` was declared three times; the shared copy in `src/lib/tools-catalog.ts` is now the one definition. The legacy launcher component is left in place, unreachable, rather than deleted here: `tests/favourites-auth-gate.dom.test.tsx` still proves real guest-access behaviour against it, and re-homing that coverage onto `/tools` is a separate change. Browser cases that loaded `/?mode=tools` now assert the same contracts on `/tools`: the shortcut row, the phone filter sheet, the detail panel launch links, and no shared search chrome on a route that owns its own filtering. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QaizDqAQZWHgfGLitWU8xf
… claude/mode-home-clear-fix
The redirect landed clinicians on a directory with no way to type a tools search. `/tools` deliberately renders no shared composer because it owns its own filtering, but its filtering was reachable only through the URL: the box lived on the `/?mode=tools` hub, and redirecting that alias took the box with it. `tests/ui-clinical-ask.spec.ts` caught it — local-only Smart search in tools mode has nowhere to be typed. So the box comes across with the shortcut row, in flow rather than fixed, as a page-owned filter and not a second shared composer. A draft typed here wins over the URL and the shared command draft, the same precedence the hub used. Submitting navigates to `/tools?q=…&run=1` so a result set stays shareable and survives reload, which is what the hub's submit control already did. Verified in a browser on the failing journey: typing a natural-language query hides the shortcut row, submitting reaches the ranked results, Answer, Documents and Favourites stay excluded, and neither the clinical-ask nor the universal-search endpoint is called. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QaizDqAQZWHgfGLitWU8xf
Summary
Follow-up to #2672. You asked me to root-cause seven items from that PR's audit and apply the smallest safe fix to each. Three were real, two were my own misreads, one is yours to rule on, and one was resolved by another session. All of that is set out below with evidence.
Issues 1 and 2 turned out to be a single root cause, and it is the same defect a clinician reported on Sources. Clearing the composer emptied the React query but left
qandrun=1in the URL.showSharedHomereadsrun=1straight off the URL (submittedUrlRunRequested), so it stayed suppressed while the mode branch beneath it, now seeing no query, fell back to the home its route had been consolidated away from. Reproduced in a browser against a running build before any change:/?mode=prescribing&q=…&run=1medication-home(the Dose/Safety/Monitoring tiles)/documents/search?q=…&run=1document-search-empty-state(the "Start here" home)Both are described as retired by the repository itself —
medications/page.tsxsays the tiles were retired,documents/page.tsxsays the idle Documents view was, andtests/ui-tools.spec.tsasserts in a comment thatmedication-homeis "retired and no longer reachable from any route". It was reachable, by that one click.The fix is one handler.
onClearQueryinClinicalDashboard.tsxnow also drops the submitted search from the URL, so the shared home renders.appModeSelectionHrefrather thanappModeHomeHref, because every mode reaching this handler is dashboard-owned so its home is the shared home at/, andappModeHomeHref("prescribing")still answers/medications, which only 307s back./documents/searchalso has to leave its own route entirely, which the selection href does.The pre-existing
answerguard is kept and is load-bearing: a generated answer is durable state the URL does not own, so the clear button must never discard one. Verified separately in a browser that answer mode is untouched.Issue 7 fixed.
tests/privacy-readiness-contract.test.tswas the third governance suite failing on a shallow clone withreviewedCommit does not exist: <sha>, which reads as a governance breach rather than a missing object.#1M0J6Dguarded the other two and left this one. It now mirrors them exactly. The gate is not weakened: on a full clone the commit resolves,checkGitstays true, and the real ancestry check runs.Issue 6 resolved, but not here. A competing
donerequest (4036f064) was already pending on#ZKR5YK, backed by an implementation of the mode-pill change. Two pending mutations on one row block reconciliation for everyone (#686WHW), so this branch cancels its own re-scope rather than fight over the row.Three items deliberately not changed
/?mode=toolslauncher. Confirmed live by browser probe:/?mode=toolsrenderstools-homefrom the legacy client-only launcher while canonical/toolsrendersToolsSearchResultsPage. It is real duplication, but removing it is an owner ruling plus a sizeable refactor, not a small safe fix: the mode pill deliberately always returns to the shared home, so special-casing Tools trades one inconsistency for another, and deleting the launcher means re-pointing six assertions intests/favourites-auth-gate.dom.test.tsxthat render it directly. Left for a decision.favourites-hubtest id. Not a defect — my misread. The three nodes are mutually exclusive branches identifying one logical surface, andexpectSingleSettledOwner(a general Next-streaming helper, not a workaround for this) asserts exactly one owner and passes./documentsand/medicationsintools-catalog.ts. Not a defect — also my misread. These render as plain<a href>links to a mode's canonical namespace root, which redirects server-side; that is normal, not the client-side push into a 307 that made the/sourcescase worth changing. Changing them would break six pinned expectations acrossui-tools.spec.tsandsearch-pins-menu.dom.test.tsxfor no user-visible gain.Flagged, not actioned
appModeHomeHref("prescribing")returns/medicationsfor the bare case while returning/?mode=prescribingfor every case that carries a query — an inconsistency inside one function, and the reason the sidebar link, the medication back link and search pins all route through a 307. Fixing it is correct but changes two pinned hrefs for no visible gain, so it is recorded here rather than bundled in.RAG impact: none — no file under
src/lib/rag/, no retrieval RPC, no ranking config, no eval harness or golden fixture is touched.Verification
npm run verify:pr-local— 40 gates completed, 0 failed, 0 not reached, includinglint,typecheck,test(the full offline suite, which now passes outright because this PR fixes the third shallow-clone suite) andbuild.if (false && submittedUrlRunRequested)) both new browser cases fail; restored, both pass. The guards are real, not vacuous.tests/ui-tools.spec.ts99 passed (2.2m) andtests/ui-smoke.spec.ts108 passed (3.5m) — 207 tests across the two specs this change touches, the two most likely to catch a dashboard regression.medication-home1 → 0 andshared-home-empty-state0 → 1, with the URL landing on/?mode=prescribing&focus=1; the Documents pair moved the same way to/?mode=documents&focus=1.UI verification not run:
npm run plan:browserreturnslevel: full, escalating becausesrc/components/ClinicalDashboard.tsxis a shared foundation, so no narrowed run of the whole suite was available and a local fullverify:uiwould only re-derive the verdict CI reaches. The focused browser proof above covers the changed surface; the complete Chromium suite is left to the Production UI job, which runs because this PR is opened ready for review rather than as a draft.Risk and rollout
/dsm/search,/services/searchand the rest) never reach this handler and are untouched, which the 207 passing browser tests bear out.9b4533aandd8367c6. The ledger commitd3e1b24is an immutable inbox record and should be left alone. No data, schema, or configuration change, and nothing to unwind on the provider side.Clinical Governance Preflight
This change touches one composer event handler and one test guard. It does not alter ingestion, answer generation, search ranking, source rendering, document access, privacy, the production environment, or clinical output. What it removes is a retired page appearing where a clinician expected their search cleared, which is a small improvement to the honesty of the interface rather than a change to any clinical content. Each box below is confirmed unchanged rather than newly assured.
Clinical KB Database(sjrfecxgysukkwxsowpy)No clinical decision-support behaviour changed, so the SaMD classification is unaffected.
Notes
Branch naming: this work belongs on
claude/vigilant-mayer-60ftd5, whose previous PR (#2672) was merged. The branch was correctly restarted from latestmain, but publishing it needed either a force-push or a remote branch deletion, and this environment denies both. With the owner's approval it was published underclaude/mode-home-clear-fixinstead. The staleclaude/vigilant-mayer-60ftd5still points atccc6a7d, whose content is already onmainvia #2672, and can be deleted safely.🤖 Generated with Claude Code
https://claude.ai/code/session_01QaizDqAQZWHgfGLitWU8xf
Generated by Claude Code
Note
Low Risk
Single composer clear handler plus test-only shallow-clone guards; answer-present guard unchanged, so generated answers are not cleared by this URL sync.
Overview
Clearing the search composer no longer leaves
run=1(and related query params) in the URL after the React query is emptied. That mismatch had keptshowSharedHomeoff while prescribing and documents modes fell back to retired idle UIs (medication-home,document-search-empty-state) instead of the consolidated shared home. TheonClearQuerypath inClinicalDashboardnow callsrouter.replacewithappModeSelectionHrefwhen a submitted run was URL-driven, while still bailing out when an answer is on screen.Tests and CI: Playwright cases lock in prescribing and documents clear-to-shared-home behavior. The privacy readiness contract test gains the same shallow-clone / missing
reviewedCommithandling as other governance suites so cloud CI does not fail spuriously without weakening checks on full clones.Docs inbox: Ledger entries update and withdraw a competing outstanding-issue mutation in favor of another close request; findings are preserved in this PR narrative.
Reviewed by Cursor Bugbot for commit d8367c6. Configure here.