fix: restore service detail fixtures and dashboard H1 copy - #855
fix: restore service detail fixtures and dashboard H1 copy#855BigSimmo wants to merge 2 commits into
Conversation
Updates to Preview Branch (codex/merge-safe-819-mainfix) ↗︎
Tasks are run on every commit but only new migration files are pushed.
❌ Branch Error • Sat, 18 Jul 2026 13:04:53 UTC View logs for this Workflow Run ↗︎. |
📝 WalkthroughWalkthroughThe changes add abort-aware RAG RPC and classifier handling, answer-coalescing telemetry, dashboard rendering and accessibility updates, broadened query-parameter typing, and synchronized Playwright, schema, audit, and UI regression expectations. ChangesRAG pipeline
Dashboard and UI contracts
Regression and test configuration
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Retrieval as RAG retrieval
participant Classifier as Classifier fallback
participant RPC as Supabase RPC
participant Metrics as Answer telemetry
Retrieval->>Classifier: forward caller AbortSignal
Classifier->>RPC: execute abortable retrieval RPC
RPC-->>Classifier: result or abort
Classifier-->>Retrieval: verdict or abort error
Retrieval->>Metrics: record coalesced waiter
Retrieval->>Metrics: record origination start and finish
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:f99d4cfb31
ℹ️ 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".
| grepInvert: mockupTag, | ||
| use: { | ||
| ...devices["Desktop Chrome"], | ||
| contextOptions: { reducedMotion: "no-preference" }, |
There was a problem hiding this comment.
When the chromium project runs the production specs, including the ui-smoke and ui-stress files selected by productionSpecPattern, this project-level contextOptions: { reducedMotion: "no-preference" } overrides the suite default that was added to keep CSS transitions from moving click targets mid-test. That puts the main Chromium gate back under real animations and reintroduces the documented flake risk for every production Chromium spec just to support one transition assertion; keep the default reduced motion and opt that specific assertion into no-preference with page.emulateMedia or a dedicated project.
Useful? React with 👍 / 👎.
BigSimmo
commented
Jul 18, 2026
@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. 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. After fixing or dispositioning a thread, reply in that thread with as the first line, followed by a concise summary; that marker authorizes the workflow to close that exact thread. If human input or new authorization is required, do not use the marker and leave the thread open with the blocker. 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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:f99d4cfb31
ℹ️ 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".
| const pendingWithAbort = | ||
| signal && typeof pending.abortSignal === "function" ? pending.abortSignal(signal) : pending; |
There was a problem hiding this comment.
This helper now knows how to attach an AbortSignal to Supabase RPC builders, but the RAG retrieval call sites still never pass args.signal into callVersionedRetrievalRpc (checked the direct searchChunksWithTelemetry calls and the candidate searchers). When an /api/answer or stream request disconnects or hits the route deadline after retrieval has started, routeDeadline.race() rejects while the expensive Supabase RPCs continue running in the background; thread the deadline/request signal through the candidate args and direct RPC calls, and cover it with a test that exercises searchChunksWithTelemetry rather than this helper directly.
Useful? React with 👍 / 👎.
| grepInvert: mockupTag, | ||
| use: { | ||
| ...devices["Desktop Chrome"], | ||
| contextOptions: { reducedMotion: "no-preference" }, |
There was a problem hiding this comment.
This project-level contextOptions overrides the top-level contextOptions: { reducedMotion: "reduce" } for the whole Chromium production project, so the CI path that runs ui-smoke, ui-stress, and the other production specs now re-enables animations despite the suite-wide race-prevention comment above. If only the new dock-transition assertion needs real motion, scope reducedMotion: "no-preference" to that test/spec instead of the entire Chromium project.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/lib/rag.ts`:
- Around line 2430-2434: Update both retrieval RPC call sites in the query
analysis flow to pass args.signal as the fifth argument to
callVersionedRetrievalRpc, including the hybrid call near
analyzeQueryWithClassifierFallback and the fallback call around the fallback
retrieval path. Preserve the existing retrieval arguments and ensure caller
cancellation reaches both RPC requests.
- Around line 431-447: Update awaitWithCallerSignal and its surrounding
error-handling flow so any caller-aborted signal bypasses classifier fallback,
regardless of whether signal.reason is an AbortError or a custom Error. Preserve
and propagate the caller’s abort reason instead of swallowing it and returning
analysis.
In `@tests/ui-tools.spec.ts`:
- Around line 21-22: Update the testServiceSlugs declaration and its use in the
testServiceRecords filter so the slug collection accepts a general string, for
example by widening it to a string collection or using Set<string>. Preserve the
existing serviceRecords filtering behavior and matching slug values.
🪄 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: 45aaaa55-68cb-4f47-a469-3f052fa0bcde
📒 Files selected for processing (13)
playwright.config.tssrc/app/page.tsxsrc/components/ClinicalDashboard.tsxsrc/components/route-error-boundary.tsxsrc/lib/rag-candidate-sources.tssrc/lib/rag.tssupabase/drift-manifest.jsontests/audit-content-services-regressions.test.tstests/audit-navigation-auth-regressions.test.tstests/supabase-schema.test.tstests/ui-accessibility.spec.tstests/ui-smoke.spec.tstests/ui-tools.spec.ts
| function awaitWithCallerSignal<T>(pending: Promise<T>, signal?: AbortSignal): Promise<T> { | ||
| if (!signal) return pending; | ||
| if (signal.aborted) throw signal.reason ?? new DOMException("The operation was aborted.", "AbortError"); | ||
| return new Promise<T>((resolve, reject) => { | ||
| const onAbort = () => reject(signal.reason ?? new DOMException("The operation was aborted.", "AbortError")); | ||
| signal.addEventListener("abort", onAbort, { once: true }); | ||
| pending.then( | ||
| (value) => { | ||
| signal.removeEventListener("abort", onAbort); | ||
| resolve(value); | ||
| }, | ||
| (error) => { | ||
| signal.removeEventListener("abort", onAbort); | ||
| reject(error); | ||
| }, | ||
| ); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
node - <<'NODE'const controller = new AbortController();const reason = new Error("caller cancelled");controller.abort(reason);if (controller.signal.reason !== reason) process.exit(1);NODERepository: BigSimmo/Database
Length of output: 155
🏁 Script executed:
#!/bin/bashset -euo pipefail
file="src/lib/rag.ts"
wc -l "$file"
sed -n '410,470p'"$file"| cat -n
printf'\n---\n'
sed -n '1370,1415p'"$file"| cat -nRepository: BigSimmo/Database
Length of output: 4351
🏁 Script executed:
#!/bin/bashset -euo pipefail
file="src/lib/rag.ts"
nl -ba "$file"| sed -n '428,450p'printf'\n---\n'
nl -ba "$file"| sed -n '1388,1405p'Repository: BigSimmo/Database
Length of output: 195
Preserve caller cancellations here. Any aborted signal should skip the classifier fallback; custom reasons like abort(new Error(...)) are not AbortError, so this catch swallows cancellation and returns analysis instead.
🤖 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 431 - 447, Update awaitWithCallerSignal and its
surrounding error-handling flow so any caller-aborted signal bypasses classifier
fallback, regardless of whether signal.reason is an AbortError or a custom
Error. Preserve and propagate the caller’s abort reason instead of swallowing it
and returning analysis.
| const queryAnalysis = await analyzeQueryWithClassifierFallback(retrievalQuery, analyzeClinicalQuery(retrievalQuery), { | ||
| corpusGrounding: corpusGroundingScope, | ||
| ownerId: args.ownerId, | ||
| signal: args.signal, | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Thread the caller signal into both retrieval RPC calls.
Line 2433 cancels classifier waiting, but the hybrid and fallback calls to callVersionedRetrievalRpc omit args.signal; their new abort support is therefore never used.
Proposed fix
const { data, error } = await callVersionedRetrievalRpc(
supabase,
"match_document_chunks_hybrid_v2",
"match_document_chunks_hybrid",
{
// ...
},
+ args.signal,
);Apply the same fifth argument to the fallback call around Line 2945.
🤖 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 2430 - 2434, Update both retrieval RPC call
sites in the query analysis flow to pass args.signal as the fifth argument to
callVersionedRetrievalRpc, including the hybrid call near
analyzeQueryWithClassifierFallback and the fallback call around the fallback
retrieval path. Preserve the existing retrieval arguments and ensure caller
cancellation reaches both RPC requests.
| const testServiceSlugs = ["13yarn", "city-east-community-mental-health-service"] as const; | ||
| const testServiceRecords = serviceRecords.filter((service) => testServiceSlugs.includes(service.slug)); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bashset -euo pipefail
npm run verify:cheapRepository: BigSimmo/Database
Length of output: 11787
🏁 Script executed:
#!/usr/bin/env bashset -euo pipefail
sed -n '1,60p' tests/ui-tools.spec.ts | cat -nRepository: BigSimmo/Database
Length of output: 3356
🏁 Script executed:
#!/usr/bin/env bashset -euo pipefail
python3 - <<'PY'from pathlib import Pathp = Path('tests/ui-tools.spec.ts')text = p.read_text()for i, line in enumerate(text.splitlines(), 1): if 'testServiceSlugs' in line or 'testServiceRecords' in line: print(f"{i}: {line}")PYRepository: BigSimmo/Database
Length of output: 568
Fix the tuple-typed includes check at tests/ui-tools.spec.ts:22.
testServiceSlugs is a readonly literal tuple, so includes(service.slug) rejects the general string type and blocks the TypeScript check. Use a Set<string> or widen the slug collection type.
🧰 Tools
🪛 GitHub Check: Static PR checks
[failure] 22-22:
Argument of type 'string' is not assignable to parameter of type '"13yarn" | "city-east-community-mental-health-service"'.
🤖 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-tools.spec.ts` around lines 21 - 22, Update the testServiceSlugs
declaration and its use in the testServiceRecords filter so the slug collection
accepts a general string, for example by widening it to a string collection or
using Set<string>. Preserve the existing serviceRecords filtering behavior and
matching slug values.
Sources: Coding guidelines, Linters/SAST tools
Summary
/api/registry/records.Clinical GuidetoClinical KBto match existing product contract assertions.Why
The branch fixed test drift between API fixture behavior and fixture-based UI coverage, and resolved a dashboard copy mismatch regression.
Verification
npm run test:e2e:pr -- tests/ui-tools.spec.ts --project=chromium --grep "service detail"✅ PASS (5/5)npm run test:e2e:pr -- tests/ui-smoke.spec.ts --project=chromium --grep "Clinical KB UI smoke coverage"✅ PASS (87/87)npm run test:e2e:pr -- tests/ui-accessibility.spec.ts --project=chromium --grep "Clinical KB"✅ PASS (7/7)npm run test -- tests/audit-navigation-auth-regressions.test.ts✅ PASS (5/5)npm run verify:uinot executed for this local-only correction.Clinical Governance Preflight
Not applicable for this scope: no ingestion, answer generation, source governance, privacy, or clinical output behavior changes.
Risk and Rollout
mainand monitor regular CI.Summary by CodeRabbit
Accessibility
Performance & Reliability
Search & Forms
Testing