chore(dx): timestamp migrations, journal-order guard, lint OOM root fix, hook + e2e locale fixes - #193
Conversation
…ix, hook + e2e locale fixes DX quick wins from a friction retrospective over recent feature cycles: - drizzle.config.ts sets migrations.prefix: 'timestamp' — parallel branches stop colliding on the next sequential migration number; the remaining merge surface is meta/_journal.json's append-only entries. 0000–0023 stay index-prefixed (apply order comes from the journal, not filenames). - New guard spec src/db/migration-journal.spec.ts pins contiguous idx and strictly increasing `when`: drizzle's migrator silently skips any entry whose `when` is older than the newest already-applied migration — the exact shape a careless rebase produces. The guard immediately caught a live instance: hand-authored 0004's `when` was stamped older than 0003's (a database parked at 0003 would have silently skipped it) — re-stamped. - apps/api/tsconfig.json excludes dist/: its .d.ts files were pulled into the tsgo/tsgolint program by the implicit include — the actual cause of local lint/typecheck OOMs after a build (previous folklore fix: rm -rf dist). - api `test` script pins jest --maxWorkers=2 (stable on memory-constrained dev machines, negligible on 4-core CI). - Pre-commit prettier hook filters staged files to existing ones (--diff-filter=ACMR): a commit deleting an api .ts file no longer aborts on a missing path while masking real drift in survivors. The .ts scoping moves into the git pathspec, which also closes a latent gap where direct children of src/ (e.g. src/main.ts) escaped the hook — lefthook's glob needs a subdirectory to match `**/`. - playwright.config.ts forces LANG/LC_ALL=en_US.UTF-8 into every spawned e2e server: local runs match CI instead of crashing SSR with Intl.Locale RangeErrors under WSL's default C.UTF-8 locale (Node >=21 derives navigator.language from the process locale). Closes #173 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EtvG7ZN3kWgAMt7ni11CdX
|
You do not have enough credits to review this pull request. Please purchase more credits to continue. |
📝 WalkthroughWalkthroughDrizzle migrations now use timestamp-prefixed filenames with journal-order validation. API typechecking and tests receive narrower resource scopes, the pre-commit formatter ignores deleted staged files, and Playwright servers use explicit UTF-8 locale settings. ChangesDeveloper safeguards
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces several developer experience (DX) improvements and guardrails. Key changes include configuring Drizzle migrations to use timestamp-prefixed filenames to prevent collisions across parallel branches, and adding a new test suite (migration-journal.spec.ts) to enforce journal invariants (contiguous indices and strictly increasing timestamps). Additionally, the API's tsconfig.json now excludes the dist/ directory to prevent local linting and type-checking out-of-memory (OOM) errors, Jest tests are limited to two workers for stability on memory-constrained machines, the pre-commit Prettier hook is optimized to filter out deleted files, and Playwright is configured to force a UTF-8 locale to prevent SSR crashes. The changelog and documentation have been updated accordingly. There are no review comments, so we have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
Pull request overview
This PR bundles several developer-experience fixes across the monorepo: migration naming/order safety, reduced local lint/typecheck memory usage, more reliable pre-commit formatting, and more stable local Playwright e2e runs by pinning process locale.
Changes:
- Make new Drizzle migrations timestamp-prefixed and add a unit test guard to prevent out-of-order migration journal entries.
- Exclude
apps/api/distfrom the API tsgo program and cap Jest workers to improve reliability on constrained machines. - Fix pre-commit prettier hooking to avoid deleted-file failures and align local Playwright server locale with CI.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| playwright.config.ts | Forces LANG/LC_ALL in spawned servers to avoid locale-derived SSR crashes locally. |
| lefthook.yml | Refines the API prettier pre-commit job to only check existing staged files. |
| CHANGELOG.md | Records the DX changes shipped on 2026-07-12. |
| apps/api/tsconfig.json | Excludes dist/ to avoid tsgo/tsgolint program bloat after builds. |
| apps/api/src/db/migrations/meta/_journal.json | Restamps migration 0004 to restore increasing apply-order timestamps. |
| apps/api/src/db/migration-journal.spec.ts | Adds a Jest guard test for journal idx contiguity and strictly increasing when. |
| apps/api/package.json | Pins Jest --maxWorkers=2 for more stable API unit test runs. |
| apps/api/drizzle.config.ts | Sets Drizzle migrations filename prefixing to timestamp. |
| apps/api/AGENTS.md | Documents the new migration naming scheme and the journal invariants/guard. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…imeouts Ride-alongs on the DX quick-wins branch, same retrospective: - rls-test.sh derives its container name from the invocation (llame-rls-test-$$) and probes 55440-55490 for a free host port (an explicit RLS_TEST_PORT still wins), so two harness runs on one machine no longer share llame-rls-test/55432 and kill each other's container mid-lifecycle. Cleanup already targeted only $CONTAINER, which is now unique per run. host_reachable() is deduplicated onto the same TCP probe. Verified end-to-end: with a decoy listener occupying 55440, the script chose 55441, ran the full suite green (126 integration + 3 active-runs + 63 e2e), and removed its own container. - The Fork-menu vitest tests (chat-item.test.tsx) get an explicit 15s timeout: userEvent's pointer sequences against the Radix menu repeatedly blew the 5s default under contended local runs (noted in #179; never failed in CI). Closes #164 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EtvG7ZN3kWgAMt7ni11CdX
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EtvG7ZN3kWgAMt7ni11CdX
DX quick wins from a friction retrospective over the last few feature cycles. Eight independent fixes, each small; none change product behavior.
Migrations: timestamp prefixes + apply-order guard
drizzle.config.tssetsmigrations.prefix: 'timestamp'→ new migrations areYYYYMMDDHHMMSS_<name>.sql, so parallel branches stop colliding on the next sequential number (this cycle alone: 0021 renumbered twice, and 0023 landed in parallel with 0022).0000–0023stay index-prefixed; apply order comes frommeta/_journal.json, not filenames. Smoke-verified with a throwawaydrizzle-kit generate --custom(produced20260712025511_….sql, then reverted).apps/api/src/db/migration-journal.spec.tspins the journal's two apply-order invariants: contiguousidxand strictly increasingwhen. drizzle-orm's migrator (pg-core/dialectmigrate) applies an entry only when itswhenis newer than the newest already-applied migration — an out-of-order entry is silently skipped on existing databases, which is exactly the shape a careless rebase produces. Runs in the normal unit suite; no CI wiring.0004's journalwhenwas stamped older than0003's, so a database parked exactly at0003would have silently skipped0004. Re-stamped to sit between0003and0005. Trade-off (deliberate): a volume parked exactly at0004would now try to re-apply it and fail loudly — better than the silent skip, and any volume parked pre-0005is a year old and needspnpm db:resetregardless. No real deployment is affected.Lint/typecheck OOM: root cause instead of folklore
apps/api/tsconfig.jsonhad noexclude, so the implicit include pulleddist/**/*.d.tsinto the tsgo program — bothtypecheckand tsgolint (type-aware lint) doubled their file set after any local build, which is what actually OOM'd memory-constrained machines (the working fix until now wasrm -rf distbefore linting). With the exclude, lint passes in ~1.8s withdist/present (~400MB max RSS).nest buildis unaffected —tsconfig.build.jsondefines its ownexclude.Related: the api
testscript pinsjest --maxWorkers=2— stable on constrained dev machines, negligible on 4-core CI, and removes thepnpm --filter api test -- --maxWorkers=2trap (jest treats the flag as a test pattern).Pre-commit prettier hook (#173)
{staged_files}includes deletions, andprettier --check <missing-path>exits 2 — blocking any commit that deletes an api.tsfile and masking real drift in surviving files (both observed). The job now sources files fromgit diff --cached --diff-filter=ACMRwith the.tsscoping moved into the git pathspec. That move also closes a latent gap the fix surfaced: lefthook's glob needs a subdirectory to match**/, so direct children ofsrc/(e.g.src/main.ts) never hit the local format gate at all — CI'sformat:checkcaught them, the hook didn't.Verified against the full matrix: deletion-only commit → job skips, hook passes; deletion + unformatted nested survivor → fails on the survivor; unformatted
src/main.ts(direct child) → now fails locally too.Local e2e locale crash
playwright.config.tsnow forcesLANG/LC_ALL=en_US.UTF-8into every spawned e2e server. Node ≥21 derivesnavigator.languagefrom the process locale; under WSL's defaultC.UTF-8it reports an invalid BCP-47 tag andnew Intl.Locale(...)throws during SSR (TanStack Query devtools undernext dev). CI already forces this at the job level — local runs now match CI instead of inheriting the shell locale.Ride-alongs (#164 + the Fork-menu timeout from #179's side note)
rls-test.shderives its container name from the invocation (llame-rls-test-$$) and probes 55440–55490 for a free host port (an explicitRLS_TEST_PORTstill wins), so two harness runs on one machine no longer sharellame-rls-test/55432 and kill each other's container mid-lifecycle.host_reachable()is deduplicated onto the same TCP probe. Proven end-to-end: with a decoy listener occupying 55440, the script chose 55441, ran fully green (126 integration + 3 active-runs + 63 e2e tests), and removed its own container.Verification
migration-journal.spec.ts3/3 green; full api unit suite via the changed script: 333 passed (DB-backed suites self-skip as usual)drizzle-kit checkgreen; timestamp-prefix generation smoke-tested and reverteddist/present; rootformat:checkgreenlefthook run pre-commitplaywright test --listcompiles the config (18 tests in 5 files)Closes #173
Closes #164
Summary by cubic
DX quick wins to reduce friction: timestamped migrations with a journal-order guard, a real fix for lint/typecheck OOMs, a reliable pre-commit formatter, stable local e2e, a collision-free RLS harness, and less flaky web tests. No product behavior changes.
DX Improvements
drizzle-kitmigrations now use timestamp prefixes viaapps/api/drizzle.config.ts;0000–0023stay index-prefixed. Addedapps/api/src/db/migration-journal.spec.tsto enforce contiguousidxand strictly increasingwhen, and re-stamped0004in the journal to satisfy it.apps/api/tsconfig.jsonexcludesdist/to prevent type-aware lint/typecheck OOMs;jest --maxWorkers=2is set inapps/api/package.jsonfor stability.apps/api/scripts/rls-test.shpicks a free port (55440–55490 orRLS_TEST_PORT) and a per-run container name to avoid collisions; integration spec comments now reference the dynamic port. Closes rls-test.sh: parameterize container name/port for concurrent runs #164.Bug Fixes
prettierhook checks only existing staged files viagit diff --cached --diff-filter=ACMRand a git pathspec that also coverssrc/*.ts; fixes false failures on deletions and closes thesrcdirect-child gap. Closes bug(hooks): pre-commit prettier hook exits 2 on any commit that deletes an apps/api ts file #173.playwright.config.tsforcesLANG/LC_ALL=en_US.UTF-8for spawned servers to avoidIntl.Localecrashes underC.UTF-8.Written for commit e4492e6. Summary will update on new commits.
Summary by CodeRabbit
Quality Improvements
Bug Fixes