improvement(db): stop declaring retired usage columns ahead of their drop - #7134
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…drop Contract-prep for the legacy usage-column drop. Removes the drizzle declarations for 19 retired user_stats counters, organization.departed_member_usage, and workflow_execution_logs.cost, so generated SELECT lists stop referencing them, plus the last incidental references: the cycle-close departed_member_usage resets, the v1 admin exposure of departedMemberUsage/currentPeriodCopilotCost (both serving dead values since the ledger cutover), the testing schema mirror, and a lock-order fixture. The DROP COLUMN migration ships separately once this is the deployed production app; contract-pending markers in schema.ts track it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
8263b71 to
3494d73
Compare
Greptile SummaryThis PR prepares a deploy-safe contract migration by replacing broad Drizzle reads with explicit live-column maps and removing legacy usage fields from admin contracts.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| packages/db/schema.ts | Marks retired columns for a later contract migration and exports live-column maps that omit them. |
| scripts/check-pending-drop-tables.ts | Adds a static audit preventing broad reads from tables containing pending-drop columns. |
| packages/db/script-migrations/0009_backfill_wel_residual_cost_total.ts | Adds a bounded, resumable data migration projecting residual legacy cost JSON into live columns. |
| apps/sim/lib/logs/execution/logger.ts | Replaces broad execution-log selections and returning clauses with the live-column map. |
| apps/sim/app/api/v1/admin/types.ts | Removes retired organization and user billing fields from admin API response types. |
Reviews (9): Last reviewed commit: "fix(ci): match relational reads by table..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…arity CI requires drizzle-kit generate to be a no-op on every PR, so the declaration removal cannot ship without its migration. Committing the generated 0309 with every DROP commented out (a SELECT 1 no-op) keeps the parity gate and future generates clean while deferring the destructive half until this change is part of an already-deployed production release; the file documents the follow-up procedure. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Restructures the contract prep per review: the no-op 0309 migration is gone and schema.ts keeps the deprecated declarations (with the contract-pending markers on the columns), so the follow-up PR ships the declaration removal and the real generated DROP migration together. What makes that safe is that no deployed query names the doomed columns anymore: every argless select()/returning()/relational read of user_stats, organization, and workflow_execution_logs (29 sites) now goes through omit(getTableColumns(...)) live-column maps, following the workspaceFileColumns precedent. scripts/check-pending-drop-tables.ts enforces this — it derives the guarded tables from the contract-pending markers themselves (picking up workspace_files' pending size drop too) and fails CI on any argless read, so the invariant holds between the two PRs and retires itself when the markers are deleted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Restructured per discussion: the no-op 0309 migration is gone. schema.ts keeps the deprecated declarations (with contract-pending markers), all 29 argless reads of the affected tables now go through omit(getTableColumns(...)) live-column maps (workspaceFileColumns precedent), and a new check:pending-drop-tables audit — derived from the markers themselves — fails CI on any argless read so the invariant holds until the follow-up PR ships the declaration removal + real DROP migration together. |
|
@cubic-dev-ai review |
@icecrasher321 I have started the AI code review. It will take a few minutes to complete. |
The 0220 procedure projected every then-existing legacy cost json into cost_total, but a transition-window writer added 23 rows (all 2026-05-30, verified on the prod replica) carrying a numeric json total with no projection. Script migration 0009 re-runs 0220's exact candidate filter and projection in bounded batches at deploy time, so the pending cost DROP abandons nothing cost_total should hold. The contract PR that drops the column must deregister the script in the same change — it reads the column; the contract-pending marker says so. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@cubic-dev-ai review |
@icecrasher321 I have started the AI code review. It will take a few minutes to complete. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 25 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…ing-drop audit Hardens check-pending-drop-tables per review: the audit now derives each guarded table's doomed columns from schema.ts (@deprecated tags and column-level contract-pending markers), so a sanctioned omit(getTableColumns(t), [...]) or destructure-omit must name every doomed column away — an unrelated or incomplete key list is reported with the columns it misses, and schema.ts itself is no longer skipped, so the <table>Columns helpers fail the audit if a later deprecation is not added to their omit lists. Inline alias(t, ...) expressions passed directly to from()/getTableColumns()/insert/update/delete now resolve to their canonical table instead of escaping the checks. The testing mirror's *Columns aliases now omit the same doomed keys as the real helpers (and gain the missing proPeriodCostSnapshotAt entry), so tests cannot keep passing against doomed-column references the app no longer serves. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@cubic-dev-ai review |
@icecrasher321 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
All reported issues were addressed across 25 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
… schema The doomed-column derivation matched `@deprecated` with a line regex, so a column documented with a multiline TSDoc block was never marked doomed and the live-column helpers were free to omit nothing for it. Derivation now runs off the parsed schema: guarded tables are pgTable calls whose enclosed comments carry a drop-flavored contract-pending marker, and doomed columns come from each property's own leading TSDoc block, so single-line and multiline forms read alike. Verified by mutation: making a deprecated column's TSDoc multiline and dropping it from the helper's omit list now fails with the column named, where the previous version reported clean. Also fills the seven live columns the testing mirror was missing (user_stats.limit_notifications, four organization settings columns, workflow_execution_logs.cost_total/models_used) so mock-derived selections have the same shape as production ones. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@cubic-dev-ai review |
@icecrasher321 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
All reported issues were addressed across 25 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Table references were matched by bare identifier only, so an argless read reached through `import * as schema` — a live pattern here, e.g. `db.insert(schema.userStats)` in lib/auth/anonymous.ts — or through a renamed import was invisible to the audit. Resolution now runs over the file's schema-module import bindings (named, renamed, and namespace) as well as alias() bindings, at every position the audit inspects. Verified by probe: argless select() via `schema.userStats`, via a renamed import, and an argless .returning() on `db.insert(schema.userStats)` are all reported now, where the previous version reported none of the three. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@cubic-dev-ai review |
@icecrasher321 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
All reported issues were addressed across 25 files
You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.
Re-trigger cubic
A table parked in an ordinary binding (`const t = userStats`) escaped the audit, since only alias() results were re-bound. Binding collection now resolves any declarator whose right side names a table — the table itself, a namespace member, an earlier binding, or an alias() call — and repeats to a fixpoint so declaration order does not matter. Verified by probe: argless reads through a direct assignment, a namespace-member assignment, and a chained binding are all reported now, where the previous version reported none; the repo stays clean, including a file where a local array shadows a pending table's name. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@cubic-dev-ai review |
@icecrasher321 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
All reported issues were addressed across 25 files
You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.
Re-trigger cubic
… chain An argless findFirst/findMany reached through a binding that holds db.query escaped the audit, because the check required the receiver to be a literal `<x>.query` member. Drizzle's relational API is keyed by table name however the root is reached, so the check now keys on the table-named property itself, closing the whole class rather than the one aliasing form. Verified by probe: an aliased root (`const q = db.query`) is reported now where it previously was not, `tx.query` still is, a read that passes `columns` still is not, and the repo stays clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@cubic-dev-ai review |
@icecrasher321 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
No issues found across 25 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.
Re-trigger cubic
1 participant
What
Contract-prep for dropping the legacy usage columns: this PR makes the app stop naming them in SQL while the schema keeps declaring them, so a follow-up PR can ship the declaration removal and the real generated
DROP COLUMNmigration together, safely.userStatsColumns,organizationColumns,workflowExecutionLogColumnsinpackages/db/schema.ts, following the existingworkspaceFileColumnsprecedent):omit(getTableColumns(t), [...doomed]). All 29 argless reads of these tables —select().from(t), argless.returning(),findFirstwithoutcolumns— acrossusage.ts,threshold-billing.ts,logger.ts, dashboard, org/admin routes, data-drains, and anonymous auth now select through them, so generated SQL never mentions the doomed columns.scripts/check-pending-drop-tables.ts(bun run check:pending-drop-tables, picked up bycheck:auditsin CI). Derives the guarded tables from thecontract-pendingmarkers inschema.ts(only markers that describe a drop — a pendingSET NOT NULLnormalization doesn't make argless reads hazardous) and fails on any argless read, including viaalias()bindings and unsanctionedgetTableColumns()spreads. It also guardsworkspace_files' pendingsizedrop for free, and retires itself when the markers are deleted.contract-pendingmarkers on the deprecated columns inschema.ts(19user_statscounters,organization.departed_member_usage,workflow_execution_logs.cost), with stale "legacy/admin reads" TSDoc corrected — those reads are removed here.departedMemberUsage: '0'resets (its last writers), and the v1 admin exposure ofdepartedMemberUsage/currentPeriodCopilotCost(routes, types, contracts — both served dead values since the ledger cutover; the admin panel references neither).packages/testingschema mirror:userStats/organization/workflowExecutionLogshoisted and the three*Columnshelpers mirrored, same asworkspaceFileColumns.Why this shape
Migrations run before images are promoted, so a
DROP COLUMNis only safe once the already-deployed app no longer names the column in SQL — and drizzle puts every declared column into the SQL of any argless read. This PR removes every such read and pins that invariant with a lint. The follow-up PR (after this one is in a deployed production release) removes the declarations + markers +*Columnshelpers, runsdrizzle-kit generate, and annotates the DROPsmigration-safe: contract of #7134.grep -rn "contract-pending" packages/dblists what's owed.Replica verification (prod, 2026-08-26)
organization.departed_member_usage: 3/116 orgs carry pre-cutover residue (largest ≈ $3.6k), recorded in this PR since the drop abandons it. Nothing reads the column; member-departure usage is attributed via ledger entity stamps.workflow_execution_logs: 94 of 4.77M rows have a legacycostjson with nocost_total(71 zero/absent totals Jul–Aug 2025, 22 small totals May 2026). Unread history; the drop abandons it.Checks
bunx turbo run type-check— 26/26 passbun run check:pending-drop-tables— clean (guardsorganization,userStats,workflowExecutionLogs,workspaceFiles)bun run check:api-validation,check:migrations— passbunx drizzle-kit generate— no-op (schema and migrations in sync; no migration in this PR)vitest runover billing, logs, admin, auth, data-drains, org routes, contracts — 192 files / 2,435 tests pass🤖 Generated with Claude Code