Skip to content

fix(ui): stabilize and accelerate mode navigation - #1607

Merged
BigSimmo merged 9 commits into
mainfrom
codex/fix-mode-switching-and-loading-issues
Aug 5, 2026
Merged

fix(ui): stabilize and accelerate mode navigation#1607
BigSimmo merged 9 commits into
mainfrom
codex/fix-mode-switching-and-loading-issues

Conversation

@BigSimmo

@BigSimmoBigSimmo commented Aug 4, 2026

Copy link
Copy Markdown
Owner

Motivation

  • Mode switches could paint the previous mode's page (or the wrong page) for a frame or longer, causing choppy layout, stale content flashes, and perceived latency when users select a new app mode.
  • The intent is to ensure immediate, stable visual feedback on mode change and to reduce navigation latency by making mode-home prefetches recoverable when Next invalidates cached payloads.

Description

  • Add a PendingModeNavigation render state in GlobalSearchShell and show an accessible ModeHomeRouteLoading skeleton while an imperative mode navigation is resolving so the previous page cannot remain visible during the client transition (src/components/clinical-dashboard/global-search-shell.tsx).
  • Keep the header marked busy while a pending mode navigation is active and suppress the secondary navigation until the destination pathname + URL-derived mode both land, with a conservative 10s recovery timeout to avoid permanent loading states (src/components/clinical-dashboard/global-search-shell.tsx).
  • Make mode-home prefetching resilient by passing an onInvalidate callback to router.prefetch that removes the href from the warmed-set so a subsequent user intent can re-warm the destination after Next invalidates its cached RSC payload (src/components/clinical-dashboard/master-search-header.tsx).
  • Preserve deep-linked and active document source disclosures across rerenders by keeping the targeted disclosure React-controlled while respecting an explicit manual close (src/components/document-viewer/source-panels.tsx).
  • Add/adjust tests to cover the invalidation→re-prefetch behaviour and the URL-sync regression guard (tests/mode-menu-prefetch.dom.test.tsx, tests/shared-search-shell-url-sync.test.ts), and make a minimal, type-safe cast to align with the runtime router.prefetch shape without importing router internals.

Testing

  • Ran npm run test -- --run tests/shared-search-shell-url-sync.test.ts tests/mode-menu-prefetch.dom.test.tsx and all tests passed (2 files, 6 tests passed).
  • Added document-viewer regression coverage that forces the memoised disclosure component to rerender with a new chunks-array reference and verifies the deep-linked passage remains open.
  • Ran TypeScript check with node ./node_modules/typescript/bin/tsc --noEmit and it exited successfully (TSC_EXIT=0).
  • Ran ESLint over the touched files and it reported no new issues.
  • Executed a focused Playwright proof (throttled) that verifies the stale Services page is hidden, the loading surface is shown, and /forms lands during a header-mode switch; the screenshot was written to /tmp/database-ui-evidence/mode-switch-loading.png and the scripted proof logged success.

Codex Task


Open in Devin Review

Summary by CodeRabbit

  • New Features

    • Added an accessible loading state when switching between clinical dashboard modes.
    • Temporarily hides secondary navigation during mode transitions.
  • Bug Fixes

    • Improved navigation reliability when switching modes, including query-string changes and stalled transitions.
    • Refreshed invalidated mode pages when revisiting navigation options.
    • Deep-linked document sections now remain open across rerenders and parent toggles.

@coderabbitai

coderabbitaiBot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in:17 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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: eaedd8a1-954b-4129-8d1b-96956eea26cc

📥 Commits

Reviewing files that changed from the base of the PR and between 04f6cdc and 0ad9e95.

📒 Files selected for processing (1)
  • tests/document-section-summary.dom.test.tsx
📝 Walkthrough

Walkthrough

The dashboard now tracks pending mode navigation using full URL state and renders destination-mode loading content until navigation completes. Prefetches can repeat after invalidation. Document disclosures preserve deep-linked open state across rerenders.

Changes

Mode navigation and prefetching

Layer / File(s)Summary
Pending navigation tracking
src/components/clinical-dashboard/global-search-shell.tsx, tests/shared-search-shell-url-sync.test.ts, tests/search-route-ownership.test.ts
Mode changes push destination URLs and record source and destination state. Pending navigation clears on exact URL arrival, superseding navigation, or timeout.
Pending navigation UI contract
src/components/clinical-dashboard/global-search-shell.tsx, tests/mode-home-loading-contract.test.ts
The shell updates loading indicators, hides secondary navigation, and renders an accessible destination-mode skeleton while navigation is pending.
Prefetch invalidation handling
src/components/clinical-dashboard/master-search-header.tsx, tests/mode-menu-prefetch.dom.test.tsx, tests/audit-navigation-auth-regressions.test.ts
Prefetch invalidation removes cached URLs. Tests verify URL matching and repeated prefetching after invalidation.

Document disclosure state

Layer / File(s)Summary
Controlled disclosure state
src/components/document-viewer/source-panels.tsx, tests/document-section-summary.dom.test.tsx
Manual-close tracking now uses React state. Deep-linked and active-search chunk disclosures receive controlled open state and remain open across rerenders and parent close events.

Review ledger

Layer / File(s)Summary
Branch review record
docs/branch-review-ledger.md
The ledger records the PR merge state and required checks still running.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
participant User
participant GlobalSearchShell
participant Router
participant SearchRoute
User->>GlobalSearchShell: Select mode
GlobalSearchShell->>GlobalSearchShell: Record destination and source URL
GlobalSearchShell->>Router: router.push(destination URL)
Router->>SearchRoute: Resolve destination URL
SearchRoute->>GlobalSearchShell: Provide pathname, mode, and query
GlobalSearchShell->>User: Render destination content after exact match
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check⚠️ WarningThe description explains the motivation, implementation, and tests but omits the required verification, risk, rollout, and governance sections.Add the template sections and complete the required verification checkboxes, risk and rollback details, production effects, and clinical governance preflight.
✅ Passed checks (3 passed)
Check nameStatusExplanation
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
Title check✅ PassedThe title clearly and concisely describes the main changes to mode navigation.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch

Comment @coderabbitai help to get the list of available commands.

@supabase

supabaseBot commented Aug 4, 2026

Copy link
Copy Markdown

This pull request has been ignored for the connected project sjrfecxgysukkwxsowpy because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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.
Inline comments:
In `@src/components/clinical-dashboard/global-search-shell.tsx`:
- Around line 485-506: The condition checking when to clear
pendingModeNavigation only verifies that the pathname and mode match the
intended destination, but does not handle the case where a different navigation
interrupts the original mode switch. To fix this, record the source pathname
when setting pendingModeNavigation, then update the clearing logic to also clear
pendingModeNavigation when the pathname changes to any destination other than
the one stored in pendingModeNavigation (indicating an interrupting navigation),
not just when the intended mode arrives. This prevents the loading skeleton from
displaying on unrelated navigations until the timeout expires.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 730b24cc-6998-4644-86c7-5d670f4e924e

📥 Commits

Reviewing files that changed from the base of the PR and between 130c774 and a771d01.

📒 Files selected for processing (3)
  • src/components/clinical-dashboard/global-search-shell.tsx
  • src/components/clinical-dashboard/master-search-header.tsx
  • tests/mode-menu-prefetch.dom.test.tsx

Comment threadsrc/components/clinical-dashboard/global-search-shell.tsx
Comment threadsrc/components/clinical-dashboard/global-search-shell.tsx Outdated

@chatgpt-codex-connectorchatgpt-codex-connectorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:a771d01a92

ℹ️ 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".

Comment threadsrc/components/clinical-dashboard/global-search-shell.tsx Outdated
@BigSimmo

Copy link
Copy Markdown
OwnerAuthor

@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 codex/fix-mode-switching-and-loading-issues at starting commit a771d01; 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:codex/fix-mode-switching-and-loading-issues, 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.

@devin-ai-integrationdevin-ai-integrationBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 2 potential issues.

View 2 additional findings in Devin Review.

Open in Devin Review

Comment threadsrc/components/clinical-dashboard/global-search-shell.tsx Outdated
Comment threadsrc/components/clinical-dashboard/global-search-shell.tsx Outdated
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Can't wait for the next one!

Reviewed commit:a771d01a92

ℹ️ 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".

@github-actions

github-actionsBot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

CI triage

CI failed on this PR. Automated classification of the 2 failed job(s):

  • Production UI (2)needs investigation: inspect the failing step and uploaded diagnostics; rerun only after classifying the cause.
  • PR requiredneeds investigation: inspect the failing step and uploaded diagnostics; rerun only after classifying the cause.

Compared with main CI run #8024 (cancelled).

Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves mode-switch UX in the shared clinical dashboard shell by preventing “stale page” flashes during imperative mode navigation and by making mode-home prefetching resilient to Next.js client cache invalidations.

Changes:

  • Adds a pendingModeNavigation render state that swaps the current page content for an accessible loading skeleton until the destination pathname + URL-derived mode have both landed (with a 10s recovery timeout).
  • Enhances mode-home prefetching by supplying an onInvalidate callback that removes invalidated hrefs from the warmed set so subsequent intent can re-prefetch.
  • Adds/updates DOM + URL sync tests to cover prefetch invalidation re-warming and URL sync regression protection.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

FileDescription
src/components/clinical-dashboard/global-search-shell.tsxAdds pending mode-navigation loading state, busy/secondary-nav suppression, and a conservative timeout recovery.
src/components/clinical-dashboard/master-search-header.tsxMakes mode-home prefetching recoverable by deleting invalidated entries from the prefetched set via onInvalidate.
tests/mode-menu-prefetch.dom.test.tsxAdds test coverage for invalidation → re-prefetch behavior and updates assertions to account for the new prefetch options argument.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@BigSimmo

Copy link
Copy Markdown
OwnerAuthor

@cursoragent run bugbot and ensure its findings are correct

Please also fix any issues, address and fix any comments and resolve once done. Ensure no regression and it makes sense according to PR for any fix in context and also fix failing CI please.

@cursor

cursorBot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Taking a look!

Open in WebOpen in Cursor

@cursor

cursorBot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Already done on this PR — no further code changes needed.

Bugbot findings: All validated as real and fixed in e2c9b003 (same-mode home nav, interrupt clearing, mid-nav re-select). Survived the later main merge at b4a6c1a6.

Review threads: 5/5 resolved, 0 open.

CI (was failing on 4284cf70): Current head run is green on the previously failing path — Unit coverage ✅, Build ✅, Static PR ✅, Production UI critical ✅. Remaining jobs are still in progress (Production UI shards / advisory).

Contract tests re-checked locally: 38/38 passed.

Open in WebOpen in Cursor

cursoragentand others added 3 commits August 4, 2026 14:35
The mode-equality early return blocked ModeActionPopup and active-mode
menu picks from returning to a clean mode home. Skip only true no-ops,
clear pending mode navigation when any other URL commits, and update
the source contracts CI was failing on.
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
@BigSimmo

Copy link
Copy Markdown
OwnerAuthor

@cursoragent Unblock the current open PR. Confirm the PR number and GitHub head first from context. If more than one open PR could apply, stop and say which one you would use and why.

Fetch and start from the remote tip that matches that GitHub head. If the named branch ref is missing or stale, use the PR head ref. Preserve unrelated local WIP; do not discard dirty work; do not treat a local-ahead commit as the reviewed tip. Do not merge the PR, force-push, rebase, or arm auto-merge. No provider-backed gates without approval. If you cannot push or resolve threads, diagnose and comment only; if inline replies fail, resolve when possible and put blocker dispositions in the summary comment. If auto-merge is already armed, push only for a real blocker and avoid cancelling in-flight required CI unless the push clears that blocker.

If the PR is already merged or closed: confirm outcome, post one summary, and stop.

Primary job: clear mergeability and required-CI blockers on this tip. Do a full product review only if needed to explain or fix a blocker. Stay light otherwise—no broad verification suites or Playwright unless needed to prove one concrete unblock fix.

Snapshot the GitHub head: tip, behind/ahead, mergeable state, merge-tree versus origin/main (real conflict versus behind-but-clean), required checks on that tip including Production UI when selected, and unresolved threads that block merge or required CI. Missing checks while dirty are not green. Tip moved → re-snapshot.

Ledger-lookup against that GitHub head under an unblock/fix scope for this PR. If already handled at this head for unblock work, merge-tree clean, required checks green, and no blocking threads: summarize, comment, stop.

Unblock once: real conflict → merge origin/main with the smallest correct resolution (prefer main’s shared queues; keep this PR’s notes); behind-but-clean → one sync, late if required CI is in flight; clean → leave. skip-branch-sync and similar labels do not block a manual sync needed to clear a real blocker. No sync thrash. After any sync or push, re-snapshot tip, merge-tree, and required checks before declaring done. Dedupe the ledger if a merge touched it.

Fix only what blocks required checks or mergeability on this tip. Ignore advisory red unless it reveals a real required failure. Prefer the smallest safe fix; if a bot/autofix commit caused the break, prefer revert. Prove with the smallest targeted check only. Format and commit before push.

Push only blocker-fix commits. Append ledger for the final GitHub head under the unblock scope. Never push a ledger-only tip.

One PR comment: tip, sync/merge-tree, what blocked and what you fixed or dispositioned, required CI status, residual risks; merge left to me.

Stop when merge-tree is clean on the current GitHub tip, no blocking threads remain for merge/required CI, and required checks are green or clearly in progress.

@cursor

cursorBot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

PR #1607 · GitHub tip 3e3b224a — no unblock push needed.

CheckResult
Merge-tree vs mainCLEAN (behind 0, ahead 5)
Unresolved threads0
Failing required CInone
Required CIin progress (Unit coverage, Build, Static PR, Production UI critical)
Auto-mergenot armed

mergeStateStatus: BLOCKED is only pending required checks, not a content conflict. Prior Bugbot fixes (e2c9b003) are already on this tip. Ledger append is local-only (not pushed). Summary posted on the PR — merge left to you.

Open in WebOpen in Cursor

Local-only ledger append for unblock/fix scope; not pushed alone.
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
@cursor

cursorBot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Unblock snapshot — PR #1607

PR:#1607 (codex/fix-mode-switching-and-loading-issues)
GitHub tip:3e3b224a2ec13928d1e28173b1fc4c75d202d7d2
Base:main · ahead 5 / behind 0 · mergeable: MERGEABLE · mergeStateStatus: BLOCKED (required checks still pending, not a content conflict)

Sync / merge-tree

  • git merge-tree --write-tree origin/main <tip>CLEAN
  • No sync/merge performed (already up to date with main)
  • Auto-merge: not armed

What blocked / what I fixed

  • Nothing actionable on this tip. Prior Bugbot/Codex/Devin findings were already fixed in e2c9b003 and remain on the tip after the later main merges.
  • Unresolved review threads:0 (5 previously resolved)
  • Failing required checks:none
  • No code push from this unblock pass (would only cancel in-flight CI)

Required CI (tip 3e3b224a)

In progress / clearly running: Static PR checks, Unit coverage, Build, Production UI critical
Already green: Change scope, Safety and config, PR policy, PR mergeability, Semgrep (+ ingestion), Gitleaks, GitGuardian
Advisory pending (ignored for unblock): Lighthouse, Visual baselines

Residual risks

  • mergeStateStatus: BLOCKED until required checks finish (and any approval rules, if configured — no approving human review is present yet; only bot COMMENTED reviews).
  • Ledger append for this unblock scope is recorded locally only (not pushed — ledger-only tip policy).

Merge left to you.

@BigSimmo

Copy link
Copy Markdown
OwnerAuthor

@cursoragent Work the current open PR. Confirm the PR number and GitHub head first from context. If more than one open PR could apply, stop and say which one you would use and why.

Fetch and start from the remote tip that matches that GitHub head. If the named branch ref is missing or stale, use the PR head ref. Preserve unrelated local WIP; do not discard dirty work; do not treat a local-ahead commit as the reviewed tip. Do not merge the PR, force-push, rebase, or arm auto-merge unless I explicitly ask. No provider-backed gates without approval. If you cannot push or resolve threads, diagnose and comment only; if inline replies fail, resolve when possible and put dispositions in the summary comment. If auto-merge is already armed, push only for a real blocker and avoid cancelling in-flight required CI unless the push clears that blocker.

If the PR is already merged or closed: confirm outcome, post one summary, and stop.

Goal: review and Bugbot the tip, fix actionable issues with the smallest correct changes, clear merge / required-CI / thread blockers, prove fixes with focused local checks, push, append ledger, and post one PR summary. Use broader offline gates only when the delta or a failed required check warrants them. Regenerate large assets only when a fix requires it.

Snapshot the GitHub head: tip, behind/ahead, mergeable state, merge-tree versus origin/main, required checks on that tip including Production UI when selected, unresolved actionable threads. Missing checks while dirty are not green. Tip moved → re-snapshot.

Ledger-lookup against that GitHub head under the review-and-fix scope for this PR. Already reviewed at this head with clean merge-tree, green required checks, and no new actionable threads → summarize, comment, stop unless I asked for a fresh superseding pass.

Unblock once: real conflict → merge origin/main (prefer main’s shared queues; keep this PR’s notes); behind-but-clean → one sync, late if required CI is in flight; clean → leave. After any sync or push, re-snapshot before declaring done. No sync thrash.

Review high-confidence issues only. Separate PR-introduced defects from pre-existing re-emitted data. For generated assets, review contracts, aliases, and cache—not every generated line. Flag RAG or governance before editing; fix PR-body policy text only when wrong or missing. Ignore bot noise.

Fix P0/P1 always; clear scoped P2 when locally provable; else disposition and resolve. Required-check failures on this tip only; ignore advisory. Prove each fix with the smallest targeted check; escalate to cheap or PR-local verification only when the change set, risk surface, or CI failure needs that breadth. Format and commit before push.

Push only this PR’s fix commits. Append ledger for the final GitHub head. Never push a ledger-only tip.

One PR comment: tip, sync/merge-tree, fixed versus dispositioned, required CI, decisive local proof, residual risks; merge left to me.

Stop when merge-tree is clean on the current GitHub tip, no actionable threads remain, required checks are green or clearly in progress, summary posted, merge left to me.

@cursor

cursorBot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

PR #1607 · tip 04f6cdc8 — required CI blocker fixed and pushed.

Merge-treeCLEAN (behind 0)
Threads0 unresolved (Bugbot already fixed)
BlockerProduction UI (2) — deep-linked source chunk stayed open: false
FixReact-control auto-open target in IndexedTextPanel + DOM test
Proofdocument-section-summary.dom.test.tsx5/5 passed
CIRestarting on new tip

Summary posted on the PR. Merge left to you.

Open in WebOpen in Cursor

@BigSimmo

Copy link
Copy Markdown
OwnerAuthor

@cursoragent Work the current open PR. Confirm the PR number and GitHub head first from context. If more than one open PR could apply, stop and say which one you would use and why.

Fetch and start from the remote tip that matches that GitHub head. If the named branch ref is missing or stale, use the PR head ref. Preserve unrelated local WIP; do not discard dirty work; do not treat a local-ahead commit as the reviewed tip. Do not merge the PR, force-push, rebase, or arm auto-merge unless I explicitly ask. No provider-backed gates without approval. If you cannot push or resolve threads, diagnose and comment only; if inline replies fail, resolve when possible and put dispositions in the summary comment. If auto-merge is already armed, push only for a real blocker and avoid cancelling in-flight required CI unless the push clears that blocker.

If the PR is already merged or closed: confirm outcome, post one summary, and stop.

Goal: review and Bugbot the tip, fix actionable issues with the smallest correct changes, clear merge / required-CI / thread blockers, prove fixes with focused local checks, push, append ledger, and post one PR summary. Use broader offline gates only when the delta or a failed required check warrants them. Regenerate large assets only when a fix requires it.

Snapshot the GitHub head: tip, behind/ahead, mergeable state, merge-tree versus origin/main, required checks on that tip including Production UI when selected, unresolved actionable threads. Missing checks while dirty are not green. Tip moved → re-snapshot.

Ledger-lookup against that GitHub head under the review-and-fix scope for this PR. Already reviewed at this head with clean merge-tree, green required checks, and no new actionable threads → summarize, comment, stop unless I asked for a fresh superseding pass.

Unblock once: real conflict → merge origin/main (prefer main’s shared queues; keep this PR’s notes); behind-but-clean → one sync, late if required CI is in flight; clean → leave. After any sync or push, re-snapshot before declaring done. No sync thrash.

Review high-confidence issues only. Separate PR-introduced defects from pre-existing re-emitted data. For generated assets, review contracts, aliases, and cache—not every generated line. Flag RAG or governance before editing; fix PR-body policy text only when wrong or missing. Ignore bot noise.

Fix P0/P1 always; clear scoped P2 when locally provable; else disposition and resolve. Required-check failures on this tip only; ignore advisory. Prove each fix with the smallest targeted check; escalate to cheap or PR-local verification only when the change set, risk surface, or CI failure needs that breadth. Format and commit before push.

Push only this PR’s fix commits. Append ledger for the final GitHub head. Never push a ledger-only tip.

One PR comment: tip, sync/merge-tree, fixed versus dispositioned, required CI, decisive local proof, residual risks; merge left to me.

Stop when merge-tree is clean on the current GitHub tip, no actionable threads remain, required checks are green or clearly in progress, summary posted, merge left to me.

@cursor

cursorBot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

PR #1607 · GitHub tip 04f6cdc8 — already worked; nothing further to push.

CheckResult
Merge-treeCLEAN (behind 0)
Threads0 unresolved
Failing required CInone
Required CIin progress on this tip
Prior fixProduction UI chunk-open fix already on tip

No new actionable Bugbot/review items. Not pushing the local ledger-only commit. Summary posted — merge left to you once CI finishes.

Open in WebOpen in Cursor

Production UI flake: nested citation details relied on imperative
.open and could stay collapsed across re-renders. Control the
auto-open target via React open state and cover it with a DOM test.
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
@cursor

cursorBot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Review-and-fix — PR #1607

PR:#1607 (codex/fix-mode-switching-and-loading-issues)
GitHub tip:04f6cdc878eb5bc88621f0b1026acba4f9906240 (was 3e3b224a)
Base:main · ahead 7 / behind 0 · merge-tree CLEAN · mergeable: MERGEABLE
Auto-merge: not armed · Unresolved threads: 0

Sync / merge-tree

  • Already up to date with main — no sync performed.

Review / Bugbot

  • Prior Bugbot/Codex/Devin findings (same-mode home nav, pending interrupt clear, mid-nav re-select) remain fixed since e2c9b003; threads already resolved.
  • No new actionable review threads on this tip.

Required CI blocker fixed

  • Blocked on:Production UI (2) / PR required at 3e3b224a
    • Fail: ui-smoke@critical — deep-linked highlighted-indexed-source-chunk stayed open: false (imperative .open lost across re-renders under condensed view).
  • Fix (04f6cdc8): React-control the auto-open target disclosure in IndexedTextPanel; track manual-close in state; DOM regression in tests/document-section-summary.dom.test.tsx.
  • Note: this surface is document-viewer (not introduced by the mode-nav delta); it became the required-CI blocker on this tip after the main merge.

Local proof

npx vitest run tests/document-section-summary.dom.test.tsx
# Test Files 1 passed (1)
# Tests 5 passed (5)

Required CI now

  • Restarting on 04f6cdc8 (checks pending after push). Prior tip had Unit coverage / Build / Static / Production UI critical green; only Production UI (2) failed.

Residual risks

  • Full Chromium shard not re-run locally (focused DOM proof only).
  • Ledger review-and-fix row recorded locally for 04f6cdc8 — not pushed alone.

Merge left to you.

@cursor

cursorBot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Review-and-fix status — PR #1607

PR:#1607 · GitHub tip:04f6cdc878eb5bc88621f0b1026acba4f9906240 (not treating local ledger-ahead c6bf2489 as tip)

Snapshot

  • ahead 7 / behind 0 vs main · merge-tree CLEAN · mergeable: MERGEABLE
  • Unresolved threads: 0
  • Failing required checks: none
  • Required CI: in progress on this tip (Unit coverage, Build, Static PR, Production UI critical, Semgrep, …)
  • Auto-merge: not armed

Already handled on this tip

  • Bugbot/Codex/Devin mode-nav findings fixed earlier (e2c9b003); threads resolved.
  • Production UI (2) blocker at 3e3b224a fixed in 04f6cdc8 (React-controlled deep-link chunk open + DOM test; local proof 5/5).
  • No new actionable findings; no sync/push this pass (would only cancel in-flight CI / would be ledger-only).

Residual

  • Wait for required CI on 04f6cdc8 to finish green.
  • Local ledger row for this tip remains unpushed (ledger-only tip policy).

Merge left to you.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (3)
tests/audit-navigation-auth-regressions.test.ts (1)

160-172: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Verify the Answer-mode behavior at runtime.

alsoMatchesGate only checks source text. It does not prove that showUniversalAlsoMatches controls rendered content or that answer and loading are evaluated at runtime. A regression can leave these strings in the extracted segment while bypassing the gate.

Add a focused DOM test for no answer, loading, and completed-answer states. Keep this source assertion only if it remains useful as a separate implementation audit.

Based on the PR objective and the existing DOM-test pattern in tests/mode-menu-prefetch.dom.test.tsx Lines 103-121, this check should verify rendered behavior.

🤖 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/audit-navigation-auth-regressions.test.ts` around lines 160 - 172, Add
a focused DOM test alongside the existing source assertion in the test covering
showUniversalAlsoMatches, following the rendering pattern from
mode-menu-prefetch.dom.test.tsx: verify the also-matches content is absent when
answer is missing, absent while loading, and rendered after a completed answer.
Ensure the test exercises the actual component output so answer and loading are
evaluated at runtime; retain the source-text assertion only as a separate
implementation audit if still useful.
tests/mode-home-loading-contract.test.ts (1)

38-42: 🎯 Functional Correctness | 🔵 Trivial | 🏗️ Heavy lift

Add a rendered loading-state assertion.

These checks only verify JSX text. They do not verify that the route renders ModeHomeRouteLoading. Add a deterministic SSR test that asserts the initial response contains role="status" and aria-label="Loading".

Also applies to: 63-63

🤖 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/mode-home-loading-contract.test.ts` around lines 38 - 42, Add a
deterministic server-rendered test in mode-home-loading-contract.test.ts that
exercises the route’s initial loading state and asserts the response contains
role="status" with aria-label="Loading". Keep the existing source-structure
assertion, and use the route/rendering entry point already present in the test
suite rather than adding a JSX-only check.

Source: Learnings

tests/shared-search-shell-url-sync.test.ts (1)

67-81: 🎯 Functional Correctness | 🔵 Trivial | 🏗️ Heavy lift

Test the production release predicate.

shouldClearPending is a second implementation of the release logic. These assertions can pass after the shell behavior changes. Extract the predicate into a dependency-free module and test that export, or render the shell through the pending-navigation transitions.

🤖 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/shared-search-shell-url-sync.test.ts` around lines 67 - 81, The tests
currently validate the duplicate `shouldClearPending` implementation rather than
production behavior. Extract the pending-navigation release predicate into a
dependency-free production module and import its export in the tests, or replace
the helper-based assertions with tests that render the shell through
pending-navigation transitions; ensure assertions exercise the production
release logic.
🤖 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.
Inline comments:
In `@tests/document-section-summary.dom.test.tsx`:
- Around line 134-165: Update the rerender call in the “keeps the deep-linked
nested chunk disclosure open under condensed view” test to pass a newly created
chunks array while preserving its entries, ensuring memoized IndexedTextPanel
actually renders again. Keep the existing prop values and open-state assertion
unchanged.
---
Nitpick comments:
In `@tests/audit-navigation-auth-regressions.test.ts`:
- Around line 160-172: Add a focused DOM test alongside the existing source
assertion in the test covering showUniversalAlsoMatches, following the rendering
pattern from mode-menu-prefetch.dom.test.tsx: verify the also-matches content is
absent when answer is missing, absent while loading, and rendered after a
completed answer. Ensure the test exercises the actual component output so
answer and loading are evaluated at runtime; retain the source-text assertion
only as a separate implementation audit if still useful.
In `@tests/mode-home-loading-contract.test.ts`:
- Around line 38-42: Add a deterministic server-rendered test in
mode-home-loading-contract.test.ts that exercises the route’s initial loading
state and asserts the response contains role="status" with aria-label="Loading".
Keep the existing source-structure assertion, and use the route/rendering entry
point already present in the test suite rather than adding a JSX-only check.
In `@tests/shared-search-shell-url-sync.test.ts`:
- Around line 67-81: The tests currently validate the duplicate
`shouldClearPending` implementation rather than production behavior. Extract the
pending-navigation release predicate into a dependency-free production module
and import its export in the tests, or replace the helper-based assertions with
tests that render the shell through pending-navigation transitions; ensure
assertions exercise the production release logic.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: ed5b98fa-6f0c-449b-ac4c-9869545a44fa

📥 Commits

Reviewing files that changed from the base of the PR and between a771d01 and 04f6cdc.

📒 Files selected for processing (8)
  • docs/branch-review-ledger.md
  • src/components/clinical-dashboard/global-search-shell.tsx
  • src/components/document-viewer/source-panels.tsx
  • tests/audit-navigation-auth-regressions.test.ts
  • tests/document-section-summary.dom.test.tsx
  • tests/mode-home-loading-contract.test.ts
  • tests/search-route-ownership.test.ts
  • tests/shared-search-shell-url-sync.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/components/clinical-dashboard/global-search-shell.tsx

Comment threadtests/document-section-summary.dom.test.tsx
@BigSimmo

Copy link
Copy Markdown
OwnerAuthor

@cursoragent review PR for me.

@cursor

cursorBot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

PR #1607 review — tip 04f6cdc8

Verdict: merge-ready on required CI. No P0/P1 findings on this tip. Merge left to you.

PR#1607 · codex/fix-mode-switching-and-loading-issues
GitHub tip04f6cdc878eb5bc88621f0b1026acba4f9906240
vs mainahead 7 / behind 0 · merge-tree CLEAN
Required CIall green (PR required, Production UI critical + shards 1–3, Static, Unit, Build, …)
Unresolved threads1 CodeRabbit advisory (test strength — below)

Findings

No P0 / P1.

P2 — test gap (not a product bug):tests/document-section-summary.dom.test.tsx re-renders IndexedTextPanel with the same prop references. The panel is memo’d, so React can bail out and the assertion never exercises a real re-render. CodeRabbit’s suggestion (chunks={[...props.chunks]}) would close that. Product fix for deep-link open already landed in 04f6cdc8 and Production UI (2) is green.

P2 residual (edge, not blocking): Pending clear treats “back on exact source URL” as still pending (supersededWhilePending is false when Back lands on the recorded source). Escape is the 10s recovery timer. Interrupt paths that change the URL (sidebar, New chat, second mode) clear correctly.

Already fixed earlier on this PR (not re-raised): same-mode home no-op, destination-only pending stall, mid-nav re-select — in e2c9b003.

What looks solid

  • Destination-based changeMode (same-mode from detail/submitted still goes home; true no-op only)
  • Pending skeleton + supersede-on-other-URL + 10s recovery
  • Prefetch onInvalidate re-warm
  • React-controlled deep-link chunk disclosure

Checks

  • Read: shell / disclosure / prefetch + tests vs origin/main...04f6cdc8
  • gh pr checks — required all pass
  • merge-tree clean
  • Not run: local Vitest/Playwright this pass (CI already green on tip); no provider gates

Ledger: local append only for PR #1607 diff-review at this tip (not pushed — ledger-only tip policy).

Open in WebOpen in Cursor

@BigSimmo
BigSimmo enabled auto-merge (squash) August 5, 2026 05:35
@BigSimmo
BigSimmo merged commit 9d4a28c into mainAug 5, 2026
25 checks passed
@BigSimmo
BigSimmo deleted the codex/fix-mode-switching-and-loading-issues branch August 5, 2026 05:56
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@BigSimmo@cursoragent