fix: complete form services regression alignment for design-audit#836
Conversation
* fix: stop Escape focus restore from closing the app-mode menu Scope dismiss deferred a focus restore onto the answer-options trigger. When a mode-menu open landed in the same frame window, that restore stole focus, blur-dismissed the menu, and flaked the Documents mode switch in Production UI CI. Skip restore when the mode menu is open or focus already moved, and wait for the scope popover to hide before opening the menu. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * fix(test): fail closed when scope popover does not dismiss Only wait for the scope popover when it is visible, and let that wait throw if dismissal times out so the mode-menu open cannot recreate the Escape focus-restore race. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
|
Updates to Preview Branch (codex/design-audit-main-safe-merge-20260719) ↗︎
Tasks are run on every commit but only new migration files are pushed.
❌ Branch Error • Sat, 18 Jul 2026 09:31:53 UTC View logs for this Workflow Run ↗︎. |
📝 WalkthroughWalkthroughUpdates presentation routing and sitemap output, dashboard and forms UI behavior, RAG RPC and classifier handling, and Supabase migrations with matching regression assertions. ChangesRouting and sitemap updates
Dashboard and forms UI behavior
RAG and Supabase changes
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
CI triageCI failed on this PR. Automated classification of the 3 failed job(s):
Compared with main CI run #3245 (success). Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/services/services-navigator-page.tsx (1)
36-37: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winRestore the missing service-metrics import
serviceNavigatorMetricsandcanCompareServicesare used inRightRail, but this file no longer imports them, so the component won’t compile.Proposed fix
+import { canCompareServices, serviceNavigatorMetrics } from "`@/lib/service-navigator-metrics`";🤖 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 `@src/components/services/services-navigator-page.tsx` around lines 36 - 37, Restore the missing import for serviceNavigatorMetrics and canCompareServices in services-navigator-page.tsx, alongside the existing service-related imports, so the RightRail component resolves both referenced symbols and compiles successfully.src/lib/rag.ts (1)
1291-1292: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winThread the request signal through classifier fallback and retrieval RPCs.
src/lib/rag.ts#L1283-L1370still waits for the shared classifier verdict after the caller aborts, andsrc/lib/rag-candidate-sources.ts#L79-L115issues the versioned/legacy Supabase RPCs withoutabortSignal, so aborted requests keep consuming LLM/DB time.
- Add an optional
signaltoanalyzeQueryWithClassifierFallbackand stop waiting once it aborts.- Pass that signal into
callVersionedRetrievalRpcand chain.abortSignal(signal)on the RPC calls.🤖 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 `@src/lib/rag.ts` around lines 1291 - 1292, Update analyzeQueryWithClassifierFallback in src/lib/rag.ts (lines 1291-1292) to accept an optional signal and stop awaiting the shared classifier verdict after abort; propagate that signal through its retrieval call sites at lines 1362-1369 and 2398-2399 into callVersionedRetrievalRpc. In src/lib/rag-candidate-sources.ts, update the versioned RPC at lines 85-92 and legacy RPC at lines 102-105 to chain abortSignal(signal), preserving existing retrieval behavior when no signal is provided.
🧹 Nitpick comments (1)
tests/audit-navigation-auth-regressions.test.ts (1)
46-46: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the new empty-query contract.
The route now intentionally ignores
qwhenqueryis present but blank. Replace the removed fallback case with an assertion for this behavior so the changed redirect contract remains protected.🤖 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` at line 46, Update the redirect test using redirectMedications to cover the empty-query contract: provide a blank query alongside a populated q value and assert that q is ignored rather than used as a fallback. Replace the removed fallback-case expectation while preserving existing redirect assertions.
🤖 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/app/page.tsx`:
- Around line 8-10: Update the HomeProps.searchParams type to include optional
q, focus, and run properties alongside mode, using the appropriate string or
string-array types expected by the page’s query-param reads.
In `@tests/audit-navigation-auth-regressions.test.ts`:
- Around line 126-128: Fix the failing H1 assertion in the “keeps the root
dashboard H1 as Clinical KB” test by aligning its regex with the actual markup
emitted by ClinicalDashboard, or update the component’s root H1 to exactly match
the asserted sr-only Clinical KB structure; ensure the test passes while
preserving the single-H1 requirement.
In `@tests/ui-accessibility.spec.ts`:
- Line 51: Update the H1 rendered by ClinicalDashboard from “Clinical Guide” to
“Clinical KB” so it matches the updated assertions. Apply the production change
in ClinicalDashboard; retain the expectations at
tests/ui-accessibility.spec.ts:51, tests/ui-smoke.spec.ts:876, and
tests/ui-smoke.spec.ts:1271 unchanged because they are corrected by the H1
update.
---
Outside diff comments:
In `@src/components/services/services-navigator-page.tsx`:
- Around line 36-37: Restore the missing import for serviceNavigatorMetrics and
canCompareServices in services-navigator-page.tsx, alongside the existing
service-related imports, so the RightRail component resolves both referenced
symbols and compiles successfully.
In `@src/lib/rag.ts`:
- Around line 1291-1292: Update analyzeQueryWithClassifierFallback in
src/lib/rag.ts (lines 1291-1292) to accept an optional signal and stop awaiting
the shared classifier verdict after abort; propagate that signal through its
retrieval call sites at lines 1362-1369 and 2398-2399 into
callVersionedRetrievalRpc. In src/lib/rag-candidate-sources.ts, update the
versioned RPC at lines 85-92 and legacy RPC at lines 102-105 to chain
abortSignal(signal), preserving existing retrieval behavior when no signal is
provided.
---
Nitpick comments:
In `@tests/audit-navigation-auth-regressions.test.ts`:
- Line 46: Update the redirect test using redirectMedications to cover the
empty-query contract: provide a blank query alongside a populated q value and
assert that q is ignored rather than used as a fallback. Replace the removed
fallback-case expectation while preserving existing redirect assertions.
🪄 Autofix (Beta)
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 Plus
Run ID: 82c5833a-c654-4429-99a0-64af3961f0dd
📒 Files selected for processing (20)
docs/site-map.mdplaywright.config.tsscripts/generate-site-map.tssrc/app/differentials/presentations/route.tssrc/app/page.tsxsrc/components/ClinicalDashboard.tsxsrc/components/clinical-dashboard/master-search-header.tsxsrc/components/services/services-navigator-page.tsxsrc/lib/rag-candidate-sources.tssrc/lib/rag.tssupabase/drift-manifest.jsonsupabase/migrations/20260714180000_patch_rag_and_corrector_scalability.sqlsupabase/migrations/20260714190000_document_table_facts_trgm_idx.sqltests/audit-content-services-regressions.test.tstests/audit-navigation-auth-regressions.test.tstests/site-map.test.tstests/supabase-schema.test.tstests/ui-accessibility.spec.tstests/ui-smoke.spec.tstests/ui-tools.spec.ts
| it("keeps the root dashboard H1 as Clinical KB", () => { | ||
| expect(clinicalDashboardSource.match(/<h1\b/g)).toHaveLength(1); | ||
| expect(clinicalDashboardSource).toMatch(/<h1 className="sr-only">\s*Clinical Guide\s*<\/h1>/); | ||
| expect(clinicalDashboardSource).toMatch(/<h1 className="sr-only">\s*Clinical KB\s*<\/h1>/); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Fix the failing H1 assertion before merging.
The exact regex on Line 128 does not match the current ClinicalDashboard.tsx source, and the test currently fails. Align the assertion with the actual markup or update the component to emit exactly the asserted <h1> structure.
🧰 Tools
🪛 GitHub Check: Unit coverage
[failure] 128-128: [node] tests/audit-navigation-auth-regressions.test.ts > audit navigation and auth regressions > keeps the root dashboard H1 as Clinical KB
AssertionError: expected '"use client";\n\nimport { useRouter, …' to match /
\s*Clinica…/h1>
- Expected:
/\sClinical KB\s</h1>/
- Received:
""use client";
import { useRouter, useSearchParams } from "next/navigation";
import dynamic from "next/dynamic";
import {
CircleAlert,
BookOpen,
ChevronDown,
Clock3,
ExternalLink,
FileImage,
FileText,
FolderOpen,
Heart,
ListChecks,
Loader2,
Quote,
RefreshCw,
Search,
ShieldAlert,
Square,
UploadCloud,
WifiOff,
Wrench,
} from "lucide-react";
import { type CSSProperties, useCallback, useEffect, useMemo, useReducer, useRef, useState } from "react";
import { type DocumentDeleteResult } from "@/components/DocumentManagementActions";
import { extractSafetyFindings } from "@/lib/clinical-safety";
import { isLocalNoAuthMode, publicUploadsEnabled } from "@/lib/client-env";
import { readLocalProjectIdentity, unsafeLocalProjectMessage } from "@/lib/local-project-identity";
import { isDeployedClinicalKb } from "@/lib/deployed-app";
import {
appBackdrop,
answerSurface,
cn,
EmptyState,
floatingControl,
InlineNotice,
primaryControl,
textMuted,
} from "@/components/ui-primitives";
import { useAuthSession } from "@/lib/supabase/client";
import { AccountSetupDialog } from "@/components/clinical-dashboard/account-setup-dialog";
import { CrossModeLinksSection } from "@/components/clinical-dashboard/cross-mode-links";
import { useEventCallback } from "@/components/clinical-dashboard/use-event-callback";
import { AuthPanel } from "@/components/clinical-dashboard/auth-panel";
import { buildMobileSectionFabState, MobileSectionFab, ToolsHub } from "@/components/clinical-dashboard/dashboard-nav";
import { SettingsDialog } from "@/components/clinical-dashboard/settings-dialog";
import { useSidebarCollapsed } from "@/components/clinical-dashboard/use-sidebar-collapsed";
import { useTheme } from "@/components/clinical-dashboard/use-theme";
import {
deriveSidebarIdentity,
ClinicalDesktopSidebar,
ClinicalMobileSidebar,
} from "@/components/clinical-dashboard/ClinicalSidebar";
import {
SetupChecklist,
UploadPanel,
IndexingMonitor,
IngestionQualityConsole,
LibraryHealthStrip,
fallbackSetupChecks,
hasReadyRequiredPublicSearchConfig,
hasReadyPublicSearchSetup,
type SetupCheck,
type IngestionQualityReviewItem,
} from "@/components/clinical-dashboard/DocumentManagerPanel";
import { GuideDialog, GuideTrigger, UtilityDrawer } from "@/components/clinical-dashboard/dashboard-shell";
import { sanitizeAnswerDisplayText, sanitizeDisplayText } from "@/components/clinical-dashboard/display-text";
import {
isPreformattedGroundedAnswer,
NaturalLanguageAnswer,
ScopeAndGovernanceNotice,
UserQuestionBubble,
} from "@/components/clinical-dashboard/answer-content";
import { AnswerEmptyState, AnswerProgressStepper, AnswerSkeleton } from "@/components/clinical-dashboard/answer-status";
import {
type AnswerProgressUpdate,
type TimedAnswerProgressUpdate,
} from "@/components/clinical-dashboard/answer-progress";
import { evidenceMapRowsFromRenderModel } from "@/components/clinical-dashboard/evidence-map-model";
import { MasterSearchHeader } from "@/components/clinical-dashboard/master-search-header";
import { UniversalSearchAlsoMatches } from "@/components/clinical-dashboard/universal-search-also-matches";
import { useScrollHideReporter } from "@/components/clinical-dashboard/use-hide-on-scroll";
import { SearchCommandProvider } from "@/components/clinical-dashboard/search-command-context";
import {
answerReferencesDocument,
answerTimedOutError,
applyRenamedDocumentToAnswer,
compactScopeFilters,
hasActiveIndexingWork,
hasNonProductionSupabaseApiKeyFallback,
isAbortError,
mergeDocumentRefresh,
normalizeNavigationHash,
setupNeedsSlowRecheck,
setupRecheckPollMs,
shorterPollDelay,
} from "@/components/clinical-dashboard/clinical-dashboard-helpers";
import { answerRecovery, errorCopy } from "@/lib/ui-copy";
imp
🤖 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 126 - 128, Fix
the failing H1 assertion in the “keeps the root dashboard H1 as Clinical KB”
test by aligning its regex with the actual markup emitted by ClinicalDashboard,
or update the component’s root H1 to exactly match the asserted sr-only Clinical
KB structure; ensure the test passes while preserving the single-H1 requirement.
Source: Linters/SAST tools
|
|
||
| async function expectDashboardUsable(page: Page) { | ||
| await expect(page.getByRole("heading", { level: 1, name: "Clinical Guide" })).toHaveCount(1); | ||
| await expect(page.getByRole("heading", { level: 1, name: "Clinical KB" })).toHaveCount(1); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Align the dashboard H1 with these updated assertions.
src/components/ClinicalDashboard.tsx Line 3495 still renders Clinical Guide, so all three updated locators fail. Rename that H1 to Clinical KB (or retain the prior test expectation consistently).
tests/ui-accessibility.spec.ts#L51-L51: keep this expectation only after the rendered H1 is updated.tests/ui-smoke.spec.ts#L876-L876: keep this expectation only after the rendered H1 is updated.tests/ui-smoke.spec.ts#L1271-L1271: keep this expectation only after the rendered H1 is updated.
📍 Affects 2 files
tests/ui-accessibility.spec.ts#L51-L51(this comment)tests/ui-smoke.spec.ts#L876-L876tests/ui-smoke.spec.ts#L1271-L1271
🤖 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-accessibility.spec.ts` at line 51, Update the H1 rendered by
ClinicalDashboard from “Clinical Guide” to “Clinical KB” so it matches the
updated assertions. Apply the production change in ClinicalDashboard; retain the
expectations at tests/ui-accessibility.spec.ts:51, tests/ui-smoke.spec.ts:876,
and tests/ui-smoke.spec.ts:1271 unchanged because they are corrected by the H1
update.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/lib/rag.ts (1)
1364-1367: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winRestore caller cancellation while waiting on a shared classifier request.
A request aborted while
pendingis unresolved now waits for the classifier/provider to finish beforesearchChunksWithTelemetryreaches its next abort check. This regresses prompt cancellation and retains abandoned request work; the changed test also removes coverage for it.
src/lib/rag.ts#L1364-L1367: restore thesignaloption and race the caller’s wait against it without cancelling the shared classifier promise.tests/rag-classifier-memo.test.ts#L168-L177: retain/add a regression test proving an aborted coalesced caller rejects promptly.🤖 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 `@src/lib/rag.ts` around lines 1364 - 1367, Restore caller cancellation in src/lib/rag.ts at lines 1364-1367 by waiting on the shared classifier request with the caller’s signal, racing the caller wait against abort without cancelling pending, and preserving verdict memoization for completed requests. In tests/rag-classifier-memo.test.ts lines 168-177, retain or add regression coverage proving an aborted coalesced caller rejects promptly.src/components/services/services-navigator-page.tsx (1)
601-609: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep
universal-also-matchesafter the main results.SearchResultsLayoutrendersheaderbeforechildren, so placing this panel in the header makes it precedeservice-search-resultsin the DOM and breakstests/ui-universal-search.spec.ts. If that order is intentional, update the regression and contract together.🤖 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 `@src/components/services/services-navigator-page.tsx` around lines 601 - 609, Move UniversalSearchAlsoMatches out of the SearchResultsLayout header and render it after the main service-search-results children so universal-also-matches follows the results in the DOM. Update the services navigator composition around UniversalSearchAlsoMatches while preserving its modeId and query props; only change the regression or contract tests if the ordering is intentionally revised.
🤖 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/rag-abort-signal.test.ts`:
- Line 38: Remove the duplicate rpcCalls const declaration in the test scope,
keeping a single Array<{ name: string; args: Record<string, unknown> }>
definition so the block-scoped redeclaration error is resolved.
---
Outside diff comments:
In `@src/components/services/services-navigator-page.tsx`:
- Around line 601-609: Move UniversalSearchAlsoMatches out of the
SearchResultsLayout header and render it after the main service-search-results
children so universal-also-matches follows the results in the DOM. Update the
services navigator composition around UniversalSearchAlsoMatches while
preserving its modeId and query props; only change the regression or contract
tests if the ordering is intentionally revised.
In `@src/lib/rag.ts`:
- Around line 1364-1367: Restore caller cancellation in src/lib/rag.ts at lines
1364-1367 by waiting on the shared classifier request with the caller’s signal,
racing the caller wait against abort without cancelling pending, and preserving
verdict memoization for completed requests. In tests/rag-classifier-memo.test.ts
lines 168-177, retain or add regression coverage proving an aborted coalesced
caller rejects promptly.
🪄 Autofix (Beta)
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 Plus
Run ID: 23cad46b-9290-4621-a1fa-b1bda5bb75f1
📒 Files selected for processing (7)
docs/site-map.mdplaywright.config.tssrc/app/page.tsxsrc/components/services/services-navigator-page.tsxsrc/lib/rag.tstests/rag-abort-signal.test.tstests/rag-classifier-memo.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/app/page.tsx
| const controller = new AbortController(); | ||
| const abortSignal = vi.fn(async () => ({ data: [], error: null })); | ||
| const supabase = { rpc: vi.fn(() => ({ abortSignal })) }; | ||
| const rpcCalls: Array<{ name: string; args: Record<string, unknown> }> = []; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Remove the duplicate rpcCalls declaration.
Line 38 declares the same const twice in one scope, causing a block-scoped redeclaration compile error.
🤖 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/rag-abort-signal.test.ts` at line 38, Remove the duplicate rpcCalls
const declaration in the test scope, keeping a single Array<{ name: string;
args: Record<string, unknown> }> definition so the block-scoped redeclaration
error is resolved.
1 participant
Summary
src/app/page.tsxservicesnavigator imports insrc/components/services/services-navigator-page.tsxsrc/lib/rag.tstests/rag-abort-signal.test.tsandtests/rag-classifier-memo.test.tsdocs/site-map.md.origin/main.Verification
npm run typechecknpm run test -- tests/rag-abort-signal.test.ts tests/rag-classifier-memo.test.tsnpm run verify:uiis intentionally deferred until these non-UI blockers are fully in-tree and the PR-policy issue is resolved.Risk and rollout
d25ef25c8if this patch needs to be rolled back.Clinical Governance Preflight
Clinical KB Database(sjrfecxgysukkwxsowpy)