docs(drift): forward-codify runbook for live-ahead retrieval RPCs - #524
Conversation
Drift backlog item 0 (docs/database-drift-detection.md): prepare the non-live-gated half of forward-codifying the 7 retrieval RPCs whose live bodies are ahead of the repo, without touching the live project or hand-authoring the (two-way-diverged, actively churning) bodies. - docs/forward-codify-retrieval-rpcs-workorder.md: operator runbook — read-only capture, fill-in-the-blanks migration skeleton, schema.sql reconcile, byte-faithful drift:manifest (Docker) + check:drift validation, allowlist cleanup, rollback. - scripts/sql/capture-live-retrieval-rpcs.sql: READ-ONLY pg_get_functiondef capture for the target set (search_path=''). - tests/forward-codify-retrieval-targets.test.ts: offline guard keeping the capture query's targets in lockstep with the "LIVE IS AHEAD" allowlist entries. - docs/database-drift-detection.md: link the runbook from backlog item 0. No runtime, schema.sql, or migration changes; the live capture + apply remain an operator step gated on a quiescent DB. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This pull request has been ignored for the connected project Preview Branches by Supabase. |
📝 WalkthroughWalkthroughAdds a staged runbook for forward-codifying seven live-ahead retrieval RPCs, a deterministic SQL capture script, and Vitest coverage ensuring the capture targets remain synchronized with the drift allowlist. ChangesRetrieval RPC forward-codification
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error)
✅ Passed checks (10 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:414a4cc837
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@docs/forward-codify-retrieval-rpcs-workorder.md`:
- Line 164: Add language identifiers to the fenced code blocks in the
documentation: use sql for the block at line 164 and shell for the blocks at
lines 197, 207, 217, and 223, preserving their existing contents.
- Around line 249-257: Update the “Rollback / safety” guidance to require
preserving pre-apply function definitions as a known-good capture and generating
an explicit rollback migration from them. Do not recommend re-capturing live
after an incorrect apply as the rollback mechanism; retain the existing
migration-only and drift-check guidance.
- Around line 86-98: Update the diagnostic query’s unnest expression to assign
an explicit name to its output column, then reference that column in both the
SELECT projection and the to_regprocedure predicate. Keep the existing
function-signature array and missing-signature behavior unchanged.
In `@scripts/sql/capture-live-retrieval-rpcs.sql`:
- Around line 39-51: The capture query should tolerate missing functions instead
of failing on the cast to regprocedure. In the values subquery used by the
query, resolve each signature with to_regprocedure, filter out null results, and
pass the resolved OID to pg_get_functiondef while preserving the existing
signature output and fewer-than-seven-rows 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 60a2cfe5-4962-4868-9269-da581bda391b
📒 Files selected for processing (4)
docs/database-drift-detection.mddocs/forward-codify-retrieval-rpcs-workorder.mdscripts/sql/capture-live-retrieval-rpcs.sqltests/forward-codify-retrieval-targets.test.ts
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…-retrieval-rpcs-05a3dc # Conflicts: # docs/forward-codify-retrieval-rpcs-workorder.md
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:333498f76e
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
Caution Docstrings generation - FAILED No docstrings were generated. |
Address PR #524 review (CodeRabbit + Codex): the capture query cast each signature with ::regprocedure, which THROWS on a missing/renamed/reconciled target instead of returning fewer rows — contradicting the file's documented "expect 7, fewer means missing" contract. Resolve via to_regprocedure + filter nulls (validated against a supabase/postgres container: 0 rows, no error when the target functions are absent). Also clarify the query covers only the allowlist "LIVE IS AHEAD" retrieval subset, not the live-only/(verify) functions the runbook fingerprint table codifies. The remaining review comments anchored to the pre-merge 265-line work-order that merging origin/main replaced with the fingerprinted version (now 113 lines); those line references (164/213/257) no longer exist. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:e8ee696dd6
ℹ️ 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".
| `get_visual_evidence_cards`, `run_visual_eval_case`, `run_all_visual_eval_cases`, | ||
| `repair_enrichment_quality_batch`, `match_document_index_units_hybrid`, | ||
| `match_document_memory_cards_hybrid[_v2]` — are captured from that table per | ||
| Step 1, not from this query. The fingerprint table remains the point-in-time |
There was a problem hiding this comment.
Capture every live-only RPC before codifying
Fresh evidence beyond the prior capture-target comment: this newly added section now directs operators to take run_visual_eval_case, run_all_visual_eval_cases, and repair_enrichment_quality_batch from the fingerprint table, but that table only contains get_visual_evidence_cards among the live-only RPCs. When Step 2 asks them to codify and then remove allowlist entries for all four live-only functions, following this path leaves three functions without current definitions or ACL evidence, so the migration cannot be byte-faithful and drift can remain red or be miscodified; either add those signatures to a live capture artifact/table or stop listing them in this work-order.
Useful? React with 👍 / 👎.
Summary
Stages the non-live-gated half of drift backlog item 0 — forward-codifying the 7 retrieval RPCs whose live bodies are ahead of the repo (
docs/database-drift-detection.md#reconciliation-backlog) — without touching the live project or hand-authoring the bodies.Those bodies have diverged in both directions (e.g.
match_document_chunks_hybridinschema.sqlcarries the fail-closedretrieval_owner_matchespredicate while live is ahead on the left-joinedquality_score) and are actively churning under concurrent editing, so a hand-written body is exactly what got migration20260705210000neutralized. Only a verbatimpg_get_functiondefcapture from a quiescent live DB is correct.What's in this PR (all offline / inert)
docs/forward-codify-retrieval-rpcs-workorder.md— operator runbook: read-only capture → fill-in-the-blanks migration skeleton →schema.sqlreconcile → byte-faithfuldrift:manifest(Docker) +check:driftvalidation → allowlist cleanup → rollback.scripts/sql/capture-live-retrieval-rpcs.sql— READ-ONLYpg_get_functiondefcapture for the target set (search_path=''), reusing the exact expressionsschema_drift_snapshot()already runs in production.tests/forward-codify-retrieval-targets.test.ts— offline guard: the capture query's targets must equal the allowlist entries taggedLIVE IS AHEAD, so the artifact can't rot as siblings get reconciled.docs/database-drift-detection.md— link the runbook from backlog item 0.Scope / risk
No runtime,
schema.sql, or migration changes. The capture query is not wired into the app; the test is offline. Clinical governance preflight: N/A — this PR does not change ingestion, answer generation, search/ranking behavior, source rendering, document access, privacy, or production config. The live capture + apply remain an operator step gated on a quiescent DB + explicit approval.Verification
tests/forward-codify-retrieval-targets.test.ts+tests/drift-detection.test.tsgreen (13/13).--max-warnings 0clean on the new test.🤖 Generated with Claude Code