Guard schema snapshot against NULL-generation artifact purge regressions - #164
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c5914ec3e7
ℹ️ 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".
| expect(sql).toContain("from public.document_table_facts replacement"); | ||
| expect(sql).toContain("from public.document_embedding_fields replacement"); | ||
| expect(sql).toContain("from public.document_index_units replacement"); | ||
| expect(sql).toContain("from public.document_memory_cards replacement"); | ||
| expect(sql).toContain("from public.document_sections replacement"); |
There was a problem hiding this comment.
Assert the NULL-generation guard for every artifact table
The test claims to guard all artifact deletes, but after document_images these assertions only prove that a same-table replacement subquery exists somewhere in the SQL. A schema regression that changes, for example, document_memory_cards back to an unconditional nullif(metadata->>'index_generation_id', '') is null delete would still pass as long as the from public.document_memory_cards replacement text remains, so the destructive drift this test is meant to catch can slip through. Please assert the full is null and exists (...) guard per table, or isolate each delete block before checking it.
Useful? React with 👍 / 👎.
Three conflicts, resolved by evidence rather than by side-preference. scripts/ensure-codex-cloud-git-remote.mjs and tests/codex-cloud-setup.test.ts: took main's version. The branch carried its own fix for the same defect (5aa3d6c, judging origin safety from the configured remote URL) on top of main's #1497, so neither side was obviously newer. main's version introduces a configuredOriginUrl() helper, already covers the url.*.insteadOf case the branch's extra test was written for, and passes 12/12 locally — verified before choosing, since taking a side wholesale is only safe when the other side's content is genuinely subsumed. This also clears the two codex-cloud-setup failures this branch has carried all session. docs/outstanding-issues.md: fifth id collision on this branch. main has since claimed #151-#153 — the exact ids these rows previously held — and now runs to #164. main's rows are kept and the follow-ups renumber to #165 (adopt a consolidated answer-home notice block), #166 (answer mode ships no verify-before-use caveat) and #167 (verify:pr-local exits 0 when its build step refuses to run), with their three cross-references and the marker (168) following. check:outstanding-issues: 165 rows (50 open, 115 archived), unique ids, next-id=168 above the highest, no ids deleted from base 23465d5. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NPyFcMfn1jMmphr6AqiWBg
Summary
commit_document_index_generationalready had replacement guards in migration logic, but the risk was schema snapshot drift back to unconditional NULL-generation deletes, which would erase legacy artifacts (for exampledocument_images) during commits.Added regression coverage to lock in the guarded-delete contract across both live migration SQL and
supabase/schema.sql.Schema snapshot parity guard
20260702000000_commit_generation_preserve_legacy_artifacts.sqlintests/supabase-schema.test.ts.document_chunksdocument_imagesdocument_table_factsdocument_embedding_fieldsdocument_index_unitsdocument_memory_cardsdocument_sectionsExample assertion pattern
Verification
npm run verify:cheapnpm run verify:uiwhen UI, routing, styling, browser behavior, reduced-motion, or forced-colors behavior changednpm run verify:releasebefore release or handoff confidence claimsnpm run format:checknpm run check:production-readinesswhen clinical workflow, privacy, environment, Supabase, source governance, or deployment behavior changednpm run check:deployment-readinesswhen deployment startup, hosting, or rollout behavior changedClinical Governance Preflight
Complete this section when the change touches ingestion, answer generation, search/ranking, source rendering, document access, privacy, production env, or clinical output.
Clinical KB Database(sjrfecxgysukkwxsowpy)Notes