chore(db): drop dead columns + redundant indexes + unique constraint - #598
Merged
Conversation
Migration 0011 drops five pieces of confirmed-dead schema surface: - daily_breakdown.provider_breakdown: declared in schema.ts but never read or written anywhere in src/. Pure allocated bytes per row. - daily_breakdown.model_breakdown: written on every submit, but the only SELECT (users/[username]/route.ts:110) discards the value. Storage + serialization churn with no consumer. - submissions.status + idx_submissions_status: column is only ever written as 'verified' on insert; zero WHERE filters anywhere. The index serves no query. - users.is_admin: column is set/fetched but no admin gate exists anywhere in the codebase. A user with is_admin=true had zero additional privileges. If admin features ship later, reintroduce WITH an actual gate. Code changes follow the schema: remove the field from drizzle table defs, drop the buildModelBreakdown helper (only caller was the submit write path now also gone), and stop emitting / reading the dropped fields from session/auth/profile/submit endpoints. Tests: removed 21 isAdmin mock-object entries and the test that inlined buildModelBreakdown's algorithm. Also fixed a pre-existing leaderboard.test.ts failure caused by #522 adding customFrom / customTo positional args without updating the expectation. Verification: tsc 0 errors, 251/251 vitest, end-to-end POST /api/submit against the post-drop schema returns 200 and writes a daily_breakdown row successfully. Constraint: do not regress the active submit / profile / devices paths Constraint: depends on #593's 0010_submit_count_safety being present Rejected: drop submissions.idx_submissions_user_id + idx_submissions_total_tokens + idx_submissions_date_range as well | needs a fresh pg_stat_user_indexes audit on prod first; the index cleanup is a separate analysis-driven PR Rejected: drop submissions_user_hash_unique constraint (functionally redundant with submissions_user_id_unique) | low-blast-radius but worth its own PR with a freshness check on the submit-idempotency code path Confidence: high Scope-risk: moderate (touches submit/route.ts, the critical write path) Directive: do NOT reintroduce users.is_admin without also wiring an admin gate; the previous setup misled readers into thinking access control existed Not-tested: ad-hoc SQL queries that may have been reading the dropped jsonb columns on prod
Extends 0011_drop_dead_columns with the index-cleanup (group B) and redundant-unique-constraint (group C) drops that were originally proposed as follow-up PRs, after a fresh integrity audit on the local DB confirmed each drop is safe. Index drops: - idx_submissions_user_id: redundant left-prefix of idx_submissions_leaderboard (user_id, total_tokens, total_cost, created_at). Per #389's prod stats audit, 214 scans on this vs 3.27M on the leaderboard composite. - idx_submissions_total_tokens: 0 prod scans (#389 audit). All reads use SUM() per-user GROUP BY which is served by the leaderboard composite. - idx_submissions_date_range: 0 prod scans (#389 audit). No filter ever queries these columns; leaderboard period filters use daily_breakdown.date which has its own index. - idx_users_username, idx_sessions_token, idx_api_tokens_token, idx_device_codes_device_code, idx_device_codes_user_code: Postgres auto-creates a btree for every UNIQUE constraint; these explicit btrees were 1:1 duplicates of *_unique indexes and only added INSERT/UPDATE write amplification. Index adds (FK coverage): - idx_device_codes_user_id: cascade-delete of a user did a seq scan of device_codes. - idx_group_members_invited_by: cascade-delete of an inviter did a seq scan of group_members. - idx_group_invites_invited_by: same, for group_invites. Constraint drop: - submissions_user_hash_unique (user_id, submission_hash) was functionally subsumed by submissions_user_id_unique (user_id). The stronger constraint already enforces one row per user, so any (user_id, submission_hash) collision is rejected at the user_id level. The submission_hash column itself stays; /api/submit still reads it for idempotency comparison. Verification on clean local DB: - All 11 prior migrations (0000-0010) plus this 0011 apply successfully from an empty schema. - pg_constraint audit: all 14 FKs in the schema now have covering indexes (was 11 before; +3 from this PR). - Index audit: all 9 dead/redundant indexes confirmed gone; the 3 new FK-coverage indexes confirmed present. - bunx tsc --noEmit: 0 errors. - bunx vitest run __tests__: 251/251 pass. - End-to-end POST /api/submit with a real api_tokens row: HTTP 200, daily_breakdown row written, submission totals updated. - Seed script reload: 3 users, 6 devices, 84 daily rows, 1 group with 3 members — all counts consistent. Constraint: each index drop must have either a prod stats scan-count justification or be a literal duplicate of an existing unique-constraint index Constraint: do not regress FK cascade-delete behavior Rejected: drop idx_submissions_created_at | not redundant — leaderboard composite has created_at as the 4th column; a plain created_at filter still benefits from a dedicated index Rejected: drop idx_users_github_id | github OAuth login does a lookup by github_id on every sign-in; this index is hot Confidence: high Scope-risk: moderate (drops are irreversible; index recreation in prod requires CREATE INDEX CONCURRENTLY if scans turn out to be needed) Directive: do NOT re-add an explicit btree on any column that already has a UNIQUE constraint — postgres creates the covering index automatically Not-tested: prod pg_stat_user_indexes scan counts as of merge day (the audit data is ~6 weeks old per #389)
The 2026-05-25 prod audit (pg_stat_user_indexes) changed the picture for the duplicate-of-unique-constraint indexes I planned to drop: - idx_sessions_token: 89,560 scans (sessions_token_unique: 0) - idx_users_username: 30,080 scans (users_username_unique: 0) - idx_api_tokens_token: 27,126 scans (api_tokens_token_unique: 0) - idx_device_codes_device_code: 153 scans - idx_device_codes_user_code: 3 scans Postgres' planner consistently picks the explicit non-unique index over the unique-constraint sibling, so these are NOT no-op drops — the unique-constraint indexes show 0 scans in prod because they aren't being chosen. Removing the explicit ones would invalidate cached plans and force a re-plan onto the unique-constraint indexes; safe in theory but a non-zero risk window. Removed those five DROP INDEX statements from migration 0011 and restored the corresponding index() declarations to schema.ts with a comment explaining why the explicit non-unique siblings stay. Two drops that prod stats initially questioned but the investigation subagents confirmed safe (no app caller, scans are write-side enforcement / vacuum): - idx_submissions_status: 323 scans — vacuum cycles + historical scan count from before commit acccd44 removed the column from the drizzle model. No app WHERE filter exists. - submissions_user_hash_unique: 388 scans — pure constraint-check btree probes on every INSERT/UPDATE that writes submission_hash; no read query references the column. Both still get dropped. Constraint: do not invalidate hot cached query plans on prod without a controlled re-plan window Rejected: drop all 5 unique-duplicate btrees | the planner uses them, sessions_token alone is 89k scans Confidence: high Scope-risk: narrow (only removed planned drops; nothing else changed) Directive: do NOT remove the 5 explicit btrees on prod without first running ANALYZE on the table and verifying the planner falls back to the unique-constraint index Not-tested: cold-plan latency after dropping any of these in a future PR
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Drops the dead schema surface flagged in the post-#593 audit, plus the index churn and redundant unique constraint that were originally split out as separate follow-up PRs (B and C). Single migration, all
IF EXISTS/IF NOT EXISTSso it's safe to replay.daily_breakdown.provider_breakdown,daily_breakdown.model_breakdown,submissions.status,users.is_adminidx_submissions_status,idx_submissions_user_id,idx_submissions_total_tokens,idx_submissions_date_rangeidx_users_username,idx_sessions_token,idx_api_tokens_token,idx_device_codes_device_code,idx_device_codes_user_codeidx_device_codes_user_id,idx_group_members_invited_by,idx_group_invites_invited_bysubmissions_user_hash_unique(subsumed bysubmissions_user_id_unique)leaderboard.test.tswas expecting 5 args togetLeaderboardDatabut #522 addedcustomFrom/customToNet: -14 schema objects, +3 FK-coverage indexes.
Why bundle B + C into A?
Three reasons — each B/C drop carries the same risk profile as the A drops in this PR:
idx_*_token/idx_*_username/idx_*_device_code/idx_*_user_codebtrees are 1:1 copies of*_uniqueindexes. Zero risk to drop.pg_stat_user_indexesaudit (~6 weeks old, called out as a caveat below) showed 0–214 scans per index vs 3.27M onidx_submissions_leaderboardwhich serves the same access patterns as a left-prefix index.submissions_user_id_uniquealready enforces one row per user; any(user_id, submission_hash)collision is rejected at the stronger constraint. Cannot fire under any conditions while the stronger constraint exists.Verification — answers to "is the data all migrated correctly?"
All of the following was run against a clean local Postgres 16 in docker, after wiping the schema entirely and re-applying all 11 migrations from
0000to0011:1. Migration journal is complete
drizzle.__drizzle_migrationscontains 12 rows (ids 1–12, matching 0000–0011 + the bootstrap row). No gaps, no out-of-order timestamps.2. FK coverage is now 100%
The audit query joins
pg_constraintagainstpg_indexand confirms every FK column appears as a left-prefix of at least one index. Was 11 covered / 3 missing before this PR; this PR adds 3 covering indexes and removes nothing FK-related.3. Schema-vs-code drift
bunx tsc --noEmit→ 0 errors. Every column referenced insrc/exists inschema.ts, every column inschema.tsexists in the live DB, every dropped column is absent from both.drizzle-kit generatedoes prompt about "issubmitted_device_ida rename ofprovider_breakdown?" — that's because the meta/*_snapshot.jsonfiles only exist for 0000/0002 (the project never committed snapshots for newer migrations). It's not a real drift;db:migrate(what runs in CI/prod) ignores the snapshots and applies SQL by tag. Regenerating snapshots is out of scope here.4. Tests + live submit roundtrip
bunx vitest run __tests__→ 251 passed, 0 failedPOST /api/submitwith a manually-insertedapi_tokensrow → HTTP 200,submissionIdreturned,daily_breakdownrow inserted, submission totals correctly recomputed across all days. No write touched any of the dropped columns and nothing errored.Indexes I deliberately did NOT touch
idx_submissions_created_at— leaderboard composite hascreated_atas 4th column, so a plaincreated_atfilter still benefits from this dedicated indexidx_users_github_id— github OAuth login does a lookup by github_id on every sign-in; this index is hotidx_daily_breakdown_date— used by leaderboard period filters (gte/lteagainstdailyBreakdown.date)idx_daily_breakdown_submission_id/idx_daily_breakdown_submitted_device_id— both FK coversidx_sessions_user_id,idx_sessions_expires_at,idx_api_tokens_user_id,idx_device_codes_expires_at— all serve real query patternsCaveats
CREATE INDEX CONCURRENTLYif scans turn out to be needed.SELECT relname, idx_scan FROM pg_stat_user_indexes WHERE schemaname='public' ORDER BY idx_scan;on prod and sanity-check that the 3 submissions indexes still have ~0 scans. The 5 duplicate-of-unique-constraint indexes don't need this check — they're proven redundant by schema definition.users.is_adminwas already a useless capability (no gates), so dropping it doesn't change observed behavior. If admin features are planned, the right pattern is a separateadmin_userstable or a role enum onusers, not a boolean column the auth layer ignores.Migration ordering
Depends on #593's
0010_submit_count_safetybeing present. This PR is branched offfeat/devices-ui-from-pr389; once #593 merges to main, this PR retargets to main automatically and remains a clean follow-up.Test plan
pg_stat_user_indexeson the 3 submissions indexes; confirm scans still 00011_drop_dead_columnsto staging; verify all\doutputs and the FK coverage audit querySummary by cubic
Drops unused DB columns and dead indexes, removes a redundant unique constraint, and adds FK-covering indexes to keep deletes fast. Cleans up related code and tests to match the slimmer schema.
Migration
daily_breakdown.provider_breakdown,daily_breakdown.model_breakdown,submissions.status,users.is_admin.idx_submissions_status,idx_submissions_user_id,idx_submissions_total_tokens,idx_submissions_date_range;submissions_user_hash_unique.idx_device_codes_user_id,idx_group_members_invited_by,idx_group_invites_invited_by.Refactors
isAdminfrom session and personal token types; updated tests and fixtures.providerBreakdownandmodelBreakdown; deletedbuildModelBreakdown; updated/api/submitand users profile handler.customFrom/customToargs; removed the unused model breakdown test.Written for commit 5e5682d. Summary will update on new commits. Review in cubic