Fix service-related options placement and test sync#843
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>
…-main-merge-safe-20260718
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
📝 WalkthroughWalkthroughThe PR updates route documentation and query handling, adjusts search UI rendering and menu behavior, removes abort plumbing and coalescing telemetry from retrieval flows, and adds Supabase trigger/index changes with corresponding regression-test updates. ChangesRouting and site-map alignment
Search interface behavior
Retrieval cancellation and memoization
Supabase migration and schema validation
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
supabase/migrations/20260714190000_document_table_facts_trgm_idx.sql (1)
1-11: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider
CONCURRENTLYfor the index creation if migrations are not transaction-wrapped.Squawk flags that
CREATE INDEXwithoutCONCURRENTLYblocks writes. Since this index is intentionally temporary (dropped by the harden migration), the blocking window is brief. If your migration runner supports non-transactional execution,CREATE INDEX CONCURRENTLY IF NOT EXISTSwould avoid write blocking. If migrations run inside transactions (common in Supabase),CONCURRENTLYcannot be used and this can be safely disregarded.🤖 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, Update the document_table_facts_text_trgm_idx creation to use concurrent index creation only if the migration runner executes migrations outside transactions; otherwise retain the current non-concurrent CREATE INDEX because PostgreSQL disallows CONCURRENTLY within transactional migrations.Source: Linters/SAST tools
🤖 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/ClinicalDashboard.tsx`:
- Line 3651: Update the error title logic in ClinicalDashboard so only
answer-result errors use “Answer unavailable”; document and differential search
failures must retain a non-answer-specific title. Use the result-kind condition
surrounding the title to select the appropriate mode-specific label.
In `@tests/supabase-schema.test.ts`:
- Around line 1399-1401: Replace the tautological conditional assertion in the
relevant schema test with an unconditional assertion that verifies the expected
corrector content, or assert a distinct related property inside the guard;
ensure the test fails when the similarity-threshold statement is absent.
---
Nitpick comments:
In `@supabase/migrations/20260714190000_document_table_facts_trgm_idx.sql`:
- Around line 1-11: Update the document_table_facts_text_trgm_idx creation to
use concurrent index creation only if the migration runner executes migrations
outside transactions; otherwise retain the current non-concurrent CREATE INDEX
because PostgreSQL disallows CONCURRENTLY within transactional migrations.
🪄 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: dc7e90aa-cee1-4c72-8ccb-4e1b513ad97a
📒 Files selected for processing (21)
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/rag-abort-signal.test.tstests/rag-classifier-memo.test.tstests/site-map.test.tstests/supabase-schema.test.tstests/ui-tools.spec.tstests/ui-universal-search.spec.ts
| <EmptyState | ||
| icon={CircleAlert} | ||
| title={activeModeResultKind === "answer" ? "Answer unavailable" : "Search unavailable"} | ||
| title="Answer unavailable" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Keep the error title mode-specific.
error is shared by document and differentials searches too, so those failures now display “Answer unavailable.” Restore a non-answer title for non-answer result kinds.
Proposed fix
- title="Answer unavailable"
+ title={activeModeResultKind === "answer" ? "Answer unavailable" : "Search unavailable"}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| title="Answer unavailable" | |
| title={activeModeResultKind === "answer" ? "Answer unavailable" : "Search unavailable"} |
🤖 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/ClinicalDashboard.tsx` at line 3651, Update the error title
logic in ClinicalDashboard so only answer-result errors use “Answer
unavailable”; document and differential search failures must retain a
non-answer-specific title. Use the result-kind condition surrounding the title
to select the appropriate mode-specific label.
| if (corrector.includes("set pg_trgm.similarity_threshold = 0.3")) { | ||
| expect(corrector).toContain("set pg_trgm.similarity_threshold = 0.3"); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Tautological assertion provides no test coverage.
The if condition and the expect assertion check the exact same string ("set pg_trgm.similarity_threshold = 0.3"). If the string is present, the assertion trivially passes; if absent, the block is skipped. This effectively tests nothing. Either remove the conditional and assert unconditionally, or assert a different related property inside the guard.
♻️ Suggested fix
- if (corrector.includes("set pg_trgm.similarity_threshold = 0.3")) {
- expect(corrector).toContain("set pg_trgm.similarity_threshold = 0.3");
- }
+ expect(corrector).toContain("set pg_trgm.similarity_threshold = 0.3");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (corrector.includes("set pg_trgm.similarity_threshold = 0.3")) { | |
| expect(corrector).toContain("set pg_trgm.similarity_threshold = 0.3"); | |
| } | |
| expect(corrector).toContain("set pg_trgm.similarity_threshold = 0.3"); |
🤖 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/supabase-schema.test.ts` around lines 1399 - 1401, Replace the
tautological conditional assertion in the relevant schema test with an
unconditional assertion that verifies the expected corrector content, or assert
a distinct related property inside the guard; ensure the test fails when the
similarity-threshold statement is absent.
1 participant
Summary
Clinical Guidetitle variant and stabilized selectors) so they reflect live rendering while preserving regression coverage.Verification
npx playwright test tests/ui-accessibility.spec.ts --project=chromium --workers=1npx playwright test tests/ui-universal-search.spec.ts --project=chromium --workers=1npm run test -- tests/site-map.test.ts tests/supabase-schema.test.tsRisk and rollout
Clinical Governance Preflight
Summary by CodeRabbit
New Features
Bug Fixes
Documentation