chore: land design-audit fixups and scoped regression hardening#839
chore: land design-audit fixups and scoped regression hardening#839BigSimmo wants to merge 3 commits into
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/ensure-unique-tree-pr-822-final) ↗︎
Tasks are run on every commit but only new migration files are pushed.
❌ Branch Error • Sat, 18 Jul 2026 09:32:10 UTC View logs for this Workflow Run ↗︎. |
📝 WalkthroughWalkthroughUpdates route handling and sitemap documentation, search UI behavior, RAG execution plumbing, Supabase schema state, and associated regression tests. ChangesProduct and platform updates
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. Comment |
CI triageCI failed on this PR. Automated classification of the 6 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. |
This reverts commit cccf120.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
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 | 🟠 Major | ⚡ Quick winRestore the metrics imports.
RightRailstill callsserviceNavigatorMetricsandcanCompareServices(Lines 296-297). Removing these bindings leaves unresolved identifiers and blocks the build.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 serviceNavigatorMetrics and canCompareServices imports in services-navigator-page.tsx so the existing RightRail references resolve and the build succeeds; leave the surrounding rankServiceRecords and useRegistryRecords imports unchanged.src/lib/rag.ts (1)
1291-1292: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRestore per-caller classifier cancellation.
This removes the contract exercised by
tests/rag-classifier-memo.test.ts:168-192: a cancelled caller must reject withAbortErrorwithout cancelling the shared classifier request. It now waits forpendingand returns a verdict/fallback instead. Restoresignaland race only the caller’s await against it.Also applies to: 1363-1366
🤖 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, Restore the optional caller AbortSignal in the RAG classifier options and classifier invocation near the ownerId field. In the pending-request await path, race only this caller’s wait against its signal so cancellation rejects with AbortError, while leaving the shared classifier request active for other callers. Preserve the existing verdict/fallback behavior for non-cancelled callers.
🤖 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/differentials/presentations/route.ts`:
- Line 6: Update the query extraction in the route handler to trim each
search-parameter candidate before choosing between them, so an empty or
whitespace-only query falls back to q. Preserve the existing behavior of
trimming the selected value and the redirect contract for valid query values.
In `@supabase/migrations/20260714190000_document_table_facts_trgm_idx.sql`:
- Around line 1-11: Move the index DDL from the existing migration into a new
migration created after the current migration head, leaving the released
migration unchanged. Create document_table_facts_text_trgm_idx concurrently and
configure the migration to run outside a transaction, using the repository’s
established convention.
In `@tests/audit-content-services-regressions.test.ts`:
- Around line 209-212: Update the source-rendering assertions in
audit-content-services-regressions.test.ts to match the current form-detail
implementation rather than expecting the stale ternary string. Use the current
normalized rendering shape or assert the resulting rendered behavior, while
preserving coverage for source-link availability.
In `@tests/audit-navigation-auth-regressions.test.ts`:
- Around line 126-128: Update the ClinicalDashboard.tsx implementation or the
corresponding clinicalDashboardSource assertion so the root dashboard contains
exactly one visually hidden H1 with the text “Clinical KB”, matching the
contract checked by the test.
In `@tests/ui-accessibility.spec.ts`:
- Line 51: Align the rendered H1 and all related assertions on the “Clinical KB”
heading: update the H1 in ClinicalDashboard, and ensure
tests/ui-accessibility.spec.ts lines 51-51, tests/ui-smoke.spec.ts lines
876-876, and tests/ui-smoke.spec.ts lines 1271-1271 use the same heading text.
---
Outside diff comments:
In `@src/components/services/services-navigator-page.tsx`:
- Around line 36-37: Restore the serviceNavigatorMetrics and canCompareServices
imports in services-navigator-page.tsx so the existing RightRail references
resolve and the build succeeds; leave the surrounding rankServiceRecords and
useRegistryRecords imports unchanged.
In `@src/lib/rag.ts`:
- Around line 1291-1292: Restore the optional caller AbortSignal in the RAG
classifier options and classifier invocation near the ownerId field. In the
pending-request await path, race only this caller’s wait against its signal so
cancellation rejects with AbortError, while leaving the shared classifier
request active for other callers. Preserve the existing verdict/fallback
behavior for non-cancelled callers.
🪄 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: e5ceafc7-5e2c-4ba5-8ad3-f43dcac43736
📒 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
| -- Migration to add GIN trigram index on document_table_facts text fields for performance optimization | ||
| create index if not exists document_table_facts_text_trgm_idx | ||
| on public.document_table_facts using gin ( | ||
| lower( | ||
| coalesce(table_title, '') || ' ' || | ||
| coalesce(row_label, '') || ' ' || | ||
| coalesce(clinical_parameter, '') || ' ' || | ||
| coalesce(threshold_value, '') || ' ' || | ||
| coalesce(action, '') | ||
| ) extensions.gin_trgm_ops | ||
| ); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Ship this as a new, concurrent migration.
Editing this previously released migration will not create the index in environments that already applied its former no-op version. Move the DDL into a new migration after the current migration head, and create it concurrently outside a transaction (or use the repository’s equivalent) so a large document_table_facts table does not block ingestion writes.
🧰 Tools
🪛 Squawk (2.59.0)
[warning] 2-11: During normal index creation, table updates are blocked, but reads are still allowed. Use concurrently to avoid blocking writes.
(require-concurrent-index-creation)
🤖 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 `@supabase/migrations/20260714190000_document_table_facts_trgm_idx.sql` around
lines 1 - 11, Move the index DDL from the existing migration into a new
migration created after the current migration head, leaving the released
migration unchanged. Create document_table_facts_text_trgm_idx concurrently and
configure the migration to run outside a transaction, using the repository’s
established convention.
Source: Linters/SAST tools
| 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 | 🔴 Critical | ⚡ Quick win
Fix the failing H1 contract before merge.
The unit-coverage check confirms that ClinicalDashboard.tsx does not contain the expected <h1 className="sr-only">Clinical KB</h1>. Update the implementation or the assertion so this test passes.
🧰 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,
Update the ClinicalDashboard.tsx implementation or the corresponding
clinicalDashboardSource assertion so the root dashboard contains exactly one
visually hidden H1 with the text “Clinical KB”, matching the contract checked by
the test.
Source: Linters/SAST tools
|
No-op/duplicate cleanup PR: head branch tree is identical to origin/main after unique-tree alignment; regression fix from #822 is already in main. |
1 participant
Summary
Verification
git statusclean on branch.git fetchand branch comparison done againstorigin/mainbefore PR creation.node_modulesmissing) and heavy-gate checks were blocked by an active external heavy test lock in another worktree.Notes
mainbranch.Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests