feat(db): auto-apply tracked script data migrations in db:migrate#5497
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryMedium Risk Overview After Drizzle SQL migrations succeed (still under the migration advisory lock), The
Reviewed by Cursor Bugbot for commit 4983f86. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
@greptile review |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 3a828bd. Configure here.
Greptile SummaryThis PR adds a run-once TypeScript script migration framework to the existing
Confidence Score: 5/5Safe to merge — the migration framework is additive and well-guarded; the backfill is idempotent and isolated per table; the only finding is a detached JSDoc block from the assertLockSessionHeld refactor. The core machinery (advisory-lock guard, timeout resets, ON CONFLICT DO NOTHING journal insert, per-table error isolation) is all correct and addresses issues raised in prior review rounds. The backfill logic is idempotent: re-runs skip fully keyed tables, the unnest-based UPDATE avoids parameter limits, and per-table failures let the migration stay pending and retry only unfinished tables. The only finding is a cosmetic orphaned comment left behind when assertLockSessionHeld was extracted from runMigrationsWithRetry. packages/db/scripts/migrate.ts has a minor doc inconsistency from the refactor; all other files are clean. Important Files Changed
Reviews (4): Last reviewed commit: "improvement(db): re-verify advisory-lock..." | Re-trigger Greptile |
|
@greptile review |
Greptile SummaryThis PR introduces a run-once TypeScript script-migration mechanism layered on top of the existing
Confidence Score: 4/5The migration framework and backfill implementation are well-structured — advisory locking, per-table isolation, idempotency, and retry semantics are all correct. Two small gaps in The core design is sound and end-to-end tested. The packages/db/scripts/migrate.ts — the two gaps described above both live here. All other files look correct. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Runner as migrate.ts
participant PG as PostgreSQL
participant SM as script_migrations table
Runner->>PG: connectWithRetry()
Runner->>PG: pg_try_advisory_lock(MIGRATION_LOCK_KEY) [session-level]
Runner->>PG: "SET statement_timeout=0, lock_timeout='5s'"
Runner->>PG: drizzle migrate() — SQL migrations
PG-->>Runner: SQL migrations applied
Note over Runner: runScriptMigrations(client)
Runner->>PG: CREATE TABLE IF NOT EXISTS script_migrations
Runner->>SM: SELECT name FROM script_migrations
SM-->>Runner: applied set
loop For each pending script migration
Runner->>Runner: check requiredEnv
Runner->>PG: BEGIN (sql.begin)
Runner->>PG: pg_advisory_xact_lock(table-key) [tx-level]
Runner->>PG: "SELECT id FROM user_table_rows WHERE table_id=X ORDER BY position"
Runner->>Runner: generateNKeysBetween(null, null, n)
loop chunks of 5000
Runner->>PG: UPDATE user_table_rows SET order_key FROM unnest(ids, keys)
end
Runner->>PG: COMMIT
Runner->>SM: INSERT INTO script_migrations (name)
end
Runner->>PG: pg_advisory_unlock(MIGRATION_LOCK_KEY)
Runner->>PG: warnOnInvalidIndexes()
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant Runner as migrate.ts
participant PG as PostgreSQL
participant SM as script_migrations table
Runner->>PG: connectWithRetry()
Runner->>PG: pg_try_advisory_lock(MIGRATION_LOCK_KEY) [session-level]
Runner->>PG: "SET statement_timeout=0, lock_timeout='5s'"
Runner->>PG: drizzle migrate() — SQL migrations
PG-->>Runner: SQL migrations applied
Note over Runner: runScriptMigrations(client)
Runner->>PG: CREATE TABLE IF NOT EXISTS script_migrations
Runner->>SM: SELECT name FROM script_migrations
SM-->>Runner: applied set
loop For each pending script migration
Runner->>Runner: check requiredEnv
Runner->>PG: BEGIN (sql.begin)
Runner->>PG: pg_advisory_xact_lock(table-key) [tx-level]
Runner->>PG: "SELECT id FROM user_table_rows WHERE table_id=X ORDER BY position"
Runner->>Runner: generateNKeysBetween(null, null, n)
loop chunks of 5000
Runner->>PG: UPDATE user_table_rows SET order_key FROM unnest(ids, keys)
end
Runner->>PG: COMMIT
Runner->>SM: INSERT INTO script_migrations (name)
end
Runner->>PG: pg_advisory_unlock(MIGRATION_LOCK_KEY)
Runner->>PG: warnOnInvalidIndexes()
Reviews (2): Last reviewed commit: "fix(db): reset session lock_timeout befo..." | Re-trigger Greptile |
|
@greptile review |

Summary
db:migrateentrypoint, so compose/helm pick it up with zero config changespackages/db/script-migrations/:ScriptMigrationinterface, explicit registry, and a runner invoked frommigrate.tsafter SQL migrations succeed, inside the same advisory-lock session. Applied scripts are recorded by name in a runner-managedscript_migrationstable (mirrors how drizzle manages__drizzle_migrations— deliberately not inschema.ts)order_keyfractional-indexing backfill as entry0001(oldapps/sim/scripts/backfill-table-order-keys.tsdeleted): same per-table advisory lock + deterministic re-key fromposition, chunked writes now via 2-paramunnestinstead of VALUES lists. Per-table failures are isolated, then the migration throws so it stays pending and retries only still-unkeyed tablesfractional-indexing.tsinto@sim/utils/fractional-indexingso it's available inside the slim migrations image;lib/table/order-key.tsstays as the app-side wrapperorder_keyunconditionally, so the pending set is empty), records the journal row, and skips thereafterType of Change
Testing
Verified end-to-end against a scratch pgvector container: all 255 SQL migrations + script migration apply on a fresh DB; seeded 12k rows (reverse-inserted, crossing the chunk boundary) with NULL
order_key, re-randb:migrate— 0 NULLs left, key order matches position order exactly, already-keyed tables untouched, third run no-ops.lint:check,check:api-validation:strict, type-check (utils/db/sim), and both package test suites passChecklist