Scenario note: the PR, row counts, and query plans below are illustrative. Nothing here is reported as executed, measured, or merged.
The setup
An agent opens a pull request titled "soft delete for users". It touches three files. One migration, one repository method, one test file. The diff reads cleanly.
That readability is the risk. Clean diffs hide schema decisions. Those decisions outlive the review window.
Step 1: Read the migration before the code
Skip the application layer on the first pass. Read the SQL first. The migration sets the constraints every later line depends on.
git fetch origin pull/812/head:review/soft-delete
git diff --stat main...review/soft-delete
git diff main...review/soft-delete -- migrations/
The illustrative migration looks like this.
-- illustrative only
ALTER TABLE users ADD COLUMN deleted_at TIMESTAMPTZ NULL;
CREATE INDEX users_deleted_at_idx ON users (deleted_at);
UPDATE users SET deleted_at = NULL WHERE deleted_at IS NULL;
The column is fine. The index is fine. The third line is a full table scan that changes zero rows. The agent added it for symmetry.
Step 2: Triage every hunk into three buckets
A working review separates trust from suspicion. This table is the artifact to keep.
| Bucket | Criteria | Example hunk |
|---|---|---|
| Trust, skim | Pure addition, no shared state | ADD COLUMN deleted_at |
| Test before merge | Changes a constraint, backfills data, or alters a query path | Unique index, UPDATE, WHERE deleted_at IS NULL
|
| Revert on sight | Touches secrets, drops a constraint unasked, or rewrites history | Dropped email uniqueness, DROP INDEX on a live path |
Most agent PRs land in the middle bucket. The reviewer's job is to widen that bucket, not to empty it.
Step 3: The constraint bugs the diff does not show
Soft delete breaks uniqueness. The old rule was one row per email. The new rule is one live row per email.
The agent does not change the constraint. So a deleted account keeps its email reserved forever. Re-registration fails with a duplicate-key error. Nobody notices in tests, because tests start empty.
The correct shape is a partial unique index, a reversible one.
-- illustrative only
CREATE UNIQUE INDEX CONCURRENTLY users_email_live_key
ON users (email)
WHERE deleted_at IS NULL;
ALTER TABLE users DROP CONSTRAINT users_email_key;
Note the order. Build the new index before dropping the old one. Note CONCURRENTLY too. It cannot run inside a transaction block, which changes how the migration runner must invoke it.
Confirm the predicate matches the application query. A partial index on deleted_at IS NULL is ignored by a query written as WHERE deleted_at = 'infinity'. Verify with the plan.
-- illustrative only
EXPLAIN (COSTS OFF)
SELECT id FROM users WHERE email = 'a@example.com' AND deleted_at IS NULL;
If the plan does not name users_email_live_key, the predicate is wrong. Fix the query, not the index.
Step 4: Reject one-shot backfills on large tables
A single UPDATE over a large table takes one long lock and writes one long transaction. Bloat follows. Replication lag follows.
Batching is the standard fix. The loop below is illustrative and unexecuted.
-- illustrative only: run repeatedly until zero rows
UPDATE users
SET deleted_at = NULL
WHERE id IN (
SELECT id FROM users WHERE deleted_at IS NULL LIMIT 5000
);
That specific statement is a no-op on a new nullable column. It becomes meaningful when the PR adds a populated column or a NOT NULL default. Test whichever one is actually in the diff. Do not approve a backfill because the statement parses.
Two checks belong in the PR description. The first is the lock level. The second is the rollback path. A migration the agent cannot reverse is a migration the agent did not finish.
Step 5: Name the test that proves nothing
The illustrative test asserts a count.
# illustrative only
def test_soft_delete_hides_user(db):
create_user(db, email="a@example.com")
soft_delete(db, email="a@example.com")
assert count_live_users(db) == 0
This passes on any implementation that writes any value to the column. It never checks re-registration. It never checks the restore path. Demand three tests instead.
The first: delete, then re-register the same email, expect success. The second: restore the deleted row, expect the old email to conflict. The third: run both against a database seeded with a pre-existing deleted row.
Step 6: Where a second opinion fits
The reviewer is one reader with one bias. A fresh pass over the raw diff sometimes surfaces a constraint case the first reader missed. That is a narrow, useful job for a hosted model.
This is where MonkeyCode's free model access is relevant here. The operator describes it as free model access plus a free server option. Disclosure: This article was prepared as part of MonkeyCode's product outreach. Verify current limits and capabilities in the project's own documentation before relying on them; this article does not restate quotas or specifications.
Keep the prompt narrow and paste no production data.
Input: this migration and this table DDL.
Task: list write paths that break if deleted_at is set.
Output: three failure cases, one sentence each.
Do not rewrite the migration.
The value is the list, not the patch. Treat every suggested fix as an unverified claim. A model that proposes a new index has not measured that index.
The free server option fits a different step. It gives a rehearsal box for running the migration against a disposable copy. Seeded fixture data is fine there. Real customer records are not. Check the current terms before uploading anything.
The revert rule
Revert when the PR removes a safety property nobody asked to remove. Revert when it drops a constraint in the same commit that adds its replacement. Revert when the rollback is described as "TBD".
Do not revert for style. Do not rewrite the agent's diff to feel productive. A small, correct PR is cheaper to merge than a large, correct one.
Limitations and who should skip this
This workflow assumes a reversible schema and a staging database. It assumes the reviewer can read SQL. It does not scale to teams without a migration runner that supports concurrent index creation.
Skip the free-server rehearsal when the data is regulated. Skip it when lock behavior on production hardware is the question. Skip it when the change requires an SLA the operator does not offer.
Close
The checklist outlives the model. Keep the three buckets, the order rule, and the re-registration test. Swap the assistant whenever a better one appears. If you try the second-opinion pass, keep the diff narrow and the claims unverified until a plan or a test proves them.
Top comments (0)