feat(migrations): data migration support with descriptor-based planner#268
Open
feat(migrations): data migration support with descriptor-based planner#268
Conversation
📝 WalkthroughWalkthroughThe PR introduces a descriptor-based migration authoring system enabling manual data transformation operations. It adds framework types for operation descriptors and schema issues, implements new Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant CLI as CLI:<br/>migration new
participant Config as Config<br/>Loader
participant Contract as Contract<br/>Reader
participant FSys as Filesystem
participant Planner as Descriptor<br/>Planner
participant Verify as CLI:<br/>migration verify
participant Resolver as Descriptor<br/>Resolver
participant Apply as CLI:<br/>migration apply
User->>CLI: migration new --name X
CLI->>Config: load config
Config->>Contract: read contract.json
Contract-->>CLI: contract object
CLI->>FSys: read existing migrations
FSys-->>CLI: bundle list
CLI->>CLI: compute from hash<br/>(latest migration)
alt from == to contract hash
CLI-->>User: error: no changes
end
CLI->>FSys: create migration directory
CLI->>FSys: write MigrationManifest<br/>(migrationId: null)
CLI->>FSys: scaffold migration.ts
CLI-->>User: migration created
User->>User: edit migration.ts<br/>add descriptors
User->>Verify: migration verify --config X
Verify->>FSys: read migration.ts
Verify->>Verify: evaluate migration.ts
Verify-->>Verify: descriptors array
Verify->>Config: load framework context
Verify->>Resolver: resolveDescriptors(descriptors)
Resolver-->>Verify: MigrationPlanOperation[]
Verify->>FSys: write ops.json
Verify->>FSys: compute migrationId
Verify->>FSys: attest (set migrationId)
Verify-->>User: verification complete
User->>Apply: migration apply
Apply->>FSys: read migration bundles
Apply->>Apply: load attested only
Apply->>Planner: execute operations
alt data transform operation
Planner->>Planner: execute check (optional)
Planner->>Planner: execute run steps
Planner->>Planner: re-execute check
else schema operation
Planner->>Planner: precheck/execute/postcheck
end
Apply-->>User: apply complete
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
@prisma-next/mongo-runtime
@prisma-next/family-mongo
@prisma-next/sql-runtime
@prisma-next/family-sql
@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/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/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-pipeline-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: |
saevarb
force-pushed
the
feat/data-migrations
branch
3 times, most recently
from
214212a to
c0811c9
Compare
saevarb
force-pushed
the
feat/data-migrations
branch
from
c0811c9 to
0b63d7f
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
packages/1-framework/3-tooling/migration/src/migration-ts.ts
Dismissed
Show dismissed
Hide dismissed
saevarb
force-pushed
the
feat/data-migrations
branch
from
0b63d7f to
db07b45
Compare
saevarb
changed the title
saevarb
force-pushed
the
feat/data-migrations
branch
from
de15296 to
cc004c7
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
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-status.ts (1)
439-466:⚠️ Potential issue | 🟠 MajorDraft-only repos still fall through the “no migrations” path.
You now collect
draftsand emit a warning, but the earlyattested.length === 0success return below still omits bothdraftsandgraph. A fresh repo aftermigration newwill therefore report “No migrations found” in JSON/default rendering even though a draft exists, which breaks the new draft-visibility workflow.🤖 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-status.ts` around lines 439 - 466, The current early return that triggers when attested.length === 0 omits drafts and graph causing draft-only repos to report "No migrations"; update the logic in migration-status.ts (around the attested/drafts/graph handling after loadAllBundles) so that the success return includes drafts and graph when drafts.length > 0 (either change the early return condition to consider drafts or construct the success payload to include drafts and graph even when attested is empty), ensuring functions/variables attested, drafts, graph and the success/notOk return path are updated accordingly.
🧹 Nitpick comments (9)
turbo.json (1)
44-46: Consider addinginputsandoutputsfor optimal caching.The new
emittask lacksinputsandoutputsarrays. Most other tasks in this config define them so Turbo can:
- Invalidate cache when relevant files change
- Skip execution when inputs haven't changed
- Track generated artifacts
Based on the AI summary, this task generates contract/migration inputs after the CLI build completes. Consider specifying which source files trigger the emit (inputs) and what artifacts it produces (outputs).
📦 Example configuration with inputs/outputs
"@prisma-next/sql-builder#emit": { - "dependsOn": ["@prisma-next/cli#build"] + "dependsOn": ["@prisma-next/cli#build"], + "inputs": ["src/**", "package.json"], + "outputs": ["dist/**"] }Note: Adjust the actual paths based on what files the emit task reads and generates.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@turbo.json` around lines 44 - 46, Add explicit inputs and outputs to the "@prisma-next/sql-builder#emit" Turbo task so caching is accurate: update the task definition for "@prisma-next/sql-builder#emit" to include an "inputs" array that lists the source files this emit step reads (e.g., schema, contract, migration, and CLI build artifacts such as "packages/sql-builder/src/**", "prisma/schema.prisma", or any contract templates) and an "outputs" array that lists the generated artifacts the task produces (e.g., "packages/sql-builder/dist/**", "packages/sql-builder/generated/**" or the specific contract/migration files), ensuring the task consumes the CLI build result (dependsOn remains) and Turbo can invalidate/skip runs correctly.packages/3-targets/6-adapters/postgres/src/core/enum-control-hooks.ts (1)
700-717: Clarify reorder-only enum diffs in the verification message.When values are only reordered, this path reports
+[ ] -[ ], which is hard to interpret. Consider a dedicated “reordered” message branch for rebuild diffs with no adds/removals.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/3-targets/6-adapters/postgres/src/core/enum-control-hooks.ts` around lines 700 - 717, The verification message currently prints +[ ] -[ ] for reorder-only enum diffs; update the message generation in the enum diff path (around determineEnumDiff, diff.kind and the object with kind 'enum_values_changed') to detect reorder-only cases (either diff.kind === 'reorder' if available, or when addedValues.length === 0 && removedValues.length === 0) and emit a dedicated message like `Enum type "${typeName}" values reordered: [${desired.join(', ')}]` instead of the +/-. Keep the existing messages for 'add_values' and rebuild cases with real adds/removals.packages/3-targets/3-targets/postgres/src/core/migrations/operation-descriptors.ts (2)
12-66: Consider consolidating duplicate type imports.Types are re-exported on lines 12-57, then imported again on lines 59-66 for internal use. This mirrors the pattern in the sql-family file and could be consolidated.
🤖 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/operation-descriptors.ts` around lines 12 - 66, The file currently duplicates type exports and imports from '@prisma-next/family-sql/operation-descriptors' — remove the second import block (lines importing Buildable, DataTransformDescriptor, SqlMigrationOpDescriptor, builders as structuralBuilders, TODO, TodoMarker) and consolidate into a single import/export so the same type symbols (e.g., Buildable, SqlMigrationOpDescriptor, TODO, TodoMarker, DataTransformDescriptor) are imported once and re-exported together with the operation helpers (addColumn, addEnumValues, createTable, etc.); update the top export/import so internal usage and re-exports reference the same imported symbols instead of importing them twice.
105-132: VerifyresolveInputhandles all Buildable cases correctly.The
resolveInputfunction checks'build' in inputbutinputcould be aFunction(callback) which also has properties. The type narrowing assumes that ifinputhas abuildmethod, it's aBuildable, but a function with a coincidentalbuildproperty could slip through.This is likely safe in practice since migration authors won't accidentally create functions with
buildmethods, but worth a note.🛡️ More defensive check
function resolveInput(input: QueryPlanInput): QueryPlanInput { if (typeof input === 'symbol' || typeof input === 'function') return input; - if ('build' in input && typeof (input as Buildable).build === 'function') { + if (typeof input === 'object' && input !== null && 'build' in input && typeof input.build === 'function') { return (input as Buildable).build(); } return input; }🤖 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/operation-descriptors.ts` around lines 105 - 132, Summary: resolveInput currently checks 'build' in input without ensuring input is an object, so a Function with a build property could be misclassified; change the guard to only treat non-null objects as Buildable. Update resolveInput to first return for typeof input === 'symbol' || typeof input === 'function' (keep existing) and then check if typeof input === 'object' && input !== null && 'build' in input && typeof (input as Buildable).build === 'function' before calling build; also scan usages such as dataTransform's handling of options.check and options.run to ensure they call resolveInput only for non-function/non-symbol inputs (or rely on the updated resolveInput) so functions aren't mistaken for Buildable.packages/2-sql/9-family/src/core/migrations/operation-descriptors.ts (1)
13-61: Consider consolidating duplicate type imports.Types are re-exported on lines 13-36, then imported again on lines 40-61 for internal use. You could use a single import with
typemodifier and re-export in one statement.♻️ Suggested consolidation
-// Re-export types and schemas from the schema source of truth -export type { - AddColumnDescriptor, - // ... all types -} from './descriptor-schemas'; - -export { MigrationDescriptorArraySchema } from './descriptor-schemas'; - -import type { - AddColumnDescriptor, - // ... same types again -} from './descriptor-schemas'; +// Re-export types and schemas from the schema source of truth +export type { + AddColumnDescriptor, + AddEnumValuesDescriptor, + // ... all types +} from './descriptor-schemas'; + +export { MigrationDescriptorArraySchema } from './descriptor-schemas'; + +// Use inline type imports in function signatures instead of +// duplicating the import block, or use a single combined import: +import type * as Schemas from './descriptor-schemas'; +// Then reference as Schemas.AddColumnDescriptor, etc.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-sql/9-family/src/core/migrations/operation-descriptors.ts` around lines 13 - 61, Duplicate type imports for descriptors (AddColumnDescriptor, AddEnumValuesDescriptor, AddForeignKeyDescriptor, AddPrimaryKeyDescriptor, AddUniqueDescriptor, AlterColumnTypeDescriptor, CreateDependencyDescriptor, CreateEnumTypeDescriptor, CreateIndexDescriptor, CreateTableDescriptor, DropColumnDescriptor, DropConstraintDescriptor, DropDefaultDescriptor, DropEnumTypeDescriptor, DropIndexDescriptor, DropNotNullDescriptor, DropTableDescriptor, RenameTypeDescriptor, SetDefaultDescriptor, SetNotNullDescriptor) are present; remove the second import block and consolidate into a single statement by re-exporting the types from './descriptor-schemas' (use a single export type { ... } from './descriptor-schemas';) while keeping the existing export { MigrationDescriptorArraySchema } from './descriptor-schemas'; so there is only one import/re-export for those descriptor types and no duplicate import type block.packages/1-framework/3-tooling/cli/test/utils/formatters/graph-migration-mapper.test.ts (1)
199-201: Strengthen dashed-edge assertions to lock endpoint behaviorThese checks confirm edge presence, but they don’t fully validate edge routing. Add
from/toendpoint assertions in both detached-node tests to avoid false positives when dashed edges are attached to the wrong node.Suggested assertion hardening
const dashedEdge = result.graph.edges.find((e) => e.style === 'dashed'); expect(dashedEdge).toBeDefined(); + expect(dashedEdge!.from).toBe(result.options.spineTarget); expect(dashedEdge!.to).toBe(sid('DETACHED_HASH')); @@ const dashedEdge = result.graph.edges.find((e) => e.style === 'dashed'); expect(dashedEdge).toBeDefined(); + expect(dashedEdge!.from).toBe(result.options.spineTarget); + expect(dashedEdge!.to).toBe(sid('OFF_GRAPH'));Also applies to: 221-223
🤖 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/utils/formatters/graph-migration-mapper.test.ts` around lines 199 - 201, The tests currently only assert that a dashed edge exists and its `to` endpoint, which can hide cases where the edge's `from` is incorrect; update the detached-node tests that find `dashedEdge` (the assertion around `result.graph.edges.find((e) => e.style === 'dashed')`) to also assert `dashedEdge!.from` equals the expected source node id (use the same `sid(...)` helper for the expected source) in addition to the existing `dashedEdge!.to === sid('DETACHED_HASH')` check; apply the same strengthening to the second detached-node test (the one near the other assertions at 221-223) so both tests lock both endpoints.packages/3-targets/3-targets/postgres/package.json (1)
48-48: Route migration-builder APIs through canonical plane entrypoints
./migration-builderslooks migration-plane specific but is exported as a standalone subpath. Prefer exposing this via./control(or moving truly shared bits to shared-plane exports) to keep plane boundaries consistent.As per coding guidelines: “Do not export plane-specific files via the wrong entrypoint… Export multi-plane entrypoints from
package.jsonwith separate./adapter,./control, and./runtimesubpaths.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/3-targets/3-targets/postgres/package.json` at line 48, Replace the standalone subpath export "./migration-builders" with a control-plane entrypoint by moving its mapping to the package.json control export (i.e., stop exporting "./migration-builders" and instead expose the same "./dist/migration-builders.mjs" via the "./control" subpath); update the package.json exports so consumers import the migration-builder API through "./control" (or refactor truly shared pieces into shared-plane exports) to keep plane boundaries consistent.test/integration/test/cli.migration-apply.e2e.test.ts (1)
289-384: Drop this migrated resume journey from the legacy suite.This now covers the same duplicate-email → unique-constraint → resume flow as
test/integration/test/cli-journeys/migration-apply-edge-cases.e2e.test.ts. Keeping both doubles a slow dev-database path and makes future behavior changes easy to update in one place but miss in the other.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/integration/test/cli.migration-apply.e2e.test.ts` around lines 289 - 384, Remove the duplicated legacy test "resume after partial apply" from the legacy suite: delete (or skip) the describe block named 'resume after partial apply' / the it block titled 'resumes from last successful migration after failure' in test/integration/test/cli.migration-apply.e2e.test.ts; this block exercises the duplicate-email → unique-constraint → resume flow already covered by cli-journeys/migration-apply-edge-cases.e2e.test.ts, so remove references to its helpers (setupTestDirectoryFromFixtures, emitContract, runMigrationPlan, runMigrationApply, withDevDatabase, withClient, readMigrationsDir, getExitCode) to avoid running the slow dev-database path twice. Ensure any imports only used by this block are cleaned up.packages/1-framework/3-tooling/cli/test/utils/formatters/graph-render.test.ts (1)
136-148: Split this formatter test file before adding more cases.This file is already over the 500-line limit, and these new dashed-edge/truncation cases add another distinct concern on top of extraction, truncation, rendering, and marker coverage. Moving the dashed-edge/truncation scenarios into a sibling file would keep snapshots and failures much easier to navigate.
As per coding guidelines, "
**/*.test.ts: Keep test files under 500 lines to maintain readability and navigability. If a test file exceeds this limit, it should be split into multiple files."Also applies to: 229-233, 411-426
🤖 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/utils/formatters/graph-render.test.ts` around lines 136 - 148, The test file has grown past the 500-line guideline and the new dashed-edge/truncation cases introduce a separate concern; split out the dashed-edge and truncation tests into a new sibling test file by moving the test(s) that reference RenderGraph, extractRelevantSubgraph and any related dashed/truncation snapshots into a new file (e.g., graph-render.dashed.test.ts), update imports to reference RenderGraph and extractRelevantSubgraph from the same module, and run/update snapshots so the original file stays under 500 lines and the new file contains only the dashed-edge/truncation scenarios.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@packages/1-framework/1-core/framework-components/src/control-migration-types.ts`:
- Around line 341-358: Remove the orphaned/duplicated JSDoc block for
resolveDescriptors that precedes the actual JSDoc for planWithDescriptors:
locate the duplicate comment text describing resolveDescriptors (the block that
is not attached to a function) and delete it so only the correct JSDoc remains
immediately above planWithDescriptors; ensure the real resolveDescriptors and
planWithDescriptors function declarations keep their proper single JSDoc each
(reference symbols: resolveDescriptors, planWithDescriptors).
In `@packages/1-framework/3-tooling/cli/src/commands/migration-apply.ts`:
- Around line 198-205: loadAllBundles() returns drafts and attested sets, but
migration-apply.ts currently uses migrations.graph (which includes drafts) for
markerHash/destinationHash checks and path selection while materializing steps
from migrations.attested, causing draft-only targets to pass early then fail
later; after calling loadAllBundles(migrationsDir) (the migrations variable),
construct an attested-only graph from migrations.attested and use that
attestedGraph for all validation and path-picking logic
(markerHash/destinationHash checks and path resolution in place of
migrations.graph), while keeping migrations.drafts only for the
ui.warn/diagnostic messages (e.g., the existing drafts warning and the “verify
this draft first” guidance); update all the decision points that currently
reference migrations.graph (the sections around the current marker/destination
validation and path selection and the code between the earlier mentioned ranges)
to reference the attested-only graph so materialization later (which uses
migrations.attested) cannot fail with “Migration package not found.”
In `@packages/1-framework/3-tooling/cli/src/commands/migration-new.ts`:
- Around line 129-138: Before using findLatestMigration to set
fromHash/fromContract, detect whether the attested graph is branched by checking
for multiple leaf packages (i.e., more than one package in attested that
represents a leaf of the migration graph); if more than one leaf exists, do not
call findLatestMigration or silently pick a branch—instead return/throw a
structured error that explains the history is branched and instructs the user to
supply --from (include the names/ids of the leaf packages for clarity). Update
the block around findLatestMigration/latestMigration/fromHash/fromContract in
migration-new.ts to perform this branch-detection and produce the error when
multiple leaves are found.
In `@packages/1-framework/3-tooling/cli/src/commands/migration-plan.ts`:
- Around line 301-368: The current flow calls writeMigrationPackage and
scaffoldMigrationTs early, which leaves a draft package on disk if the non-draft
path (evaluateMigrationTs, migrations.resolveDescriptors, writeMigrationOps,
attestMigration) fails; update the logic so that when
descriptorResult.needsDataMigration === false you either create the package in a
temporary directory and atomically rename/move it into place only after
resolveDescriptors, writeMigrationOps and attestMigration all succeed, or ensure
you delete/cleanup the packageDir on any thrown error before returning
notOk(mapMigrationToolsError(error)); target the functions
writeMigrationPackage, scaffoldMigrationTs, evaluateMigrationTs,
migrations.resolveDescriptors, writeMigrationOps, attestMigration and the catch
block that calls mapMigrationToolsError to implement the temp-dir-or-cleanup
behavior.
In `@packages/1-framework/3-tooling/cli/src/commands/migration-verify.ts`:
- Around line 114-123: The code currently calls attestMigration(dir) and returns
an attested result when the stored migrationId mismatches ops.json; instead,
change migration-verify.ts so that on mismatch you do not auto-attest for
non-draft packages: check package draft status (e.g., a function/flag used
elsewhere in this file or project) and if the package is non-draft, return or
throw a verification error (fail the command) describing the stale migrationId
and include both stored and expected IDs; only when the package is a draft
should you call attestMigration(dir) and return the ok(...) attested response
with migrationId.
In
`@packages/1-framework/3-tooling/cli/src/utils/formatters/graph-migration-mapper.ts`:
- Around line 219-227: The current guard drops edges when matchingDraft.from
references another draft not present in graph.nodes; update the logic around
matchingDraft / fromHash in graph-migration-mapper.ts to instead traverse
input.draftEdges (or input.draftEdges keyed by .from) from matchingDraft.from up
the chain until you find an ancestor that exists in graph.nodes or reach
spineTargetHash, and use that ancestor as fromHash, or alternatively materialize
the intermediate draft nodes/edges (creating node entries and dashed edges for
each draft in the chain) before pushing to edgeList; ensure you preserve the
dashed style and label (matchingDraft.dirName + " [draft]") for draft edges and
fall back to spineTargetHash only if no attested ancestor is found.
In `@packages/1-framework/3-tooling/cli/src/utils/formatters/graph-render.ts`:
- Around line 913-923: The truncation step currently prunes nodes reachable only
from the visible spine and thus can remove dashed (draft) edges and their
off-graph endpoints; update truncateGraph to mirror extractRelevantSubgraph’s
behavior by first collecting dashed edges (graph.edges.filter(e => e.style ===
'dashed')) and adding both endpoints (e.from/e.to) to the kept node set before
computing reachable nodes from the spine window, and ensure the final edge
filter always includes edges whose pair is in edgePairs OR where e.style ===
'dashed' so dashed edges and their endpoints survive truncation.
In `@packages/1-framework/3-tooling/migration/src/exports/types.ts`:
- Around line 5-7: Remove the legacy aliases for BaseMigrationBundle from the
export surface: drop the re-exports "BaseMigrationBundle as MigrationBundle" and
"BaseMigrationBundle as MigrationPackage" so only the canonical types
(BaseMigrationBundle, AttestedMigrationBundle, DraftMigrationBundle) are
exported; then update any call sites that import MigrationBundle or
MigrationPackage to import the appropriate new symbol (BaseMigrationBundle /
AttestedMigrationBundle / DraftMigrationBundle) instead. Ensure no new
compatibility aliases are introduced in the exports file and run a project-wide
search/replace for uses of MigrationBundle and MigrationPackage to switch them
to the explicit types.
In `@packages/1-framework/3-tooling/migration/src/migration-ts.ts`:
- Around line 119-125: The generated import for the Contract type uses
relativeContractDts which can produce bare specifiers like "contract.d" or
"types/contract.d"; update the code around hasDataTransform and
options.contractJsonPath to ensure the import path is a local relative path by
prefixing "./" when relativeContractDts does not already start with "./" or
"../" before calling lines.push(`import type { Contract } from
"${relativeContractDts}"`), so the generated migration.ts imports the local .d
file instead of resolving via node_modules.
In `@packages/2-sql/4-lanes/relational-core/src/query-operation-registry.ts`:
- Around line 24-33: The entries() method currently returns
Object.freeze(Object.fromEntries(map)) which only shallow-freezes the top-level
record and because register(descriptor) stores caller-derived nested objects via
const { method: _, ...entry } = descriptor those nested fields (args, returns,
lowering) remain mutable; to fix, deep-freeze the stored entry at registration
(in register) using the project's recursive freeze helper/pattern (see
plan-helpers.ts) before map.set so map holds immutable nested structures, and
ensure entries() still returns a frozen snapshot (you can keep Object.freeze on
Object.fromEntries(map) but the stored entries must be recursively frozen in
register to prevent external mutation).
In `@packages/2-sql/9-family/src/core/migrations/contract-to-schema-ir.ts`:
- Around line 263-268: When re-keying storage.types by nativeType (the
byNativeType map built from storage.types and typeInstance.nativeType) add a
guard that detects collisions: while iterating Object.values(storage.types)
check if byNativeType already has that nativeType and, if so, throw a clear
error that includes the conflicting nativeType and the involved type identifiers
(e.g., the existing byNativeType[nativeType] and the new typeInstance) so the
failure is fail-fast; keep using annotationNamespace and return shape unchanged
once no collisions are found.
In
`@packages/3-targets/3-targets/postgres/src/core/migrations/descriptor-planner.ts`:
- Around line 418-460: The final ordering places dropOps before patternOps which
causes destructive drops (e.g., column drops) to run before generated
dataTransform patternOps can read data; update the descriptors assembly so
patternOps comes before dropOps (i.e., build descriptors from ...depOps,
...tableOps, ...columnOps, ...patternOps, ...dropOps, ...alterOps,
...constraintOps) or otherwise ensure dropOps that target columns referenced by
patternOps are moved after patternOps; modify the array construction around the
symbols depOps, dropOps, patternOps, tableOps, columnOps, alterOps,
constraintOps and update any related ordering logic to preserve that patternOps
execute before destructive drops.
In
`@packages/3-targets/3-targets/postgres/src/core/migrations/operation-descriptors.ts`:
- Around line 134-140: The descriptor currently hardcodes source: 'migration.ts'
which breaks traceability; update the factory that returns the dataTransform
descriptor in operation-descriptors.ts to accept a source string (e.g., add a
source?: string parameter to the function that builds the returned object) and
set source to that parameter instead of the literal, and if source is not
provided derive a fallback using a caller location (Error().stack) or a
lightweight caller utility so generated descriptors contain the originating file
path; ensure the returned object still has kind: 'dataTransform', name, check,
and run fields unchanged.
In
`@packages/3-targets/3-targets/postgres/src/core/migrations/operation-resolver.ts`:
- Around line 695-727: The generated enum-add operation from
resolveAddEnumValues creates ALTER TYPE ... ADD VALUE steps that must run
outside a transaction; add a non-transactional marker to the returned ResolvedOp
(e.g., set nonTransactional: true or similar) and propagate that flag into the
SqlMigrationPlanOperation so the runner can execute such operations without
BEGIN/COMMIT; update resolveAddEnumValues to include the new property alongside
id/label/operationClass/target/execute/postcheck and then update the migration
planner/runner to honor that property when deciding transactional boundaries so
ALTER TYPE steps run non-transactionally.
In
`@packages/3-targets/3-targets/postgres/src/core/migrations/planner-strategies.ts`:
- Around line 84-94: Update enumRebuildRecipe to drop and recreate column
defaults: add dropDefault and setDefault to the imports from
./operation-descriptors, then modify enumRebuildRecipe so that for each entry in
columnRefs you emit a dropDefault(ref.table, ref.column) before the
alterColumnType(...) call and, after renameType(tempName, nativeType) (and after
dropEnumType/native recreation), emit setDefault(ref.table, ref.column,
ref.default) for each columnRef to restore the original default; keep the
existing createEnumType, alterColumnType, dropEnumType, and renameType steps but
surround alters with the dropDefault/setDefault pair to avoid type-binding
errors.
In `@packages/3-targets/3-targets/postgres/src/core/migrations/runner.ts`:
- Around line 279-343: The current executeDataTransform allows op.run to execute
without a query-based op.check, which permits silent partial/no-op backfills;
require a query-based check when a run is present: in executeDataTransform
(handling DataTransformOperation), add an early validation so if op.run is
non-empty and op.check is not an object with sql/params (i.e., op.check === null
|| op.check === false || op.check === true) return a runnerFailure (e.g., new
code path 'INVALID_TRANSFORM') with a clear message and meta including
operationId and dataTransformName; this ensures callers must provide a check
query for run steps and preserves the check→run→check lifecycle (alternatively,
if you prefer to force post-run validation, ensure op.check.sql is executed
after any run by treating boolean false as an error and running the check only
when an SQL check object exists).
In
`@packages/3-targets/3-targets/postgres/test/migrations/descriptor-planner.scenarios.md`:
- Around line 45-46: Update the enum scenarios in
descriptor-planner.scenarios.md to reflect the new rebuild-based enum evolution
instead of marking "Enum values added" and "Enum values removed" as conflicts:
remove or replace the two bullets that say "conflict (not yet supported)" and
state that enum additions/removals are handled by the rebuild recipe (mentioning
"rebuild recipe" / "enum evolution via rebuild") and briefly note any relevant
behavior expected during rebuilds so tests/readers know the current flow.
In
`@packages/3-targets/3-targets/postgres/test/migrations/descriptor-planner.scenarios.test.ts`:
- Around line 131-1183: The test file is too large and bundles many distinct
scenario groups; split the big describe-suite into multiple focused test files
(e.g., additive, reconciliation, types, dependencies, ordering,
data-safety-gaps, mixed/complex, old-planner-parity) by moving each top-level
describe(...) block (names like 'additive — fresh database', 'additive —
existing contract', 'reconciliation — drops', 'reconciliation — alters',
'types', 'dependencies', 'ordering', 'combined / realistic', 'data-safety gaps',
'mixed / complex', 'old planner parity') into its own test file that preserves
all its it(...) cases and test titles; ensure each new file imports the shared
test helpers and symbols used by the tests (functions/constructors like plan,
contract, table, uuidCol, textCol, col, intCol, boolCol, descriptorKinds,
descriptorSummary, defaultComponents, pgvectorDescriptor, and any local helpers
such as enumType or componentsWithPgvector) by either moving those shared
helpers into a test-utils module or re-declaring/duplicating minimal shared
fixtures in the new files; keep behavior identical (no code changes to tests)
and run the suite to confirm tests still execute.
---
Outside diff comments:
In `@packages/1-framework/3-tooling/cli/src/commands/migration-status.ts`:
- Around line 439-466: The current early return that triggers when
attested.length === 0 omits drafts and graph causing draft-only repos to report
"No migrations"; update the logic in migration-status.ts (around the
attested/drafts/graph handling after loadAllBundles) so that the success return
includes drafts and graph when drafts.length > 0 (either change the early return
condition to consider drafts or construct the success payload to include drafts
and graph even when attested is empty), ensuring functions/variables attested,
drafts, graph and the success/notOk return path are updated accordingly.
---
Nitpick comments:
In
`@packages/1-framework/3-tooling/cli/test/utils/formatters/graph-migration-mapper.test.ts`:
- Around line 199-201: The tests currently only assert that a dashed edge exists
and its `to` endpoint, which can hide cases where the edge's `from` is
incorrect; update the detached-node tests that find `dashedEdge` (the assertion
around `result.graph.edges.find((e) => e.style === 'dashed')`) to also assert
`dashedEdge!.from` equals the expected source node id (use the same `sid(...)`
helper for the expected source) in addition to the existing `dashedEdge!.to ===
sid('DETACHED_HASH')` check; apply the same strengthening to the second
detached-node test (the one near the other assertions at 221-223) so both tests
lock both endpoints.
In
`@packages/1-framework/3-tooling/cli/test/utils/formatters/graph-render.test.ts`:
- Around line 136-148: The test file has grown past the 500-line guideline and
the new dashed-edge/truncation cases introduce a separate concern; split out the
dashed-edge and truncation tests into a new sibling test file by moving the
test(s) that reference RenderGraph, extractRelevantSubgraph and any related
dashed/truncation snapshots into a new file (e.g., graph-render.dashed.test.ts),
update imports to reference RenderGraph and extractRelevantSubgraph from the
same module, and run/update snapshots so the original file stays under 500 lines
and the new file contains only the dashed-edge/truncation scenarios.
In `@packages/2-sql/9-family/src/core/migrations/operation-descriptors.ts`:
- Around line 13-61: Duplicate type imports for descriptors
(AddColumnDescriptor, AddEnumValuesDescriptor, AddForeignKeyDescriptor,
AddPrimaryKeyDescriptor, AddUniqueDescriptor, AlterColumnTypeDescriptor,
CreateDependencyDescriptor, CreateEnumTypeDescriptor, CreateIndexDescriptor,
CreateTableDescriptor, DropColumnDescriptor, DropConstraintDescriptor,
DropDefaultDescriptor, DropEnumTypeDescriptor, DropIndexDescriptor,
DropNotNullDescriptor, DropTableDescriptor, RenameTypeDescriptor,
SetDefaultDescriptor, SetNotNullDescriptor) are present; remove the second
import block and consolidate into a single statement by re-exporting the types
from './descriptor-schemas' (use a single export type { ... } from
'./descriptor-schemas';) while keeping the existing export {
MigrationDescriptorArraySchema } from './descriptor-schemas'; so there is only
one import/re-export for those descriptor types and no duplicate import type
block.
In `@packages/3-targets/3-targets/postgres/package.json`:
- Line 48: Replace the standalone subpath export "./migration-builders" with a
control-plane entrypoint by moving its mapping to the package.json control
export (i.e., stop exporting "./migration-builders" and instead expose the same
"./dist/migration-builders.mjs" via the "./control" subpath); update the
package.json exports so consumers import the migration-builder API through
"./control" (or refactor truly shared pieces into shared-plane exports) to keep
plane boundaries consistent.
In
`@packages/3-targets/3-targets/postgres/src/core/migrations/operation-descriptors.ts`:
- Around line 12-66: The file currently duplicates type exports and imports from
'@prisma-next/family-sql/operation-descriptors' — remove the second import block
(lines importing Buildable, DataTransformDescriptor, SqlMigrationOpDescriptor,
builders as structuralBuilders, TODO, TodoMarker) and consolidate into a single
import/export so the same type symbols (e.g., Buildable,
SqlMigrationOpDescriptor, TODO, TodoMarker, DataTransformDescriptor) are
imported once and re-exported together with the operation helpers (addColumn,
addEnumValues, createTable, etc.); update the top export/import so internal
usage and re-exports reference the same imported symbols instead of importing
them twice.
- Around line 105-132: Summary: resolveInput currently checks 'build' in input
without ensuring input is an object, so a Function with a build property could
be misclassified; change the guard to only treat non-null objects as Buildable.
Update resolveInput to first return for typeof input === 'symbol' || typeof
input === 'function' (keep existing) and then check if typeof input === 'object'
&& input !== null && 'build' in input && typeof (input as Buildable).build ===
'function' before calling build; also scan usages such as dataTransform's
handling of options.check and options.run to ensure they call resolveInput only
for non-function/non-symbol inputs (or rely on the updated resolveInput) so
functions aren't mistaken for Buildable.
In `@packages/3-targets/6-adapters/postgres/src/core/enum-control-hooks.ts`:
- Around line 700-717: The verification message currently prints +[ ] -[ ] for
reorder-only enum diffs; update the message generation in the enum diff path
(around determineEnumDiff, diff.kind and the object with kind
'enum_values_changed') to detect reorder-only cases (either diff.kind ===
'reorder' if available, or when addedValues.length === 0 && removedValues.length
=== 0) and emit a dedicated message like `Enum type "${typeName}" values
reordered: [${desired.join(', ')}]` instead of the +/-. Keep the existing
messages for 'add_values' and rebuild cases with real adds/removals.
In `@test/integration/test/cli.migration-apply.e2e.test.ts`:
- Around line 289-384: Remove the duplicated legacy test "resume after partial
apply" from the legacy suite: delete (or skip) the describe block named 'resume
after partial apply' / the it block titled 'resumes from last successful
migration after failure' in
test/integration/test/cli.migration-apply.e2e.test.ts; this block exercises the
duplicate-email → unique-constraint → resume flow already covered by
cli-journeys/migration-apply-edge-cases.e2e.test.ts, so remove references to its
helpers (setupTestDirectoryFromFixtures, emitContract, runMigrationPlan,
runMigrationApply, withDevDatabase, withClient, readMigrationsDir, getExitCode)
to avoid running the slow dev-database path twice. Ensure any imports only used
by this block are cleaned up.
In `@turbo.json`:
- Around line 44-46: Add explicit inputs and outputs to the
"@prisma-next/sql-builder#emit" Turbo task so caching is accurate: update the
task definition for "@prisma-next/sql-builder#emit" to include an "inputs" array
that lists the source files this emit step reads (e.g., schema, contract,
migration, and CLI build artifacts such as "packages/sql-builder/src/**",
"prisma/schema.prisma", or any contract templates) and an "outputs" array that
lists the generated artifacts the task produces (e.g.,
"packages/sql-builder/dist/**", "packages/sql-builder/generated/**" or the
specific contract/migration files), ensuring the task consumes the CLI build
result (dependsOn remains) and Turbo can invalidate/skip runs correctly.
🪄 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: 2a5ecb5c-6b69-4972-9f86-d65446d04814
⛔ Files ignored due to path filters (15)
packages/1-framework/3-tooling/cli/test/utils/formatters/__snapshots__/graph-render.test.ts.snapis excluded by!**/*.snappnpm-lock.yamlis excluded by!**/pnpm-lock.yamlprojects/graph-based-migrations/plans/data-migrations-plan.mdis excluded by!projects/**projects/graph-based-migrations/specs/april-milestone.mdis excluded by!projects/**projects/graph-based-migrations/specs/chat.mdis excluded by!projects/**projects/graph-based-migrations/specs/data-migration-scenarios.mdis excluded by!projects/**projects/graph-based-migrations/specs/data-migrations-response.mdis excluded by!projects/**projects/graph-based-migrations/specs/data-migrations-solutions.mdis excluded by!projects/**projects/graph-based-migrations/specs/data-migrations-spec.mdis excluded by!projects/**projects/graph-based-migrations/specs/data-migrations.mdis excluded by!projects/**projects/graph-based-migrations/specs/mongodb-applicability.mdis excluded by!projects/**projects/graph-based-migrations/specs/planner-replacement-proposal.mdis excluded by!projects/**projects/graph-based-migrations/walkthrough.mdis excluded by!projects/**test/integration/test/sql-builder/fixtures/generated/contract.d.tsis excluded by!**/generated/**test/integration/test/sql-builder/fixtures/generated/contract.jsonis excluded by!**/generated/**
📒 Files selected for processing (85)
packages/1-framework/1-core/framework-components/src/control-migration-types.tspackages/1-framework/1-core/framework-components/src/control-result-types.tspackages/1-framework/1-core/framework-components/src/exports/control.tspackages/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-new.tspackages/1-framework/3-tooling/cli/src/commands/migration-plan.tspackages/1-framework/3-tooling/cli/src/commands/migration-status.tspackages/1-framework/3-tooling/cli/src/commands/migration-verify.tspackages/1-framework/3-tooling/cli/src/control-api/operations/migration-apply.tspackages/1-framework/3-tooling/cli/src/utils/command-helpers.tspackages/1-framework/3-tooling/cli/src/utils/formatters/graph-migration-mapper.tspackages/1-framework/3-tooling/cli/src/utils/formatters/graph-render.tspackages/1-framework/3-tooling/cli/src/utils/formatters/graph-types.tspackages/1-framework/3-tooling/cli/test/utils/formatters/graph-migration-mapper.test.tspackages/1-framework/3-tooling/cli/test/utils/formatters/graph-render.test.tspackages/1-framework/3-tooling/cli/test/utils/formatters/test-graphs.tspackages/1-framework/3-tooling/cli/tsdown.config.tspackages/1-framework/3-tooling/migration/package.jsonpackages/1-framework/3-tooling/migration/src/attestation.tspackages/1-framework/3-tooling/migration/src/errors.tspackages/1-framework/3-tooling/migration/src/exports/io.tspackages/1-framework/3-tooling/migration/src/exports/migration-ts.tspackages/1-framework/3-tooling/migration/src/exports/types.tspackages/1-framework/3-tooling/migration/src/io.tspackages/1-framework/3-tooling/migration/src/migration-ts.tspackages/1-framework/3-tooling/migration/src/types.tspackages/1-framework/3-tooling/migration/tsdown.config.tspackages/2-sql/2-authoring/contract-ts/package.jsonpackages/2-sql/4-lanes/relational-core/package.jsonpackages/2-sql/4-lanes/relational-core/src/exports/query-operations.tspackages/2-sql/4-lanes/relational-core/src/query-operation-registry.tspackages/2-sql/4-lanes/relational-core/tsdown.config.tspackages/2-sql/4-lanes/sql-builder/package.jsonpackages/2-sql/4-lanes/sql-builder/src/exports/types.tspackages/2-sql/4-lanes/sql-builder/src/runtime/index.tspackages/2-sql/4-lanes/sql-builder/test/fixtures/contract.tspackages/2-sql/9-family/package.jsonpackages/2-sql/9-family/src/core/migrations/contract-to-schema-ir.tspackages/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/migrations/types.tspackages/2-sql/9-family/src/core/schema-verify/verify-helpers.tspackages/2-sql/9-family/src/exports/operation-descriptors.tspackages/2-sql/9-family/test/contract-to-schema-ir.test.tspackages/2-sql/9-family/tsdown.config.tspackages/3-extensions/pgvector/package.jsonpackages/3-extensions/pgvector/src/exports/control.tspackages/3-targets/3-targets/postgres/package.jsonpackages/3-targets/3-targets/postgres/src/core/migrations/descriptor-planner.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/planner-reconciliation.tspackages/3-targets/3-targets/postgres/src/core/migrations/planner-strategies.tspackages/3-targets/3-targets/postgres/src/core/migrations/runner.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/test/migrations/descriptor-planner.scenarios.mdpackages/3-targets/3-targets/postgres/test/migrations/descriptor-planner.scenarios.test.tspackages/3-targets/3-targets/postgres/tsdown.config.tspackages/3-targets/6-adapters/postgres/package.jsonpackages/3-targets/6-adapters/postgres/src/core/enum-control-hooks.tspackages/3-targets/6-adapters/postgres/test/enum-control-hooks.basic.test.tstest/integration/test/cli-journeys/data-transform.e2e.test.tstest/integration/test/cli-journeys/migration-apply-edge-cases.e2e.test.tstest/integration/test/cli.migration-apply.e2e.test.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/cli-journeys/contract-unique-email.tstest/integration/test/sql-builder/distinct.test.tstest/integration/test/sql-builder/execution.test.tstest/integration/test/sql-builder/extension-functions.test.tstest/integration/test/sql-builder/fixtures/contract.tstest/integration/test/sql-builder/fixtures/prisma-next.config.tstest/integration/test/sql-builder/group-by.test.tstest/integration/test/sql-builder/join.test.tstest/integration/test/sql-builder/mutation-defaults.test.tstest/integration/test/sql-builder/mutation.test.tstest/integration/test/sql-builder/order-by.test.tstest/integration/test/sql-builder/pagination.test.tstest/integration/test/sql-builder/select.test.tstest/integration/test/sql-builder/setup.tstest/integration/test/sql-builder/subquery.test.tstest/integration/test/sql-builder/where.test.tstest/integration/test/utils/journey-test-helpers.tsturbo.json
💤 Files with no reviewable changes (3)
- packages/3-extensions/pgvector/package.json
- packages/3-targets/6-adapters/postgres/package.json
- packages/2-sql/2-authoring/contract-ts/package.json
packages/1-framework/1-core/framework-components/src/control-migration-types.ts
Outdated
Show resolved
Hide resolved
packages/3-targets/3-targets/postgres/src/core/migrations/operation-resolver.ts
Show resolved
Hide resolved
packages/3-targets/3-targets/postgres/src/core/migrations/planner-strategies.ts
Show resolved
Hide resolved
packages/3-targets/3-targets/postgres/test/migrations/descriptor-planner.scenarios.md
Show resolved
Hide resolved
packages/3-targets/3-targets/postgres/test/migrations/descriptor-planner.scenarios.test.ts
Show resolved
Hide resolved
saevarb
force-pushed
the
feat/data-migrations
branch
2 times, most recently
from
032661e to
6c0e2e0
Compare
…clean up dependencies Remove phantom @prisma-next/cli dependencies from target-postgres, adapter-postgres, extension-pgvector, family-sql. Move sql-builder integration tests to integration test package to break dependency cycle. Restore unit test fixtures with simplified contract.ts re-export. Export Db type from sql-builder runtime for cross-package type portability.
Descriptor types defined as arktype schemas (descriptor-schemas.ts) in sql-family — TypeScript types derived via typeof schema.infer. Operation resolver, migration.ts scaffolding/evaluation, migration verify wiring, runner data transform lifecycle, migration new command, shared IO utilities. Framework types: DataTransformOperation, SerializedQueryPlan, OperationDescriptor, EnumValuesChangedIssue discriminated union. contractToSchemaIR re-keys storage types by nativeType for correct verifier hook resolution. Enum control hooks produce EnumValuesChangedIssue.
planDescriptors with pluggable strategies: notNullBackfill, typeChange, nullableTightening, enumChange. Enum change strategy uses rebuild recipe (createEnumType + alterColumnType + dropEnumType + renameType) with dataTransform for removals. 48 scenario tests with 2 known failures (.fails) as gap map. Wired into migration plan replacing old planner fallback.
loadAllBundles returns MigrationBundleSet with attested and drafts. Status shows MIGRATION.DRAFTS diagnostic, apply warns/errors on drafts, plan guards against duplicate draft targets. Graph rendering uses DIR.dashed bitmask for draft edges, replacing detached node rendering.
queryOperations on SqlControlStaticContributions for extension function support. createBuilders<Contract>() for typed dataTransform callbacks. SQL lowered at verify time via postgres adapter. Journey test fixes: migration-new reads storage.storageHash correctly, resume-after-failure tests use unique constraint violation scenario.
saevarb
force-pushed
the
feat/data-migrations
branch
from
6c0e2e0 to
0b4515e
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/3-targets/3-targets/postgres/src/core/migrations/planner-reconciliation.ts (1)
99-107:⚠️ Potential issue | 🟠 MajorDon't drop
enum_values_changedon the reconciliation path.
buildReconciliationPlan()still backs the olddb update/db initflows, and classifyingenum_values_changedas additive makes those commands ignore enum drift entirely: no operation is emitted and no conflict is returned. Until reconciliation can rebuild enums, this should stay visible as a conflict instead of being skipped.🤖 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-reconciliation.ts` around lines 99 - 107, The switch in isAdditiveIssue currently classifies 'enum_values_changed' as additive which causes buildReconciliationPlan to ignore enum drift; remove 'enum_values_changed' from the case list in isAdditiveIssue so that it is not treated as additive (i.e., do not return true for 'enum_values_changed') and allow the reconciliation flow to surface it as a conflict; locate the isAdditiveIssue function and delete the 'enum_values_changed' case branch so other code (e.g., buildReconciliationPlan) will handle it as a non-additive issue.
♻️ Duplicate comments (7)
packages/1-framework/3-tooling/cli/src/utils/formatters/graph-render.ts (1)
913-923:⚠️ Potential issue | 🟠 MajorTruncation can still prune the only dashed draft edge.
Keeping dashed edges here is not sufficient because
truncateGraph()later retains only nodes reachable from the visible spine window. When a draft edge starts from an older migration outside that window, the dashed edge and off-graph contract node still disappear from the rendered default view.🤖 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/utils/formatters/graph-render.ts` around lines 913 - 923, The truncation step later (truncateGraph) can still remove dashed (draft) edges and their off-graph endpoints because only nodes reachable from the visible spine window are kept; to fix this, ensure dashed edges and their endpoints are treated as reachable before or inside truncateGraph by adding dashed endpoints to the visible node set used by truncateGraph (or by expanding truncateGraph's reachability logic to always include nodes referenced by any graph.edges with style === 'dashed'); update the code that builds nodeSet/edgePairs (or truncateGraph) so dashedEdges and their e.from/e.to IDs are injected into the reachable/visible node collection so filteredNodes/filteredEdges cannot prune them.packages/1-framework/3-tooling/cli/src/utils/formatters/graph-migration-mapper.ts (1)
219-227:⚠️ Potential issue | 🟠 MajorNested draft chains still collapse into an orphan contract node.
If the matching draft points to another draft hash,
fromHashis not ingraph.nodes, so Lines 221-228 drop the dashed edge entirely. The common draft-on-top-of-draft case still loses its lineage instead of rendering from the nearest attested ancestor (or materializing the intermediate draft node).🤖 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/utils/formatters/graph-migration-mapper.ts` around lines 219 - 227, The dashed edge creation drops lineage when a draft points to another draft because fromHash is set to matchingDraft?.from without resolving draft chains; update the logic using matchingDraft (and input.draftEdges) to walk the draft chain: start at matchingDraft.from and follow draftEdges (find by d.to) until you reach a hash that exists in graph.nodes or equals spineTargetHash (guard against cycles/lengths), then set fromHash to that resolved attested ancestor (or spineTargetHash) before pushing into edgeList; use the resolved fromHash (and keep the dashed style and label logic) so chained drafts render from the nearest attested ancestor instead of being dropped.packages/1-framework/3-tooling/migration/src/migration-ts.ts (1)
119-125:⚠️ Potential issue | 🟠 MajorFix the generated
Contractimport to stay relative.When
relative()returnscontract.dortypes/contract.d, Line 124 emits a bare specifier, so Node resolves it via package lookup instead of the migration directory. Prefix./when the path does not already start with..Proposed fix
if (hasDataTransform && options.contractJsonPath) { const relativeContractDts = relative(packageDir, options.contractJsonPath).replace( /\.json$/, '.d', ); - lines.push(`import type { Contract } from "${relativeContractDts}"`); + const contractImportPath = relativeContractDts.startsWith('.') + ? relativeContractDts + : `./${relativeContractDts}`; + lines.push(`import type { Contract } from "${contractImportPath}"`); lines.push(`import { createBuilders } from "@prisma-next/target-postgres/migration-builders"`);In Node.js ESM / native TypeScript import resolution, is `import type { X } from "contract.d"` treated as a bare package specifier rather than a relative local file import?🤖 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-ts.ts` around lines 119 - 125, The generated Contract import sometimes becomes a bare specifier; update the code that builds relativeContractDts (used when hasDataTransform && options.contractJsonPath) to ensure the import stays relative by prefixing './' when relativeContractDts does not start with '.'; adjust the lines.push call that emits `import type { Contract } from "${relativeContractDts}"` so it uses the possibly-prefixed path (e.g., compute a safeImportPath = relativeContractDts.startsWith('.') ? relativeContractDts : `./${relativeContractDts}` and use safeImportPath in the import string).packages/3-targets/3-targets/postgres/src/core/migrations/descriptor-planner.ts (1)
418-460:⚠️ Potential issue | 🟠 MajorKeep destructive drops after pattern-generated backfills.
Putting
dropOpsahead ofpatternOpsmeans a generateddataTransformcan lose the source column/table it needs to read. That still makes split/reshape migrations unsafe; the final ordering needs to preserve pattern-generated backfills before destructive drops.🤖 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/descriptor-planner.ts` around lines 418 - 460, The current descriptor ordering places dropOps before patternOps, which can remove columns/tables needed by pattern-generated dataTransform backfills; update the final descriptors array so patternOps come before destructive dropOps (i.e., build descriptors as [...depOps, ...tableOps, ...columnOps, ...patternOps, ...alterOps, ...constraintOps, ...dropOps] or otherwise ensure patternOps precede dropOps), adjusting only the order of depOps/dropOps/patternOps in the descriptors assembly to preserve backfill reads.packages/1-framework/3-tooling/cli/src/commands/migration-verify.ts (1)
114-123:⚠️ Potential issue | 🟠 MajorDon't re-attest non-draft migrations on mismatch.
After the refresh step, a stored/computed
migrationIdmismatch still means the package contents changed. Auto-attesting here silently blesses an edited historical migration instead of failing integrity verification; only drafts should be attested automatically.🤖 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-verify.ts` around lines 114 - 123, The current mismatch-handling code in migration-verify.ts re-attests any migration by calling attestMigration(dir) which silently blesses edited historical migrations; change this so only draft migrations are auto-attested: detect whether the migration in dir is a draft (e.g., via the migration metadata or an existing helper like isDraftMigration or by reading the migration manifest in the same code path), and if it's a draft call attestMigration(dir) and return the 'attested' response, but if it is not a draft do not call attestMigration — instead return a failure/verification error (e.g., ok: false or status: 'mismatch' with a clear message) so integrity verification fails for non-draft migrations.packages/1-framework/3-tooling/cli/src/commands/migration-plan.ts (1)
301-368:⚠️ Potential issue | 🟠 MajorClean up the draft package when the non-draft path fails.
writeMigrationPackage()andscaffoldMigrationTs()happen before evaluate/resolve/attest. If any later step throws, the partially written draft remains on disk and the earlier “existing draft” check blocks every retry for the same target hash until someone deletes it manually.🤖 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 301 - 368, A partially written draft package created by writeMigrationPackage + scaffoldMigrationTs is left on disk if later steps (evaluateMigrationTs, migrations.resolveDescriptors, writeMigrationOps, attestMigration) throw, blocking retries; after creating the draft and before returning notOk(mapMigrationToolsError(error)) ensure the draft is cleaned up on failure. Concretely: after the early draft-return for descriptorResult.needsDataMigration keep current behavior, but for the non-draft path wrap evaluateMigrationTs/resolveDescriptors/writeMigrationOps/attestMigration in a try/catch/finally and on any thrown error remove the draft packageDir (or call a helper like removeMigrationPackage(packageDir) / fs.rm) before rethrowing or returning notOk(mapMigrationToolsError(error)); reference functions/vars: writeMigrationPackage, scaffoldMigrationTs, evaluateMigrationTs, migrations.resolveDescriptors, writeMigrationOps, attestMigration, mapMigrationToolsError, and packageDir.packages/3-targets/3-targets/postgres/src/core/migrations/planner-strategies.ts (1)
84-94:⚠️ Potential issue | 🟠 MajorDrop and restore defaults around enum rebuilds.
ALTER COLUMN ... TYPE ... USING ...does not carry an enum-typed default through the cast path. Rebuilds will fail on columns with existing defaults unless those defaults are dropped before the type swap and recreated afterward.In PostgreSQL, when changing a column from one enum type to another with `ALTER TABLE ... ALTER COLUMN ... TYPE ... USING ...`, do existing column defaults need to be dropped and recreated because the `USING` clause does not convert the default expression?🤖 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 84 - 94, The enum-rebuild sequence must drop per-column defaults before ALTER COLUMN ... TYPE and restore them after the type swap because the USING cast does not convert enum-typed defaults; modify the sequence around createEnumType/alterColumnType/dropEnumType/renameType to (1) read and store each column's current default expression, (2) emit an operation to drop the default for each column (e.g., alterColumnDropDefault for the refs) before calling alterColumnType(ref.table, ref.column, ...), and (3) after renameType(native swap) emit operations to set the saved defaults back (e.g., alterColumnSetDefault(ref.table, ref.column, savedDefaultExpr)) so defaults are preserved through the enum rebuild.
🧹 Nitpick comments (2)
packages/2-sql/4-lanes/sql-builder/test/fixtures/contract.ts (1)
1-1: Avoid re-exporting from a non-exports/file.Line 1 uses a direct re-export. In this repo, non-
exports/modules should import then export explicitly.♻️ Suggested adjustment
-export { default as contract } from './generated/contract.json' with { type: 'json' }; +import contractJson from './generated/contract.json' with { type: 'json' }; + +export const contract = contractJson;As per coding guidelines, "Do not reexport things from one file in another, except in the exports/ folders".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-sql/4-lanes/sql-builder/test/fixtures/contract.ts` at line 1, Replace the direct re-export in the module (the `export { default as contract } from './generated/contract.json' with { type: 'json' };` statement) with an explicit import followed by an explicit export: import the default from './generated/contract.json' into a local binding (e.g., `contract`) and then export that binding as the module's export; ensure you keep the JSON import hint (`with { type: 'json' }`) when importing so the JSON is loaded correctly.packages/1-framework/3-tooling/cli/test/utils/formatters/test-graphs.ts (1)
28-29: UseifDefined()for the optional label spread.This helper introduces the inline conditional spread pattern the repo explicitly avoids in TS files. Switching to
ifDefined('label', label)keeps it consistent with the formatter codepaths.♻️ Suggested cleanup
+import { ifDefined } from '@prisma-next/utils/defined'; + function dashedEdge(from: string, to: string, label?: string): GraphEdge { - return { from, to, style: 'dashed', ...(label !== undefined ? { label } : {}) }; + return { + from, + to, + style: 'dashed', + ...ifDefined('label', label), + }; }As per coding guidelines, "Use
ifDefined()from@prisma-next/utils/definedfor conditional object spreads instead of inline conditional spread patterns".🤖 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/utils/formatters/test-graphs.ts` around lines 28 - 29, The dashedEdge helper uses an inline conditional spread for the optional label; replace that pattern by importing and using ifDefined from `@prisma-next/utils/defined` and spread the result (e.g., ...ifDefined('label', label)) so the optional label is added consistently with other formatter codepaths; update the dashedEdge function to use ifDefined('label', label) instead of ...(label !== undefined ? { label } : {}).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/1-framework/3-tooling/migration/src/migration-ts.ts`:
- Around line 130-137: The generated migration.ts can reference the placeholder
identifier TODO from serializeQueryInput() when a descriptor has kind
'dataTransform' and contractJsonPath is missing; ensure the import list includes
TODO in that untyped scaffold path. Modify the branch that builds importList
(the code using descriptors.map((d) => d.kind) and
importList.push('createTable')) to detect when any descriptor.kind ===
'dataTransform' and contractJsonPath is absent (the untyped path) and push
'TODO' into importList before emitting the import line so migration.ts imports
TODO from "@prisma-next/target-postgres/migration-builders".
In
`@packages/3-targets/3-targets/postgres/src/core/migrations/operation-descriptors.ts`:
- Around line 98-99: Remove the deprecated type alias MigrationOpDescriptor and
update all call sites to use PostgresMigrationOpDescriptor directly;
specifically delete the line exporting "export type MigrationOpDescriptor =
PostgresMigrationOpDescriptor" and replace any remaining references to
MigrationOpDescriptor in the codebase with PostgresMigrationOpDescriptor (check
imports, type annotations, and public API exports) so only
PostgresMigrationOpDescriptor remains as the canonical exported type.
In
`@packages/3-targets/3-targets/postgres/src/core/migrations/planner-strategies.ts`:
- Around line 75-82: The loop that builds columnRefs currently only matches
columns by column.typeRef === typeName; update the condition so it also matches
direct native-type enum columns by checking column.nativeType === nativeType &&
column.codecId === codecId (or the equivalent codec identifier used in this
scope), e.g. change the if to include an OR branch combining both checks,
keeping the pushed object ({ table: tableName, column: columnName }) the same;
reference the columnRefs array, ctx.toContract.storage.tables iteration, and the
typeName/nativeType/codecId symbols to locate where to modify the logic.
In `@packages/3-targets/3-targets/postgres/src/core/migrations/runner.ts`:
- Around line 206-216: The current branch treats any successful
executeDataTransform() result as "executed" and always pushes operation into
executedOperations and increments operationsExecuted; change it so that after
calling executeDataTransform(driver, operation, { runIdempotency }) you first
check the result to determine whether the transform actually ran (e.g. inspect a
field like dtResult.ran / dtResult.skipped / dtResult.status) and only push the
operation into executedOperations and increment operationsExecuted when the
result indicates it ran; keep returning dtResult on failure as-is and continue
the loop on skipped results without counting them.
In `@packages/3-targets/3-targets/postgres/src/exports/control.ts`:
- Around line 178-183: The return uses a blind cast
"planResult.value.descriptors as unknown as OperationDescriptor[]" which
violates the no-blind-cast rule; replace it by providing proper type narrowing:
add a type predicate function (e.g., isOperationDescriptorArray(descr: unknown):
descr is OperationDescriptor[]) that validates the shape against the already-run
MigrationDescriptorArraySchema (or call parseDescriptors() result) and use that
predicate to assert the type of planResult.value.descriptors, or alternatively
refactor the shared base types so PostgresMigrationOpDescriptor and
DataTransformDescriptor share a common OperationDescriptor-compatible base
(e.g., ensure DataTransformDescriptor is the base interface) and then remove the
cast and return planResult.value.descriptors directly; reference
planResult.value.descriptors, OperationDescriptor,
PostgresMigrationOpDescriptor, parseDescriptors(),
MigrationDescriptorArraySchema and DataTransformDescriptor when making the
change.
---
Outside diff comments:
In
`@packages/3-targets/3-targets/postgres/src/core/migrations/planner-reconciliation.ts`:
- Around line 99-107: The switch in isAdditiveIssue currently classifies
'enum_values_changed' as additive which causes buildReconciliationPlan to ignore
enum drift; remove 'enum_values_changed' from the case list in isAdditiveIssue
so that it is not treated as additive (i.e., do not return true for
'enum_values_changed') and allow the reconciliation flow to surface it as a
conflict; locate the isAdditiveIssue function and delete the
'enum_values_changed' case branch so other code (e.g., buildReconciliationPlan)
will handle it as a non-additive issue.
---
Duplicate comments:
In `@packages/1-framework/3-tooling/cli/src/commands/migration-plan.ts`:
- Around line 301-368: A partially written draft package created by
writeMigrationPackage + scaffoldMigrationTs is left on disk if later steps
(evaluateMigrationTs, migrations.resolveDescriptors, writeMigrationOps,
attestMigration) throw, blocking retries; after creating the draft and before
returning notOk(mapMigrationToolsError(error)) ensure the draft is cleaned up on
failure. Concretely: after the early draft-return for
descriptorResult.needsDataMigration keep current behavior, but for the non-draft
path wrap
evaluateMigrationTs/resolveDescriptors/writeMigrationOps/attestMigration in a
try/catch/finally and on any thrown error remove the draft packageDir (or call a
helper like removeMigrationPackage(packageDir) / fs.rm) before rethrowing or
returning notOk(mapMigrationToolsError(error)); reference functions/vars:
writeMigrationPackage, scaffoldMigrationTs, evaluateMigrationTs,
migrations.resolveDescriptors, writeMigrationOps, attestMigration,
mapMigrationToolsError, and packageDir.
In `@packages/1-framework/3-tooling/cli/src/commands/migration-verify.ts`:
- Around line 114-123: The current mismatch-handling code in migration-verify.ts
re-attests any migration by calling attestMigration(dir) which silently blesses
edited historical migrations; change this so only draft migrations are
auto-attested: detect whether the migration in dir is a draft (e.g., via the
migration metadata or an existing helper like isDraftMigration or by reading the
migration manifest in the same code path), and if it's a draft call
attestMigration(dir) and return the 'attested' response, but if it is not a
draft do not call attestMigration — instead return a failure/verification error
(e.g., ok: false or status: 'mismatch' with a clear message) so integrity
verification fails for non-draft migrations.
In
`@packages/1-framework/3-tooling/cli/src/utils/formatters/graph-migration-mapper.ts`:
- Around line 219-227: The dashed edge creation drops lineage when a draft
points to another draft because fromHash is set to matchingDraft?.from without
resolving draft chains; update the logic using matchingDraft (and
input.draftEdges) to walk the draft chain: start at matchingDraft.from and
follow draftEdges (find by d.to) until you reach a hash that exists in
graph.nodes or equals spineTargetHash (guard against cycles/lengths), then set
fromHash to that resolved attested ancestor (or spineTargetHash) before pushing
into edgeList; use the resolved fromHash (and keep the dashed style and label
logic) so chained drafts render from the nearest attested ancestor instead of
being dropped.
In `@packages/1-framework/3-tooling/cli/src/utils/formatters/graph-render.ts`:
- Around line 913-923: The truncation step later (truncateGraph) can still
remove dashed (draft) edges and their off-graph endpoints because only nodes
reachable from the visible spine window are kept; to fix this, ensure dashed
edges and their endpoints are treated as reachable before or inside
truncateGraph by adding dashed endpoints to the visible node set used by
truncateGraph (or by expanding truncateGraph's reachability logic to always
include nodes referenced by any graph.edges with style === 'dashed'); update the
code that builds nodeSet/edgePairs (or truncateGraph) so dashedEdges and their
e.from/e.to IDs are injected into the reachable/visible node collection so
filteredNodes/filteredEdges cannot prune them.
In `@packages/1-framework/3-tooling/migration/src/migration-ts.ts`:
- Around line 119-125: The generated Contract import sometimes becomes a bare
specifier; update the code that builds relativeContractDts (used when
hasDataTransform && options.contractJsonPath) to ensure the import stays
relative by prefixing './' when relativeContractDts does not start with '.';
adjust the lines.push call that emits `import type { Contract } from
"${relativeContractDts}"` so it uses the possibly-prefixed path (e.g., compute a
safeImportPath = relativeContractDts.startsWith('.') ? relativeContractDts :
`./${relativeContractDts}` and use safeImportPath in the import string).
In
`@packages/3-targets/3-targets/postgres/src/core/migrations/descriptor-planner.ts`:
- Around line 418-460: The current descriptor ordering places dropOps before
patternOps, which can remove columns/tables needed by pattern-generated
dataTransform backfills; update the final descriptors array so patternOps come
before destructive dropOps (i.e., build descriptors as [...depOps, ...tableOps,
...columnOps, ...patternOps, ...alterOps, ...constraintOps, ...dropOps] or
otherwise ensure patternOps precede dropOps), adjusting only the order of
depOps/dropOps/patternOps in the descriptors assembly to preserve backfill
reads.
In
`@packages/3-targets/3-targets/postgres/src/core/migrations/planner-strategies.ts`:
- Around line 84-94: The enum-rebuild sequence must drop per-column defaults
before ALTER COLUMN ... TYPE and restore them after the type swap because the
USING cast does not convert enum-typed defaults; modify the sequence around
createEnumType/alterColumnType/dropEnumType/renameType to (1) read and store
each column's current default expression, (2) emit an operation to drop the
default for each column (e.g., alterColumnDropDefault for the refs) before
calling alterColumnType(ref.table, ref.column, ...), and (3) after
renameType(native swap) emit operations to set the saved defaults back (e.g.,
alterColumnSetDefault(ref.table, ref.column, savedDefaultExpr)) so defaults are
preserved through the enum rebuild.
---
Nitpick comments:
In `@packages/1-framework/3-tooling/cli/test/utils/formatters/test-graphs.ts`:
- Around line 28-29: The dashedEdge helper uses an inline conditional spread for
the optional label; replace that pattern by importing and using ifDefined from
`@prisma-next/utils/defined` and spread the result (e.g., ...ifDefined('label',
label)) so the optional label is added consistently with other formatter
codepaths; update the dashedEdge function to use ifDefined('label', label)
instead of ...(label !== undefined ? { label } : {}).
In `@packages/2-sql/4-lanes/sql-builder/test/fixtures/contract.ts`:
- Line 1: Replace the direct re-export in the module (the `export { default as
contract } from './generated/contract.json' with { type: 'json' };` statement)
with an explicit import followed by an explicit export: import the default from
'./generated/contract.json' into a local binding (e.g., `contract`) and then
export that binding as the module's export; ensure you keep the JSON import hint
(`with { type: 'json' }`) when importing so the JSON is loaded correctly.
🪄 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: 67705864-41da-42cb-a5ed-9a6395e10c28
⛔ Files ignored due to path filters (13)
packages/1-framework/3-tooling/cli/test/utils/formatters/__snapshots__/graph-render.test.ts.snapis excluded by!**/*.snappnpm-lock.yamlis excluded by!**/pnpm-lock.yamlprojects/graph-based-migrations/plans/data-migrations-plan.mdis excluded by!projects/**projects/graph-based-migrations/specs/april-milestone.mdis excluded by!projects/**projects/graph-based-migrations/specs/chat.mdis excluded by!projects/**projects/graph-based-migrations/specs/data-migration-scenarios.mdis excluded by!projects/**projects/graph-based-migrations/specs/data-migrations-solutions.mdis excluded by!projects/**projects/graph-based-migrations/specs/data-migrations-spec.mdis excluded by!projects/**projects/graph-based-migrations/specs/data-migrations.mdis excluded by!projects/**projects/graph-based-migrations/specs/planner-replacement-proposal.mdis excluded by!projects/**projects/graph-based-migrations/walkthrough.mdis excluded by!projects/**test/integration/test/sql-builder/fixtures/generated/contract.d.tsis excluded by!**/generated/**test/integration/test/sql-builder/fixtures/generated/contract.jsonis excluded by!**/generated/**
📒 Files selected for processing (80)
packages/1-framework/1-core/framework-components/src/control-migration-types.tspackages/1-framework/1-core/framework-components/src/control-result-types.tspackages/1-framework/1-core/framework-components/src/exports/control.tspackages/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-new.tspackages/1-framework/3-tooling/cli/src/commands/migration-plan.tspackages/1-framework/3-tooling/cli/src/commands/migration-status.tspackages/1-framework/3-tooling/cli/src/commands/migration-verify.tspackages/1-framework/3-tooling/cli/src/control-api/operations/migration-apply.tspackages/1-framework/3-tooling/cli/src/utils/command-helpers.tspackages/1-framework/3-tooling/cli/src/utils/formatters/graph-migration-mapper.tspackages/1-framework/3-tooling/cli/src/utils/formatters/graph-render.tspackages/1-framework/3-tooling/cli/src/utils/formatters/graph-types.tspackages/1-framework/3-tooling/cli/test/utils/formatters/graph-migration-mapper.test.tspackages/1-framework/3-tooling/cli/test/utils/formatters/graph-render.test.tspackages/1-framework/3-tooling/cli/test/utils/formatters/test-graphs.tspackages/1-framework/3-tooling/cli/tsdown.config.tspackages/1-framework/3-tooling/migration/package.jsonpackages/1-framework/3-tooling/migration/src/attestation.tspackages/1-framework/3-tooling/migration/src/errors.tspackages/1-framework/3-tooling/migration/src/exports/io.tspackages/1-framework/3-tooling/migration/src/exports/migration-ts.tspackages/1-framework/3-tooling/migration/src/exports/types.tspackages/1-framework/3-tooling/migration/src/io.tspackages/1-framework/3-tooling/migration/src/migration-ts.tspackages/1-framework/3-tooling/migration/src/types.tspackages/1-framework/3-tooling/migration/tsdown.config.tspackages/2-sql/4-lanes/sql-builder/package.jsonpackages/2-sql/4-lanes/sql-builder/src/exports/types.tspackages/2-sql/4-lanes/sql-builder/src/runtime/index.tspackages/2-sql/4-lanes/sql-builder/test/fixtures/contract.tspackages/2-sql/9-family/package.jsonpackages/2-sql/9-family/src/core/migrations/contract-to-schema-ir.tspackages/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/migrations/types.tspackages/2-sql/9-family/src/core/schema-verify/verify-helpers.tspackages/2-sql/9-family/src/exports/operation-descriptors.tspackages/2-sql/9-family/test/contract-to-schema-ir.test.tspackages/2-sql/9-family/tsdown.config.tspackages/3-extensions/pgvector/package.jsonpackages/3-extensions/pgvector/src/exports/control.tspackages/3-targets/3-targets/postgres/package.jsonpackages/3-targets/3-targets/postgres/src/core/migrations/descriptor-planner.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/planner-reconciliation.tspackages/3-targets/3-targets/postgres/src/core/migrations/planner-strategies.tspackages/3-targets/3-targets/postgres/src/core/migrations/runner.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/test/migrations/descriptor-planner.scenarios.mdpackages/3-targets/3-targets/postgres/test/migrations/descriptor-planner.scenarios.test.tspackages/3-targets/3-targets/postgres/tsdown.config.tspackages/3-targets/6-adapters/postgres/package.jsonpackages/3-targets/6-adapters/postgres/src/core/enum-control-hooks.tspackages/3-targets/6-adapters/postgres/test/enum-control-hooks.basic.test.tstest/integration/test/cli-journeys/data-transform.e2e.test.tstest/integration/test/cli-journeys/migration-apply-edge-cases.e2e.test.tstest/integration/test/cli.migration-apply.e2e.test.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/cli-journeys/contract-unique-email.tstest/integration/test/sql-builder/distinct.test.tstest/integration/test/sql-builder/execution.test.tstest/integration/test/sql-builder/extension-functions.test.tstest/integration/test/sql-builder/fixtures/contract.tstest/integration/test/sql-builder/fixtures/prisma-next.config.tstest/integration/test/sql-builder/group-by.test.tstest/integration/test/sql-builder/join.test.tstest/integration/test/sql-builder/mutation-defaults.test.tstest/integration/test/sql-builder/mutation.test.tstest/integration/test/sql-builder/order-by.test.tstest/integration/test/sql-builder/pagination.test.tstest/integration/test/sql-builder/select.test.tstest/integration/test/sql-builder/setup.tstest/integration/test/sql-builder/subquery.test.tstest/integration/test/sql-builder/where.test.tstest/integration/test/utils/journey-test-helpers.tsturbo.json
💤 Files with no reviewable changes (2)
- packages/3-targets/6-adapters/postgres/package.json
- packages/3-extensions/pgvector/package.json
✅ Files skipped from review due to trivial changes (15)
- packages/1-framework/3-tooling/cli/tsdown.config.ts
- packages/2-sql/9-family/tsdown.config.ts
- packages/1-framework/3-tooling/migration/tsdown.config.ts
- packages/2-sql/4-lanes/sql-builder/src/runtime/index.ts
- packages/1-framework/3-tooling/cli/package.json
- packages/2-sql/9-family/test/contract-to-schema-ir.test.ts
- packages/1-framework/3-tooling/migration/src/exports/io.ts
- test/integration/test/cli-journeys/migration-apply-edge-cases.e2e.test.ts
- packages/1-framework/3-tooling/migration/package.json
- test/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/cli-journeys/contract-unique-email.ts
- packages/1-framework/3-tooling/migration/src/exports/migration-ts.ts
- packages/2-sql/9-family/src/exports/operation-descriptors.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/1-framework/3-tooling/cli/src/commands/migration-apply.ts
🚧 Files skipped from review as they are similar to previous changes (29)
- packages/1-framework/3-tooling/cli/src/control-api/operations/migration-apply.ts
- packages/2-sql/9-family/package.json
- turbo.json
- packages/2-sql/9-family/src/core/migrations/contract-to-schema-ir.ts
- packages/1-framework/3-tooling/migration/src/attestation.ts
- packages/1-framework/3-tooling/cli/src/cli.ts
- packages/2-sql/4-lanes/sql-builder/src/exports/types.ts
- packages/3-targets/3-targets/postgres/package.json
- packages/3-extensions/pgvector/src/exports/control.ts
- packages/1-framework/3-tooling/migration/src/errors.ts
- packages/3-targets/6-adapters/postgres/test/enum-control-hooks.basic.test.ts
- packages/1-framework/3-tooling/cli/test/utils/formatters/graph-migration-mapper.test.ts
- test/integration/test/utils/journey-test-helpers.ts
- packages/3-targets/3-targets/postgres/tsdown.config.ts
- test/integration/test/cli.migration-apply.e2e.test.ts
- packages/1-framework/1-core/framework-components/src/exports/control.ts
- test/integration/test/sql-builder/fixtures/contract.ts
- packages/1-framework/3-tooling/cli/src/utils/command-helpers.ts
- packages/1-framework/3-tooling/migration/src/exports/types.ts
- packages/1-framework/3-tooling/cli/src/commands/migration-new.ts
- packages/1-framework/3-tooling/migration/src/types.ts
- packages/1-framework/1-core/framework-components/src/control-migration-types.ts
- test/integration/test/cli-journeys/data-transform.e2e.test.ts
- packages/1-framework/3-tooling/cli/src/utils/formatters/graph-types.ts
- packages/1-framework/3-tooling/cli/src/commands/migration-status.ts
- packages/3-targets/3-targets/postgres/src/core/migrations/operation-resolver.ts
- packages/3-targets/3-targets/postgres/test/migrations/descriptor-planner.scenarios.test.ts
- packages/2-sql/4-lanes/sql-builder/package.json
- packages/1-framework/3-tooling/cli/test/utils/formatters/graph-render.test.ts
Comment on lines
+130
to
+137
| } else { | ||
| const importList = [...new Set(descriptors.map((d) => d.kind))]; | ||
| if (importList.length === 0) { | ||
| importList.push('createTable'); | ||
| } | ||
| lines.push( | ||
| `import { ${importList.join(', ')} } from "@prisma-next/target-postgres/migration-builders"`, | ||
| ); |
There was a problem hiding this comment.
Import TODO in the untyped data-transform scaffold path.
serializeQueryInput() renders symbol placeholders as TODO, but this branch only imports descriptor builders. If descriptors contains a dataTransform with a TODO placeholder and contractJsonPath is absent, the generated migration.ts references an undefined identifier and fails before verify.
🤖 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-ts.ts` around lines
130 - 137, The generated migration.ts can reference the placeholder identifier
TODO from serializeQueryInput() when a descriptor has kind 'dataTransform' and
contractJsonPath is missing; ensure the import list includes TODO in that
untyped scaffold path. Modify the branch that builds importList (the code using
descriptors.map((d) => d.kind) and importList.push('createTable')) to detect
when any descriptor.kind === 'dataTransform' and contractJsonPath is absent (the
untyped path) and push 'TODO' into importList before emitting the import line so
migration.ts imports TODO from
"@prisma-next/target-postgres/migration-builders".
Comment on lines
+98
to
+99
| /** @deprecated Use PostgresMigrationOpDescriptor */ | ||
| export type MigrationOpDescriptor = PostgresMigrationOpDescriptor; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove the deprecated alias instead of shipping a compatibility shim.
MigrationOpDescriptor is just a second public name for PostgresMigrationOpDescriptor. This repo explicitly avoids backward-compatibility shims in new APIs; update the remaining call sites to the new name instead of exporting both.
As per coding guidelines, "Do not add backward-compatibility shims, migration scaffolding, deprecation warnings, or comments explaining legacy vs. new approaches" and "Update all references immediately to use new approaches instead of maintaining parallel implementations."
🤖 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/operation-descriptors.ts`
around lines 98 - 99, Remove the deprecated type alias MigrationOpDescriptor and
update all call sites to use PostgresMigrationOpDescriptor directly;
specifically delete the line exporting "export type MigrationOpDescriptor =
PostgresMigrationOpDescriptor" and replace any remaining references to
MigrationOpDescriptor in the codebase with PostgresMigrationOpDescriptor (check
imports, type annotations, and public API exports) so only
PostgresMigrationOpDescriptor remains as the canonical exported type.
Comment on lines
+75
to
+82
| const columnRefs: { table: string; column: string }[] = []; | ||
| for (const [tableName, table] of Object.entries(ctx.toContract.storage.tables)) { | ||
| for (const [columnName, column] of Object.entries(table.columns)) { | ||
| if (column.typeRef === typeName) { | ||
| columnRefs.push({ table: tableName, column: columnName }); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Match enum columns by nativeType + codecId here too.
This recipe only follows column.typeRef === typeName, but the enum control hooks treat direct native-type enum columns as dependencies as well. A contract column declared with nativeType === nativeType and the enum codec will be missed here, and dropEnumType(nativeType) can fail because that column still points at the old enum.
🤖 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 75 - 82, The loop that builds columnRefs currently only matches
columns by column.typeRef === typeName; update the condition so it also matches
direct native-type enum columns by checking column.nativeType === nativeType &&
column.codecId === codecId (or the equivalent codec identifier used in this
scope), e.g. change the if to include an OR branch combining both checks,
keeping the pushed object ({ table: tableName, column: columnName }) the same;
reference the columnRefs array, ctx.toContract.storage.tables iteration, and the
typeName/nativeType/codecId symbols to locate where to modify the logic.
Comment on lines
+206
to
+216
| // Data transform operations have a different execution lifecycle | ||
| if (operation.operationClass === 'data' && isDataTransformOperation(operation)) { | ||
| const dtResult = await this.executeDataTransform(driver, operation, { | ||
| runIdempotency, | ||
| }); | ||
| if (!dtResult.ok) { | ||
| return dtResult; | ||
| } | ||
| executedOperations.push(operation); | ||
| operationsExecuted += 1; | ||
| continue; |
There was a problem hiding this comment.
Don't count skipped data transforms as executed.
executeDataTransform() returns success for both “ran” and “skipped”, but this branch always appends the original operation and increments operationsExecuted. That makes check === true and already-applied transforms show up as executed in the ledger/summary, unlike the normal postcheck-pre-satisfied path.
🤖 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/runner.ts` around
lines 206 - 216, The current branch treats any successful executeDataTransform()
result as "executed" and always pushes operation into executedOperations and
increments operationsExecuted; change it so that after calling
executeDataTransform(driver, operation, { runIdempotency }) you first check the
result to determine whether the transform actually ran (e.g. inspect a field
like dtResult.ran / dtResult.skipped / dtResult.status) and only push the
operation into executedOperations and increment operationsExecuted when the
result indicates it ran; keep returning dtResult on failure as-is and continue
the loop on skipped results without counting them.
Comment on lines
+178
to
+183
| return { | ||
| ok: true as const, | ||
| // Safe — PostgresMigrationOpDescriptor satisfies OperationDescriptor structurally. | ||
| // TS can't verify due to discriminated-union-to-index-signature limitation. | ||
| descriptors: planResult.value.descriptors as unknown as OperationDescriptor[], | ||
| needsDataMigration: planResult.value.needsDataMigration, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect the planner descriptor type and the public control-plane descriptor type.
sed -n '1,240p' packages/3-targets/3-targets/postgres/src/core/migrations/operation-descriptors.ts
printf '\n---\n'
sed -n '1,180p' packages/1-framework/1-core/framework-components/src/control-migration-types.tsRepository: prisma/prisma-next
Length of output: 1784
🏁 Script executed:
find . -type f -name "*.ts" | grep -E "(operation-descriptors|control-migration-types)" | head -20Repository: prisma/prisma-next
Length of output: 404
🏁 Script executed:
find . -type f -path "*postgres*" -name "control.ts" | head -5Repository: prisma/prisma-next
Length of output: 251
🏁 Script executed:
find . -type f -name "control.ts" | grep -E "exports|src" | head -10Repository: prisma/prisma-next
Length of output: 662
🏁 Script executed:
# Check the specific file and lines under review
wc -l packages/3-targets/3-targets/postgres/src/exports/control.tsRepository: prisma/prisma-next
Length of output: 126
🏁 Script executed:
# Read the context around lines 178-183
sed -n '170,200p' packages/3-targets/3-targets/postgres/src/exports/control.tsRepository: prisma/prisma-next
Length of output: 1365
🏁 Script executed:
# Inspect the operation descriptor types
head -100 packages/3-targets/3-targets/postgres/src/core/migrations/operation-descriptors.tsRepository: prisma/prisma-next
Length of output: 3277
🏁 Script executed:
# Inspect the public control migration types
head -120 packages/1-framework/1-core/framework-components/src/control-migration-types.tsRepository: prisma/prisma-next
Length of output: 4761
🏁 Script executed:
# Check if there are any converter/type predicate patterns used elsewhere for similar situations
rg "is never" packages/3-targets/3-targets/postgres/src/exports/control.ts -A 3 -B 3Repository: prisma/prisma-next
Length of output: 44
🏁 Script executed:
# Look for how MigrationOpDescriptor is defined and if it includes a kind discriminator
rg -A 10 "type PostgresMigrationOpDescriptor" packages/3-targets/3-targets/postgres/src/core/migrations/operation-descriptors.tsRepository: prisma/prisma-next
Length of output: 607
🏁 Script executed:
# Check if SqlMigrationOpDescriptor and DataTransformDescriptor both have 'kind' property
rg "readonly kind" packages/2-sql/9-family/src/core/migrations/operation-descriptors.ts | head -20Repository: prisma/prisma-next
Length of output: 44
🏁 Script executed:
# Check how OperationDescriptor is used elsewhere in the codebase
rg "OperationDescriptor\[\]" packages/ | head -10Repository: prisma/prisma-next
Length of output: 1398
🏁 Script executed:
# Search for type predicates or converter functions for descriptor types
rg "is.*Descriptor|satisfies.*Descriptor" packages/ -t ts | head -15Repository: prisma/prisma-next
Length of output: 1839
🏁 Script executed:
# Check the SQL family operation descriptors to see if they have 'kind' property
sed -n '50,200p' packages/2-sql/9-family/src/core/migrations/operation-descriptors.tsRepository: prisma/prisma-next
Length of output: 4624
🏁 Script executed:
# Check DataTransformDescriptor definition
rg -B 5 -A 15 "type DataTransformDescriptor" packages/2-sql/9-family/src/core/migrations/operation-descriptors.tsRepository: prisma/prisma-next
Length of output: 44
🏁 Script executed:
# Check the complete OperationDescriptor interface definition
sed -n '50,120p' packages/1-framework/1-core/framework-components/src/control-migration-types.tsRepository: prisma/prisma-next
Length of output: 2662
🏁 Script executed:
# Look for how the discriminated union issue manifests
rg "discriminated-union" packages/ -B 3 -A 3Repository: prisma/prisma-next
Length of output: 882
🏁 Script executed:
# Check if there are any type predicates or guards for descriptor validation elsewhere
rg -B 5 -A 10 "parseDescriptors|isDescriptor|guardDescriptor" packages/3-targets/3-targets/postgres/src/exports/control.tsRepository: prisma/prisma-next
Length of output: 1537
🏁 Script executed:
# Check if the SQL family exports typed descriptor arrays or converter functions
rg -B 3 -A 10 "export.*Descriptor\[\]|export.*function.*Descriptor" packages/2-sql/9-family/src/exports/operation-descriptors.ts | head -40Repository: prisma/prisma-next
Length of output: 44
🏁 Script executed:
# Verify if TypeScript can actually narrow the discriminated union to OperationDescriptor
cat > /tmp/test-discriminated-union.ts << 'EOF'
// Simulating the structure
type SqlMigrationOpDescriptor =
| { kind: 'createTable'; table: string }
| { kind: 'dropTable'; table: string };
interface DataTransformDescriptor {
kind: string;
name: string;
}
type PostgresMigrationOpDescriptor = SqlMigrationOpDescriptor | DataTransformDescriptor;
interface OperationDescriptor {
readonly kind: string;
readonly [key: string]: unknown;
}
// Can we assign without a cast?
function test(descriptors: PostgresMigrationOpDescriptor[]): OperationDescriptor[] {
// This should fail without the cast
return descriptors;
}
EOF
cat /tmp/test-discriminated-union.tsRepository: prisma/prisma-next
Length of output: 645
🏁 Script executed:
# Check if DataTransformDescriptor is properly defined with kind property
rg -B 3 -A 15 "interface DataTransformDescriptor" packages/2-sql/9-family/src/core/migrations/operation-descriptors.tsRepository: prisma/prisma-next
Length of output: 44
🏁 Script executed:
# Verify the actual return type from planWithDescriptors
sed -n '140,175p' packages/3-targets/3-targets/postgres/src/exports/control.tsRepository: prisma/prisma-next
Length of output: 1522
🏁 Script executed:
# Verify the exact signature of planDescriptors to see what it returns
rg -B 5 -A 10 "function planDescriptors|export.*planDescriptors" packages/ -t ts | head -30Repository: prisma/prisma-next
Length of output: 1868
🏁 Script executed:
# Check if there's a converter helper or type guard anywhere for this conversion
rg -B 3 -A 8 "type.*Guard|predicate.*Descriptor|as.*Descriptor\[\]" packages/3-targets/3-targets/postgres/src/exports/control.tsRepository: prisma/prisma-next
Length of output: 675
🏁 Script executed:
# Look at how mongo adapter handles similar descriptor conversions
rg -B 5 -A 5 "OperationDescriptor\[\]" packages/3-mongo-target/2-mongo-adapter/src/exports/control.tsRepository: prisma/prisma-next
Length of output: 44
🏁 Script executed:
# Check what MigrationDescriptorArraySchema validates
rg -B 5 -A 10 "MigrationDescriptorArraySchema" packages/3-targets/3-targets/postgres/src/exports/control.tsRepository: prisma/prisma-next
Length of output: 1510
🏁 Script executed:
# Find the actual schema definition
find . -type f -name "*.ts" -path "*descriptor*" | xargs grep -l "MigrationDescriptorArraySchema" | head -5Repository: prisma/prisma-next
Length of output: 263
🏁 Script executed:
# Check the structure of SqlMigrationOpDescriptor to see if all variants have 'kind'
rg -B 2 "kind.*:" packages/2-sql/9-family/src/core/migrations/operation-descriptors.ts | head -30Repository: prisma/prisma-next
Length of output: 1033
Remove the blind cast on the public descriptor return.
The cast planResult.value.descriptors as unknown as OperationDescriptor[] violates the no-blind-cast rule. While TypeScript cannot directly assign a discriminated union (PostgresMigrationOpDescriptor) to an index-signature type (OperationDescriptor), the solution is not a blind cast but rather a type predicate or better type structure.
Since parseDescriptors() validates the descriptors using MigrationDescriptorArraySchema before they reach this code, you can either:
- Add a type predicate function that checks the shape and lets TypeScript narrow to
OperationDescriptor[] - Ensure
DataTransformDescriptoris a proper base interface that both SQL and data descriptors extend, eliminating the union-to-index-signature conflict
Replace the cast with one of these approaches instead.
🤖 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/exports/control.ts` around lines
178 - 183, The return uses a blind cast "planResult.value.descriptors as unknown
as OperationDescriptor[]" which violates the no-blind-cast rule; replace it by
providing proper type narrowing: add a type predicate function (e.g.,
isOperationDescriptorArray(descr: unknown): descr is OperationDescriptor[]) that
validates the shape against the already-run MigrationDescriptorArraySchema (or
call parseDescriptors() result) and use that predicate to assert the type of
planResult.value.descriptors, or alternatively refactor the shared base types so
PostgresMigrationOpDescriptor and DataTransformDescriptor share a common
OperationDescriptor-compatible base (e.g., ensure DataTransformDescriptor is the
base interface) and then remove the cast and return planResult.value.descriptors
directly; reference planResult.value.descriptors, OperationDescriptor,
PostgresMigrationOpDescriptor, parseDescriptors(),
MigrationDescriptorArraySchema and DataTransformDescriptor when making the
change.
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-2173
Intent
Introduce data migration support into the graph-based migration system. Schema changes involving data transformations (NOT NULL backfills, type conversions, enum value changes) are authored as TypeScript operation chains using a new descriptor-based planner, serialized to SQL at verify time, and executed at apply time with retry safety.
Note: the old reconciliation planner is still used by
db updateanddb init. Those two commands are unchanged. The new descriptor planner is used exclusively bymigration plan. The old planner will be removed in a follow-up once dev-push strategies are implemented.What this enables
migration plandetects data migration needs via pluggable strategies and scaffoldsmigration.tswith TODO placeholdersmigration newcreates empty migrations for manual authoringmigration verifyevaluates TypeScript, validates with arktype schemas, resolves to SQL, attestsmigration applyexecutes lowered SQL with check → run → check retry safetymigration statusshows draft migrations as dashed graph edgescreateBuilders<Contract>()provides full autocomplete on tables, columns, and extension functionsChange map
Implementation:
migration newcommandTests:
.failsas gap map)The story
Move sql-builder integration tests to break a dependency cycle. This enables
target-postgresto depend onsql-builderfor the typed query builder needed by data transform callbacks. Phantom CLI dependencies are cleaned up from 4 packages.Introduce descriptor types as arktype schemas at the sql-family level. 21 descriptor types (createTable, addColumn, alterColumnType, addEnumValues, dropEnumType, renameType, etc.) defined as arktype schemas in
descriptor-schemas.ts— TypeScript types derived viatypeof schema.infer. Shared across SQL targets. User-authored migration.ts output is validated at theresolveDescriptorsboundary.Build the operation resolver and migration.ts round-trip. The resolver converts thin descriptors into
SqlMigrationPlanOperationobjects with SQL, prechecks, and postchecks.scaffoldMigrationTs()serializes descriptors as TypeScript builder calls.evaluateMigrationTs()loads migration.ts via Node's native TypeScript import and returns the descriptor array.Build the descriptor-based planner with pluggable strategies. Four migration strategies detect data migration needs:
notNullBackfillStrategy(addColumn nullable + dataTransform + setNotNull),typeChangeStrategy(safe widenings → alterColumnType, unsafe → dataTransform + alterColumnType),nullableTighteningStrategy(dataTransform + setNotNull),enumChangeStrategy(add → addEnumValues, removal → dataTransform + rebuild recipe, reorder → rebuild recipe). The enum rebuild recipe uses primitive descriptors: createEnumType(temp) → alterColumnType(USING cast) → dropEnumType(old) → renameType(temp, old).Extend the runner with data transform lifecycle. The runner handles
operationClass: 'data'with check → skip/run → check. The first check provides retry safety; the second validates violations are resolved.Add draft migration visibility. Draft packages appear as dashed graph edges in
migration status, with[draft]labels. Status diagnostics, apply warnings, and plan guards for duplicate drafts.Wire query builder integration.
createBuilders<Contract>()returns typed builder functions wheredataTransformcallbacks receiveDb<Contract>with full autocomplete. SQL lowered at verify time via the Postgres adapter. Extension functions (e.g., pgvector cosineDistance) available viaqueryOperationson control descriptors.Known issues for follow-up
Blocking further work
ALTER TYPE ADD VALUEcan't run in a transaction — enum value addition works at the planner/resolver level but fails at apply time. NeedsnoTransactiondescriptor support. Enum removal and reorder work fine (rebuild recipe uses transactional DDL).db update/db initstill use the old reconciliation planner — 2 production call sites. The descriptor planner supports this viaplanDescriptors({ strategies: devPushStrategies })but the dev-push strategies (temp defaults, destructive changes without data transforms) haven't been implemented yet.Minor gaps
.fails) — verifier doesn't flag types referenced by amissing_table.fails) —dropColumnordered before pattern opsmigration verifyre-attests unconditionally — even for already-applied migrationsresolveSetDefaultandresolveDropDefaulthave empty postchecks — relies on post-apply schema verification as safety netNot in scope
db updatedev-push strategies — separate follow-upTest plan
pnpm typecheck:packagespassespnpm lint:packagespassespnpm lint:depspassespnpm -w run test:packagespasses (2 known.failsin scenario tests)pnpm test:journeyspasses (57/57)pnpm test:integrationpassesSummary by CodeRabbit
New Features
migration newcommand to scaffold manual migration packagesImprovements
Chores