Skip to content

fix: resolve design-audit and CI regression guard blockers#841

Merged
7 commits merged into
mainfrom
codex/answer-progress-main-safe-20260718b
Jul 18, 2026
Merged

fix: resolve design-audit and CI regression guard blockers#841
7 commits merged into
mainfrom
codex/answer-progress-main-safe-20260718b

Conversation

@BigSimmo

@BigSimmo BigSimmo commented Jul 18, 2026

Copy link
Copy Markdown
Owner

Summary

  • Fixes related-options positioning/regression behavior and keeps service navigation and differential route checks stable after merge-sync adjustments.
  • Carries forward prior CI-drift and merge-policy guard fixes needed to keep this stream merge-safe on main.

Verification

  • npm run check:runtime
  • npm run check:owner-scope
  • npm run typecheck
  • UI verification not run: npm run verify:ui repeatedly hit cross-worktree heavy-lock contention in active bb47 worktree and could not complete in this local branch.
  • Lint verification not run: npm run lint is currently blocked by a concurrent heavy lock from another worktree process and was deferred.

Risk and rollout

  • Risk: medium; broad UI and clinical-path touchpoints mean unintended positioning regressions are possible if menu alignment assumptions drift in routing shells.
  • Rollback: revert this PR and re-run the full gate (verify:cheap + required policy checklist) before reattempting merge.

Clinical Governance Preflight

  • Source-backed claims still require linked source verification before clinical use
  • No patient-identifiable document workflow was introduced or expanded without explicit governance approval
  • Supabase target remains Clinical KB Database (sjrfecxgysukkwxsowpy)
  • Service-role keys and private document access remain server-only
  • Demo/synthetic content remains clearly separated from real clinical sources
  • Source metadata, review status, and outdated/unknown-source behavior remain conservative
  • Deployment classification/TGA SaMD impact was checked when clinical decision-support behavior changed

BigSimmo and others added 7 commits July 18, 2026 16:36
* 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>
@supabase

supabase Bot commented Jul 18, 2026

Copy link
Copy Markdown

This pull request has been ignored for the connected project sjrfecxgysukkwxsowpy due to reaching the limit of concurrent preview branches.
Go to Project Integrations Settings ↗︎ if you wish to update this limit.


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

@BigSimmo
BigSimmo enabled auto-merge July 18, 2026 09:33
@coderabbitai

coderabbitai Bot commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The PR aligns sitemap and redirect metadata, normalizes home and presentations query handling, adjusts search UI behavior, removes RAG abort and coalescing instrumentation paths, adds a trigram index migration, changes trigger comparisons, and updates regression expectations.

Changes

Route and sitemap contracts

Layer / File(s) Summary
Route and sitemap contracts
src/app/page.tsx, src/app/differentials/presentations/route.ts, scripts/generate-site-map.ts, docs/site-map.md, tests/site-map.test.ts, tests/audit-navigation-auth-regressions.test.ts
Search parameters are normalized, the presentations redirect uses [workflow-slug], and factsheet descriptions are removed from generated sitemap metadata. Tests update the related route expectations.

Search UI behavior and assertions

Layer / File(s) Summary
Search UI behavior and assertions
src/components/ClinicalDashboard.tsx, src/components/clinical-dashboard/master-search-header.tsx, src/components/services/services-navigator-page.tsx, playwright.config.ts, tests/audit-content-services-regressions.test.ts, tests/ui-*.spec.ts
Dashboard match/error rendering, header menu focus behavior, service navigator placement, reduced-motion formatting, and UI regression assertions are updated.

RAG RPC and classifier cancellation

Layer / File(s) Summary
RAG RPC and classifier cancellation
src/lib/rag-candidate-sources.ts, src/lib/rag.ts, tests/rag-*.test.ts
RPC and classifier calls no longer use caller abort signals; coalescing observability calls are removed, and tests cover direct RPC arguments and fallback classification.

Database trigger and search index updates

Layer / File(s) Summary
Database trigger and search index updates
supabase/migrations/*, supabase/drift-manifest.json, tests/supabase-schema.test.ts
Document title-word trigger comparisons use <>, a document facts trigram index is created idempotently, and schema/replay expectations are refreshed.

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

Possibly related PRs

  • BigSimmo/Database#814: Overlaps on presentations query handling, RAG signal wiring, and service navigator remount behavior.
  • BigSimmo/Database#817: Overlaps on RAG abort-signal removal, classifier fallback changes, migrations, and sitemap updates.
  • BigSimmo/Database#833: Directly overlaps the dashboard error title and UniversalSearchAlsoMatches rendering changes.

Suggested labels: codex

Suggested reviewers: copilot, claude

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 38.46% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title matches the PR’s main theme: resolving design-audit and CI regression guard blockers.
Description check ✅ Passed The description follows the template headings and includes summary, verification, risk/rollback, governance, and notes.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/answer-progress-main-safe-20260718b

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

@coderabbitai coderabbitai Bot 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: 6

🧹 Nitpick comments (1)
supabase/migrations/20260714190000_document_table_facts_trgm_idx.sql (1)

1-11: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Consider CREATE INDEX CONCURRENTLY to avoid blocking writes during deployment.

Non-concurrent CREATE INDEX blocks all writes to document_table_facts for the duration of index builds. For production tables with ongoing writes, this can cause noticeable downtime. Supabase migrations support transaction-less execution — add -- transaction: false as the first line and use CONCURRENTLY:

♻️ Proposed refactor
+-- transaction: false
 -- 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
+create index concurrently if not exists document_table_facts_text_trgm_idx
   on public.document_table_facts using gin (

If the table is small or this migration only runs in preview/staging, the current approach is acceptable.

🤖 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 index migration to run outside a
transaction and create document_table_facts_text_trgm_idx with CREATE INDEX
CONCURRENTLY, preserving the existing GIN trigram expression and IF NOT EXISTS
behavior.

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/app/differentials/presentations/route.ts`:
- Line 6: Update the query extraction expression in the route handler to use
logical-OR fallback between the trimmed query parameters, so an empty or
whitespace-only query falls back to q as before. Preserve the existing trimming
behavior and parameter precedence for non-empty values.

In `@tests/audit-navigation-auth-regressions.test.ts`:
- Around line 95-109: The regression expectations in
tests/audit-navigation-auth-regressions.test.ts at lines 95-109 and 126-128 are
stale relative to ClinicalDashboard.tsx. Update the canUsePrivateApis assertion
to include localNoAuthMode and localDevCanAttemptPrivateApis, preserve the
current polling expectations, and change the dashboard heading expectation to
Clinical Guide rather than Clinical KB.

In `@tests/site-map.test.ts`:
- Around line 125-131: In the expectedProductHandlers assertions, change the
productSection check back to not.toContain for each redirect route. Keep the
redirects assertion and apiSection exclusion unchanged so redirect handlers
remain only in the Redirects section.

In `@tests/supabase-schema.test.ts`:
- Around line 1372-1374: Update the cleanup SQL assertion in the test around the
registry-record CASE expression to match the migration’s qualified function
call, including the pg_catalog. prefix before to_jsonb(old). Keep the existing
assertions for registry_record_id and registry_record_kind unchanged.
- Around line 131-132: Update the migration-order assertion in
tests/supabase-schema.test.ts to reflect the current migration set: either
expect 20260717173000_reassert_supabase_admin_default_privileges.sql as the
latest migration or revise the assertion to target the specific migration
covered by defaultAclAssertionMigration.

In `@tests/ui-accessibility.spec.ts`:
- Line 51: Align the heading-name locators with the rendered h1 from
ClinicalDashboard: update 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
to expect “Clinical Guide” instead of “Clinical KB”.

---

Nitpick comments:
In `@supabase/migrations/20260714190000_document_table_facts_trgm_idx.sql`:
- Around line 1-11: Update the document_table_facts index migration to run
outside a transaction and create document_table_facts_text_trgm_idx with CREATE
INDEX CONCURRENTLY, preserving the existing GIN trigram expression and IF NOT
EXISTS behavior.
🪄 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: 3f32d839-4e92-4ed7-ae02-686581b13981

📥 Commits

Reviewing files that changed from the base of the PR and between 7815204 and d25ef25.

📒 Files selected for processing (22)
  • docs/site-map.md
  • playwright.config.ts
  • scripts/generate-site-map.ts
  • src/app/differentials/presentations/route.ts
  • src/app/page.tsx
  • src/components/ClinicalDashboard.tsx
  • src/components/clinical-dashboard/master-search-header.tsx
  • src/components/services/services-navigator-page.tsx
  • src/lib/rag-candidate-sources.ts
  • src/lib/rag.ts
  • supabase/drift-manifest.json
  • supabase/migrations/20260714180000_patch_rag_and_corrector_scalability.sql
  • supabase/migrations/20260714190000_document_table_facts_trgm_idx.sql
  • tests/audit-content-services-regressions.test.ts
  • tests/audit-navigation-auth-regressions.test.ts
  • tests/rag-abort-signal.test.ts
  • tests/rag-classifier-memo.test.ts
  • tests/site-map.test.ts
  • tests/supabase-schema.test.ts
  • tests/ui-accessibility.spec.ts
  • tests/ui-smoke.spec.ts
  • tests/ui-tools.spec.ts

const legacyQuery = request.nextUrl.searchParams.get("q");
// Prefer `query`, but fall through on empty/whitespace so `q` remains usable.
const query = (rawQuery?.trim() || legacyQuery?.trim())?.trim();
const query = (request.nextUrl.searchParams.get("query") ?? request.nextUrl.searchParams.get("q"))?.trim();

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.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

?? skips q fallback when query is an empty string.

searchParams.get("query") returns "" (not null) when ?query= is present, so ?? won't fall back to q. The previous logic explicitly fell back to q on empty/whitespace. Use || to preserve the original fallback semantics.

🔧 Proposed fix
-  const query = (request.nextUrl.searchParams.get("query") ?? request.nextUrl.searchParams.get("q"))?.trim();
+  const query = (request.nextUrl.searchParams.get("query") || request.nextUrl.searchParams.get("q"))?.trim();
📝 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.

Suggested change
const query = (request.nextUrl.searchParams.get("query") ?? request.nextUrl.searchParams.get("q"))?.trim();
const query = (request.nextUrl.searchParams.get("query") || request.nextUrl.searchParams.get("q"))?.trim();
🤖 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/app/differentials/presentations/route.ts` at line 6, Update the query
extraction expression in the route handler to use logical-OR fallback between
the trimmed query parameters, so an empty or whitespace-only query falls back to
q as before. Preserve the existing trimming behavior and parameter precedence
for non-empty values.

Comment on lines +95 to +109
"// Local/demo guests can read the public library",
"const canRunSearch =",
);
expect(privateCapabilityContract).toContain("const canUsePrivateApis =");
expect(privateCapabilityContract).toContain(
'localNoAuthMode || localDevCanAttemptPrivateApis || authStatus === "authenticated"',
'const canUsePrivateApis = localProjectReady && authStatus === "authenticated";',
);
expect(privateCapabilityContract).not.toMatch(/localNoAuth|clientDemoMode/);

const pollingContract = sourceSegment(
clinicalDashboardSource,
"if (!nextDemoMode && !canUsePrivateApis) {",
"const shouldRefreshWorkState =",
"const [documentsResponse",
);
expect(pollingContract).toContain("if (!nextDemoMode && !canUsePrivateApis) {");
expect(pollingContract).toContain("setDocuments([]);");
expect(pollingContract).toContain("return;");
expect(pollingContract).toContain("(canUsePrivateApis || serverDemoMode)");
expect(pollingContract).not.toMatch(/localNoAuth|clientDemoMode/);

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.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== files =="
git ls-files tests/audit-navigation-auth-regressions.test.ts src components app | sed -n '1,200p'

echo
echo "== outline: test file =="
ast-grep outline tests/audit-navigation-auth-regressions.test.ts --view expanded || true

echo
echo "== outline: ClinicalDashboard source candidates =="
fd -a 'ClinicalDashboard.tsx' . || true

echo
echo "== relevant excerpts from test =="
sed -n '1,220p' tests/audit-navigation-auth-regressions.test.ts

echo
echo "== search for cited symbols/patterns =="
rg -n 'canUsePrivateApis|localNoAuthMode|clientDemoMode|serverDemoMode|Clinical KB|<h1 className="sr-only"|isLocalNoAuthMode|localProjectReady &&' -S .

echo
echo "== show candidate ClinicalDashboard source if found =="
for f in $(fd -a 'ClinicalDashboard.tsx' .); do
  echo "--- $f ---"
  wc -l "$f"
  sed -n '1,260p' "$f"
done

Repository: BigSimmo/Database

Length of output: 50373


🏁 Script executed:

#!/bin/bash
set -euo pipefail

f=src/components/ClinicalDashboard.tsx

echo "== file size =="
wc -l "$f"

echo
echo "== around private-api gate =="
rg -n -C 8 'canUsePrivateApis|localNoAuthMode|clientDemoMode|serverDemoMode|canRunSearch|shouldRefreshWorkState|mutateDocumentLabel|openUploadDrawer|<h1 className="sr-only"|Clinical KB' "$f"

echo
echo "== source excerpt (private-api / polling / h1 regions) =="
sed -n '1,260p' "$f" | nl -ba | sed -n '1,260p'

echo
echo "== test file excerpt with line numbers =="
sed -n '88,132p' tests/audit-navigation-auth-regressions.test.ts | nl -ba

Repository: BigSimmo/Database

Length of output: 30011


Align the regression expectations with src/components/ClinicalDashboard.tsx

  • canUsePrivateApis still includes localNoAuthMode and localDevCanAttemptPrivateApis, so the simplified expectation here is stale.
  • The dashboard H1 is still Clinical Guide, not Clinical KB.

Update the test to match the current source, or include the ClinicalDashboard.tsx changes in this PR.

📍 Affects 1 file
  • tests/audit-navigation-auth-regressions.test.ts#L95-L109 (this comment)
  • tests/audit-navigation-auth-regressions.test.ts#L126-L128
🤖 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 95 - 109, The
regression expectations in tests/audit-navigation-auth-regressions.test.ts at
lines 95-109 and 126-128 are stale relative to ClinicalDashboard.tsx. Update the
canUsePrivateApis assertion to include localNoAuthMode and
localDevCanAttemptPrivateApis, preserve the current polling expectations, and
change the dashboard heading expectation to Clinical Guide rather than Clinical
KB.

Source: Linters/SAST tools

Comment thread tests/site-map.test.ts
Comment on lines +125 to +131
for (const [route, file, target] of expectedProductHandlers) {
expect(data.publicRouteHandlers).toContainEqual({ route, file });
expect(data.apiRoutes).not.toContainEqual({ route, file });
expect(data.redirects).toContainEqual({ route, file, target });
expect(productSection).toContain(`\`${route}\``);
expect(apiSection).not.toContain(`\`${route}\``);
}

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.

🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

productSection.toContain assertion will fail for redirect handlers.

The "Main product routes" section only lists page routes — redirect handlers like /applications and /medications appear exclusively in the "## Redirects" section. Changing from not.toContain to toContain (line 129) causes the test to fail, confirmed by the CI failure: expected '## Main product routes\n...' to contain '/applications'.

Revert line 129 to not.toContain — redirect handlers should not appear in the product routes section.

🔧 Proposed fix
-      expect(productSection).toContain(`\`${route}\``);
+      expect(productSection).not.toContain(`\`${route}\``);
📝 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.

Suggested change
for (const [route, file, target] of expectedProductHandlers) {
expect(data.publicRouteHandlers).toContainEqual({ route, file });
expect(data.apiRoutes).not.toContainEqual({ route, file });
expect(data.redirects).toContainEqual({ route, file, target });
expect(productSection).toContain(`\`${route}\``);
expect(apiSection).not.toContain(`\`${route}\``);
}
for (const [route, file, target] of expectedProductHandlers) {
expect(data.publicRouteHandlers).toContainEqual({ route, file });
expect(data.apiRoutes).not.toContainEqual({ route, file });
expect(data.redirects).toContainEqual({ route, file, target });
expect(productSection).not.toContain(`\`${route}\``);
expect(apiSection).not.toContain(`\`${route}\``);
}
🧰 Tools
🪛 GitHub Check: Unit coverage

[failure] 129-129: [node] tests/site-map.test.ts > tracked sitemap > keeps public redirect handlers in product routes and API handlers in the API section
AssertionError: expected '## Main product routes\n\n- / - Mai…' to contain '/applications'

  • Expected
  • Received
  • /applications
  • Main product routes

    • / - Main Clinical KB shell. Source: src/app/page.tsx.
    • /differentials - Differentials home and search surface. Source: src/app/differentials/page.tsx.
    • /differentials/diagnoses - Diagnosis stream. Source: src/app/differentials/diagnoses/page.tsx.
    • /differentials/presentations/[slug] - Route discovered from app directory Source: src/app/differentials/presentations/[slug]/page.tsx.
    • /documents/search - Documents search command centre. Source: src/app/documents/search/page.tsx.
    • /documents/source - Compatibility redirect to the canonical live document viewer when a valid id is supplied. Source: src/app/documents/source/page.tsx.
    • /documents/source/evidence - Compatibility redirect sharing the canonical live document viewer handoff. Source: src/app/documents/source/evidence/page.tsx.
    • /dsm - DSM-5 Diagnosis home. Source: src/app/dsm/page.tsx.
    • /dsm/compare - DSM diagnosis comparison. Source: src/app/dsm/compare/page.tsx.
    • /dsm/search - DSM diagnosis search and catalogue browser. Source: src/app/dsm/search/page.tsx.
    • /factsheets - Route discovered from app directory Source: src/app/factsheets/page.tsx.
    • /factsheets/[slug] - Route discovered from app directory Source: src/app/factsheets/[slug]/page.tsx.
    • /factsheets/search - Route discovered from app directory Source: src/app/factsheets/search/page.tsx.
    • /favourites - Saved clinical items and sets. Source: src/app/favourites/page.tsx.
    • /forms - Forms home and search surface. Source: src/app/forms/page.tsx.
    • /formulation - Clinical formulation home and local mechanism search surface. Source: src/app/formulation/page.tsx.
    • /formulation/builder - Structured clinical formulation builder. Source: src/app/formulation/builder/page.tsx.
    • /formulation/compare - Side-by-side mechanism comparison. Source: src/app/formulation/compare/page.tsx.
    • /formulation/map - Formulation mechanism domain map. Source: src/app/formulation/map/page.tsx.
    • /privacy - Privacy and data-processing governance draft. Source: src/app/privacy/page.tsx.
    • /reference/colour-coding - Route discovered from app directory Source: src/app/reference/colour-coding/page.tsx.
    • /services - Services home and search surface. Source: src/app/services/page.tsx.
    • /specifiers - Psychiatric specifier home and local search surface. Source: src/app/specifiers/page.tsx.
    • /specifiers/builder - Structured diagnostic wording builder. Source: src/app/specifiers/builder/page.tsx.
    • /specifiers/compare - Side-by-side psychiatric specifier comparison. Source: src/app/specifiers/compare/page.tsx.
    • /specifiers/map - Psychiatric specifier family map. Source: src/app/specifiers/map/page.tsx.
    • /therapy-compass - Route discovered from app directory Source: src/app/therapy-compass/page.tsx.
    • /tools - Route discovered from app directory Source: src/app/tools/page.tsx.

❯ tests/site-map.test.ts:129:30

🤖 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/site-map.test.ts` around lines 125 - 131, In the
expectedProductHandlers assertions, change the productSection check back to
not.toContain for each redirect route. Keep the redirects assertion and
apiSection exclusion unchanged so redirect handlers remain only in the Redirects
section.

Source: Linters/SAST tools

Comment on lines 131 to +132
const defaultAclAssertionMigration = readFileSync(
new URL("../supabase/migrations/20260717173000_reassert_supabase_admin_default_privileges.sql", import.meta.url),
new URL("../supabase/migrations/20260717161000_assert_supabase_admin_default_privileges.sql", import.meta.url),

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.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# List migration files to confirm both exist
ls -1 supabase/migrations/ | grep -E '20260717161000|20260717173000'

Repository: BigSimmo/Database

Length of output: 277


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the relevant test assertions and the migration file list.
sed -n '1200,1245p' tests/supabase-schema.test.ts
printf '\n---\n'
sed -n '1360,1425p' tests/supabase-schema.test.ts
printf '\n--- migrations ---\n'
ls -1 supabase/migrations/ | grep -E '20260717161000|20260717173000' | sort

Repository: BigSimmo/Database

Length of output: 6457


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path
p = Path('tests/supabase-schema.test.ts')
text = p.read_text()
for needle in [
    "20260717161000_assert_supabase_admin_default_privileges.sql",
    "20260717173000_reassert_supabase_admin_default_privileges.sql",
    "to_jsonb(old)->>'kind'",
    "pg_catalog.to_jsonb(old)->>'kind'",
    "set pg_trgm.similarity_threshold = 0.3",
    "min_sim is null or min_sim < 0.3 or min_sim > 1",
]:
    print(f"NEEDLE: {needle}")
    idx = text.find(needle)
    print("FOUND:", idx)
PY

Repository: BigSimmo/Database

Length of output: 540


Align the migration-order assertion with the current file set. tests/supabase-schema.test.ts:1222 still expects 20260717161000_assert_supabase_admin_default_privileges.sql to be the last migration, but 20260717173000_reassert_supabase_admin_default_privileges.sql sorts after it. Update the expectation to the actual latest migration, or target the specific file this test is meant to cover.

🤖 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 131 - 132, Update the
migration-order assertion in tests/supabase-schema.test.ts to reflect the
current migration set: either expect
20260717173000_reassert_supabase_admin_default_privileges.sql as the latest
migration or revise the assertion to target the specific migration covered by
defaultAclAssertionMigration.

Source: Pipeline failures

Comment on lines +1372 to +1374
expect(cleanup).toContain("metadata->>'registry_record_id' = old.id::text");
expect(cleanup).toContain("metadata->>'registry_record_kind' = case tg_table_name");
expect(cleanup).toContain("when 'clinical_registry_records' then to_jsonb(old)->>'kind'");

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.

🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

Assertion doesn't account for pg_catalog. schema prefix.

The test expects when 'clinical_registry_records' then to_jsonb(old)->>'kind' but the actual migration SQL uses pg_catalog.to_jsonb(old)->>'kind'. The pg_catalog. prefix between then and to_jsonb breaks the substring match. This is a confirmed pipeline failure.

🐛 Proposed fix
-      expect(cleanup).toContain("when 'clinical_registry_records' then to_jsonb(old)->>'kind'");
+      expect(cleanup).toContain("when 'clinical_registry_records' then pg_catalog.to_jsonb(old)->>'kind'");
📝 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.

Suggested change
expect(cleanup).toContain("metadata->>'registry_record_id' = old.id::text");
expect(cleanup).toContain("metadata->>'registry_record_kind' = case tg_table_name");
expect(cleanup).toContain("when 'clinical_registry_records' then to_jsonb(old)->>'kind'");
expect(cleanup).toContain("metadata->>'registry_record_id' = old.id::text");
expect(cleanup).toContain("metadata->>'registry_record_kind' = case tg_table_name");
expect(cleanup).toContain("when 'clinical_registry_records' then pg_catalog.to_jsonb(old)->>'kind'");
🧰 Tools
🪛 GitHub Check: Unit coverage

[failure] 1374-1374: [node] tests/supabase-schema.test.ts > Supabase Preview replay guards > hardens registry cleanup without UUID casts or cross-registry collisions
AssertionError: expected 'create or replace function public.cle…' to contain 'when 'clinical_registry_records' th…'

Expected: "when 'clinical_registry_records' then to_jsonb(old)->>'kind'"
Received: "create or replace function public.cleanup_registry_corpus_document() returns trigger language plpgsql security definer set search_path = '' as $$ begin delete from public.documents where metadata->>'source_kind' = 'registry_record' and metadata->>'registry_record_kind' = case tg_table_name when 'clinical_registry_records' then pg_catalog.to_jsonb(old)->>'kind' when 'medication_records' then 'medication' when 'differential_records' then 'differential' else null end and metadata->>'registry_record_id' = old.id::text; return old; end; $$; "

❯ tests/supabase-schema.test.ts:1374:23

🤖 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 1372 - 1374, Update the cleanup
SQL assertion in the test around the registry-record CASE expression to match
the migration’s qualified function call, including the pg_catalog. prefix before
to_jsonb(old). Keep the existing assertions for registry_record_id and
registry_record_kind unchanged.

Source: Pipeline failures


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);

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.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Align these locators with the rendered h1.

src/components/ClinicalDashboard.tsx Line 3495 still renders the sole level-1 heading as “Clinical Guide”, so these updated locators time out. Revert them to that name, or rename the production h1 consistently.

  • tests/ui-accessibility.spec.ts#L51-L51: expect the actual heading name.
  • tests/ui-smoke.spec.ts#L876-L876: expect the actual heading name.
  • tests/ui-smoke.spec.ts#L1271-L1271: expect the actual heading name.
📍 Affects 2 files
  • tests/ui-accessibility.spec.ts#L51-L51 (this comment)
  • tests/ui-smoke.spec.ts#L876-L876
  • tests/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, Align the heading-name locators
with the rendered h1 from ClinicalDashboard: update
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 to expect “Clinical Guide”
instead of “Clinical KB”.

@github-actions

Copy link
Copy Markdown

CI triage

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

  • Unit coverageneeds investigation: inspect the failing step and uploaded diagnostics; rerun only after classifying the cause.
  • Production UIneeds 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 #3245 (success).

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

@BigSimmo BigSimmo closed this pull request by merging all changes into main in 5c3a0e5 Jul 18, 2026
@BigSimmo
BigSimmo deleted the codex/answer-progress-main-safe-20260718b branch July 18, 2026 10:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant