Skip to content

feat(db): Drizzle tooling, and the tables everything else hangs off - #44

Closed
Willi363363 wants to merge 1 commit into
feat/rewrite-phase-2from
feat/rewrite-phase-2-schema
Closed

Willi363363 wants to merge 1 commit into
feat/rewrite-phase-2from
feat/rewrite-phase-2-schema

Conversation

@Willi363363

Copy link
Copy Markdown
Owner

Steps 2.1 and 2.2 of plans/rewrite/phase-02-data.md. Phase 2 opens here.

feat/rewrite-phase-2 branches off the top of phase 1 (#43), since phase 2
depends on it and nothing is merged yet. This is a step branch onto it.

Why the two steps are one change

2.1's criterion is "drizzle-kit migrate runs on a fresh database". On an
empty schema it does nothing and proves nothing, so the tooling is only really
verified by the first table. The phase sheet says so now.

One driver, one code path

postgres.js speaks plain Postgres — what Neon serves over TCP and what a
container serves in a test. So the code that runs in production is the code
the tests exercise
. A serverless HTTP driver alongside would mean two paths
with one of them tested; phase 9 revisits it if a deployment needs one.

The Better Auth schema was read, not remembered

Worth doing: account carries an issuer column, in a compound unique with
account_id, and I would have left it out. Getting that wrong means phase 5
migrating on arrival instead of configuring an adapter.

profile is a separate table on purpose. user belongs to Better Auth, and
adding columns to it means the adapter and the migration disagree the day its
core schema changes.

The integration tests run against a real Postgres

Migrated from the committed migrations, on a fresh database. The pitfall the
sheet names is exactly this: a suite running against a database somebody already
migrated proves the queries and nothing about the schema.

They assert on SQLSTATE, not on error text:

23505 unique_violation      two accounts on one email, two links on one (issuer, accountId)
23503 foreign_key_violation a profile with no user to hang off

My first version matched /unique|duplicate/ on the message and failed — the
constraints were working, but postgres.js reports "Failed query: …" and keeps
the class in a property. Matching the message would have passed for any
failure, including a typo in the query. Drizzle wraps the driver's error, so the
code is read by walking the cause chain rather than off the top error.

Also asserted: a deleted account takes its sessions, provider links and profile
with it — a session pointing at nobody is a way in.

A skip that cannot hide in CI

DATABASE_URL absent locally skips the integration tests; absent in CI it
fails them. A suite that quietly skips its integration tests on the machine
that decides whether to merge reports green for work it never did.

CI gets a postgres:17-alpine service, and runs drizzle-kit migrate before
the tests — the exit gate asks for the CLI on a never-migrated database, and a
programmatic migrator passing is not the same proof.

Two things drizzle-kit made necessary

Vitest split in two. drizzle-kit pulls in tsx, which is a peer of Vite,
which gave pnpm reason to hand two packages two instances of Vitest. Since
vitest.base.ts exported a type across package boundaries, every package
re-exporting it stopped typechecking — for a reason with nothing to do with its
own code:

../config/vitest.base.ts(7,3): error TS2769: 'test' does not exist in type 'UserConfigExport'

The baseline is a plain object now. Vitest reads it identically, a typo still
fails loudly (no test found), and the type no longer crosses a package boundary.

Trailing newlines. drizzle writes files without one and the hygiene check
refuses that, so generate normalises them afterwards. Its snapshots are kept
out of Prettier: drizzle reads them back to compute the next diff, and
reformatting one is how a migration chain starts disagreeing with the database it
describes.

Also fixed: turbo.json did not pass DATABASE_URL through, so the integration
tests were silently skipped under pnpm test even with the variable set.
Caught because 8 tests reported as skipped when they should have run.

Checks

On Node 22, against a container Postgres 17, replaying CI's exact sequence on a
brand-new database:

pnpm --filter @wikifake/db migrate    applied
CI=true pnpm test    db 11 · domain 233 · protocol 220 · config 17 · env 7
pnpm typecheck       5 packages
pnpm lint            5 packages
pnpm format:check
bash scripts/checks.sh staged

Three new dependencies, all named by 00-overview.md's decision table:
drizzle-orm, postgres, and drizzle-kit (dev).

Not run: the Python backend tests — no pytest here. This change touches no
Python; CI covers them.

Next

2.3 (game tables, with the negative assertion on in-progress reads), 2.4 (audit),
2.5 (llm_call and the cost queries), 2.6 (seed).

🤖 Generated with Claude Code

https://claude.ai/code/session_01533WTTbgmfCaDP5tmK8Gkv

Steps 2.1 and 2.2 of phase 2, in one change: the first half of 2.1's
criterion needs a table to migrate, since `drizzle-kit migrate` on an empty
schema does nothing and proves nothing.

One driver for every environment. `postgres.js` speaks plain Postgres, which
is what Neon serves over TCP and what a container serves in a test, so the
code that runs in production is the code the tests exercise. A separate
serverless driver would mean two paths and only one of them tested; phase 9
revisits that if a deployment needs the HTTP driver.

The Better Auth core schema was read rather than remembered, which was worth
doing: `account` carries an `issuer` column in a compound unique with
`account_id`, and I would have left it out. Phase 5 configures the adapter
instead of migrating on arrival.

`profile` is a separate table because `user` belongs to Better Auth: adding
columns to it means the adapter and the migration disagree the day its core
schema changes.

The integration tests run against a real Postgres, migrated from the
committed migrations — the pitfall being that a suite running against a
database somebody already migrated proves the queries and nothing about the
schema. They assert on SQLSTATE rather than on error text: `postgres.js`
reports "Failed query: …" and keeps the class in a property, so matching the
message would have passed for any failure at all, including a typo in the
query. Drizzle wraps the driver's error, so the code is read by walking the
`cause` chain.

Absent locally, `DATABASE_URL` skips those tests; absent in CI, it fails
them. A suite that quietly skips its integration tests on the machine that
decides whether to merge reports green for work it never did.

Two things `drizzle-kit` made necessary. It pulls in `tsx`, which is a peer
of Vite, which gave pnpm reason to hand two packages two instances of
Vitest — and `vitest.base.ts` exported a *type* across package boundaries,
so every package re-exporting it stopped typechecking for a reason unrelated
to its own code. The baseline is a plain object now. And drizzle writes files
without a trailing newline, which the hygiene check refuses, so `generate`
normalises them; its snapshots are kept out of Prettier, since drizzle reads
them back to compute the next diff.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01533WTTbgmfCaDP5tmK8Gkv
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Add Drizzle-based DB package, initial auth/profile schema, and CI migrations

✨ Enhancement 🧪 Tests ⚙️ Configuration changes 📝 Documentation 🕐 40+ Minutes

Grey Divider

AI Description

• Introduce @wikifake/db with Drizzle schema, migrations, and a single Postgres driver.
• Add real-Postgres integration tests that validate constraints via SQLSTATE codes.
• Update CI to provision a fresh Postgres service and run drizzle-kit migrate pre-test.
Diagram

graph TD
  EnvVars["Env vars (DATABASE_URL, CI)"] --> Tests["@wikifake/db integration tests"] --> DbPkg["@wikifake/db (client + schema)"] --> Pg[("Postgres (Neon/CI)")]
  CI["GitHub Actions CI"] --> DrizzleCli["drizzle-kit migrate"] --> MigDir["packages/db/migrations/"] --> Pg
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Use Neon serverless HTTP driver in production
  • ➕ Potentially simpler serverless deployment story on some platforms
  • ➕ May reduce connection management concerns in edge environments
  • ➖ Introduces a second DB driver/code path (harder to keep parity)
  • ➖ Tests would likely exercise only one path unless duplicated
2. Use an embedded DB for integration tests (e.g., pglite/sqlite)
  • ➕ Faster tests with no container/service dependency
  • ➕ Easier local setup
  • ➖ Weaker proof of Postgres-specific DDL/constraint behavior
  • ➖ Higher risk of schema drift vs actual production Postgres
3. Rely only on programmatic migrator (skip CLI migrate in CI)
  • ➕ Less CI time and one fewer step
  • ➕ Avoids CLI-specific environment quirks
  • ➖ Does not prove the stated requirement that drizzle-kit migrate works on a fresh DB
  • ➖ CLI and programmatic migrator can diverge in behavior/options

Recommendation: Keep the PR’s approach: a single postgres.js driver across environments, real-Postgres integration tests, and an explicit CI step running drizzle-kit migrate on a fresh database. This maximizes confidence that both the committed migrations and the operational migration workflow work end-to-end, and avoids maintaining parallel DB driver code paths.

Files changed (25) +1871 / -27

Enhancement (7) +276 / -0
0000_chubby_risque.sqlIntroduce initial auth/profile schema migration +65/-0

Introduce initial auth/profile schema migration

• Adds the first SQL migration creating user/session/account/verification tables plus a separate profile table. Includes unique constraints, foreign keys with cascade deletes, and supporting indexes.

packages/db/migrations/0000_chubby_risque.sql

normalise-migrations.tsNormalize Drizzle-generated migrations to satisfy repo newline hygiene +33/-0

Normalize Drizzle-generated migrations to satisfy repo newline hygiene

• Adds a script that walks the migrations directory and appends missing trailing newlines without altering content. Wired into the generate script to avoid manual fixes and prevent snapshot drift.

packages/db/scripts/normalise-migrations.ts

client.tsAdd postgres.js + Drizzle client with env-validated connection path +50/-0

Add postgres.js + Drizzle client with env-validated connection path

• Implements connect() using postgres.js and drizzle-orm, returning both a typed db handle and a close() hook. Adds connectFromEnv() using @wikifake/env to validate DATABASE_URL at startup and to share one driver path across prod/tests.

packages/db/src/client.ts

index.tsExport schema and connection helpers from @wikifake/db +8/-0

Export schema and connection helpers from @wikifake/db

• Exports schema tables and the database connection helpers/types as the package public API, keeping business logic out of the persistence layer.

packages/db/src/index.ts

auth.tsDefine Better Auth-compatible tables in Drizzle schema +84/-0

Define Better Auth-compatible tables in Drizzle schema

• Defines user/session/account/verification tables with snake_case SQL column names mapped to camelCase properties. Implements key constraints and indexes, including the compound (issuer, accountId) uniqueness on account.

packages/db/src/schema/auth.ts

index.tsCentralize schema exports for Drizzle and runtime client +3/-0

Centralize schema exports for Drizzle and runtime client

• Aggregates and re-exports all tables from a single schema entrypoint used by drizzle-kit and the runtime client.

packages/db/src/schema/index.ts

profile.tsAdd separate profile table for user preferences and display data +33/-0

Add separate profile table for user preferences and display data

• Creates a profile table keyed by userId, with displayName, accent default, and jsonb preferences default. Keeps it separate from Better Auth’s user table to avoid adapter/schema drift as auth evolves.

packages/db/src/schema/profile.ts

Bug fix (1) +13 / -4
vitest.base.tsAvoid defineConfig export to fix cross-package Vitest type conflicts +13/-4

Avoid defineConfig export to fix cross-package Vitest type conflicts

• Switches the shared Vitest base config export from defineConfig(...) to a plain object. Prevents pnpm from creating conflicting Vitest type instances (triggered by drizzle-kit pulling in tsx/vite peers) across package boundaries.

packages/config/vitest.base.ts

Tests (3) +184 / -0
workspace-graph.test.tsRegister db package dependencies in workspace graph test +1/-0

Register db package dependencies in workspace graph test

• Updates the expected dependency graph to include the new db package and its direct dependencies (env, drizzle-orm, postgres).

packages/config/src/workspace-graph.test.ts

client.test.tsTest env validation behavior for DATABASE_URL without leaking secrets +39/-0

Test env validation behavior for DATABASE_URL without leaking secrets

• Adds unit tests ensuring connectFromEnv fails by naming DATABASE_URL when missing or malformed, and that error messages never include credential values.

packages/db/src/client.test.ts

auth.test.tsAdd real-Postgres integration tests for auth/profile tables +144/-0

Add real-Postgres integration tests for auth/profile tables

• Introduces integration tests running against a migrated database, validating defaults, JSONB behavior, cascade deletes, and uniqueness/foreign-key failures. Asserts failures by SQLSTATE codes to avoid brittle message matching through driver/ORM wrappers.

packages/db/src/schema/auth.test.ts

Documentation (2) +9 / -3
README.mdMark phase 2 (Data) as in progress +1/-1

Mark phase 2 (Data) as in progress

• Updates the plans index to reflect that phase 2 work has started.

plans/README.md

phase-02-data.mdMark steps 2.1 and 2.2 complete and clarify acceptance criteria +8/-2

Mark steps 2.1 and 2.2 complete and clarify acceptance criteria

• Marks Drizzle tooling/client and initial auth/profile tables as complete. Adds rationale for delivering together and documents the single-driver strategy across prod/tests.

plans/rewrite/phase-02-data.md

Other (12) +1389 / -20
ci.ymlRun CI against a fresh Postgres and apply migrations pre-test +29/-0

Run CI against a fresh Postgres and apply migrations pre-test

• Adds a postgres:17-alpine service and exports DATABASE_URL for the job. Runs @wikifake/db migrate before typecheck/lint/tests to ensure drizzle-kit migrate succeeds on a never-migrated database.

.github/workflows/ci.yml

.prettierignoreIgnore Drizzle migrations from Prettier formatting +5/-0

Ignore Drizzle migrations from Prettier formatting

• Excludes packages/db/migrations from formatting to avoid Drizzle snapshot/migration diffs diverging from what Drizzle expects to read back.

.prettierignore

drizzle.config.tsAdd drizzle-kit config with explicit DATABASE_URL enforcement +21/-0

Add drizzle-kit config with explicit DATABASE_URL enforcement

• Introduces Drizzle configuration pointing to the schema entrypoint and migrations output directory. Reads DATABASE_URL directly and fails with a targeted error if missing, while enabling strict/verbose generation.

packages/db/drizzle.config.ts

eslint.config.jsAdopt repository ESLint configuration for db package +1/-0

Adopt repository ESLint configuration for db package

• Adds a local ESLint config that re-exports the shared @wikifake/config ESLint setup.

packages/db/eslint.config.js

0000_snapshot.jsonAdd Drizzle schema snapshot for migration 0000 +458/-0

Add Drizzle schema snapshot for migration 0000

• Stores Drizzle’s snapshot representation of the schema used to compute future diffs. Captures tables, columns, constraints, foreign keys, and indexes.

packages/db/migrations/meta/0000_snapshot.json

_journal.jsonAdd Drizzle migration journal metadata +13/-0

Add Drizzle migration journal metadata

• Introduces the Drizzle migrations journal tracking applied entries and breakpoints for the committed migration chain.

packages/db/migrations/meta/_journal.json

package.jsonCreate @wikifake/db package with Drizzle tooling scripts +30/-0

Create @wikifake/db package with Drizzle tooling scripts

• Defines the new workspace package, its exports, and scripts for generate/migrate/test/typecheck/lint. Adds drizzle-kit/drizzle-orm/postgres dependencies and shared config dev dependencies.

packages/db/package.json

database.tsAdd test DB harness with migrations, truncation, and SQLSTATE helpers +93/-0

Add test DB harness with migrations, truncation, and SQLSTATE helpers

• Implements openTestDatabase() to migrate from committed migrations and provide deterministic truncation per test. Enforces DATABASE_URL presence in CI (fail) vs locally (skip), and provides rejectionCode() that walks error causes to extract SQLSTATE robustly.

packages/db/src/testing/database.ts

tsconfig.jsonAdd TypeScript configuration for db package +8/-0

Add TypeScript configuration for db package

• Extends the shared base tsconfig, includes source and scripts, and configures node types with noEmit for package typechecking.

packages/db/tsconfig.json

vitest.config.tsWire db tests to shared Vitest base configuration +1/-0

Wire db tests to shared Vitest base configuration

• Adds a Vitest config that re-exports the shared base config from @wikifake/config.

packages/db/vitest.config.ts

pnpm-lock.yamlLockfile updates for Drizzle tooling and transitive deps +728/-19

Lockfile updates for Drizzle tooling and transitive deps

• Adds drizzle-kit, drizzle-orm, postgres, and associated transitive dependencies (including tsx/vite peer interactions) and registers the new packages/db importer.

pnpm-lock.yaml

turbo.jsonDeclare DATABASE_URL and CI as inputs to Turbo test task +2/-1

Declare DATABASE_URL and CI as inputs to Turbo test task

• Adds DATABASE_URL and CI to Turbo’s test task environment inputs to ensure correct caching/invalidation when integration test execution depends on these variables.

turbo.json

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Windows migration generation fails 🐞 Bug ☼ Reliability
Description
The normalisation script passes new URL(...).pathname directly to Node filesystem APIs, which
produces a URL pathname such as /C:/repo/... rather than a native Windows path. Consequently `pnpm
--filter @wikifake/db generate` fails while scanning the generated migrations on Windows.
Code

packages/db/scripts/normalise-migrations.ts[11]

+const MIGRATIONS = new URL('../migrations/', import.meta.url).pathname;
Evidence
The new package's generate command always invokes this script after Drizzle generation. The script
feeds MIGRATIONS to readdirSync, while the test helper already demonstrates the correct
cross-platform conversion using fileURLToPath(new URL(...)).

packages/db/package.json[9-14]
packages/db/scripts/normalise-migrations.ts[8-16]
packages/db/src/testing/database.ts[6-12]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`normalise-migrations.ts` derives a filesystem directory with `URL.pathname`. On Windows this is not a valid native path, so the `generate` script fails during migration normalisation.

## Issue Context
Use Node's `fileURLToPath()` for file-URL-to-path conversion, matching the existing migration test helper.

## Fix Focus Areas
- packages/db/scripts/normalise-migrations.ts[8-11]
- packages/db/src/testing/database.ts[8-12]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
✅ Web pages:
  +14 more
Review mode: 🧠 Deep: This is a broad, bug-dense schema/tooling and CI change spanning migrations, database runtime code, integration tests, configuration, and multiple independent execution paths.

Grey Divider

Tip of the day
💡 Did you know, you can switch off images and animations for a plain-text comment

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

import { readdirSync, readFileSync, statSync, writeFileSync } from 'node:fs';
import { join } from 'node:path';

const MIGRATIONS = new URL('../migrations/', import.meta.url).pathname;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

1. Windows migration generation fails 🐞 Bug ☼ Reliability

The normalisation script passes new URL(...).pathname directly to Node filesystem APIs, which
produces a URL pathname such as /C:/repo/... rather than a native Windows path. Consequently `pnpm
--filter @wikifake/db generate` fails while scanning the generated migrations on Windows.
Agent Prompt
## Issue description
`normalise-migrations.ts` derives a filesystem directory with `URL.pathname`. On Windows this is not a valid native path, so the `generate` script fails during migration normalisation.

## Issue Context
Use Node's `fileURLToPath()` for file-URL-to-path conversion, matching the existing migration test helper.

## Fix Focus Areas
- packages/db/scripts/normalise-migrations.ts[8-11]
- packages/db/src/testing/database.ts[8-12]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

@Willi363363

Copy link
Copy Markdown
Owner Author

Closed by the stack collapse, not abandoned.

Every commit of this pull request is in willi363/refonte via #114, which
merged the whole rewrite in one go: the stack was strictly linear, so the top
branch was an ancestor-of-nothing and a descendant-of-everything, and the merge
was a fast-forward with no conflict possible. The history keeps one commit per
step, which is what squash-per-step was there to produce.

Three commit messages were reworded on the way — a wip: type, a 74-character
subject and a (web,realtime) scope, none of which scripts/checks.sh allows.
The trees are byte-identical.

This description and its review thread stay readable here.

@Willi363363
Willi363363 deleted the feat/rewrite-phase-2-schema branch August 29, 2026 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant