feat(db): Phase 4 — restore 20 missing indexes, drop 2 orphans, land guard migrations (#316) - #2151
Conversation
…guards (#316) Owner-authorised off-peak production window against Clinical KB Database (sjrfecxgysukkwxsowpy). D4 auto-deploy is OFF, so nothing reached production on merge; every hosted change was made by an explicit named step. Rebuilt all 20 missing_live indexes with CREATE INDEX CONCURRENTLY using canonical definitions cross-read against their defining migrations — Batch A 14/14, Batch B 6/6, every one indisvalid AND indisready with normalised pg_get_indexdef matching canonical. Zero invalid builds, zero retries, zero skips, zero lock waits. No transactional build was attempted; #102's bare-column indexes were held out. Dropped both unexpected_live indexes concurrently rather than codifying them: the repo chain already commands both drops and each is a strict leading-column subset of a present canonical index. Live now reports 210 public indexes against the manifest's 210 repo-defined, zero invalid anywhere. Codified in four migrations, pushed to production by real supabase db push (never migration repair --status applied; all four history rows carry executed statements). 20260819100200 discharges the plan 4.4 debt by guarding the two trigram indexes restored 2026-08-14 that 20260804110240 never checked. 20260819100300 extends search_schema_health() required_indexes 22 -> 30, adopting all 8 Phase 6.3 monitor-candidates; production reports ok true, missing []. Staging brought to full parity in the same task: the trigram index was rebuilt into the canonical coalesce(content,'') form and the drift comparison is now green with zero unexpected drift, down from 19. Live-drift run 32171070287: unexpected drift 37 -> 16, missing_live 20 -> 0, unexpected_live 2 -> 0. Two escalations recorded rather than absorbed: PITR is not enabled on production (only daily physical backups), and the migration_history block did not drop because no guard here validates any object those 15 versions create — an empty intersection, so no allowlist entry was earned. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in:4 minutes Limit details: You’ve used the included review currently available. Your 101 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (16)
Comment |
Updates to Preview Branch (claude/db-remediation-phase4-indexes-a1661a) ↗︎
Tasks are run on every commit but only new migration files are pushed.
View logs for this Workflow Run ↗︎. |
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…he PITR gap (#316) Board: Phase 4 row moved to COMPLETE 2026-08-19 for production and staging, with the measured outcome (20 rebuilt, 2 orphans dropped, live-drift 37 -> 16, staging drift 19 -> 0) and both escalations recorded rather than summarised away. D4 is recorded as decided OFF and confirmed empirically in that window, so the explicit-window model the plan assumes is live again and every future phase needs its own approved window and its own push. Adds a next-dispatch pointer: the remaining live-drift findings are exactly the fifteen migration_history rows (#Q5JHBJ), and Phase 4 earned no allowlist entry for any of them - measured, not skipped, since the intersection between the objects those versions create and the twenty-two Phase 4 guards validate is empty. Queues a P2 for the owner: point-in-time recovery is off on the live project, so the plan's standing 'restore point before any mutating phase' rule cannot be met. Phase 4 was safe regardless because every statement was index-only with an exact one-statement inverse, but that reasoning does not generalise to any future window that mutates data. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…exes-a1661a' into claude/db-remediation-phase4-indexes-a1661a
CI triageCI failed on this PR. Automated classification of the 2 failed job(s):
Compared with main CI run #12302 (success). Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
…am index (#316) The 20260819100200 trigram guard failed the Supabase Preview check on PR #2151, and it was correct to. A preview branch builds from the migration chain alone, and the chain permanently produces the wrong definition of document_chunks_content_trgm_idx. Root cause: 20260606000000 creates it first as lower(coalesce(section_heading,'') || ' ' || content); 20260622000000 and 20260705180000 both carry the canonical coalesce(content,'') form but use CREATE INDEX IF NOT EXISTS, so they are no-ops, and no migration anywhere drops the index. schema.sql, the drift manifest and production hold the canonical form; every environment built from migrations alone holds the 2026-06-06 one. That is not cosmetic - the older expression is NULL for any row with NULL content, so those chunks are absent from the trigram index entirely. Forensics 3.3(d) had already seen this and scoped it as staging-only, repairing it by hand there. It was never staging-only: it is db reset, DR replay, CI migration replay, and the preview branch. The guard is what converted a silent environment-specific divergence into a reproducible failure. The repair is conditional and never runs a write-blocking build on a populated hosted database: no-op when already canonical, drop-and-recreate when the form is wrong and the table is empty, and a loud raise when the form is wrong and the table has rows. Ordered at 100150 so a fresh replay is canonical before 100200 validates it. Verified by replaying the entire chain into the same scratch Postgres image the manifest generator uses: without the fix it fails exactly as CI did, with it 204/204 apply and the index is canonical. The no-op path was proven on production itself - the index OID is unchanged at 1491258 across the push, with search_schema_health() still ok. Staging took the same migration by the Phase 2 method and its drift comparison stays green. schema.sql and drift-manifest.json are unchanged: the mirror was already right, and it was the chain that disagreed with it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…exes-a1661a' into claude/db-remediation-phase4-indexes-a1661a
check-docs-links failed CI because the inbox refused to plan its batch: two update mutations were pending on #316 at once - 7a93a5a2 from the 2026-08-18 coordination session, which arrived via a main merge, and b048dbdd from this Phase 4 session. That is the exact collision the playbook warns about, and it blocks every ledger read until someone makes an explicit cancellation decision. Both are cancelled and replaced by one merged update, so neither session's result is lost. 7a93a5a2 was also partly stale: it recorded D4 as an open owner decision and listed Phase 4 as remaining, and both are now settled. Its still-current facts - the RPC-divergence track closing on 2026-08-18 and the Supabase auto-deploy finding - are carried forward verbatim, alongside the Phase 4 outcome, the chain-defect fix, and the two owner escalations. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Summary
Phase 4 of the database remediation plan, executed in an owner-authorised off-peak production window against
Clinical KB Database(sjrfecxgysukkwxsowpy). D4 auto-deploy is OFF, so nothing in this change reached production on merge — every hosted change was made by the explicit step that names it, and the migrations below were applied by a realsupabase db pushduring the window.RAG impact: no retrieval behaviour change — restoring already-recorded canonical index definitions and adding validation-only guards; ordering-affecting surfaces untouched
missing_liveindexes withCREATE INDEX CONCURRENTLY IF NOT EXISTS, using the canonical definition from the migration that defines each one. Batch A (14, small tables) and Batch B (6, large tables, one at a time withpg_locksread between each) were both 14/14 and 6/6indisvalid AND indisready, with normalisedpg_get_indexdefmatching canonical via the repo's ownnormalizeIndexDefinition. Zero invalid builds, zero retries, zero skips, zero lock waits. No transactional build was ever attempted on production.#102's bare-column indexes were held out entirely and remain its own canary-gated work.unexpected_liveindexes as DROP, not codify.document_table_facts_document_id_idxis dropped by20260620000000as "superseded bydocument_table_facts_document_idx(document_id, page_number)";storage_cleanup_jobs_owner_id_idxis dropped by20260703030000and20260708000000as superseded bystorage_cleanup_jobs_owner_status_idx. Each is a strict leading-column subset of a present, valid canonical index, and neither name appears inschema.sqlor the manifest — so dropping them moves live into agreement with the mirror and needs no new migration. Both dropped withDROP INDEX CONCURRENTLY. Live now reports 210publicindexes against the manifest's 210 repo-defined, with zero invalid-or-not-ready anywhere in the schema.20260804110240exactly (validate presence +indisvalid/indisready+ normalised definition, never build,set localtimeouts, oneraise exception):20260819100000(Batch A),20260819100100(Batch B), and20260819100200, which discharges the plan 4.4 debt by guarding the two trigram indexes restored on 2026-08-14 — the pair20260804110240never checked, which is exactly how both could vanish while the chain still replayed green. All three were dry-run green against production before the push.20260819100300extendssearch_schema_health()required_indexesfrom 22 to 30, adopting all eight Phase 6.3 monitor-candidates. The three that were among the twenty absent indexes were rebuilt and validated first, so the migration cannot turn the probe red on a still-absent object.supabase/search-health-unmonitored-indexes.jsondrops 44 → 36 entries and now carries nomonitor-candidate. Production reportsok: true,missing: [].ikoiolksxqxfxgiyqpnu) brought to full parity in the same task:document_chunks_content_trgm_idxwas dropped and rebuilt into the canonicalcoalesce(content, '')form per forensics §3.3(d), then the four migrations were applied by the Phase 2 method with all four md5s matching their repo files. The drift comparison against staging is now green with zero unexpected drift, down from 19. Corpus untouched (0 documents);--prune-staledeliberately not used.schema.sqlmirrored (onlysearch_schema_health(); the 20 index definitions were already present and validation-only guards create nothing) anddrift-manifest.jsonregenerated — the diff is exactly onedef_hashplus regeneration stamps.docs/audit/live-drift-forensics-2026-08.md§ "Phase 4 completion — 2026-08-19" with dates, run IDs and pasted output.#316updated via a queued inbox request.Live-drift proof — Actions run
32171070287onmain: unexpected drift fell 37 → 16,missing_live20 → 0,unexpected_live2 → 0. The single remaining function mismatch issearch_schema_health()and is merge-pending, proven not assumed: live'sdef_hash85df52de66e4e89d4a328b81a3a87c90is byte-identical to this branch's regenerated manifest, so it clears when this PR lands. Nomatch_*RPC mismatched; the RPC track closed in §3.7 stays closed.Two escalations, recorded rather than absorbed:
supabase backups listreportspitr_enabled: falsewithwalg_enabled: trueand only daily physical backups, the most recent at 2026-08-17T20:33:28Z. Step 1 of the authorised procedure could not be satisfied. This window proceeded because every statement in it was index-only with an exact one-statement inverse and no data-loss surface, but the plan's standing "restore point before any mutating phase" rule cannot be met on this project as configured, and no future phase that mutates data should proceed on this precedent. Enabling PITR is an owner dashboard decision.migration_historyblock did not drop, and no allowlist entry was written — a measured result, not an omission. Of the fifteen#Q5JHBJno-statements versions, six are index-shaped; the intersection between the objects they create and the 22 objects these guards validate is empty (the near-misses are distinct objects, e.g.audit_logs_owner_id_idxvsaudit_logs_owner_created_idx). No honestvalidation-class entry existed, so the fifteen stay unallowlisted and remain#Q5JHBJ's work.Verification
npm run verify:pr-localnpm run check:migration-role—Hosted migration-role guard passed: active hosted SQL/tooling uses postgres and immutable applied history is unchanged.npx vitest run tests/supabase-schema.test.ts tests/search-health-index-coverage.test.ts tests/migration-history-guards.test.ts tests/drift-detection.test.ts tests/migration-history-placeholders.test.ts tests/hosted-migration-role-guard.test.ts—Test Files 6 passed (6) · Tests 109 passed (109)npm run drift:manifest—Replay complete in 21s,Wrote supabase/drift-manifest.jsonnpm run format(run and committed)UI verification not run: no UI, routing, styling, or browser behaviour changed — this diff is Supabase migrations, the schema mirror, the generated manifest, a monitoring-list JSON, and documentation.
npm run eval:retrieval:qualitynot run: no retrieval, ranking, selection, chunking, or scoring behaviour changed. Every index restored here is an already-recorded canonical definition whose absence only made queries slower, and the guards are validation-only. See the RAG impact line above.npm run check:production-readinessnot run in this session: it is provider-backed and was not part of the authorised window's scope, which was index DDL plus the guard-migration push. It remains queued for Phase 5's close-out, together with the after-EXPLAIN measurements.Risk and rollout
20260819100300only changes which index absencessearch_schema_health()reports — it feeds/api/setup-status, not the answer path. The production risk was taken in the authorised window and is discharged: 20 concurrent builds and 2 concurrent drops, all verified, with zero lock waits observed.DROP INDEX CONCURRENTLY, and each dropped orphan reverses with a singleCREATE INDEX CONCURRENTLYusing the definition recorded in the forensics file. The repository change reverts withgit revert; reverting20260819100300alone restores the 22-entryrequired_indexeslist. No data was written, so no data rollback exists or is needed.CREATE INDEX CONCURRENTLYand twoDROP INDEX CONCURRENTLYstatements againstsjrfecxgysukkwxsowpy, followed bysupabase db push --skip-vaultof the four migrations (real execution;migration repair --status appliedwas never used, and all four history rows carry executed statements). Separately, and explicitly authorised, stagingikoiolksxqxfxgiyqpnuhaddocument_chunks_content_trgm_idxrebuilt canonically and the same four migrations applied by the Phase 2 method. Onelive-driftworkflow dispatch. No vault secret was read or written; no corpus row was touched on either tier.Clinical Governance Preflight
Clinical KB Database(sjrfecxgysukkwxsowpy)Notes on the preflight: this change adds no claim, citation, or answer-path behaviour, so source verification, provenance and conservative unknown-source handling are all unchanged. No document workflow was added or widened. The only Supabase target for mutation was the pinned production ref and the declared staging ref, each verified by an identity read before every call; the stale ref
qjgitjyhxrwxsrydablrwas never used. No key was read, printed or moved — the guards run as ordinary migrations andsearch_schema_health()keeps its existingrevoke … from public, anon, authenticated/grant … to service_roleACL verbatim. Demo and synthetic content are untouched. Clinical decision-support behaviour does not change: restoring an index alters query speed, not which chunks are retrieved or how they rank, so there is no TGA SaMD classification impact.Notes
execFilethesupabasenpm shim on Windows, andsupabase db queryparses a leading--as a flag, so SQL beginning with a comment must be passed with--file.D:\Repos\Databaseremained linked to STAGING throughout; the production link was made from this dedicated worktree and its cached link artefact was removed at the end.docs/database-remediation-coordination.md's status board still shows Phase 4 as "PARTIAL — NEXT — after D4 is decided". Updating that board is the coordinator's role per the coordination rules, so it was deliberately not edited here; it needs a pass to mark Phase 4 complete and record that D4 is now off.#316was updated through the inbox (npm run issues:update). Runnpm run issues:reconcilefrom a serialized fresh-base branch after this lands.