Skip to content

perf(db): drop redundant clinical_registry_record_sources_record_idx#491

Merged
BigSimmo merged 2 commits into
mainfrom
claude/supabase-postgres-practices-3eeeb0
Jul 11, 2026
Merged

perf(db): drop redundant clinical_registry_record_sources_record_idx#491
BigSimmo merged 2 commits into
mainfrom
claude/supabase-postgres-practices-3eeeb0

Conversation

@BigSimmo

Copy link
Copy Markdown
Owner

Summary

Drops the redundant clinical_registry_record_sources_record_idx on (record_id). It is a strict prefix of the index backing the UNIQUE (record_id, document_id) constraint on the same table, so Postgres already serves every record_id equality lookup from the unique index — the standalone index only adds write overhead. Same rationale as 20260702110000_drop_redundant_indexes.sql.

Surfaced by the supabase-postgres-best-practices review (query-covering / schema-foreign-key-index rules).

Changes

  • supabase/migrations/20260711000000_drop_redundant_registry_sources_record_index.sqldrop index if exists (with the manual CONCURRENTLY alternative documented, since Supabase migrations run in a transaction).
  • supabase/schema.sql — index removed; comment notes the unique index now serves record_id lookups.
  • supabase/drift-manifest.json — regenerated from a scratch schema.sql replay.

clinical_registry_record_sources_document_idx is kept — document_id is not a leading column of any other index and backs the documents ON DELETE CASCADE.

Verification

  • tests/supabase-schema.test.ts — 43/43 passing against the merged tree.
  • Branch merged up to current main (conflict-free; disjoint file changes) before this PR.

Not in this PR

  • Live apply. The migration is not yet applied to the live Clinical KB Database project — that goes through the approved live migration workflow. check:drift will report the index as live-only until then (expected pending-migration window).

🤖 Generated with Claude Code

BigSimmo and others added 2 commits July 11, 2026 10:45
The standalone (record_id) index is a strict prefix of the index backing
the UNIQUE (record_id, document_id) constraint, so it adds write overhead
with no read benefit (same rationale as 20260702110000). Schema.sql is
reconciled and drift-manifest.json regenerated from a scratch replay;
tests/supabase-schema.test.ts 43/43 green. check:drift will transiently
report the index as live-only until the migration is applied.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@supabase

supabase Bot commented Jul 11, 2026

Copy link
Copy Markdown

Updates to Preview Branch (claude/supabase-postgres-practices-3eeeb0) ↗︎

Deployments Status Updated
Database Sat, 11 Jul 2026 13:37:06 UTC
Services Sat, 11 Jul 2026 13:37:06 UTC
APIs Sat, 11 Jul 2026 13:37:06 UTC

Tasks are run on every commit but only new migration files are pushed.
Close and reopen this PR if you want to apply changes from existing seed or migration files.

Tasks Status Updated
Configurations Sat, 11 Jul 2026 13:37:14 UTC
Migrations Sat, 11 Jul 2026 13:38:09 UTC
Seeding Sat, 11 Jul 2026 13:38:12 UTC
Edge Functions Sat, 11 Jul 2026 13:38:13 UTC

View logs for this Workflow Run ↗︎.
Learn more about Supabase for Git ↗︎.

@coderabbitai

coderabbitai Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The PR removes a redundant record_id index from the registry sources table, adds a migration to drop it, documents reliance on the existing composite unique index, and refreshes drift-manifest metadata.

Changes

Registry source index cleanup

Layer / File(s) Summary
Remove redundant record_id index
supabase/migrations/..., supabase/schema.sql, supabase/drift-manifest.json
Adds a migration to drop the standalone index, removes its schema declaration, and updates drift metadata after the schema change.

Estimated code review effort: 2 (Simple) | ~10 minutes


Caution

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

  • Ignore

❌ Failed checks (1 error, 1 warning)

Check name Status Explanation Resolution
Verification Claims ❌ Error PR description claims “tests pass 43/43” without naming the exact command/result, which this check requires. Rewrite the verification note with the exact command and result, e.g. Ran npm run test -- tests/supabase-schema.test.ts: passed (43/43) or Not run: reason.
Description check ⚠️ Warning The description matches the main sections but omits the required Clinical Governance Preflight section for a Supabase change. Add the Clinical Governance Preflight section and complete the Supabase, service-role, source-governance, and deployment checks required by the template.
✅ Passed checks (9 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately summarizes the main change: dropping the redundant clinical_registry_record_sources_record_idx.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Generated And Sensitive Files ✅ Passed Only schema, migration, and justified generated drift manifest changed; no secrets, credentials, dumps, or forbidden artifact files were introduced.
Risky Git Or Deployment Actions ✅ Passed The PR files only drop a redundant index and update metadata; no force-push/reset/branch-delete/deploy-without-confirmation instructions were added.
Supabase Project And Schema Safety ✅ Passed Only a redundant index drop; the migration explains the non-concurrent choice and manual rollback path, schema keeps required indexes, and no touched file retargets Supabase.
Runtime And Package Manager Integrity ✅ Passed PASS: packageManager/engines stayed npm@11.17.0 + Node 24/npm 11, .npmrc still engine-strict=true, and no legacy/force flags or lockfile bypass appeared.
Api Route Failure Handling ✅ Passed Changed routes validate input and catch errors; search/RAG/provider paths degrade or return controlled errors on auth, timeout, quota, and empty-result cases.
✨ 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 claude/supabase-postgres-practices-3eeeb0
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch claude/supabase-postgres-practices-3eeeb0

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.

🧹 Nitpick comments (1)
supabase/migrations/20260711000000_drop_redundant_registry_sources_record_index.sql (1)

1-19: 🧹 Nitpick | 🔵 Trivial

Sound cleanup; lock-mode tradeoff already documented.

The rationale for the redundant index removal is well-reasoned, and the plain DROP INDEX vs. CONCURRENTLY tradeoff is already called out with a manual alternative for zero-impact removal. No further code change needed here.

Since this migration touches supabase/**, confirm the clinical governance preflight and production-readiness checks are completed before merge/apply.

As per coding guidelines, "Pull requests touching ingestion, answer generation, search/ranking, source rendering, document access, privacy, production environment, or clinical output must complete the clinical governance preflight and relevant production-readiness checks."

🤖 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/20260711000000_drop_redundant_registry_sources_record_index.sql`
around lines 1 - 19, Confirm that the clinical governance preflight and
applicable production-readiness checks have been completed for this Supabase
migration before merging or applying it; no code changes are needed to the
migration itself.

Source: Coding guidelines

🤖 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.

Nitpick comments:
In
`@supabase/migrations/20260711000000_drop_redundant_registry_sources_record_index.sql`:
- Around line 1-19: Confirm that the clinical governance preflight and
applicable production-readiness checks have been completed for this Supabase
migration before merging or applying it; no code changes are needed to the
migration itself.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 2c66c0b6-90ce-4274-a600-77ab23557686

📥 Commits

Reviewing files that changed from the base of the PR and between 7dcea93 and 6d26f87.

📒 Files selected for processing (3)
  • supabase/drift-manifest.json
  • supabase/migrations/20260711000000_drop_redundant_registry_sources_record_index.sql
  • supabase/schema.sql

@BigSimmo
BigSimmo merged commit 70ec640 into main Jul 11, 2026
15 checks passed
@BigSimmo
BigSimmo deleted the claude/supabase-postgres-practices-3eeeb0 branch July 11, 2026 13:55
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