Repository navigation
feat(db): the development seed, idempotent for real - #48
Closed
Willi363363 wants to merge 1 commit into
Closed
Willi363363 wants to merge 1 commit into
Willi363363 wants to merge 1 commit into
Conversation
Step 2.6 of phase 2, and the last one. A room, a finished game with its solution, two participants — one account and one guest — their answers, the hints one of them bought, the items they threw, the model calls it cost, and a flag report. Enough to develop phases 3 to 8 without playing a game first, which is the point: not test data. Tests build exactly the rows they assert on. Fixed identifiers, literal timestamps, no randomness — replaying has to produce the same database, or "idempotent" is a claim rather than a property. That claim was false in my first version, and the test is what caught it. `item_use`, `llm_call` and `flag_report` default their primary key to a random UUID, so `onConflictDoNothing` had no conflict to find: the seed printed "seeded" twice, exited zero both times, and quietly doubled three tables. Every row carries an explicit id now, and the test counts rows across all eleven tables instead of trusting the exit code. Verified biting by removing one id: `flag_report` goes from 1 to 3. The seed also could not run at all at first. It went through `connectFromEnv`, which validates the whole environment, so a command whose only job is to talk to Postgres refused to start without a Redis URL and a model key — and named two variables the reader would then go looking for in the wrong place. `requireDatabaseUrl` is the tool-shaped version, now shared with `drizzle.config.ts` instead of duplicated in it. Run through `tsx`: Node's type stripping executes TypeScript but does not resolve a `.js` import onto a `.ts` file, and the package imports that way throughout. `tsx` was already in the tree under `drizzle-kit`; declaring it is better than relying on someone else's dependency. The seeded scores are C2.1 applied by hand, and a test checks them — if the scale ever moves, that is where the seed stops describing a real game. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01533WTTbgmfCaDP5tmK8Gkv
PR Summary by QodoAdd truly idempotent development DB seed with dedicated DATABASE_URL helper
AI Description
Diagram
High-Level Assessment
Files changed (10)
|
Code Review by Qodo
1. Seed skips required parents
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+47
to
+54
| await tx | ||
| .insert(user) | ||
| .values([...SEED_USERS]) | ||
| .onConflictDoNothing(); | ||
| await tx | ||
| .insert(profile) | ||
| .values([...SEED_PROFILES]) | ||
| .onConflictDoNothing(); |
There was a problem hiding this comment.
1. Seed skips required parents 🐞 Bug ☼ Reliability
seed() uses onConflictDoNothing() without a conflict target for user, so a conflict on user.email can skip inserting the seed_user_* row; the subsequent profile insert then fails its FK to user.id, breaking seeding on non-empty dev DBs.
Agent Prompt
## Issue description
`seed()` currently uses `.onConflictDoNothing()` without specifying a conflict target. In Postgres this ignores *any* unique-constraint conflict, including `user.email`. If a developer database already contains a row with `ada@example.org` under a different `user.id`, the user insert will be skipped, and the subsequent `profile` insert (which references `seed_user_ada`) will fail with a FK violation.
## Issue Context
This shows up specifically because `user.email` is unique, and `profile.userId` must reference `user.id`.
## Fix Focus Areas
- packages/db/src/seed/seed.ts[47-54]
- packages/db/src/schema/auth.ts[18-23]
- packages/db/src/schema/profile.ts[10-14]
## Suggested fix
1) For tables that are referenced by later inserts (notably `user`), use `onConflictDoNothing({ target: <primary key> })` so only *id-based* replays are treated as no-ops.
2) (Optional but robust) Make the seed emails even less likely to collide (e.g., `seed-ada@example.org`), or add a preflight check that if `seed_user_ada` does not exist after the user insert, throw a clear error indicating an email conflict.
Example direction:
- Change the user insert to `.onConflictDoNothing({ target: user.id })` so an email collision becomes an immediate, clear unique-violation error instead of a later FK failure.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Owner
Author
|
Closed by the stack collapse, not abandoned. Every commit of this pull request is in Three commit messages were reworded on the way — a This description and its review thread stay readable here. |
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.
Step 2.6 of
plans/rewrite/phase-02-data.md— the last step of phase 2.Stacked on #47. Base is
feat/rewrite-phase-2-usage, so the diff here is step 2.6alone.
What it seeds
A room, a finished game with its solution, two participants — one account and one
guest — their answers, the hints one of them bought, the items they threw, the
model calls it cost, and a flag report. Enough to develop phases 3 to 8 without
playing a game first, which is the point: not test data. Tests build exactly
the rows they assert on.
"Idempotent" was false, and the test is what caught it
My first version printed
seededtwice, exited zero both times, and quietlydoubled three tables.
item_use,llm_callandflag_reportdefault theirprimary key to a random UUID, so
onConflictDoNothinghad no conflict tofind.
That is exactly the failure mode "replays without error" cannot detect — which is
why the test counts rows across all eleven tables instead of trusting the exit
code:
Verified biting by removing one id:
flag_reportgoes from 1 to 3.(A note on my own process: my first attempt at that verification reported the
test as still passing. The regex had not matched, so the file was never modified
— the test was fine, my check was not. Worth redoing rather than reporting a
green I had not earned.)
It could not run at all at first
The seed went through
connectFromEnv, which validates the wholeenvironment. So a command whose only job is to talk to Postgres refused to start
without a Redis URL and a model key — and named two variables the reader would
then go hunting for in the wrong place.
requireDatabaseUrlis the tool-shaped version.drizzle.config.tshad the samelogic inline; it now shares this one instead.
tsx, declaredNode's type stripping executes TypeScript but does not resolve a
.jsimportonto a
.tsfile, and this package imports that way throughout — as the rest ofthe monorepo does.
tsxwas already in the tree underdrizzle-kit; declaringit is better than relying on someone else's dependency.
The seeded scores are the scale, applied by hand
A test checks both. If the scale ever moves, that is where the seed stops
describing a real game.
The phase exit gate
drizzle-kit migrateon a fresh database, in CIpostgres:17service since #44queries/only; the onesql.rawis the test harness's truncate2.5 — the cost of a game answers, and the failure is not in itbuild && test && lint && typecheckPhase 2's six steps are all ticked. I left the phase state as in progress:
the gate is passed, but nothing is merged.
Something you should read before merging anything
I had an agent audit the whole 18-PR stack read-only, and it found three things
that would have bitten. Summarised in my message rather than buried here — the
short version is: merge commits, not squash, and two of the base branches
(
feat/rewrite-phase-1,feat/rewrite-phase-2) currently lead nowhere.🤖 Generated with Claude Code
https://claude.ai/code/session_01533WTTbgmfCaDP5tmK8Gkv