feat(db): migration runner and core schema#7
Conversation
- Install node-pg-migrate@8.0.4 as devDependency - Implement pnpm migrate and pnpm migrate:down (previously stubs) using --tsx flag for TypeScript migration files - Create 4 reversible TypeScript migrations: csi_sections, specs, paragraphs, paragraph_versions with FK constraints and indexes - Fix db/index.test.ts: mock env module to prevent process.exit(1) during unit test module evaluation (pre-existing test bug) Deviations from plan: node-pg-migrate v8 uses --tsx boolean flag instead of --ts-runner tsx --migration-file-language ts (v8 API).
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdds node-pg-migrate CLI scripts, four reversible DB migrations (csi_sections, specs, paragraphs, paragraph_versions), and refactors DB test mocks to use a hoisted pg.Pool mock and shared mocked pool for pingDatabase tests. ChangesDatabase Schema Migrations & Setup
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Note 🎁 Summarized by CodeRabbit FreeYour organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/login. Comment |
Migrations:
- 002_create_specs: add notNull to section, title, source columns
- 003_create_paragraphs: add onDelete CASCADE to parent_id FK
- 004_create_paragraph_versions: make (paragraph_id, version) index UNIQUE
- all down migrations: add { cascade: true } to dropTable calls
Tests:
- Remove MockPool.prototype mutation; use vi.fn(function(){}) constructor
- Remove as unknown as casts; access mock pool via vi.mocked(Pool).mock.results
…hed (#204) Clears four dev-only Dependabot advisories via pnpm-workspace.yaml overrides — no source or runtime deps touched: esbuild >=0.28.1 GHSA-gv7w-rqvm-qjhr (high), -g7r4-m6w7-qqqr (#7, #8) dompurify >=3.4.9 GHSA-vxr8-fq34-vvx9 (#13) js-yaml >=4.2.0 GHSA-h67p-54hq-rp68 (eslint's 4.x line) (#11) None ship in dist/ or run in prod: esbuild is a vitest/tsx test-runner internal, dompurify lives in redoc's docs preview. Overrides applied surgically — fast-xml-parser and all other runtime deps stay pinned. Deferred (dev-only, not reachable): vite #9/#10 (Windows-only vite dev server we never run; bumping it needs a full lockfile regen that also trips an XMLValidator deprecation) → #203. depcheck's js-yaml@3.x is a different major the 4.2.0 fix doesn't apply to. Closes #200 Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address two CodeRabbit nitpicks on PR #423 and record a third as deferred: - #8 (logger.ts): the pino-roll file transport runs on a worker thread that buffers writes, so the bare process.exit(0) in shutdown() could kill it before the buffer flushed — dropping the final JSONL lines the corpus fix-loop reads back. Build the transport explicitly so we hold its handle, and drain it via closeLogger() (awaited in index.ts shutdown, guarded to always terminate). No-op when logging to stdout. Pinned by a unit test. - #7 (ADR-056): document the retention/redaction posture now that the user-supplied filename lands in durable JSONL — bounded by pino-roll rotation, namespaced under an app-controlled `doc` key, not redacted by default (spec filenames are not secrets), with a redact path noted for deployments that disagree. - #3 (ADR-056 P2): recorded as the first deferred item — carry the caught error's cause/detail on ParseWarning so core-metadata-unreadable surfaces *why* it failed, not just *that* it did. Deferred as an AST/contract change beyond this PR's boundary-only scope. The other 6 nitpicks are correctly skippable (one, the error.ts `declare override`, does not compile under useDefineForClassFields — the load-bearing `= undefined` initializer is documented in the Task 4 gotcha). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Summary
pnpm migrateandpnpm migrate:down(previously stubs that echoed and exited)csi_sections,specs,paragraphs,paragraph_versionswith FK constraints, CASCADE deletes, and indexessrc/db/index.test.ts: static import of./index.jstriggered env validation →process.exit(1)during unit tests; fixed by mocking../lib/env.jsviavi.mock(hoisted before static imports)Deviations from plan
node-pg-migrate v8 changed the CLI API. The plan specified
--ts-runner tsx --migration-file-language tsbut v8 uses--tsx(boolean flag). Updated scripts accordingly.Test plan
DATABASE_URL=postgres://specr:specr@localhost:5432/specr pnpm migrate— expect "Migrations complete!" after running all 4DATABASE_URL=postgres://specr:specr@localhost:5432/specr pnpm migrate:down— rolls back004_create_paragraph_versionsonlypnpm migrate— re-applies004cleanlypnpm lint— ESLint + tsc + prettier all passpnpm test— 5 test files, 15 tests, all passSummary by CodeRabbit
New Features
Tests
Chores