refactor(postgres): collapse planner and drop legacy flows#371
Open
refactor(postgres): collapse planner and drop legacy flows#371
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughConsolidates migration authoring to executable migration.ts self-emission: removes descriptor-flow types/emit/scaffolding/resolver and the Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant CLI as CLI
participant FS as Filesystem
participant MigrationTs as migration.ts
participant Attestor as AttestService
Dev->>CLI: scaffold migration (prisma-next migration new)
Dev->>MigrationTs: edit migrations/<dir>/migration.ts
Dev->>MigrationTs: node migrations/<dir>/migration.ts (exec)
MigrationTs->>FS: write ops.json
MigrationTs->>Attestor: compute & request attestation (migrationId)
Attestor->>FS: write migration.json (migrationId)
CLI->>FS: read ops.json / migration.json for plan/status/apply
CLI->>Attestor: verify attest state when applying
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
wmadden
force-pushed
the
tml-2286-phase-3-consolidate
branch
from
49526a6 to
24909d7
Compare
@prisma-next/mongo-runtime
@prisma-next/family-mongo
@prisma-next/sql-runtime
@prisma-next/family-sql
@prisma-next/middleware-telemetry
@prisma-next/mongo
@prisma-next/extension-paradedb
@prisma-next/extension-pgvector
@prisma-next/postgres
@prisma-next/sql-orm-client
@prisma-next/sqlite
@prisma-next/target-mongo
@prisma-next/adapter-mongo
@prisma-next/driver-mongo
@prisma-next/contract
@prisma-next/utils
@prisma-next/config
@prisma-next/errors
@prisma-next/framework-components
@prisma-next/operations
@prisma-next/ts-render
@prisma-next/contract-authoring
@prisma-next/ids
@prisma-next/psl-parser
@prisma-next/psl-printer
@prisma-next/cli
@prisma-next/emitter
@prisma-next/migration-tools
prisma-next
@prisma-next/vite-plugin-contract-emit
@prisma-next/runtime-executor
@prisma-next/mongo-codec
@prisma-next/mongo-contract
@prisma-next/mongo-value
@prisma-next/mongo-contract-psl
@prisma-next/mongo-contract-ts
@prisma-next/mongo-emitter
@prisma-next/mongo-schema-ir
@prisma-next/mongo-query-ast
@prisma-next/mongo-orm
@prisma-next/mongo-query-builder
@prisma-next/mongo-lowering
@prisma-next/mongo-wire
@prisma-next/sql-contract
@prisma-next/sql-errors
@prisma-next/sql-operations
@prisma-next/sql-schema-ir
@prisma-next/sql-contract-psl
@prisma-next/sql-contract-ts
@prisma-next/sql-contract-emitter
@prisma-next/sql-lane-query-builder
@prisma-next/sql-relational-core
@prisma-next/sql-builder
@prisma-next/target-postgres
@prisma-next/target-sqlite
@prisma-next/adapter-postgres
@prisma-next/adapter-sqlite
@prisma-next/driver-postgres
@prisma-next/driver-sqlite
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/1-framework/3-tooling/cli/src/commands/migration-plan.ts (1)
343-364:⚠️ Potential issue | 🟠 MajorKeep the TTY output aligned with the new self-emit workflow.
executeMigrationPlanCommand()now returns draft packages, but the human-facing output still points placeholder users at the removedprisma-next migration emit --dir ..., and the normal success path never tells users they still neednode "<dir>/migration.ts"beforemigration apply. That leaves the non-JSON flow either wrong or incomplete.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/1-framework/3-tooling/cli/src/commands/migration-plan.ts` around lines 343 - 364, The CLI output needs to be updated to reflect the self-emit workflow: in executeMigrationPlanCommand() replace any instructions that point users to the removed `prisma-next migration emit --dir ...` and instead instruct placeholder authors to run `node "<dir>/migration.ts"` (use the same dir computed as relative(process.cwd(), packageDir) / dir in MigrationPlanResult when hasPlaceholders is true); additionally, update the normal success path (the non-placeholder branch that uses plannerResult.plan.operations and the success messaging after planning) to explicitly tell users they must run `node "<dir>/migration.ts"` to self-emit the migration before running `migration apply`, and adjust any human-facing strings referencing "emit" or "draft packages" to mention "self-emit via node migration.ts" so TTY and non-JSON flows are consistent with the new draft package return value.
🧹 Nitpick comments (4)
test/integration/test/cli-journeys/mongo-migration.e2e.test.ts (2)
134-153: Duplicated helper logic — consider using the sharedrunMigrationEmitfromjourney-test-helpers.ts.This local
migrationEmithelper is nearly identical torunMigrationEmitinjourney-test-helpers.ts(lines 395-423). Both:
- Extract
--dirfrom args- Validate it's present
- Execute
tsxagainstmigration.ts- Return
{ exitCode, stdout, stderr }The only difference is the context type (
JourneyCtxvsJourneyContext). Consider either:
- Importing and using the shared helper (adapting the context type)
- Extracting the core tsx-execution logic into a shared utility
This would reduce maintenance burden when the emission logic changes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/integration/test/cli-journeys/mongo-migration.e2e.test.ts` around lines 134 - 153, Duplicate migration execution logic in migrationEmit should be consolidated with the shared runMigrationEmit from journey-test-helpers.ts; update this file to import and call runMigrationEmit (or extract the TSX execution into a shared util) instead of defining migrationEmit, adapting the context type from JourneyCtx to JourneyContext or wrapping/transforming JourneyCtx to the expected type, and ensure the returned shape { exitCode, stdout, stderr } is preserved so tests behave identically.
38-38: Path import inconsistency with established pattern.The learning for this directory states to prefer
node:pathfor path operations in tests undertest/integration/test/. This file usespatheinstead.While both work correctly, consider using
node:pathfor consistency with other test files in this directory. Based on learnings: "for tests under test/integration/test/ (including cli-journeys and all subdirectories), prefer using node:path for path operations."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/integration/test/cli-journeys/mongo-migration.e2e.test.ts` at line 38, Replace the pathe import with the node:path import to match the directory convention: change the import of join and resolve from 'pathe' to import them from 'node:path' (i.e., use join and resolve from node:path wherever they are used in this file, referencing the existing import line that currently reads "import { join, resolve } from 'pathe'"). Ensure no other references to pathe remain in this test file.test/integration/test/utils/journey-test-helpers.ts (1)
30-31: Hardcoded path to tsx binary may break in different workspace layouts.The path
../../../../node_modules/.bin/tsxassumes a specific directory depth from this helper file to the repository root. If the test utilities are moved or the monorepo structure changes, this will silently fail.Consider using a more robust resolution strategy:
♻️ Suggested improvement
-const TSX_BIN = resolve(import.meta.dirname, '../../../../node_modules/.bin/tsx'); +import { createRequire } from 'node:module'; +const require = createRequire(import.meta.url); +const TSX_BIN = require.resolve('tsx/cli');Alternatively, use
npx tsxor resolve from the workspace root dynamically.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/integration/test/utils/journey-test-helpers.ts` around lines 30 - 31, The TSX_BIN constant currently uses a hardcoded relative path that will break if files are moved; update the assignment for TSX_BIN in this file so it discovers the tsx executable dynamically (e.g., prefer resolving the installed package/bin at runtime or falling back to "npx tsx"): use a helper that checks require.resolve('tsx/package.json') or a platform "which"/"where" lookup to locate the binary and fall back to invoking "npx tsx" if not found; keep execFileAsync as-is and ensure TSX_BIN is a reliable executable path or command string used by any consumers in this module.test/integration/test/cli.db-update.preflight-gaps.e2e.test.ts (1)
156-159: Use an object matcher for related column properties.Line 157 and Line 158 assert two related fields from the same row; this can be consolidated into a single object matcher.
Proposed refactor
expect(col.rows).toHaveLength(1); - expect(col.rows[0]!.is_nullable).toBe('NO'); - expect(col.rows[0]!.column_default).toBeNull(); + expect(col.rows[0]).toMatchObject({ + is_nullable: 'NO', + column_default: null, + });As per coding guidelines,
**/*.test.ts: prefer object matchers (toMatchObject) over multiple individual assertions for 2+ related values.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/integration/test/cli.db-update.preflight-gaps.e2e.test.ts` around lines 156 - 159, The two separate assertions on the same row (col.rows[0] is_nullable and column_default) should be consolidated into a single object matcher: keep the existing length check on col.rows, then replace the two expect calls that check col.rows[0] with one expect that uses toMatchObject on col.rows[0] and asserts both is_nullable equals 'NO' and column_default equals null; update the test in test/integration/.../cli.db-update.preflight-gaps.e2e.test.ts to use col.rows and toMatchObject for the combined assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/architecture` docs/subsystems/7. Migration System.md:
- Around line 64-68: The doc still references the old inline `migration
plan`/`migration emit` flow; update the subsystem doc to remove all mentions
that `migration plan` inline-emits `ops.json` or that users must run
`prisma-next migration emit`, and instead state that `ops.json` is produced only
via self-emission when running the authored migration (e.g., `node
<dir>/migration.ts` or via Migration.run(...) detecting main module), and ensure
lifecycle/command sections (including the passages that mention `migration
apply`, `migration.json`, `migrationId`, and the runner) consistently describe
self-emission as the canonical path and not a draft/attested discrepancy.
In `@packages/1-framework/3-tooling/cli/README.md`:
- Around line 1044-1059: Update the earlier "migration plan" section to reflect
the new self-emission workflow: clarify that `migration plan` and `migration
new` only scaffold the migration files (they do NOT write `ops.json` or attest
`migrationId`) and instruct readers to run the scaffolded `migration.ts` (which
calls `Migration.run(import.meta.url, ...)`) to emit `ops.json` and produce the
content-addressed `migrationId`; mention that unfilled `placeholder()` slots
cause exit with `PN-MIG-2001` and add a pointer/link from the plan section to
the "Emitting `ops.json` and computing `migrationId`" section for full steps.
In `@packages/3-targets/3-targets/postgres/src/core/migrations/issue-planner.ts`:
- Around line 489-495: The code currently converts an unresolved foreign-key
contract lookup into a successful AddForeignKeyCall with empty references (see
AddForeignKeyCall, fkName, columns and the return ok([...])), which hides the
real error; instead, detect when the contract lookup cannot resolve the
referenced table/columns and do not emit a placeholder AddForeignKeyCall —
return a planner conflict (use the planner's conflict/error return path used
elsewhere in issue-planner.ts) describing the missing reference so the planner
surfaces the failure immediately rather than producing invalid SQL.
In `@packages/3-targets/3-targets/postgres/src/core/migrations/planner.ts`:
- Around line 111-128: The planner is forcing `db update` behavior by passing
`fromContract: null` and `dbUpdateCallStrategies` into `planIssues`; change it
to accept and thread the previous contract and a migration-specific strategy
set: stop hard-coding `fromContract: null` and instead use a passed-in previous
contract (e.g., `options.fromContract` or a new `options.previousContract`) and
replace `dbUpdateCallStrategies` with a migration-specific strategies array
(e.g., `migrationPlanCallStrategies`) so authored migrations can use
reconciliation strategies that synthesize data/placeholder plans; update
`createPlanner`/call sites to supply the previous contract or the migration
strategy set so `planIssues` gets the correct inputs.
In
`@test/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/db-update-preflight-gaps/prisma-next.config.with-db.ts`:
- Around line 14-16: The contract fixture uses contract.source as a bare async
function but must implement the ContractSourceProvider shape; replace the
current contract.source: async () => ({ ok: true, value: contract }) with a
wrapper object implementing { load: async () => ({ ok: true, value: contract })
} so the ContractSourceProvider contract.source exposes a load method (keep the
existing output: 'src/prisma/contract.json' untouched).
---
Outside diff comments:
In `@packages/1-framework/3-tooling/cli/src/commands/migration-plan.ts`:
- Around line 343-364: The CLI output needs to be updated to reflect the
self-emit workflow: in executeMigrationPlanCommand() replace any instructions
that point users to the removed `prisma-next migration emit --dir ...` and
instead instruct placeholder authors to run `node "<dir>/migration.ts"` (use the
same dir computed as relative(process.cwd(), packageDir) / dir in
MigrationPlanResult when hasPlaceholders is true); additionally, update the
normal success path (the non-placeholder branch that uses
plannerResult.plan.operations and the success messaging after planning) to
explicitly tell users they must run `node "<dir>/migration.ts"` to self-emit the
migration before running `migration apply`, and adjust any human-facing strings
referencing "emit" or "draft packages" to mention "self-emit via node
migration.ts" so TTY and non-JSON flows are consistent with the new draft
package return value.
---
Nitpick comments:
In `@test/integration/test/cli-journeys/mongo-migration.e2e.test.ts`:
- Around line 134-153: Duplicate migration execution logic in migrationEmit
should be consolidated with the shared runMigrationEmit from
journey-test-helpers.ts; update this file to import and call runMigrationEmit
(or extract the TSX execution into a shared util) instead of defining
migrationEmit, adapting the context type from JourneyCtx to JourneyContext or
wrapping/transforming JourneyCtx to the expected type, and ensure the returned
shape { exitCode, stdout, stderr } is preserved so tests behave identically.
- Line 38: Replace the pathe import with the node:path import to match the
directory convention: change the import of join and resolve from 'pathe' to
import them from 'node:path' (i.e., use join and resolve from node:path wherever
they are used in this file, referencing the existing import line that currently
reads "import { join, resolve } from 'pathe'"). Ensure no other references to
pathe remain in this test file.
In `@test/integration/test/cli.db-update.preflight-gaps.e2e.test.ts`:
- Around line 156-159: The two separate assertions on the same row (col.rows[0]
is_nullable and column_default) should be consolidated into a single object
matcher: keep the existing length check on col.rows, then replace the two expect
calls that check col.rows[0] with one expect that uses toMatchObject on
col.rows[0] and asserts both is_nullable equals 'NO' and column_default equals
null; update the test in
test/integration/.../cli.db-update.preflight-gaps.e2e.test.ts to use col.rows
and toMatchObject for the combined assertion.
In `@test/integration/test/utils/journey-test-helpers.ts`:
- Around line 30-31: The TSX_BIN constant currently uses a hardcoded relative
path that will break if files are moved; update the assignment for TSX_BIN in
this file so it discovers the tsx executable dynamically (e.g., prefer resolving
the installed package/bin at runtime or falling back to "npx tsx"): use a helper
that checks require.resolve('tsx/package.json') or a platform "which"/"where"
lookup to locate the binary and fall back to invoking "npx tsx" if not found;
keep execFileAsync as-is and ensure TSX_BIN is a reliable executable path or
command string used by any consumers in this module.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 2a7630fa-8bfd-4931-a192-52df99952e62
📒 Files selected for processing (104)
docs/architecture docs/adrs/ADR 193 - Class-flow as the canonical migration authoring strategy.mddocs/architecture docs/subsystems/10. MongoDB Family.mddocs/architecture docs/subsystems/7. Migration System.mdpackages/1-framework/1-core/errors/src/exports/migration.tspackages/1-framework/1-core/errors/src/migration.tspackages/1-framework/1-core/framework-components/src/control-migration-types.tspackages/1-framework/1-core/framework-components/src/exports/control.tspackages/1-framework/1-core/ts-render/README.mdpackages/1-framework/1-core/ts-render/src/render-imports.tspackages/1-framework/3-tooling/cli/README.mdpackages/1-framework/3-tooling/cli/package.jsonpackages/1-framework/3-tooling/cli/src/cli.tspackages/1-framework/3-tooling/cli/src/commands/migration-apply.tspackages/1-framework/3-tooling/cli/src/commands/migration-emit.tspackages/1-framework/3-tooling/cli/src/commands/migration-new.tspackages/1-framework/3-tooling/cli/src/commands/migration-plan.tspackages/1-framework/3-tooling/cli/src/commands/migration-show.tspackages/1-framework/3-tooling/cli/src/commands/migration-status.tspackages/1-framework/3-tooling/cli/src/lib/migration-emit.tspackages/1-framework/3-tooling/cli/src/lib/migration-strategy.tspackages/1-framework/3-tooling/cli/src/utils/formatters/help.tspackages/1-framework/3-tooling/cli/src/utils/formatters/migrations.tspackages/1-framework/3-tooling/cli/test/commands/migration-apply.test.tspackages/1-framework/3-tooling/cli/test/commands/migration-e2e.test.tspackages/1-framework/3-tooling/cli/test/commands/migration-emit.test.tspackages/1-framework/3-tooling/cli/test/commands/migration-plan-command.test.tspackages/1-framework/3-tooling/cli/test/commands/migration-plan.test.tspackages/1-framework/3-tooling/cli/test/commands/migration-ref.test.tspackages/1-framework/3-tooling/cli/test/commands/migration-show.test.tspackages/1-framework/3-tooling/cli/test/lib/migration-emit.test.tspackages/1-framework/3-tooling/cli/test/lib/migration-strategy.test.tspackages/1-framework/3-tooling/cli/test/output.migration-commands.test.tspackages/1-framework/3-tooling/cli/tsdown.config.tspackages/1-framework/3-tooling/cli/vitest.config.tspackages/1-framework/3-tooling/migration/src/exports/migration-ts.tspackages/1-framework/3-tooling/migration/src/io.tspackages/1-framework/3-tooling/migration/src/migration-base.tspackages/1-framework/3-tooling/migration/src/migration-ts.tspackages/1-framework/3-tooling/migration/src/types.tspackages/1-framework/3-tooling/migration/test/attestation.test.tspackages/1-framework/3-tooling/migration/test/fixtures.tspackages/1-framework/3-tooling/migration/test/migration-base.test.tspackages/1-framework/3-tooling/migration/test/migration-ts.test.tspackages/2-mongo-family/9-family/src/core/mongo-emit.tspackages/2-mongo-family/9-family/src/core/mongo-migration.tspackages/2-mongo-family/9-family/src/core/mongo-target-descriptor.tspackages/2-mongo-family/9-family/test/mongo-emit.test.tspackages/2-sql/9-family/package.jsonpackages/2-sql/9-family/src/core/migrations/descriptor-schemas.tspackages/2-sql/9-family/src/core/migrations/operation-descriptors.tspackages/2-sql/9-family/src/core/sql-migration.tspackages/2-sql/9-family/src/exports/operation-descriptors.tspackages/2-sql/9-family/tsdown.config.tspackages/3-mongo-target/1-mongo-target/src/core/mongo-planner.tspackages/3-mongo-target/1-mongo-target/src/core/op-factory-call.tspackages/3-mongo-target/1-mongo-target/src/core/render-typescript.tspackages/3-mongo-target/1-mongo-target/test/mongo-planner.test.tspackages/3-mongo-target/1-mongo-target/test/op-factory-call.test.tspackages/3-targets/3-targets/postgres/package.jsonpackages/3-targets/3-targets/postgres/src/core/migrations/issue-planner.tspackages/3-targets/3-targets/postgres/src/core/migrations/op-factory-call.tspackages/3-targets/3-targets/postgres/src/core/migrations/operation-descriptors.tspackages/3-targets/3-targets/postgres/src/core/migrations/operation-resolver.tspackages/3-targets/3-targets/postgres/src/core/migrations/operations/data-transform.tspackages/3-targets/3-targets/postgres/src/core/migrations/operations/raw.tspackages/3-targets/3-targets/postgres/src/core/migrations/operations/shared.tspackages/3-targets/3-targets/postgres/src/core/migrations/planner-produced-postgres-migration.tspackages/3-targets/3-targets/postgres/src/core/migrations/planner-reconciliation.tspackages/3-targets/3-targets/postgres/src/core/migrations/planner-strategies.tspackages/3-targets/3-targets/postgres/src/core/migrations/planner.tspackages/3-targets/3-targets/postgres/src/core/migrations/postgres-emit.tspackages/3-targets/3-targets/postgres/src/core/migrations/postgres-migration.tspackages/3-targets/3-targets/postgres/src/core/migrations/render-typescript.tspackages/3-targets/3-targets/postgres/src/core/migrations/scaffolding.tspackages/3-targets/3-targets/postgres/src/exports/control.tspackages/3-targets/3-targets/postgres/src/exports/migration-builders.tspackages/3-targets/3-targets/postgres/src/exports/migration.tspackages/3-targets/3-targets/postgres/test/migrations/data-transform.test.tspackages/3-targets/3-targets/postgres/test/migrations/descriptor-planner.scenarios.mdpackages/3-targets/3-targets/postgres/test/migrations/descriptor-planner.scenarios.test.tspackages/3-targets/3-targets/postgres/test/migrations/op-factory-call.test.tspackages/3-targets/3-targets/postgres/test/migrations/operation-resolver.integration.test.tspackages/3-targets/3-targets/postgres/test/migrations/planner.authoring-surface.test.tspackages/3-targets/3-targets/postgres/test/migrations/planner.behavior.test.tspackages/3-targets/3-targets/postgres/test/migrations/planner.case1.test.tspackages/3-targets/3-targets/postgres/test/migrations/planner.reconciliation-unit.test.tspackages/3-targets/3-targets/postgres/test/migrations/planner.reconciliation.issue-to-call.test.tspackages/3-targets/3-targets/postgres/test/migrations/postgres-emit.test.tspackages/3-targets/3-targets/postgres/test/migrations/render-typescript.roundtrip.test.tspackages/3-targets/3-targets/postgres/test/migrations/scaffolding.test.tspackages/3-targets/3-targets/postgres/tsdown.config.tstest/integration/test/cli-journeys/data-transform-not-null-backfill.e2e.test.tstest/integration/test/cli-journeys/data-transform-nullable-tightening.e2e.test.tstest/integration/test/cli-journeys/data-transform-type-change.e2e.test.tstest/integration/test/cli-journeys/data-transform.e2e.test.tstest/integration/test/cli-journeys/migration-round-trip.e2e.test.tstest/integration/test/cli-journeys/mongo-migration.e2e.test.tstest/integration/test/cli.db-update.preflight-gaps.e2e.test.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/cli-journeys/contract-typechange-text.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/db-update-preflight-gaps/contract-add-fk.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/db-update-preflight-gaps/contract-add-required-unique.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/db-update-preflight-gaps/contract.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/db-update-preflight-gaps/prisma-next.config.with-db.tstest/integration/test/utils/journey-test-helpers.ts
💤 Files with no reviewable changes (43)
- packages/1-framework/3-tooling/cli/vitest.config.ts
- packages/1-framework/3-tooling/migration/src/io.ts
- packages/1-framework/3-tooling/migration/test/fixtures.ts
- packages/1-framework/3-tooling/cli/src/utils/formatters/help.ts
- packages/1-framework/1-core/framework-components/src/exports/control.ts
- packages/1-framework/3-tooling/migration/test/migration-base.test.ts
- packages/3-targets/3-targets/postgres/package.json
- packages/1-framework/3-tooling/cli/tsdown.config.ts
- packages/2-sql/9-family/tsdown.config.ts
- packages/1-framework/3-tooling/migration/src/types.ts
- packages/3-targets/3-targets/postgres/tsdown.config.ts
- packages/1-framework/3-tooling/cli/test/commands/migration-ref.test.ts
- packages/3-targets/3-targets/postgres/test/migrations/descriptor-planner.scenarios.md
- packages/1-framework/3-tooling/migration/test/attestation.test.ts
- packages/3-targets/3-targets/postgres/test/migrations/scaffolding.test.ts
- packages/1-framework/3-tooling/cli/test/lib/migration-emit.test.ts
- packages/3-targets/3-targets/postgres/src/exports/control.ts
- packages/1-framework/3-tooling/cli/test/lib/migration-strategy.test.ts
- packages/1-framework/3-tooling/cli/src/lib/migration-strategy.ts
- packages/3-targets/3-targets/postgres/src/core/migrations/scaffolding.ts
- packages/3-targets/3-targets/postgres/src/core/migrations/postgres-emit.ts
- packages/1-framework/3-tooling/cli/src/utils/formatters/migrations.ts
- packages/3-targets/3-targets/postgres/test/migrations/descriptor-planner.scenarios.test.ts
- packages/1-framework/3-tooling/cli/package.json
- packages/2-sql/9-family/package.json
- packages/1-framework/1-core/errors/src/exports/migration.ts
- packages/2-mongo-family/9-family/src/core/mongo-emit.ts
- packages/3-targets/3-targets/postgres/test/migrations/operation-resolver.integration.test.ts
- packages/3-targets/3-targets/postgres/test/migrations/postgres-emit.test.ts
- packages/1-framework/3-tooling/cli/test/commands/migration-emit.test.ts
- packages/3-targets/3-targets/postgres/test/migrations/planner.reconciliation-unit.test.ts
- test/integration/test/cli-journeys/data-transform.e2e.test.ts
- packages/3-targets/3-targets/postgres/src/core/migrations/planner-reconciliation.ts
- packages/3-targets/3-targets/postgres/test/migrations/planner.reconciliation.issue-to-call.test.ts
- packages/2-mongo-family/9-family/test/mongo-emit.test.ts
- packages/3-targets/3-targets/postgres/src/exports/migration-builders.ts
- packages/1-framework/3-tooling/cli/src/commands/migration-emit.ts
- packages/3-targets/3-targets/postgres/src/core/migrations/operation-resolver.ts
- packages/1-framework/3-tooling/cli/src/lib/migration-emit.ts
- packages/2-sql/9-family/src/exports/operation-descriptors.ts
- packages/2-sql/9-family/src/core/migrations/descriptor-schemas.ts
- packages/2-sql/9-family/src/core/migrations/operation-descriptors.ts
- packages/3-targets/3-targets/postgres/src/core/migrations/operation-descriptors.ts
wmadden
force-pushed
the
tml-2286-phase-3-consolidate
branch
from
24909d7 to
4922134
Compare
wmadden
changed the title
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/1-framework/3-tooling/cli/src/commands/migration-plan.ts (1)
450-456:⚠️ Potential issue | 🟡 MinorStale reference to removed
migration emitcommand.The output message still references
prisma-next migration emit --dirwhich contradicts the PR's goal of removing that command. This should be updated to usenode migration.tsfor consistency with all other guidance in this file and across the CLI.Proposed fix
lines.push( 'Open migration.ts and replace each `placeholder(...)` call with your actual query.', ); lines.push( - `Then run: ${green_('prisma-next migration emit --dir ' + (result.dir ?? '<dir>'))}`, + `Then run: ${green_('node ' + (result.dir ? `${result.dir}/migration.ts` : '<dir>/migration.ts'))}`, );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/1-framework/3-tooling/cli/src/commands/migration-plan.ts` around lines 450 - 456, The printed guidance still references the removed command `prisma-next migration emit --dir`; update the string pushed into lines (the lines.push call that currently builds `Then run: ${green_('prisma-next migration emit --dir ' + (result.dir ?? '<dir>'))}`) to instead instruct running the generated script, e.g. use `node migration.ts` (wrapped with green_ for color) and keep the same conditional dir handling if needed so the message is consistent with other guidance in migration-plan.ts and the CLI.
♻️ Duplicate comments (2)
packages/3-targets/3-targets/postgres/src/core/migrations/planner-strategies.ts (1)
434-444:⚠️ Potential issue | 🟠 MajorOnly consume dependency issues that this strategy actually handled.
Filtering out every
dependency_missinghere makes the default fallback inissue-planner.tsunreachable for plainext:/schema:issues, sodb updatecan silently skip required extension/schema installs whenever there is no matching component install op. ThehandledDependencyIdsset is built but not used in the filter.🐛 Proposed fix
- // Consume ALL `dependency_missing` issues — even non-postgres ones. The - // walk-schema predecessor silently skipped non-postgres deps; leaving those - // issues in the stream would let `mapIssueToCall` reject them as - // "Unknown dependency type". - const remaining = issues.filter((issue) => issue.kind !== 'dependency_missing'); + // Consume only the dependency issues this strategy handled. Let ext:/schema: + // issues without a component install op flow through to mapIssueToCall. + const remaining = issues.filter((issue) => { + if (issue.kind !== 'dependency_missing' || !issue.dependencyId) { + return true; + } + return !handledDependencyIds.has(issue.dependencyId); + });,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/3-targets/3-targets/postgres/src/core/migrations/planner-strategies.ts` around lines 434 - 444, The filter currently drops all dependency_missing issues so the fallback in issue-planner.ts never runs; change the filter to only remove dependency_missing issues whose ids are in the handledDependencyIds set (the set built earlier in this function) instead of removing every dependency_missing; update the variable used for the remaining array (replace issues.filter((issue) => issue.kind !== 'dependency_missing') with a predicate that keeps non-dependency_missing OR dependency_missing whose id is NOT in handledDependencyIds) so that unhandled ext:/schema: dependency issues still reach the default fallback.packages/3-targets/3-targets/postgres/src/core/migrations/issue-planner.ts (1)
489-495:⚠️ Potential issue | 🔴 CriticalDon't turn an unresolved FK into a fake
AddForeignKeyCall.This returns a "successful" plan even though the generated call has
references: { table: '', columns: [] }, so the migration only fails later with invalid SQL. If the contract lookup cannot resolve the FK, surface a planner conflict here instead of emitting a placeholder operation.🐛 Proposed fix
- return ok([ - new AddForeignKeyCall(schemaName, issue.table, { - name: fkName, - columns, - references: { table: '', columns: [] }, - }), - ]); + return notOk( + issueConflict( + 'foreignKeyConflict', + `Foreign key "${fkName}" on "${issue.table}" could not be resolved from the destination contract`, + { table: issue.table }, + ), + );,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/3-targets/3-targets/postgres/src/core/migrations/issue-planner.ts` around lines 489 - 495, The code currently emits a successful AddForeignKeyCall with empty references ({ table: '', columns: []}) when the FK contract lookup fails; instead detect when the referenced contract/table/columns are unresolved (the branch that computes fkName and columns) and return a planner conflict/error rather than constructing AddForeignKeyCall. Replace the placeholder AddForeignKeyCall return with a failure return that surfaces a planner conflict (use the module's existing planner conflict/error helper or return a Result.err with a clear conflict message mentioning the fkName and unresolved reference), so the planner fails fast instead of emitting invalid SQL later.
🧹 Nitpick comments (2)
docs/architecture docs/subsystems/10. MongoDB Family.md (1)
11-11: Consider clarifying the ADR 193 reference scope.The ADR 193 reference appears immediately after "self-emit via
./migration.ts", which could suggest it only applies to that specific path. However, per the ADR title ("Class-flow as the canonical migration authoring strategy"), it describes the overall authoring approach that encompasses both emit paths. Consider adjusting the placement or wording to make this clearer.Additionally, the sentence is quite dense with nested parentheticals. Breaking it into multiple sentences might improve readability.
♻️ Suggested rewording for clarity
-**Migrations are supported.** MongoDB does have server-side schema state — indexes, JSON Schema validators, collection options (capped, time-series, collation) — and Mongo participates in the migration system on the same footing as SQL targets. `prisma-next migration new --target mongo` and `prisma-next migration plan --target mongo` scaffold a runnable `migration.ts` (extending `Migration` from `@prisma-next/family-mongo/migration`). Two paths produce identical `ops.json` artifacts — the authoritative migration contract (see [ADR 192](../adrs/ADR%20192%20-%20ops.json%20is%20the%20migration%20contract.md)): inline emit at the end of `migration plan` and self-emit via `./migration.ts` (see [ADR 193](../adrs/ADR%20193%20-%20Class-flow%20as%20the%20canonical%20migration%20authoring%20strategy.md) for the authoring strategy). Data migrations are authored as `dataTransform({ check, run })` operations whose pipelines invariant-guard the transition (see [ADR 176](../adrs/ADR%20176%20-%20Data%20migrations%20as%20invariant-guarded%20transitions.md)). See the [Migration System](7.%20Migration%20System.md) subsystem doc for the full canonical pipeline. +**Migrations are supported.** MongoDB does have server-side schema state — indexes, JSON Schema validators, collection options (capped, time-series, collation) — and Mongo participates in the migration system on the same footing as SQL targets. `prisma-next migration new --target mongo` and `prisma-next migration plan --target mongo` scaffold a runnable `migration.ts` (extending `Migration` from `@prisma-next/family-mongo/migration`). Two paths produce identical `ops.json` artifacts (the authoritative migration contract, see [ADR 192](../adrs/ADR%20192%20-%20ops.json%20is%20the%20migration%20contract.md)): inline emit at the end of `migration plan` and self-emit by running `./migration.ts`. See [ADR 193](../adrs/ADR%20193%20-%20Class-flow%20as%20the%20canonical%20migration%20authoring%20strategy.md) for the authoring strategy. Data migrations are authored as `dataTransform({ check, run })` operations whose pipelines invariant-guard the transition (see [ADR 176](../adrs/ADR%20176%20-%20Data%20migrations%20as%20invariant-guarded%20transitions.md)). See the [Migration System](7.%20Migration%20System.md) subsystem doc for the full canonical pipeline.This version:
- Replaces the em dash + colon with parentheses for the ADR 192 clarification
- Moves the ADR 193 reference to a separate sentence after listing both paths
- Changes "self-emit via" to "self-emit by running" for clarity
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/architecture` docs/subsystems/10. MongoDB Family.md at line 11, Split and rephrase the long sentence around the two emit paths to make ADR 193's scope explicit: after listing both paths that produce identical ops.json artifacts (inline emit at the end of `migration plan` and self-emit by running `./migration.ts`), add a separate sentence stating that ADR 193 ("Class-flow as the canonical migration authoring strategy") describes the overall authoring approach that applies to both paths; also simplify the parentheticals (keep the ADR 192 reference tied to `ops.json` and retain `migration.ts` extending `Migration` and `dataTransform({ check, run })` mentions) so the paragraph reads as two clearer sentences rather than one dense, nested-parenthetical sentence.packages/3-targets/3-targets/postgres/src/core/migrations/issue-planner.ts (1)
615-639: Consider a type guard forrawSqlclassification.The
as unknown as { ... }cast works but could be replaced with a type predicate for safer property access. However, given this is read-only inspection of an internal op structure and the code is well-documented, this is acceptable.♻️ Optional: Type guard approach
+interface RawSqlCallWithOp { + op?: { + id?: string; + target?: { details?: { objectType?: string } }; + }; +} + +function hasOpMetadata(call: PostgresOpFactoryCall): call is PostgresOpFactoryCall & RawSqlCallWithOp { + return call.factoryName === 'rawSql' && 'op' in call; +} + function classifyCall(call: PostgresOpFactoryCall): CallCategory { switch (call.factoryName) { // ... other cases ... case 'rawSql': { - const op = ( - call as unknown as { - op?: { - id?: string; - target?: { details?: { objectType?: string } }; - }; - } - ).op; + const op = hasOpMetadata(call) ? call.op : undefined; const objectType = op?.target?.details?.objectType; // ... } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/3-targets/3-targets/postgres/src/core/migrations/issue-planner.ts` around lines 615 - 639, Replace the ad-hoc cast inside the 'rawSql' case with a small type predicate to safely narrow the call shape: add a function (e.g., isRawSqlOp(call): call is { op?: { id?: string; target?: { details?: { objectType?: string } } } }) that checks for the expected properties, then use that guard to access op, objectType and id in the rawSql branch (referencing the rawSql case, the op local variable, objectType and id identifiers); return 'dep' or 'alter' as before but only after the guard confirms the structure to avoid unsafe casting.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/1-framework/3-tooling/cli/src/commands/migration-plan.ts`:
- Around line 450-456: The printed guidance still references the removed command
`prisma-next migration emit --dir`; update the string pushed into lines (the
lines.push call that currently builds `Then run: ${green_('prisma-next migration
emit --dir ' + (result.dir ?? '<dir>'))}`) to instead instruct running the
generated script, e.g. use `node migration.ts` (wrapped with green_ for color)
and keep the same conditional dir handling if needed so the message is
consistent with other guidance in migration-plan.ts and the CLI.
---
Duplicate comments:
In `@packages/3-targets/3-targets/postgres/src/core/migrations/issue-planner.ts`:
- Around line 489-495: The code currently emits a successful AddForeignKeyCall
with empty references ({ table: '', columns: []}) when the FK contract lookup
fails; instead detect when the referenced contract/table/columns are unresolved
(the branch that computes fkName and columns) and return a planner
conflict/error rather than constructing AddForeignKeyCall. Replace the
placeholder AddForeignKeyCall return with a failure return that surfaces a
planner conflict (use the module's existing planner conflict/error helper or
return a Result.err with a clear conflict message mentioning the fkName and
unresolved reference), so the planner fails fast instead of emitting invalid SQL
later.
In
`@packages/3-targets/3-targets/postgres/src/core/migrations/planner-strategies.ts`:
- Around line 434-444: The filter currently drops all dependency_missing issues
so the fallback in issue-planner.ts never runs; change the filter to only remove
dependency_missing issues whose ids are in the handledDependencyIds set (the set
built earlier in this function) instead of removing every dependency_missing;
update the variable used for the remaining array (replace issues.filter((issue)
=> issue.kind !== 'dependency_missing') with a predicate that keeps
non-dependency_missing OR dependency_missing whose id is NOT in
handledDependencyIds) so that unhandled ext:/schema: dependency issues still
reach the default fallback.
---
Nitpick comments:
In `@docs/architecture` docs/subsystems/10. MongoDB Family.md:
- Line 11: Split and rephrase the long sentence around the two emit paths to
make ADR 193's scope explicit: after listing both paths that produce identical
ops.json artifacts (inline emit at the end of `migration plan` and self-emit by
running `./migration.ts`), add a separate sentence stating that ADR 193
("Class-flow as the canonical migration authoring strategy") describes the
overall authoring approach that applies to both paths; also simplify the
parentheticals (keep the ADR 192 reference tied to `ops.json` and retain
`migration.ts` extending `Migration` and `dataTransform({ check, run })`
mentions) so the paragraph reads as two clearer sentences rather than one dense,
nested-parenthetical sentence.
In `@packages/3-targets/3-targets/postgres/src/core/migrations/issue-planner.ts`:
- Around line 615-639: Replace the ad-hoc cast inside the 'rawSql' case with a
small type predicate to safely narrow the call shape: add a function (e.g.,
isRawSqlOp(call): call is { op?: { id?: string; target?: { details?: {
objectType?: string } } } }) that checks for the expected properties, then use
that guard to access op, objectType and id in the rawSql branch (referencing the
rawSql case, the op local variable, objectType and id identifiers); return 'dep'
or 'alter' as before but only after the guard confirms the structure to avoid
unsafe casting.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 686cd6d8-8bc1-4e77-9ca9-9727b18183a7
📒 Files selected for processing (104)
docs/architecture docs/adrs/ADR 193 - Class-flow as the canonical migration authoring strategy.mddocs/architecture docs/subsystems/10. MongoDB Family.mddocs/architecture docs/subsystems/7. Migration System.mdpackages/1-framework/1-core/errors/src/exports/migration.tspackages/1-framework/1-core/errors/src/migration.tspackages/1-framework/1-core/framework-components/src/control-migration-types.tspackages/1-framework/1-core/framework-components/src/exports/control.tspackages/1-framework/1-core/ts-render/README.mdpackages/1-framework/1-core/ts-render/src/render-imports.tspackages/1-framework/3-tooling/cli/README.mdpackages/1-framework/3-tooling/cli/package.jsonpackages/1-framework/3-tooling/cli/src/cli.tspackages/1-framework/3-tooling/cli/src/commands/migration-apply.tspackages/1-framework/3-tooling/cli/src/commands/migration-emit.tspackages/1-framework/3-tooling/cli/src/commands/migration-new.tspackages/1-framework/3-tooling/cli/src/commands/migration-plan.tspackages/1-framework/3-tooling/cli/src/commands/migration-show.tspackages/1-framework/3-tooling/cli/src/commands/migration-status.tspackages/1-framework/3-tooling/cli/src/lib/migration-emit.tspackages/1-framework/3-tooling/cli/src/lib/migration-strategy.tspackages/1-framework/3-tooling/cli/src/utils/formatters/help.tspackages/1-framework/3-tooling/cli/src/utils/formatters/migrations.tspackages/1-framework/3-tooling/cli/test/commands/migration-apply.test.tspackages/1-framework/3-tooling/cli/test/commands/migration-e2e.test.tspackages/1-framework/3-tooling/cli/test/commands/migration-emit.test.tspackages/1-framework/3-tooling/cli/test/commands/migration-plan-command.test.tspackages/1-framework/3-tooling/cli/test/commands/migration-plan.test.tspackages/1-framework/3-tooling/cli/test/commands/migration-ref.test.tspackages/1-framework/3-tooling/cli/test/commands/migration-show.test.tspackages/1-framework/3-tooling/cli/test/lib/migration-emit.test.tspackages/1-framework/3-tooling/cli/test/lib/migration-strategy.test.tspackages/1-framework/3-tooling/cli/test/output.migration-commands.test.tspackages/1-framework/3-tooling/cli/tsdown.config.tspackages/1-framework/3-tooling/cli/vitest.config.tspackages/1-framework/3-tooling/migration/src/exports/migration-ts.tspackages/1-framework/3-tooling/migration/src/io.tspackages/1-framework/3-tooling/migration/src/migration-base.tspackages/1-framework/3-tooling/migration/src/migration-ts.tspackages/1-framework/3-tooling/migration/src/types.tspackages/1-framework/3-tooling/migration/test/attestation.test.tspackages/1-framework/3-tooling/migration/test/fixtures.tspackages/1-framework/3-tooling/migration/test/migration-base.test.tspackages/1-framework/3-tooling/migration/test/migration-ts.test.tspackages/2-mongo-family/9-family/src/core/mongo-emit.tspackages/2-mongo-family/9-family/src/core/mongo-migration.tspackages/2-mongo-family/9-family/src/core/mongo-target-descriptor.tspackages/2-mongo-family/9-family/test/mongo-emit.test.tspackages/2-sql/9-family/package.jsonpackages/2-sql/9-family/src/core/migrations/descriptor-schemas.tspackages/2-sql/9-family/src/core/migrations/operation-descriptors.tspackages/2-sql/9-family/src/core/sql-migration.tspackages/2-sql/9-family/src/exports/operation-descriptors.tspackages/2-sql/9-family/tsdown.config.tspackages/3-mongo-target/1-mongo-target/src/core/mongo-planner.tspackages/3-mongo-target/1-mongo-target/src/core/op-factory-call.tspackages/3-mongo-target/1-mongo-target/src/core/render-typescript.tspackages/3-mongo-target/1-mongo-target/test/mongo-planner.test.tspackages/3-mongo-target/1-mongo-target/test/op-factory-call.test.tspackages/3-targets/3-targets/postgres/package.jsonpackages/3-targets/3-targets/postgres/src/core/migrations/issue-planner.tspackages/3-targets/3-targets/postgres/src/core/migrations/op-factory-call.tspackages/3-targets/3-targets/postgres/src/core/migrations/operation-descriptors.tspackages/3-targets/3-targets/postgres/src/core/migrations/operation-resolver.tspackages/3-targets/3-targets/postgres/src/core/migrations/operations/data-transform.tspackages/3-targets/3-targets/postgres/src/core/migrations/operations/raw.tspackages/3-targets/3-targets/postgres/src/core/migrations/operations/shared.tspackages/3-targets/3-targets/postgres/src/core/migrations/planner-produced-postgres-migration.tspackages/3-targets/3-targets/postgres/src/core/migrations/planner-reconciliation.tspackages/3-targets/3-targets/postgres/src/core/migrations/planner-strategies.tspackages/3-targets/3-targets/postgres/src/core/migrations/planner.tspackages/3-targets/3-targets/postgres/src/core/migrations/postgres-emit.tspackages/3-targets/3-targets/postgres/src/core/migrations/postgres-migration.tspackages/3-targets/3-targets/postgres/src/core/migrations/render-typescript.tspackages/3-targets/3-targets/postgres/src/core/migrations/scaffolding.tspackages/3-targets/3-targets/postgres/src/exports/control.tspackages/3-targets/3-targets/postgres/src/exports/migration-builders.tspackages/3-targets/3-targets/postgres/src/exports/migration.tspackages/3-targets/3-targets/postgres/test/migrations/data-transform.test.tspackages/3-targets/3-targets/postgres/test/migrations/descriptor-planner.scenarios.mdpackages/3-targets/3-targets/postgres/test/migrations/descriptor-planner.scenarios.test.tspackages/3-targets/3-targets/postgres/test/migrations/op-factory-call.test.tspackages/3-targets/3-targets/postgres/test/migrations/operation-resolver.integration.test.tspackages/3-targets/3-targets/postgres/test/migrations/planner.authoring-surface.test.tspackages/3-targets/3-targets/postgres/test/migrations/planner.behavior.test.tspackages/3-targets/3-targets/postgres/test/migrations/planner.case1.test.tspackages/3-targets/3-targets/postgres/test/migrations/planner.reconciliation-unit.test.tspackages/3-targets/3-targets/postgres/test/migrations/planner.reconciliation.issue-to-call.test.tspackages/3-targets/3-targets/postgres/test/migrations/postgres-emit.test.tspackages/3-targets/3-targets/postgres/test/migrations/render-typescript.roundtrip.test.tspackages/3-targets/3-targets/postgres/test/migrations/scaffolding.test.tspackages/3-targets/3-targets/postgres/tsdown.config.tstest/integration/test/cli-journeys/data-transform-not-null-backfill.e2e.test.tstest/integration/test/cli-journeys/data-transform-nullable-tightening.e2e.test.tstest/integration/test/cli-journeys/data-transform-type-change.e2e.test.tstest/integration/test/cli-journeys/data-transform.e2e.test.tstest/integration/test/cli-journeys/migration-round-trip.e2e.test.tstest/integration/test/cli-journeys/mongo-migration.e2e.test.tstest/integration/test/cli.db-update.preflight-gaps.e2e.test.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/cli-journeys/contract-typechange-text.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/db-update-preflight-gaps/contract-add-fk.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/db-update-preflight-gaps/contract-add-required-unique.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/db-update-preflight-gaps/contract.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/db-update-preflight-gaps/prisma-next.config.with-db.tstest/integration/test/utils/journey-test-helpers.ts
💤 Files with no reviewable changes (43)
- packages/2-sql/9-family/tsdown.config.ts
- packages/1-framework/3-tooling/cli/test/commands/migration-ref.test.ts
- packages/1-framework/3-tooling/cli/tsdown.config.ts
- packages/1-framework/3-tooling/migration/test/fixtures.ts
- packages/1-framework/3-tooling/cli/src/utils/formatters/help.ts
- packages/1-framework/3-tooling/cli/vitest.config.ts
- packages/1-framework/3-tooling/migration/src/io.ts
- packages/3-targets/3-targets/postgres/package.json
- packages/1-framework/3-tooling/migration/test/migration-base.test.ts
- packages/3-targets/3-targets/postgres/tsdown.config.ts
- packages/1-framework/1-core/errors/src/exports/migration.ts
- packages/1-framework/3-tooling/migration/test/attestation.test.ts
- packages/1-framework/3-tooling/cli/test/lib/migration-strategy.test.ts
- packages/1-framework/1-core/framework-components/src/exports/control.ts
- packages/1-framework/3-tooling/cli/src/utils/formatters/migrations.ts
- packages/3-targets/3-targets/postgres/src/exports/control.ts
- packages/1-framework/3-tooling/cli/package.json
- packages/2-mongo-family/9-family/src/core/mongo-emit.ts
- packages/3-targets/3-targets/postgres/src/core/migrations/postgres-emit.ts
- packages/3-targets/3-targets/postgres/src/core/migrations/scaffolding.ts
- packages/3-targets/3-targets/postgres/test/migrations/operation-resolver.integration.test.ts
- packages/2-sql/9-family/package.json
- packages/1-framework/3-tooling/cli/test/commands/migration-emit.test.ts
- packages/1-framework/3-tooling/cli/src/lib/migration-strategy.ts
- packages/3-targets/3-targets/postgres/test/migrations/descriptor-planner.scenarios.test.ts
- packages/3-targets/3-targets/postgres/test/migrations/descriptor-planner.scenarios.md
- packages/3-targets/3-targets/postgres/test/migrations/scaffolding.test.ts
- packages/1-framework/3-tooling/cli/src/lib/migration-emit.ts
- packages/1-framework/3-tooling/cli/test/lib/migration-emit.test.ts
- packages/3-targets/3-targets/postgres/src/core/migrations/planner-reconciliation.ts
- packages/3-targets/3-targets/postgres/src/core/migrations/operation-resolver.ts
- test/integration/test/cli-journeys/data-transform.e2e.test.ts
- packages/2-mongo-family/9-family/test/mongo-emit.test.ts
- packages/3-targets/3-targets/postgres/test/migrations/postgres-emit.test.ts
- packages/3-targets/3-targets/postgres/test/migrations/planner.reconciliation.issue-to-call.test.ts
- packages/3-targets/3-targets/postgres/test/migrations/planner.reconciliation-unit.test.ts
- packages/1-framework/3-tooling/cli/src/commands/migration-emit.ts
- packages/3-targets/3-targets/postgres/src/exports/migration-builders.ts
- packages/2-sql/9-family/src/core/migrations/descriptor-schemas.ts
- packages/2-sql/9-family/src/exports/operation-descriptors.ts
- packages/2-sql/9-family/src/core/migrations/operation-descriptors.ts
- packages/1-framework/3-tooling/migration/src/types.ts
- packages/3-targets/3-targets/postgres/src/core/migrations/operation-descriptors.ts
✅ Files skipped from review due to trivial changes (36)
- packages/3-targets/3-targets/postgres/src/core/migrations/operations/raw.ts
- packages/3-targets/3-targets/postgres/src/exports/migration.ts
- packages/1-framework/3-tooling/migration/test/migration-ts.test.ts
- test/integration/test/cli-journeys/data-transform-type-change.e2e.test.ts
- test/integration/test/cli-journeys/data-transform-nullable-tightening.e2e.test.ts
- test/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/cli-journeys/contract-typechange-text.ts
- packages/1-framework/1-core/ts-render/src/render-imports.ts
- packages/3-mongo-target/1-mongo-target/src/core/op-factory-call.ts
- packages/2-mongo-family/9-family/src/core/mongo-migration.ts
- packages/3-targets/3-targets/postgres/test/migrations/op-factory-call.test.ts
- packages/1-framework/3-tooling/cli/src/commands/migration-status.ts
- packages/3-mongo-target/1-mongo-target/test/op-factory-call.test.ts
- packages/3-targets/3-targets/postgres/src/core/migrations/render-typescript.ts
- packages/1-framework/1-core/ts-render/README.md
- packages/3-targets/3-targets/postgres/test/migrations/render-typescript.roundtrip.test.ts
- test/integration/test/cli-journeys/data-transform-not-null-backfill.e2e.test.ts
- packages/3-mongo-target/1-mongo-target/src/core/mongo-planner.ts
- packages/3-targets/3-targets/postgres/test/migrations/data-transform.test.ts
- packages/2-sql/9-family/src/core/sql-migration.ts
- packages/1-framework/3-tooling/cli/test/commands/migration-e2e.test.ts
- packages/3-targets/3-targets/postgres/src/core/migrations/postgres-migration.ts
- packages/3-mongo-target/1-mongo-target/test/mongo-planner.test.ts
- packages/3-targets/3-targets/postgres/src/core/migrations/planner-produced-postgres-migration.ts
- packages/3-targets/3-targets/postgres/test/migrations/planner.authoring-surface.test.ts
- packages/1-framework/3-tooling/cli/test/commands/migration-plan.test.ts
- packages/3-targets/3-targets/postgres/test/migrations/planner.behavior.test.ts
- packages/3-targets/3-targets/postgres/src/core/migrations/operations/data-transform.ts
- packages/1-framework/3-tooling/cli/src/commands/migration-apply.ts
- packages/3-targets/3-targets/postgres/src/core/migrations/op-factory-call.ts
- test/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/db-update-preflight-gaps/prisma-next.config.with-db.ts
- packages/3-targets/3-targets/postgres/src/core/migrations/operations/shared.ts
- test/integration/test/cli-journeys/migration-round-trip.e2e.test.ts
- test/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/db-update-preflight-gaps/contract-add-required-unique.ts
- packages/1-framework/3-tooling/cli/test/output.migration-commands.test.ts
- packages/3-mongo-target/1-mongo-target/src/core/render-typescript.ts
- docs/architecture docs/subsystems/7. Migration System.md
🚧 Files skipped from review as they are similar to previous changes (18)
- packages/1-framework/3-tooling/cli/src/cli.ts
- packages/2-mongo-family/9-family/src/core/mongo-target-descriptor.ts
- packages/1-framework/3-tooling/cli/test/commands/migration-apply.test.ts
- packages/1-framework/3-tooling/cli/test/commands/migration-show.test.ts
- packages/1-framework/3-tooling/migration/src/exports/migration-ts.ts
- packages/1-framework/1-core/errors/src/migration.ts
- test/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/db-update-preflight-gaps/contract.ts
- test/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/db-update-preflight-gaps/contract-add-fk.ts
- packages/1-framework/3-tooling/cli/README.md
- docs/architecture docs/adrs/ADR 193 - Class-flow as the canonical migration authoring strategy.md
- packages/1-framework/3-tooling/migration/src/migration-ts.ts
- packages/1-framework/1-core/framework-components/src/control-migration-types.ts
- packages/1-framework/3-tooling/migration/src/migration-base.ts
- test/integration/test/cli.db-update.preflight-gaps.e2e.test.ts
- packages/1-framework/3-tooling/cli/test/commands/migration-plan-command.test.ts
- test/integration/test/utils/journey-test-helpers.ts
- packages/3-targets/3-targets/postgres/src/core/migrations/planner.ts
- test/integration/test/cli-journeys/mongo-migration.e2e.test.ts
Contributor
Author
|
Review implementer summary (review bodies A05 / A06):
Non- |
wmadden
added a commit
that referenced
this pull request
Apr 22, 2026
…` references Phase 6 removed both the `migration emit` CLI and the inline-emit plumbing from `migration plan`. Update the subsystem doc so it describes self-emission via `./migration.ts` (or `node <dir>/migration.ts`) as the only path that produces `ops.json` and attests `migrationId`. Addresses PR #371 review (coderabbitai) on `7. Migration System.md`.
wmadden
added a commit
that referenced
this pull request
Apr 22, 2026
…on contract lacks matching FK
Previously, the `foreign_key_mismatch` branch of `mapIssueToCall`
fell through to emitting an `AddForeignKeyCall` with empty
`references.table` / `references.columns` when the destination
contract did not carry the FK. That yields a "successful" plan
whose rendered SQL is invalid.
Mirror the sibling fallback behavior at L498-504 and return
`notOk(issueConflict("foreignKeyConflict", ...))` so the planner
surfaces the conflict instead of producing a placeholder call.
Addresses PR #371 review (coderabbitai) on `issue-planner.ts` L489-495.
wmadden
added a commit
that referenced
this pull request
Apr 22, 2026
…ontractSourceProvider
The fixture passed a bare async function where the config type
requires `{ load }` (ContractSourceProvider). Config validation
explicitly checks `contract.source.load must be a function`, so
the fixture would fail as soon as the CLI loaded it.
Wrap the loader in the provider shape so the fixture satisfies
the declared type and the e2e tests that load it can proceed.
Addresses PR #371 review (coderabbitai) on
test/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/db-update-preflight-gaps/prisma-next.config.with-db.ts L14-16.
wmadden
added a commit
that referenced
this pull request
Apr 22, 2026
…th self-emit instruction Phase 6 removed the `migration emit` CLI. `formatMigrationPlanOutput` still instructed users to run `prisma-next migration emit --dir ...` in the placeholder branch, and never told users they needed to self-emit before `migration apply` in the normal success branch. Replace the placeholder-branch instruction with `node <dir>/migration.ts`, and add a `Next:` line to the success branch pointing to the same self-emit command followed by `migration apply`. `rg "migration emit"` in migration-plan.ts is now empty. Addresses PR #371 review bodies (coderabbitai) A05a + A06a.
wmadden
added a commit
that referenced
this pull request
Apr 22, 2026
…r split field asserts Collapse the two `col.rows[0]!.is_nullable` / `column_default` assertions into a single `expect(col.rows[0]).toMatchObject(...)`, consistent with the `prefer-object-matcher` repo rule for 2+ related fields on the same row. Addresses PR #371 review body (coderabbitai) nitpick on test/integration/test/cli.db-update.preflight-gaps.e2e.test.ts L157-158.
…ranches Adds three e2e scenarios that pin the externally-visible behavior of two walk-schema planner branches not covered by the existing db update test suites: - FK backing-index creation on an existing table when a new foreign key with default index is added via contract evolution. - Empty-table-guarded NOT-NULL column add on an empty table (unique constraint makes the shared temp-default strategy unsafe). - Empty-table guard failure path on a non-empty table. These tests act as the regression net for the Phase 4 issue-planner absorption; all three pass against todays walk-schema planner.
Phase 4 of postgres class-flow migrations: collapses the old walk-schema PostgresMigrationPlanner onto planIssues by (a) porting the gap-branch logic into class-flow strategies and (b) teaching planIssues to enforce operation-class policy and produce walk-schema-compatible DDL ordering. Strategies (planner-strategies.ts): - storageTypePlanCallStrategy delegates to codec hooks planTypeOperations for non-enum storage types and lifts emitted ops to RawSqlCall so component-declared labels / prechecks / postchecks survive untouched. - dependencyInstallCallStrategy re-invokes collectInitDependencies at plan time and consumes every dependency_missing issue (including non-Postgres ones) to prevent the default mapper from misreporting them as conflicts. - notNullAddColumnCallStrategy covers the two walk-schema branches for NOT NULL columns without a default: shared-temp-default on non-empty tables and empty-table-guarded add on fresh/empty tables. - StrategyContext now carries schema, policy and frameworkComponents so strategies can introspect live schema state and dispatch component dependencies without retrieving them from hooks. - Added a recipe flag on strategy results so sequential composites (enumChangeCallStrategy, notNullBackfillCallStrategy, typeChangeCallStrategy, nullableTighteningCallStrategy) stay contiguous, while independent dep/type strategies flow into the DDL sequencing buckets. planIssues (issue-planner.ts): - Accepts schema, policy and frameworkComponents; defaults remain lenient so direct callers (tests, planDescriptors) keep working. - Classifies calls into dep / drop / table / column / alter / primaryKey / unique / index / foreignKey buckets and emits them in walk-schema-compatible order; recipe strategy output is inserted verbatim between the column and alter buckets. - Gates calls against policy.allowedOperationClasses when a policy is explicitly provided, surfacing disallowed ops as typed conflicts via conflictKindForCall / locationForCall (typeMismatch, nullabilityConflict, foreignKeyConflict, indexIncompatible). - type_mismatch handler now computes formatTypeExpected via buildExpectedFormatType so the alterColumnType postcheck matches Postgres format_type output (fixing text to timestamp/timestamptz reconciliation). - extra_primary_key falls back to <table>_pkey when the verifier reports no constraint name, matching the old reconciliation planner. - classifyCall inspects RawSqlCall underlying op ids / objectType to route extension, schema and type install ops into the dep bucket. PostgresMigrationPlanner (planner.ts): - Reduced to a thin wrapper that verifies the live schema, builds codec hooks / storage types, and delegates to planIssues with dbUpdateCallStrategies (storageTypePlan, dependencyInstall, notNullAddColumn). fromContract is passed as null because db update has no old contract; type_mismatch and nullability_mismatch fall through to mapIssueToCall for direct destructive DDL, matching the old walk-schema reconciliation path. - Drops every buildX method and PlanItem intermediate; the class no longer renders DDL directly. Unit tests (planner.behavior.test.ts, planner.case1.test.ts): - Pinned to the new canonical DDL ordering (deps -> drops -> tables -> columns -> recipe -> alters -> PKs -> uniques -> indexes -> FKs) and to RawSqlCall label/SQL for component-declared install ops. Policy gating expectations reflect the new typed conflict kinds.
planner-reconciliation.ts (buildReconciliationPlan + buildX helpers) was the old walk-schema reconciliation path. PostgresMigrationPlanner now routes reconciliation through planIssues (dbUpdateCallStrategies + mapIssueToCall), so this file and its dedicated unit tests are dead. - Remove planner-reconciliation.ts - Remove planner.reconciliation-unit.test.ts (direct unit tests of buildReconciliationPlan - covered end-to-end by the integration suite now) - Remove planner.reconciliation.issue-to-call.test.ts (same rationale: class-flow IR for reconciliation is pinned by issue-planner.test.ts and the integration tests)
Removes the Postgres descriptor-flow authoring surface now that the class-flow issue planner handles every db update and migration plan scenario: - operation-descriptors.ts / operation-resolver.ts / scaffolding.ts (renderDescriptorTypeScript) and the migration-builders export bundle - operations/data-transform.ts (only consumed by the old resolver) - descriptor-planner scenarios, operation-resolver integration, and scaffolding tests Package exports, tsdown entries, and the internal planDescriptors stub inside issue-planner.ts are trimmed in lockstep. issue-planner.ts now documents itself as a class-flow planner and exports only the types that the class-flow call strategies consume.
Trim the framework-side descriptor authoring surface now that Postgres (the last descriptor-flow target) has migrated to class-flow: - TargetMigrationsCapability: drop planWithDescriptors, resolveDescriptors, and renderDescriptorTypeScript hooks. The emit docstring is reworded to describe class-flow-only dispatch. - control-migration-types: remove the OperationDescriptor framework shape and its re-export. - family-sql: delete the operation-descriptors export bundle (descriptor-schemas.ts, operation-descriptors.ts, exports/*) and its tsdown entry plus package.json export. - migration-tools: drop evaluateMigrationTs (legacy descriptor-flow loader) from migration-ts.ts and its re-export. - errors: remove errorTargetHasIncompleteMigrationCapabilities (PN-MIG-2011) and errorPlanDoesNotSupportAuthoringSurface (PN-MIG-2010); both are internal wiring errors that only fire when the CLI has to choose between the two flows. Callers will be cleaned up in the CLI-surface commit.
Drop the `migrationStrategy` selector and descriptor-flow branch from `migration plan` / `migration emit`. Remove `planningStrategy` from `MigrationHints`, its schema, and all test fixtures. Purge `descriptor-flow` references from docstrings on migration-new, mongo-target-descriptor, framework planner types, and postgres operation shared utilities.
Deletes the `migration emit` CLI command, the `lib/migration-emit.ts` helper, and the per-target `emit` capability (`postgresEmit`, `mongoEmit`). Under class-flow, migrations self-emit: running `node migration.ts` serializes `ops.json` and attests `migration.json` via `Migration.run(import.meta.url, ...)`. Also rewrites user-facing strings in `migration new/show/apply/status` and the CLI README to point authors at `node migration.ts` instead of the removed command, and drops the companion exports, build config, vitest coverage entries, and tests.
- Replace `runMigrationEmit` with a direct `tsx migration.ts` spawn now that `migration emit` has been removed; callers keep the same `--dir <path>` interface. - Delete `data-transform.e2e.test.ts`: it drove the deprecated `createBuilders()` descriptor-flow API which no longer exists. Postgres `notNullBackfill` recipe (addColumn → dataTransform → setNotNull) remains covered by `planner.reconciliation.integration.test.ts`, and hand-authored `dataTransform` is covered end-to-end in `mongo-migration.e2e.test.ts`. - Rewrite `mongo-migration.e2e.test.ts` emit helper to spawn `tsx` on `migration.ts`; update docblock to describe the self-emit flow. - Scrub stale `emitMigration` reference from `PN-MIG-2002` docstring.
Class-flow is now simply how migrations are authored — descriptor-flow is deleted and the qualifier no longer carries any contrast. Scrub the phrase from docstrings, code comments, and test labels across the framework, family, and target packages. No behavior or public API changes.
…d data-transform tests Rename class-flow-round-trip.e2e.test.ts to migration-round-trip.e2e.test.ts and scrub class-flow phrasing from the journey and fixture docstrings and test labels. No coverage changes.
Class-flow is now simply how migrations are authored, so the subsystem docs no longer frame it as the "new way" or contrast it with descriptor-flow. Update subsystem 7 (Migration System) and subsystem 10 (MongoDB Family) to describe the migration authoring surface plainly, and drop the stale migration emit path references. Scrub the same phrasing from the CLI README. ADR 193 and ADR 196 link titles are preserved as historical record.
Class-flow is now simply how migrations are authored — the contrast with descriptor-flow no longer exists. Add a short Status block at the top of ADR 193 recording that the decision stands but the terminology has been scrubbed from code and current docs; the ADR body is preserved verbatim as a historical record.
…` references Phase 6 removed both the `migration emit` CLI and the inline-emit plumbing from `migration plan`. Update the subsystem doc so it describes self-emission via `./migration.ts` (or `node <dir>/migration.ts`) as the only path that produces `ops.json` and attests `migrationId`. Addresses PR #371 review (coderabbitai) on `7. Migration System.md`.
…on contract lacks matching FK
Previously, the `foreign_key_mismatch` branch of `mapIssueToCall`
fell through to emitting an `AddForeignKeyCall` with empty
`references.table` / `references.columns` when the destination
contract did not carry the FK. That yields a "successful" plan
whose rendered SQL is invalid.
Mirror the sibling fallback behavior at L498-504 and return
`notOk(issueConflict("foreignKeyConflict", ...))` so the planner
surfaces the conflict instead of producing a placeholder call.
Addresses PR #371 review (coderabbitai) on `issue-planner.ts` L489-495.
…ontractSourceProvider
The fixture passed a bare async function where the config type
requires `{ load }` (ContractSourceProvider). Config validation
explicitly checks `contract.source.load must be a function`, so
the fixture would fail as soon as the CLI loaded it.
Wrap the loader in the provider shape so the fixture satisfies
the declared type and the e2e tests that load it can proceed.
Addresses PR #371 review (coderabbitai) on
test/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/db-update-preflight-gaps/prisma-next.config.with-db.ts L14-16.
…th self-emit instruction Phase 6 removed the `migration emit` CLI. `formatMigrationPlanOutput` still instructed users to run `prisma-next migration emit --dir ...` in the placeholder branch, and never told users they needed to self-emit before `migration apply` in the normal success branch. Replace the placeholder-branch instruction with `node <dir>/migration.ts`, and add a `Next:` line to the success branch pointing to the same self-emit command followed by `migration apply`. `rg "migration emit"` in migration-plan.ts is now empty. Addresses PR #371 review bodies (coderabbitai) A05a + A06a.
…r split field asserts Collapse the two `col.rows[0]!.is_nullable` / `column_default` assertions into a single `expect(col.rows[0]).toMatchObject(...)`, consistent with the `prefer-object-matcher` repo rule for 2+ related fields on the same row. Addresses PR #371 review body (coderabbitai) nitpick on test/integration/test/cli.db-update.preflight-gaps.e2e.test.ts L157-158.
wmadden
force-pushed
the
tml-2286-phase-3-consolidate
branch
from
a227d2c to
c8a850d
Compare
|
Caution Review failedAn error occurred during the review process. Please try again later. 📝 WalkthroughWalkthroughRemoved descriptor-flow and related emit plumbing; deleted the Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Developer as Dev
participant CLI as CLI
participant FS as Filesystem
participant MigrationTs as migration.ts
participant Attestor as AttestService
Dev->>CLI: scaffold / edit migration.ts
Dev->>MigrationTs: node migrations/<dir>/migration.ts (exec)
MigrationTs->>FS: write ops.json
MigrationTs->>Attestor: request attest (compute migrationId)
Attestor->>FS: write migration.json (migrationId)
CLI->>FS: read migration.json / ops.json when plan/apply/status
CLI->>Attestor: verify attest state when applying
(Colored blocks not required in sequence output; core interactions shown.) Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
…urney tests Before phase 3, `migration plan` auto-emitted the draft migration inline, so journey tests could call `migration plan` → `migration apply` directly. After the `migration emit` CLI was removed, plan writes an un-attested `migration.ts` and `migration apply` refuses drafts (PN-RUN-3000). The draft must be self-emitted by running `tsx migration.ts` before apply can attest and run it. Introduce a shared `runMigrationPlanAndEmit` helper in journey-test-helpers that wraps `runMigrationPlan` + `runMigrationEmit` against the latest migration dir, and swap it in for the 13 journey tests that plan-then-apply (or plan- then-status-asserting-pending/applied). Tests that intentionally check plan failure, plan no-op, or feed a draft into a status-only path are left on the plain helper. Also fixes the mongo migration journey by adding an explicit `migrationEmit` call between `plan0` and `apply0` using the test file's local helpers.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/1-framework/3-tooling/migration/src/migration-base.ts (1)
168-172:⚠️ Potential issue | 🟡 MinorNormalize
existing.hintsbefore reuse to avoid re-emitting removed keys.
buildAttestedManifest()currently carriesexisting?.hintsas-is. If an oldermigration.jsonincludes legacyhints.planningStrategy, this path will keep writing it back, even thoughMigrationHintsnow only includesused,applied, andplannerVersion(packages/1-framework/3-tooling/migration/src/types.ts:4-8).Suggested patch
- hints: existing?.hints ?? { - used: [], - applied: [], - plannerVersion: '2.0.0', - }, + hints: existing?.hints + ? { + used: existing.hints.used, + applied: existing.hints.applied, + plannerVersion: existing.hints.plannerVersion, + } + : { + used: [], + applied: [], + plannerVersion: '2.0.0', + },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/1-framework/3-tooling/migration/src/migration-base.ts` around lines 168 - 172, buildAttestedManifest currently reuses existing?.hints wholesale which preserves legacy keys (e.g. planningStrategy); instead, normalize existing.hints by creating a new object that explicitly picks/assigns only the MigrationHints fields (used, applied, plannerVersion) and falls back to defaults when absent (referencing buildAttestedManifest and the MigrationHints shape in types.ts) so removed keys are not re-emitted.test/integration/test/cli-journeys/mongo-migration.e2e.test.ts (1)
203-246:⚠️ Potential issue | 🟠 MajorSelf-emit before asserting
ops.json/migrationId.After this PR,
migration planonly scaffolds the draft package; it does not writeops.jsonor attestmigrationId. These assertions will fail unless the test runs the generatedmigration.tsfirst, like the live-database flow below already does.Suggested fix
const migrationDir = getLatestMigrationDir(ctx); + const emitPlan = await migrationEmit(ctx, [ + '--dir', + `migrations/${basename(migrationDir)}`, + ]); + expect( + emitPlan.exitCode, + `migration emit: ${emitPlan.stdout}\n${emitPlan.stderr}`, + ).toBe(0); const migrationTs = readFileSync(join(migrationDir, 'migration.ts'), 'utf-8');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/integration/test/cli-journeys/mongo-migration.e2e.test.ts` around lines 203 - 246, The test currently asserts ops.json and migration.json/migrationId immediately after calling migrationPlan and reading the scaffolded migration.ts; after the PR the plan step only scaffolds the draft package and does not emit ops.json or attest migrationId, so update the test to execute the generated migration script before those assertions — i.e., after calling migrationPlan(ctx, ['--name','initial']) and locating getLatestMigrationDir(ctx), run the generated migration.ts (which contains Migration.run(import.meta.url)) so it writes ops.json and produces a migrationId, then continue to read and assert ops.json and migration.json.packages/1-framework/3-tooling/cli/test/commands/migration-plan-command.test.ts (1)
295-318:⚠️ Potential issue | 🟡 MinorTest title overstates behavior compared to assertions.
Line 295 says the test “returns pendingPlaceholders”, but the body only asserts
writeMigrationTscall count (Line 317). Rename the test or also assert the returned envelope.Suggested minimal fix (rename for accuracy)
- it('writes migration.ts and returns pendingPlaceholders when placeholders are present', async () => { + it('writes migration.ts when placeholders are present', async () => {As per coding guidelines, "Make each
it()statement read as a behavior assertion under its specific context without requiring readers to inspect implementation details."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/1-framework/3-tooling/cli/test/commands/migration-plan-command.test.ts` around lines 295 - 318, The test title overstates behavior; update the it() description to reflect what's actually asserted (that writeMigrationTs is called) or add an assertion verifying the returned pendingPlaceholders envelope from executeCommand; specifically, in the test that uses createMigrationPlanCommand() and executeCommand(command, ['--json']), either rename the test to something like "writes migration.ts when placeholders are present" to match the existing expectation of mocks.writeMigrationTs being called, or keep the current title and add an assertion on the command result (the returned envelope/pendingPlaceholders) in addition to the existing expect(mocks.writeMigrationTs).toHaveBeenCalledTimes(1) to ensure the test title matches behavior.
♻️ Duplicate comments (2)
docs/architecture docs/subsystems/7. Migration System.md (1)
62-62:⚠️ Potential issue | 🟡 MinorMake the diagram match the self-emit-only explanation.
Line 62 says self-emission is the only path that produces
ops.json, but the flowchart above still showsrenderOps()writingops.jsondirectly. The diagram and the prose should describe the same lifecycle.As per coding guidelines "Keep docs current (READMEs, rules, links) and prefer links to canonical docs over long comments".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/architecture` docs/subsystems/7. Migration System.md at line 62, Update the diagram so it matches the prose that "self-emission is the only path that produces ops.json": remove or change the arrow that shows renderOps() writing ops.json directly and instead show Migration.run (in migration.ts) detecting "is main" and performing self-emission to produce ops.json and compute/persist the content-addressed migrationId; ensure renderOps() is depicted as an internal step called by Migration.run during self-emission rather than an independent external writer of ops.json.packages/1-framework/3-tooling/cli/README.md (1)
1044-1059:⚠️ Potential issue | 🟡 MinorUpdate the earlier
migration planwalkthrough to match this self-emit flow.This section now says attestation happens only after running
node migrations/<dir>/migration.ts, but the earliermigration plansection still says the command writesops.jsonand attestsmigrationId. Right now the README documents both behaviors.As per coding guidelines "Keep docs current (READMEs, rules, links) and prefer links to canonical docs over long comments".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/1-framework/3-tooling/cli/README.md` around lines 1044 - 1059, The README is inconsistent: earlier "migration plan" walkthrough claims it writes ops.json and attests migrationId, but the later "Emitting ops.json and computing migrationId" explains that migrations self-emit when you run node migrations/<dir>/migration.ts and Migration.run(import.meta.url, ...) writes ops.json and migration.json (and exits with PN-MIG-2001 if placeholder() exists). Update the earlier "migration plan" section to reflect the self-emit flow — remove or change any wording that says the plan command writes ops.json/attests migrationId, and instead instruct users to run the generated migration.ts (or call Migration.run(import.meta.url, ...)) to produce ops.json and migration.json, mention placeholder() behavior and PN-MIG-2001 so both sections match.
🧹 Nitpick comments (2)
packages/3-targets/3-targets/postgres/test/migrations/planner.case1.test.ts (1)
1-728: Split this test file; it exceeds the repository’s test size limit.At ~728 lines, this file is over the 500-line limit. Please split by concern (for example: additive-ordering vs column-default rendering) to keep maintenance and failure triage manageable.
As per coding guidelines: "
**/*.test.ts: Keep test files under 500 lines ... split into multiple files."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/3-targets/3-targets/postgres/test/migrations/planner.case1.test.ts` around lines 1 - 728, This test file exceeds the repo limit; split it into multiple smaller test files by concern (e.g., move the "PostgresMigrationPlanner - when database is empty" describe block into one file, "PostgresMigrationPlanner - column defaults" into another, and "composite unique constraint DDL" / "parameterized column types" into their own files) while preserving behavior of helpers like createPostgresMigrationPlanner, createTestContract, contractWithTable, and planTableSql; extract shared helpers (createTestContract, emptySchema, createFrameworkComponentWithDependencies, createPostgresMigrationPlanner import) into a small test-utils file if needed, update imports in the new test files, and ensure each resulting *.test.ts is under 500 lines and retains the same describe/it names and assertions so CI and local runs behave identically.packages/3-targets/3-targets/postgres/src/core/migrations/issue-planner.ts (1)
615-638: Consider typingRawSqlCall.opto avoid the inline type assertion.The classification logic accesses
call.op?.target?.details?.objectTypevia a manual type assertion. While this works, it's fragile — changes toRawSqlCall's internal structure could silently break classification.If
RawSqlCallexposes a typedopproperty, consider importing the type rather than inline-asserting. Otherwise, this pattern is acceptable for internal classification.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/3-targets/3-targets/postgres/src/core/migrations/issue-planner.ts` around lines 615 - 638, The code in the 'rawSql' branch is using an inline type assertion to access call.op?.target?.details?.objectType which is fragile; replace the ad-hoc assertion by importing/using the declared RawSqlCall type (or the specific Op type) and type the variable `call` (or the extracted `op`) accordingly so you can safely reference `RawSqlCall.op` without casting; update the case 'rawSql' block to declare `const op: RawSqlCall['op']` (or the appropriate OpInterface) before reading `objectType`, and remove the inline assertion to make classification logic (the `objectType` check, id prefix checks and returned 'dep'/'alter' values) statically typed and resilient to structural changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/architecture` docs/subsystems/7. Migration System.md:
- Around line 312-313: Update the "Plan" documentation to reflect the current
behavior: the CLI command implemented in migration-plan.ts returns a successful
pendingPlaceholders result (it catches placeholder throws) so `prisma-next
migration plan` warns that the draft still needs edits instead of surfacing
`PN-MIG-2001`; note that `PN-MIG-2001` is now only raised when the developer
later self-emits the draft that still contains `placeholder(...)`. Mention the
relevant symbols `migration-plan.ts`, `pendingPlaceholders`, `PN-MIG-2001`, and
`placeholder(...)` in the doc so readers know where the behavior is implemented
and where the error can appear.
In `@packages/3-targets/3-targets/postgres/src/core/migrations/issue-planner.ts`:
- Around line 656-658: The emptySchemaIR() function currently returns an extra
unused property `types: {}` that does not exist on the SqlSchemaIR interface;
update emptySchemaIR() to return exactly the properties required by SqlSchemaIR
(e.g., `{ tables: {}, dependencies: [] }`) so the shape matches the interface
and remove the `types` key, keeping the function name `emptySchemaIR` and type
annotation `: SqlSchemaIR` intact.
In `@test/integration/test/cli-journeys/mongo-migration.e2e.test.ts`:
- Around line 134-152: migrationEmit constructs migrationTs by always joining
ctx.testDir and relDir, which breaks when relDir is an absolute path; update
migrationEmit to detect absolute paths (use path.isAbsolute) and, if relDir is
absolute, build migrationTs as path.join(relDir, 'migration.ts'), otherwise keep
the existing path.join(ctx.testDir, relDir, 'migration.ts'); ensure you
import/is-using path.isAbsolute so TSX_BIN is invoked with the correct file
path.
---
Outside diff comments:
In
`@packages/1-framework/3-tooling/cli/test/commands/migration-plan-command.test.ts`:
- Around line 295-318: The test title overstates behavior; update the it()
description to reflect what's actually asserted (that writeMigrationTs is
called) or add an assertion verifying the returned pendingPlaceholders envelope
from executeCommand; specifically, in the test that uses
createMigrationPlanCommand() and executeCommand(command, ['--json']), either
rename the test to something like "writes migration.ts when placeholders are
present" to match the existing expectation of mocks.writeMigrationTs being
called, or keep the current title and add an assertion on the command result
(the returned envelope/pendingPlaceholders) in addition to the existing
expect(mocks.writeMigrationTs).toHaveBeenCalledTimes(1) to ensure the test title
matches behavior.
In `@packages/1-framework/3-tooling/migration/src/migration-base.ts`:
- Around line 168-172: buildAttestedManifest currently reuses existing?.hints
wholesale which preserves legacy keys (e.g. planningStrategy); instead,
normalize existing.hints by creating a new object that explicitly picks/assigns
only the MigrationHints fields (used, applied, plannerVersion) and falls back to
defaults when absent (referencing buildAttestedManifest and the MigrationHints
shape in types.ts) so removed keys are not re-emitted.
In `@test/integration/test/cli-journeys/mongo-migration.e2e.test.ts`:
- Around line 203-246: The test currently asserts ops.json and
migration.json/migrationId immediately after calling migrationPlan and reading
the scaffolded migration.ts; after the PR the plan step only scaffolds the draft
package and does not emit ops.json or attest migrationId, so update the test to
execute the generated migration script before those assertions — i.e., after
calling migrationPlan(ctx, ['--name','initial']) and locating
getLatestMigrationDir(ctx), run the generated migration.ts (which contains
Migration.run(import.meta.url)) so it writes ops.json and produces a
migrationId, then continue to read and assert ops.json and migration.json.
---
Duplicate comments:
In `@docs/architecture` docs/subsystems/7. Migration System.md:
- Line 62: Update the diagram so it matches the prose that "self-emission is the
only path that produces ops.json": remove or change the arrow that shows
renderOps() writing ops.json directly and instead show Migration.run (in
migration.ts) detecting "is main" and performing self-emission to produce
ops.json and compute/persist the content-addressed migrationId; ensure
renderOps() is depicted as an internal step called by Migration.run during
self-emission rather than an independent external writer of ops.json.
In `@packages/1-framework/3-tooling/cli/README.md`:
- Around line 1044-1059: The README is inconsistent: earlier "migration plan"
walkthrough claims it writes ops.json and attests migrationId, but the later
"Emitting ops.json and computing migrationId" explains that migrations self-emit
when you run node migrations/<dir>/migration.ts and
Migration.run(import.meta.url, ...) writes ops.json and migration.json (and
exits with PN-MIG-2001 if placeholder() exists). Update the earlier "migration
plan" section to reflect the self-emit flow — remove or change any wording that
says the plan command writes ops.json/attests migrationId, and instead instruct
users to run the generated migration.ts (or call Migration.run(import.meta.url,
...)) to produce ops.json and migration.json, mention placeholder() behavior and
PN-MIG-2001 so both sections match.
---
Nitpick comments:
In `@packages/3-targets/3-targets/postgres/src/core/migrations/issue-planner.ts`:
- Around line 615-638: The code in the 'rawSql' branch is using an inline type
assertion to access call.op?.target?.details?.objectType which is fragile;
replace the ad-hoc assertion by importing/using the declared RawSqlCall type (or
the specific Op type) and type the variable `call` (or the extracted `op`)
accordingly so you can safely reference `RawSqlCall.op` without casting; update
the case 'rawSql' block to declare `const op: RawSqlCall['op']` (or the
appropriate OpInterface) before reading `objectType`, and remove the inline
assertion to make classification logic (the `objectType` check, id prefix checks
and returned 'dep'/'alter' values) statically typed and resilient to structural
changes.
In `@packages/3-targets/3-targets/postgres/test/migrations/planner.case1.test.ts`:
- Around line 1-728: This test file exceeds the repo limit; split it into
multiple smaller test files by concern (e.g., move the "PostgresMigrationPlanner
- when database is empty" describe block into one file,
"PostgresMigrationPlanner - column defaults" into another, and "composite unique
constraint DDL" / "parameterized column types" into their own files) while
preserving behavior of helpers like createPostgresMigrationPlanner,
createTestContract, contractWithTable, and planTableSql; extract shared helpers
(createTestContract, emptySchema, createFrameworkComponentWithDependencies,
createPostgresMigrationPlanner import) into a small test-utils file if needed,
update imports in the new test files, and ensure each resulting *.test.ts is
under 500 lines and retains the same describe/it names and assertions so CI and
local runs behave identically.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: d0490c00-5b61-4cd8-99d0-ac7626a054e5
📒 Files selected for processing (117)
docs/architecture docs/adrs/ADR 193 - Class-flow as the canonical migration authoring strategy.mddocs/architecture docs/subsystems/10. MongoDB Family.mddocs/architecture docs/subsystems/7. Migration System.mdpackages/1-framework/1-core/errors/src/exports/migration.tspackages/1-framework/1-core/errors/src/migration.tspackages/1-framework/1-core/framework-components/src/control-migration-types.tspackages/1-framework/1-core/framework-components/src/exports/control.tspackages/1-framework/1-core/ts-render/README.mdpackages/1-framework/1-core/ts-render/src/render-imports.tspackages/1-framework/3-tooling/cli/README.mdpackages/1-framework/3-tooling/cli/package.jsonpackages/1-framework/3-tooling/cli/src/cli.tspackages/1-framework/3-tooling/cli/src/commands/migration-apply.tspackages/1-framework/3-tooling/cli/src/commands/migration-emit.tspackages/1-framework/3-tooling/cli/src/commands/migration-new.tspackages/1-framework/3-tooling/cli/src/commands/migration-plan.tspackages/1-framework/3-tooling/cli/src/commands/migration-show.tspackages/1-framework/3-tooling/cli/src/commands/migration-status.tspackages/1-framework/3-tooling/cli/src/lib/migration-emit.tspackages/1-framework/3-tooling/cli/src/lib/migration-strategy.tspackages/1-framework/3-tooling/cli/src/utils/formatters/help.tspackages/1-framework/3-tooling/cli/src/utils/formatters/migrations.tspackages/1-framework/3-tooling/cli/test/commands/migration-apply.test.tspackages/1-framework/3-tooling/cli/test/commands/migration-e2e.test.tspackages/1-framework/3-tooling/cli/test/commands/migration-emit.test.tspackages/1-framework/3-tooling/cli/test/commands/migration-plan-command.test.tspackages/1-framework/3-tooling/cli/test/commands/migration-plan.test.tspackages/1-framework/3-tooling/cli/test/commands/migration-ref.test.tspackages/1-framework/3-tooling/cli/test/commands/migration-show.test.tspackages/1-framework/3-tooling/cli/test/lib/migration-emit.test.tspackages/1-framework/3-tooling/cli/test/lib/migration-strategy.test.tspackages/1-framework/3-tooling/cli/test/output.migration-commands.test.tspackages/1-framework/3-tooling/cli/tsdown.config.tspackages/1-framework/3-tooling/cli/vitest.config.tspackages/1-framework/3-tooling/migration/src/exports/migration-ts.tspackages/1-framework/3-tooling/migration/src/io.tspackages/1-framework/3-tooling/migration/src/migration-base.tspackages/1-framework/3-tooling/migration/src/migration-ts.tspackages/1-framework/3-tooling/migration/src/types.tspackages/1-framework/3-tooling/migration/test/attestation.test.tspackages/1-framework/3-tooling/migration/test/fixtures.tspackages/1-framework/3-tooling/migration/test/migration-base.test.tspackages/1-framework/3-tooling/migration/test/migration-ts.test.tspackages/2-mongo-family/9-family/src/core/mongo-emit.tspackages/2-mongo-family/9-family/src/core/mongo-migration.tspackages/2-mongo-family/9-family/src/core/mongo-target-descriptor.tspackages/2-mongo-family/9-family/test/mongo-emit.test.tspackages/2-sql/9-family/package.jsonpackages/2-sql/9-family/src/core/migrations/descriptor-schemas.tspackages/2-sql/9-family/src/core/migrations/operation-descriptors.tspackages/2-sql/9-family/src/core/sql-migration.tspackages/2-sql/9-family/src/exports/operation-descriptors.tspackages/2-sql/9-family/tsdown.config.tspackages/3-mongo-target/1-mongo-target/src/core/mongo-planner.tspackages/3-mongo-target/1-mongo-target/src/core/op-factory-call.tspackages/3-mongo-target/1-mongo-target/src/core/render-typescript.tspackages/3-mongo-target/1-mongo-target/test/mongo-planner.test.tspackages/3-mongo-target/1-mongo-target/test/op-factory-call.test.tspackages/3-targets/3-targets/postgres/package.jsonpackages/3-targets/3-targets/postgres/src/core/migrations/issue-planner.tspackages/3-targets/3-targets/postgres/src/core/migrations/op-factory-call.tspackages/3-targets/3-targets/postgres/src/core/migrations/operation-descriptors.tspackages/3-targets/3-targets/postgres/src/core/migrations/operation-resolver.tspackages/3-targets/3-targets/postgres/src/core/migrations/operations/data-transform.tspackages/3-targets/3-targets/postgres/src/core/migrations/operations/raw.tspackages/3-targets/3-targets/postgres/src/core/migrations/operations/shared.tspackages/3-targets/3-targets/postgres/src/core/migrations/planner-produced-postgres-migration.tspackages/3-targets/3-targets/postgres/src/core/migrations/planner-reconciliation.tspackages/3-targets/3-targets/postgres/src/core/migrations/planner-strategies.tspackages/3-targets/3-targets/postgres/src/core/migrations/planner.tspackages/3-targets/3-targets/postgres/src/core/migrations/postgres-emit.tspackages/3-targets/3-targets/postgres/src/core/migrations/postgres-migration.tspackages/3-targets/3-targets/postgres/src/core/migrations/render-typescript.tspackages/3-targets/3-targets/postgres/src/core/migrations/scaffolding.tspackages/3-targets/3-targets/postgres/src/exports/control.tspackages/3-targets/3-targets/postgres/src/exports/migration-builders.tspackages/3-targets/3-targets/postgres/src/exports/migration.tspackages/3-targets/3-targets/postgres/test/migrations/data-transform.test.tspackages/3-targets/3-targets/postgres/test/migrations/descriptor-planner.scenarios.mdpackages/3-targets/3-targets/postgres/test/migrations/descriptor-planner.scenarios.test.tspackages/3-targets/3-targets/postgres/test/migrations/op-factory-call.test.tspackages/3-targets/3-targets/postgres/test/migrations/operation-resolver.integration.test.tspackages/3-targets/3-targets/postgres/test/migrations/planner.authoring-surface.test.tspackages/3-targets/3-targets/postgres/test/migrations/planner.behavior.test.tspackages/3-targets/3-targets/postgres/test/migrations/planner.case1.test.tspackages/3-targets/3-targets/postgres/test/migrations/planner.reconciliation-unit.test.tspackages/3-targets/3-targets/postgres/test/migrations/planner.reconciliation.issue-to-call.test.tspackages/3-targets/3-targets/postgres/test/migrations/postgres-emit.test.tspackages/3-targets/3-targets/postgres/test/migrations/render-typescript.roundtrip.test.tspackages/3-targets/3-targets/postgres/test/migrations/scaffolding.test.tspackages/3-targets/3-targets/postgres/tsdown.config.tstest/integration/test/cli-journeys/adopt-migrations.e2e.test.tstest/integration/test/cli-journeys/converging-paths.e2e.test.tstest/integration/test/cli-journeys/data-transform-not-null-backfill.e2e.test.tstest/integration/test/cli-journeys/data-transform-nullable-tightening.e2e.test.tstest/integration/test/cli-journeys/data-transform-type-change.e2e.test.tstest/integration/test/cli-journeys/data-transform.e2e.test.tstest/integration/test/cli-journeys/diamond-convergence.e2e.test.tstest/integration/test/cli-journeys/divergence-and-refs.e2e.test.tstest/integration/test/cli-journeys/drift-deleted-root.e2e.test.tstest/integration/test/cli-journeys/drift-migration-dag.e2e.test.tstest/integration/test/cli-journeys/interleaved-db-update.e2e.test.tstest/integration/test/cli-journeys/migration-apply-edge-cases.e2e.test.tstest/integration/test/cli-journeys/migration-round-trip.e2e.test.tstest/integration/test/cli-journeys/migration-status-diagnostics.e2e.test.tstest/integration/test/cli-journeys/mongo-migration.e2e.test.tstest/integration/test/cli-journeys/multi-step-migration.e2e.test.tstest/integration/test/cli-journeys/ref-routing.e2e.test.tstest/integration/test/cli-journeys/rollback-cycle.e2e.test.tstest/integration/test/cli-journeys/schema-evolution-migrations.e2e.test.tstest/integration/test/cli.db-update.preflight-gaps.e2e.test.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/cli-journeys/contract-typechange-text.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/db-update-preflight-gaps/contract-add-fk.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/db-update-preflight-gaps/contract-add-required-unique.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/db-update-preflight-gaps/contract.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/db-update-preflight-gaps/prisma-next.config.with-db.tstest/integration/test/utils/journey-test-helpers.ts
💤 Files with no reviewable changes (43)
- packages/1-framework/1-core/framework-components/src/exports/control.ts
- packages/1-framework/3-tooling/migration/src/io.ts
- packages/1-framework/3-tooling/cli/vitest.config.ts
- packages/3-targets/3-targets/postgres/tsdown.config.ts
- packages/1-framework/3-tooling/migration/test/migration-base.test.ts
- packages/2-sql/9-family/src/core/migrations/descriptor-schemas.ts
- packages/1-framework/3-tooling/cli/test/commands/migration-ref.test.ts
- packages/1-framework/3-tooling/migration/test/fixtures.ts
- packages/1-framework/3-tooling/cli/src/utils/formatters/help.ts
- packages/3-targets/3-targets/postgres/src/exports/control.ts
- packages/1-framework/3-tooling/cli/test/lib/migration-strategy.test.ts
- packages/2-mongo-family/9-family/test/mongo-emit.test.ts
- packages/2-sql/9-family/package.json
- packages/1-framework/3-tooling/migration/test/attestation.test.ts
- packages/3-targets/3-targets/postgres/test/migrations/scaffolding.test.ts
- packages/3-targets/3-targets/postgres/package.json
- packages/1-framework/3-tooling/cli/package.json
- packages/1-framework/1-core/errors/src/exports/migration.ts
- packages/1-framework/3-tooling/cli/src/lib/migration-strategy.ts
- packages/1-framework/3-tooling/cli/src/lib/migration-emit.ts
- packages/1-framework/3-tooling/migration/src/types.ts
- packages/3-targets/3-targets/postgres/src/core/migrations/scaffolding.ts
- packages/1-framework/3-tooling/cli/test/lib/migration-emit.test.ts
- test/integration/test/cli-journeys/data-transform.e2e.test.ts
- packages/2-sql/9-family/tsdown.config.ts
- packages/1-framework/3-tooling/cli/tsdown.config.ts
- packages/1-framework/3-tooling/cli/src/utils/formatters/migrations.ts
- packages/1-framework/3-tooling/cli/test/commands/migration-emit.test.ts
- packages/3-targets/3-targets/postgres/src/core/migrations/operation-descriptors.ts
- packages/3-targets/3-targets/postgres/test/migrations/planner.reconciliation-unit.test.ts
- packages/3-targets/3-targets/postgres/test/migrations/postgres-emit.test.ts
- packages/3-targets/3-targets/postgres/test/migrations/operation-resolver.integration.test.ts
- packages/3-targets/3-targets/postgres/test/migrations/planner.reconciliation.issue-to-call.test.ts
- packages/3-targets/3-targets/postgres/src/core/migrations/operation-resolver.ts
- packages/2-sql/9-family/src/core/migrations/operation-descriptors.ts
- packages/3-targets/3-targets/postgres/src/core/migrations/planner-reconciliation.ts
- packages/2-mongo-family/9-family/src/core/mongo-emit.ts
- packages/3-targets/3-targets/postgres/src/exports/migration-builders.ts
- packages/3-targets/3-targets/postgres/test/migrations/descriptor-planner.scenarios.md
- packages/3-targets/3-targets/postgres/test/migrations/descriptor-planner.scenarios.test.ts
- packages/2-sql/9-family/src/exports/operation-descriptors.ts
- packages/1-framework/3-tooling/cli/src/commands/migration-emit.ts
- packages/3-targets/3-targets/postgres/src/core/migrations/postgres-emit.ts
✅ Files skipped from review due to trivial changes (37)
- packages/1-framework/1-core/ts-render/src/render-imports.ts
- packages/1-framework/3-tooling/migration/test/migration-ts.test.ts
- packages/3-mongo-target/1-mongo-target/test/op-factory-call.test.ts
- packages/3-targets/3-targets/postgres/src/core/migrations/operations/raw.ts
- packages/2-sql/9-family/src/core/sql-migration.ts
- packages/3-targets/3-targets/postgres/src/exports/migration.ts
- packages/3-targets/3-targets/postgres/test/migrations/op-factory-call.test.ts
- test/integration/test/cli-journeys/data-transform-nullable-tightening.e2e.test.ts
- packages/3-mongo-target/1-mongo-target/src/core/render-typescript.ts
- packages/3-targets/3-targets/postgres/src/core/migrations/operations/data-transform.ts
- test/integration/test/cli-journeys/data-transform-not-null-backfill.e2e.test.ts
- packages/2-mongo-family/9-family/src/core/mongo-migration.ts
- packages/1-framework/1-core/ts-render/README.md
- docs/architecture docs/adrs/ADR 193 - Class-flow as the canonical migration authoring strategy.md
- packages/1-framework/3-tooling/cli/test/commands/migration-apply.test.ts
- test/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/cli-journeys/contract-typechange-text.ts
- packages/3-mongo-target/1-mongo-target/src/core/mongo-planner.ts
- packages/3-targets/3-targets/postgres/src/core/migrations/postgres-migration.ts
- packages/1-framework/3-tooling/migration/src/exports/migration-ts.ts
- packages/3-targets/3-targets/postgres/src/core/migrations/op-factory-call.ts
- packages/3-targets/3-targets/postgres/test/migrations/render-typescript.roundtrip.test.ts
- packages/3-targets/3-targets/postgres/test/migrations/planner.authoring-surface.test.ts
- packages/1-framework/3-tooling/cli/test/output.migration-commands.test.ts
- packages/3-mongo-target/1-mongo-target/src/core/op-factory-call.ts
- test/integration/test/cli-journeys/data-transform-type-change.e2e.test.ts
- packages/3-targets/3-targets/postgres/src/core/migrations/operations/shared.ts
- packages/3-targets/3-targets/postgres/src/core/migrations/render-typescript.ts
- packages/3-targets/3-targets/postgres/src/core/migrations/planner-produced-postgres-migration.ts
- packages/1-framework/3-tooling/cli/test/commands/migration-plan.test.ts
- packages/3-targets/3-targets/postgres/test/migrations/data-transform.test.ts
- test/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/db-update-preflight-gaps/contract-add-required-unique.ts
- test/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/db-update-preflight-gaps/prisma-next.config.with-db.ts
- packages/3-mongo-target/1-mongo-target/test/mongo-planner.test.ts
- test/integration/test/cli.db-update.preflight-gaps.e2e.test.ts
- test/integration/test/cli-journeys/migration-round-trip.e2e.test.ts
- packages/1-framework/3-tooling/cli/src/commands/migration-apply.ts
- packages/1-framework/1-core/framework-components/src/control-migration-types.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- packages/2-mongo-family/9-family/src/core/mongo-target-descriptor.ts
- packages/3-targets/3-targets/postgres/test/migrations/planner.behavior.test.ts
- packages/1-framework/3-tooling/cli/test/commands/migration-e2e.test.ts
- docs/architecture docs/subsystems/10. MongoDB Family.md
- packages/1-framework/3-tooling/cli/test/commands/migration-show.test.ts
- packages/1-framework/3-tooling/cli/src/commands/migration-status.ts
- packages/1-framework/3-tooling/migration/src/migration-ts.ts
- test/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/db-update-preflight-gaps/contract-add-fk.ts
- test/integration/test/utils/journey-test-helpers.ts
- test/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/db-update-preflight-gaps/contract.ts
…smatches in CI Extends the previous self-emit sweep to cover the remaining tests failing in CI after PR 3 removed `migration emit` / `emit` SPI: - Pick the latest migration dir by mtime instead of alphabetical sort. Same-minute sibling directories share the `YYYYMMDDTHHMM_` prefix and tie-break on the slug, which caused `runMigrationPlanAndEmit` to emit the wrong dir (and leave the freshly-planned migration as a draft). Fixes cascading apply/status failures across converging-paths, diamond-convergence, drift-*, multi-step, ref-routing, rollback-cycle, interleaved-db-update, adopt-migrations, divergence-and-refs, and migration-status-diagnostics journeys. - Switch `plan0` → `apply0` pairs to `runMigrationPlanAndEmit` in four dataTransform journey tests and migration-round-trip. - Add an inline self-emit step to `cli.migration-apply.e2e.test.ts` by having its local `runMigrationPlan` helper spawn `tsx migration.ts` on the freshly-planned draft after a successful plan. - Mongo offline journey: assert the `users` literal with single quotes (prettier rewrites it on write) and self-emit the initial plan so `ops.json` exists for the downstream assertions. - `migration-plan-details` H/I journeys: remove the obsolete `migrationId` assertion (the field was removed from `MigrationPlanResult` in PR 3), self-emit H.02 so H.03s `verifyMigration` sees an attested package, and self-emit I.01 so I.03 plans from the expected base.
…tegies `migration plan` needs to emit `DataTransformCall` placeholder stubs for data-losing schema changes (enum shrink, unsafe type change, nullable tightening, NOT-NULL backfill) so users can hand-fill the transform logic. `db update` / `db init` need direct destructive DDL instead. Phase 4 accidentally collapsed both onto `dbUpdateCallStrategies`, which silently regressed `migration plan` to emit pre-filled `rawSql` enum-rebuild ops (etc.) without any placeholders. This broke every data-transform e2e journey and the migration round-trip test, because scaffolded `migration.ts` no longer contained `placeholder(...)` calls for the user to fill. Restore the `data` operation-class dispatch the planner had prior to Phase 4: - `PostgresMigrationPlanner` now picks `migrationPlanCallStrategies` when `policy.allowedOperationClasses` includes `data`, and `dbUpdateCallStrategies` otherwise. - `SqlMigrationPlannerPlanOptions` gains an optional `fromContract` so `typeChangeCallStrategy` / `nullableTighteningCallStrategy` can compare from/to column shapes (only `migration plan` supplies one). - The CLI `migration plan` command threads the resolved `fromContract` through to the planner. Also fix two tangential CI failures that blocked the same run: - `runMigrationEmit` / mongo `migrationEmit` test helpers were joining `ctx.testDir` with the caller-provided `--dir` path unconditionally. When callers pass an absolute migration-dir path, POSIX `join` concatenates them and tsx fails with `ERR_MODULE_NOT_FOUND` on the duplicated path. Detect absolute paths and skip the prefix. - `migration-plan-details` journey I.04 reads `ops.json` from disk to assert destructive ops survived rendering. Since `migration plan` no longer auto-emits, insert an explicit `runMigrationEmit` between the plan step and the on-disk read.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
test/integration/test/cli-journeys/mongo-migration.e2e.test.ts (2)
141-162:migrationEmitimplementation is functionally identical torunMigrationEmitin journey-test-helpers.ts.This inline implementation duplicates the logic from
test/integration/test/utils/journey-test-helpers.ts(context snippet 1 shows they're nearly identical). Consider importingrunMigrationEmitfrom the shared helper to reduce duplication, unless this test intentionally stays standalone for MongoDB-specific context isolation.♻️ Proposed refactor to use shared helper
If the test can use the shared helper, replace the inline
migrationEmitwith:+import { runMigrationEmit } from '../utils/journey-test-helpers'; + +// Adapt JourneyCtx to JourneyContext shape if needed +async function migrationEmit(ctx: JourneyCtx, args: readonly string[] = []): Promise<RunResult> { + return runMigrationEmit({ testDir: ctx.testDir, configPath: ctx.configPath, outputDir: ctx.outputDir }, args); +} -async function migrationEmit(ctx: JourneyCtx, args: readonly string[] = []): Promise<RunResult> { - const rest = [...args]; - const dirIdx = rest.indexOf('--dir'); - ... // ~20 lines removed -}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/integration/test/cli-journeys/mongo-migration.e2e.test.ts` around lines 141 - 162, The local migrationEmit function duplicates logic from runMigrationEmit in the shared helper; remove the inline migrationEmit and import runMigrationEmit from test/integration/test/utils/journey-test-helpers.ts (or the module that exports it), then replace calls to migrationEmit with runMigrationEmit so the test reuses the shared implementation (ensure the imported name matches the exported symbol and that any differences in args/ctx handling are adapted).
45-45: Consider usingnode:pathper repo conventions for test files.Per learnings, "for tests under test/integration/test/ (including cli-journeys and all subdirectories), prefer using node:path for path operations." This file imports from
patheon line 45.-import { basename, isAbsolute, join, resolve } from 'pathe'; +import { basename, isAbsolute, join, resolve } from 'node:path';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/integration/test/cli-journeys/mongo-migration.e2e.test.ts` at line 45, The test imports path helpers from 'pathe' which violates the repo convention for integration tests; change the import statement "import { basename, isAbsolute, join, resolve } from 'pathe';" to use the Node built-in (e.g. "import { basename, isAbsolute, join, resolve } from 'node:path';") and ensure usages of basename, isAbsolute, join, and resolve in this file (mongo-migration.e2e.test.ts) continue to reference those functions unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@test/integration/test/cli-journeys/mongo-migration.e2e.test.ts`:
- Around line 141-162: The local migrationEmit function duplicates logic from
runMigrationEmit in the shared helper; remove the inline migrationEmit and
import runMigrationEmit from test/integration/test/utils/journey-test-helpers.ts
(or the module that exports it), then replace calls to migrationEmit with
runMigrationEmit so the test reuses the shared implementation (ensure the
imported name matches the exported symbol and that any differences in args/ctx
handling are adapted).
- Line 45: The test imports path helpers from 'pathe' which violates the repo
convention for integration tests; change the import statement "import {
basename, isAbsolute, join, resolve } from 'pathe';" to use the Node built-in
(e.g. "import { basename, isAbsolute, join, resolve } from 'node:path';") and
ensure usages of basename, isAbsolute, join, and resolve in this file
(mongo-migration.e2e.test.ts) continue to reference those functions unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 818b963c-c58f-47ba-8430-24c1a66f0e70
📒 Files selected for processing (6)
packages/1-framework/3-tooling/cli/src/commands/migration-plan.tspackages/2-sql/9-family/src/core/migrations/types.tspackages/3-targets/3-targets/postgres/src/core/migrations/planner.tstest/integration/test/cli-journeys/migration-plan-details.e2e.test.tstest/integration/test/cli-journeys/mongo-migration.e2e.test.tstest/integration/test/utils/journey-test-helpers.ts
…n Plan step Line 312 claimed `migration plan` reports PN-MIG-2001 when placeholders remain, but `migration-plan.ts` catches the placeholder throw and returns a successful `pendingPlaceholders` envelope instead. PN-MIG-2001 is only raised when the developer runs the draft `migration.ts` (self-emit) with an unfilled `placeholder(...)`. Rewrite step 2 of the Planner-led flow to match this behavior and reference the relevant symbols.
…aIR()`
`SqlSchemaIR` only defines `tables`, optional `annotations`, and
`dependencies`. The extra `types: {}` literal was hidden behind an
`as SqlSchemaIR` cast. Returning the literal directly matches the
interface and drops the cast.
`buildAttestedManifest()` previously spread `existing?.hints` wholesale, which preserved any legacy keys (e.g. `planningStrategy`) lingering in migration.json files scaffolded by older CLI versions. Introduce a `normalizeHints()` helper that projects to the known `MigrationHints` shape (`used`, `applied`, `plannerVersion`) with safe defaults, so a refreshed `migration.json` is always schema-clean regardless of what was on disk before. Add a test covering an existing manifest with a legacy `planningStrategy` key and assert it does not leak through self-emit.
The flowchart previously showed `renderOps() --> ops.json` as an independent writer, contradicting the prose at step 5 that says self-emission is the only path that produces `ops.json`. Replace the direct edge with a self-emit edge labelled "./migration.ts (self-emit, invokes renderOps())" so the diagram and text describe the same lifecycle.
The `prisma-next migration plan` section claimed the command writes `migration.json` + `ops.json` and attests `migrationId`, contradicting the later "Emitting ops.json and computing migrationId" section which documents self-emit as the only writer. Rewrite step 5 to describe the scaffolded outputs (draft `migration.ts`, draft `migration.json`, contract bookends) and link to the self-emit section. Add a bullet covering `pendingPlaceholders` behavior and clarify that `PN-MIG-2001` is only raised at self-emit.
Match the `test/integration/test/` directory convention, which uses `node:path` for all path operations. No behavioral change.
Contributor
Author
Review implementation updateAddressed the pending Implemented
Already fixed in earlier commits (acknowledgement)
Intentionally deferred (out_of_scope / not_actionable)
All four CodeRabbit review threads (#4155211732, #4155271029, #4156378323, #4156798478) are now reconciled; the two unresolved inline threads (A01, A02) have been replied to and resolved. |
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
2 participants
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.
closes TML-2286
Intent
After PR 2 ("the flip"), Postgres
migration planruns through the class-flow / issue-based planner, but a lot of legacy machinery is now dead weight: two parallel planners (walk-schema fordb update, issue-based formigration plan), the whole descriptor-flow surface, themigration emitCLI command, and "class-flow" as a distinguishing qualifier that no longer contrasts with anything. This PR collapses them into a single pipeline, deletes the dead surfaces, and makes the terminology implicit.By the end: one Postgres planner, one authoring surface (
Migration+Migration.run), one framework capability shape (createPlanner,createRunner,contractToSchema), and noclass-flow/descriptor-flowqualifiers in prose.Change map
dbUpdateCallStrategiesabsorbed from walk-schemaTargetMigrationsCapabilityreduced to 3 membersrunMigrationEmitrewritten as self-emit bridgeplanIssuesper strategy +strategies: []overrideThe story
db updatee2e doesn't reach: FK backing-index creation on a pre-existing table, and the empty-table-guarded NOT-NULL column add under an unsafe shared-temp-default. The docstring of cli.db-update.preflight-gaps.e2e.test.ts calls this out as the regression net for the absorption.planIssues. New strategies appear in planner-strategies.ts —storageTypePlanCallStrategy(codec-hook storage types),dependencyInstallCallStrategy(component-declared install ops),notNullAddColumnCallStrategy(shared-temp-default / empty-table-guarded NOT-NULL add) — and are exported asdbUpdateCallStrategies.planIssuesgains an optionalstrategiesparameter sodb updatecan pass this list whilemigration plancontinues to use the default recipe chain.planner-reconciliation.ts. With the walk-schema logic absorbed, the 798-line reconciliation module has no remaining callers and is removed wholesale.planner.tsreduces to a 181-line shell that runsverifySqlSchema, builds aSchemaIssue[], and callsplanIssues.operation-descriptors.ts,operation-resolver.ts,descriptor-planner.ts, andscaffolding.tsleave the Postgres package. Their only callers were deleted in PR 2; this removes the definition sites.OperationDescriptor,DataTransformDescriptor, and the capability membersplanWithDescriptors,resolveDescriptors,renderDescriptorTypeScript, andemitare removed from control-migration-types.ts. The capability is nowcreatePlanner+createRunner+contractToSchema.migrationStrategyselector goes away. migration-plan.ts drops its "if descriptor else class-flow" branch and becomes linear. The manifest writer stops emittinghints.planningStrategy(now that there is only one strategy).migration emit. The CLI command file, its lib module, its registration incli.ts, theTargetMigrationsCapability.emitmember, and both target implementations (postgresEmit,mongoEmit) leave the repo.Migration.run(import.meta.url, M)at the bottom of eachmigration.tsis now the only sanctioned emit driver (ADR 196).runMigrationEmitis reimplemented as a wrapper aroundtsx <migration.ts>. Existing callers keep their['--dir', relDir]invocation shape so test-side commit churn stays small; the helper name is intentionally preserved as a source-compatibility bridge, flagged in its docstring.packages/**andtest/**. Theclass-flow-round-trip.e2e.test.tsjourney is renamed to describe the behavior it tests rather than the implementation strategy it was originally introduced to cover.migration emitCLI" and the framing of class-flow as "the new way". The CLI README drops similar framing. ADR link titles are preserved as historical record.Behavior changes & evidence
Single planner pipeline:
db updateandmigration planmerge — walk-schema deleted, issue-based planner consumes both.buildDatabaseDependencyOperationsvsdependency_missingduplication). After PR 2 left the walk-schema planner as the only non-class-flow code path, the spec's R2.10 ("the planner is a single pipeline") became the final cleanup.planSqlpath, which verifies the schema and delegates toplanIssuesdb updatestrategies absorbed from walk-schema, and thedbUpdateCallStrategiesexportplanIssuesharness, now reached by both entrypointsstrategies: []override testdb updatee2es continue to run unchangedRemoved CLI command: Before →
prisma-next migration emit --dir <dir>regeneratedops.json. After → the user runs./migration.ts(ornode migration.ts/tsx migration.ts);Migration.run(...)at the file'"'"'s tail produces the artifacts in-process.migration emitexisted to drive the descriptor flow, where evaluatingmigration.tsyielded a descriptor list rather than a self-emitting class. With descriptor-flow gone and everymigration.tsclass-flow, self-emit is the only path that matters (ADR 196). The command would only have been a confusing second way to do the same thing.packages/1-framework/3-tooling/cli/src/commands/migration-emit.tsandpackages/1-framework/3-tooling/cli/src/lib/migration-emit.tsTargetMigrationsCapability.emitremoved from control-migration-types.tspostgresEmitandmongoEmitfiles deletedrunMigrationEmitrewired to runtsx <migration.ts>, exercising the self-emit path in all data-transform e2esDescriptor-flow surface is gone: The framework-level interface members (
planWithDescriptors,resolveDescriptors,renderDescriptorTypeScript,emit) and all descriptor types (OperationDescriptor,DataTransformDescriptor,PostgresMigrationOpDescriptor,MigrationDescriptorArraySchema) are deleted. The CLI losesmigrationStrategy,emitDescriptorFlow,evaluateMigrationTs, and the manifest loseshints.planningStrategy.operation-descriptors.ts,operation-resolver.ts,descriptor-planner.ts,planner-reconciliation.ts,scaffolding.ts(+ its test) underpackages/3-targets/3-targets/postgres/src/core/migrations/if (strategy === '"'"'descriptor'"'"')branch is removedpackages/andtest/on this branch returns zero production matches.Terminology change (non-behavioral): "class-flow" is removed as a qualifier from code comments, test labels, test-file names, and subsystem docs. ADRs are unchanged except for a Status addendum on ADR 193.
Compatibility / migration / risk
migration emit: anyone who hadprisma-next migration emit --dir <path>in their scripts or CI must switch to running the migration file directly (./migration.ts,node migration.ts, ortsx migration.ts). The migration file'"'"'s shebang +Migration.run(import.meta.url, M)produce byte-identicalops.jsonandmigration.jsonto whatmigration emitproduced in PR 2.migration applyis untouched. Operators who already applied migrations built with the descriptor flow or themigration emitcommand are unaffected — the runner readsops.jsonandmigration.json, both of which remain in the same format and are still attested viamigrationId.TargetMigrationsCapabilityshape changed. Any downstream plugin that implementedplanWithDescriptors,resolveDescriptors,renderDescriptorTypeScript, oremitmust remove those methods — the compiler will enforce it, since the type no longer has them. No downstream plugin shipped outside the monorepo is known.Follow-ups / open questions
dbUpdateCallStrategiesand the default recipe chain are still pending (noted in the local review as F01).migration emitreferences remain in subsystem 7 (Mermaid + Planner-led flow + Helpful Commands +migrationIdsection), ADRs 192 / 199 / 200, and the final hint line inmigration-plan.ts(F02, F03, F06). Happy to scrub these in this PR or a doc-only follow-up — reviewer'"'"'s call.locationForCallduck-typing inissue-planner.tsis listed as a deferred follow-up for the cross-targetTypeScriptRenderableMigration<TCall, TOp>consolidation (already enumerated inplan.md §"Known follow-ups").Non-goals / intentionally out of scope
plan.md §Phase 7).projects/postgres-class-flow-migrations/(archived project record).OpFactoryCallor any public type name (theclass-flowqualifier does not appear in them).Summary by CodeRabbit
Chores
Documentation
Tests