TML-2771: migrate --show — read-only preview of the migration path#735
Merged
wmadden merged 42 commits intoJun 7, 2026
Merged
Conversation
Signed-off-by: Will Madden <madden@prisma.io>
Signed-off-by: Will Madden <madden@prisma.io>
…+ legend (TML-2771) The shared Tier-3 overlay previously drew reserved markers in angle-brackets (`<contract, db>`). Now draws them with the `@`-sigil — `@contract @db` — matching the token spelling users will type into `--from`/`--to`. User refs keep parentheses. The `--legend` example markers and explanatory text are updated in lockstep: the legend now reads `@contract @db reserved markers — also typeable as --from/--to tokens`. All tests updated; `graph`/`status`/ `list` snapshot coverage regenerated. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Will Madden <madden@prisma.io>
Extends the contract-reference grammar accepted by --from/--to with two reserved sigil tokens: - `@contract` resolves offline to the on-disk working contract hash. Requires `ctx.contractHash` in the resolution context; returns `not-found` when absent. - `@db` signals "live database marker, connection required." Returns a sentinel `ContractRef` with `provenance.kind === 'reserved-db'`; callers must check the provenance and resolve the actual hash via `readAllMarkers()`. Unit tests added to `contract-ref.test.ts` covering both tokens and the offline absence of `contractHash` for `@contract`. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Will Madden <madden@prisma.io>
… ordered list (TML-2771) Adds `migrate --show [--from <ref>] [--to <ref>]`: a preview of the path `migrate` will take, with no writes of any kind. Design: - Default from-state: reads the live DB marker via `readAllMarkers()` (requires `--db`). Explicit `--from X` runs offline — no connection needed. - `@contract`/`@db` tokens from D2 are accepted as `--from` values. - Path computed by `graphWalkStrategy()` — the exact same seam real `migrate` uses. The command returns before `runMigration()` (the write boundary). - Output: a linear, ordered list of migrations that will execute, plus a summary line. `--json` emits the structured `MigrateShowResult`. Edge cases handled: no path (structured error), already-at-target (empty list + "nothing to run"), `@db`/no-connection (structured error). Tests prove: (a) `runMigration` is never called when `--show` is passed, and (b) `graphWalkStrategy` is called for path computation. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Will Madden <madden@prisma.io>
… (TML-2771) Extends the renderer's existing `MigrationEdgeAnnotation` / `edgeAnnotationsByHash` hook (no parallel renderer) with a `pathHighlight` field: - `'on-path'`: edge rendered with bright-green dirName + `↑ will run` suffix. - `'off-path'`: edge rendered with spaces for the dirName (unlabelled) and a dimmed hash column. `migrate --show` builds the annotation map from the on-path migration hash set (from D3's `graphWalkStrategy` output) and calls `renderMigrationGraphTree` via the existing shared Tier-3 renderer. The graph is output before the ordered execution list, using the @-vocabulary from D1 for @contract/@db markers. Snapshot test: DB one migration behind target — on-path edge labelled, off-path edge unlabelled, ordered list present. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Will Madden <madden@prisma.io>
…o migrate (TML-2771) Extract `planMemberPath` from `executeMigrate` in `control-api/operations/migrate.ts` and call it from both `executeMigrate` and `executeMigrateShowCommand`, so it is structurally impossible for the preview's graphWalkStrategy inputs to diverge from the real apply path. Before: the show command stripped the live marker's invariants (always passing `invariants: []` to graphWalkStrategy) and ignored the --to ref's invariants (always using `headRef.invariants = []` from the contract-derived app head ref). Both divergences produce a wrong `required` set inside graphWalkStrategy and can select a different path — or return unsatisfiable when apply would succeed. After: `planMemberPath` assembles targetHash, targetInvariants (from refInvariants when a --to ref was resolved), and currentMarker (the full live marker with invariants) identically for both callers. The empty-graph at-head/never-planned cases are also aligned: the show command now fails loudly for never-planned members instead of silently continuing. Strengthen the faithfulness test to assert the actual graphWalkStrategy call arguments on an invariant-bearing fixture (--to ref with invariants: ['inv-a']). The old bug would have called graphWalkStrategy with headRef.invariants = [], failing the new expect(callArg.member.headRef.invariants).toEqual(['inv-a']) assertion. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Will Madden <madden@prisma.io>
Signed-off-by: Will Madden <madden@prisma.io>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds a read-only ChangesMigrate --show preview mode
🎯 4 (Complex) | ⏱️ ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant Config
participant Aggregate
participant Planner as planMemberPath
participant DB as LiveDB
participant Renderer
CLI->>Config: load config, contract.json
CLI->>Aggregate: load on-disk aggregates
alt from omitted or `@db`
CLI->>DB: readAllMarkers()
DB-->>CLI: live markers
else from explicit ref/hash
CLI->>Config: parse `--from` (may use contractHash)
end
CLI->>Planner: planMemberPath(member, targetHash, refInvariants, liveMarker)
Planner-->>CLI: MemberPathOutcome (plan / unsatisfiable / at-head)
CLI->>Renderer: renderMigrationGraphTree(edgeAnnotations, isAppSpace)
Renderer-->>CLI: graphOutput
CLI->>stdout: formatted show output (graph + ordered migrations)
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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/extension-author-tools
@prisma-next/mongo-runtime
@prisma-next/family-mongo
@prisma-next/sql-runtime
@prisma-next/family-sql
@prisma-next/extension-arktype-json
@prisma-next/middleware-cache
@prisma-next/mongo
@prisma-next/extension-paradedb
@prisma-next/extension-pgvector
@prisma-next/extension-postgis
@prisma-next/postgres
@prisma-next/sql-orm-client
@prisma-next/sqlite
@prisma-next/extension-supabase
@prisma-next/target-mongo
@prisma-next/adapter-mongo
@prisma-next/driver-mongo
@prisma-next/contract
@prisma-next/utils
@prisma-next/config
@prisma-next/errors
@prisma-next/framework-components
@prisma-next/operations
@prisma-next/ts-render
@prisma-next/contract-authoring
@prisma-next/ids
@prisma-next/psl-parser
@prisma-next/psl-printer
@prisma-next/cli
@prisma-next/cli-telemetry
@prisma-next/emitter
@prisma-next/migration-tools
prisma-next
@prisma-next/vite-plugin-contract-emit
@prisma-next/mongo-codec
@prisma-next/mongo-contract
@prisma-next/mongo-value
@prisma-next/mongo-contract-psl
@prisma-next/mongo-contract-ts
@prisma-next/mongo-emitter
@prisma-next/mongo-schema-ir
@prisma-next/mongo-query-ast
@prisma-next/mongo-orm
@prisma-next/mongo-query-builder
@prisma-next/mongo-lowering
@prisma-next/mongo-wire
@prisma-next/sql-contract
@prisma-next/sql-errors
@prisma-next/sql-operations
@prisma-next/sql-schema-ir
@prisma-next/sql-contract-psl
@prisma-next/sql-contract-ts
@prisma-next/sql-contract-emitter
@prisma-next/sql-lane-query-builder
@prisma-next/sql-relational-core
@prisma-next/sql-builder
@prisma-next/target-postgres
@prisma-next/target-sqlite
@prisma-next/adapter-postgres
@prisma-next/adapter-sqlite
@prisma-next/driver-postgres
@prisma-next/driver-sqlite
commit: |
size-limit report 📦
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/1-framework/3-tooling/cli/src/utils/formatters/migration-graph-tree-render.ts (1)
453-497:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse the actual edge kind for the
will runsuffix.Line 486 hardcodes a forward-only glyph. If a preview path contains a rollback or self edge, the gutter still renders the correct
↓/⟲direction, but the suffix becomes↑ will runor> will run, which makes the preview inconsistent with the path it is showing. Pass the edge kind, or the already-resolved arrow glyph, intoformatEdgeAnnotationSuffix(...)instead of synthesizing a forward arrow.Suggested fix
-function formatEdgeAnnotationSuffix( - migrationHash: string, +function formatEdgeAnnotationSuffix( + edge: ClassifiedEdge, opts: RenderMigrationGraphTreeOptions, style: MigrationListStyler, + palette: MigrationGraphTreeGlyphPalette, ): string { - const annotation = opts.edgeAnnotationsByHash?.get(migrationHash); + const annotation = opts.edgeAnnotationsByHash?.get(edge.migrationHash); if (annotation === undefined) { return ''; } @@ if (annotation.pathHighlight === 'on-path') { - const glyph = opts.glyphMode === 'ascii' ? '>' : '↑'; + const glyph = arrowForEdgeKind(edge.kind, palette); const label = 'will run'; if (!opts.colorize) { segments.push(`${glyph} ${label}`); @@ - const annotationSuffix = formatEdgeAnnotationSuffix(edge.migrationHash, opts, style); + const annotationSuffix = formatEdgeAnnotationSuffix(edge, opts, style, palette);Also applies to: 768-768
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/1-framework/3-tooling/cli/src/utils/formatters/migration-graph-tree-render.ts` around lines 453 - 497, formatEdgeAnnotationSuffix currently hardcodes a forward/up glyph for the "will run" suffix, causing mismatches when the preview path edge is a rollback or self-edge; change the function signature of formatEdgeAnnotationSuffix to accept the resolved edge kind or the already-resolved arrow glyph (e.g., add a parameter like edgeKind or edgeGlyph), update all callers to pass the edge's kind/glyph (respecting opts.glyphMode) instead of letting the function synthesize '>'/'↑', and use that passed-in glyph when building the "will run" segment so the suffix matches the rendered gutter arrow.
🧹 Nitpick comments (3)
packages/1-framework/3-tooling/cli/test/commands/migrate-show.test.ts (1)
3-3: ⚡ Quick winUse
patheinstead ofnode:pathfor consistency.This file imports
joinfromnode:path, but other test files in this PR (migration-graph.test.ts, migration-legend-commands.test.ts) usepathe. The guidelines preferpatheover Node's built-in path module across the codebase, including tests.📦 Proposed fix
-import { join } from 'node:path'; +import { join } from 'pathe';Based on learnings, the Prisma-next repository prefers importing and using 'pathe' over the built-in 'node:path' module for path operations across the codebase (including CLI commands and tooling files), and the learning states this applies to "any TypeScript file (including tests, test helpers, and integration tooling)".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/1-framework/3-tooling/cli/test/commands/migrate-show.test.ts` at line 3, Replace the import of join from Node's built-in path with the project's preferred pathe module: change the import that currently pulls join from 'node:path' to import join from 'pathe' so the test uses pathe like the other migration tests (migration-graph.test.ts, migration-legend-commands.test.ts) and update any usages if necessary to match pathe semantics.packages/1-framework/3-tooling/cli/src/commands/migrate.ts (1)
445-445: 💤 Low valuePrefer
ifDefinedfor conditional spreads.The ternary-based spread
...(graphOutput !== undefined ? { graphOutput } : {})can be replaced with...ifDefined('graphOutput', graphOutput)for consistency with the codebase convention.♻️ Suggested refactor
return ok({ ok: true, migrations: orderedMigrations, summary, - ...(graphOutput !== undefined ? { graphOutput } : {}), + ...ifDefined('graphOutput', graphOutput), });Based on learnings: When using the Prisma Next
ifDefinedhelper fromprisma-next/utils/defined, prefer it for conditional object spreads instead of inline ternary-based spreads.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/1-framework/3-tooling/cli/src/commands/migrate.ts` at line 445, Replace the inline ternary spread ...(graphOutput !== undefined ? { graphOutput } : {}) with the prisma-next helper spread ...ifDefined('graphOutput', graphOutput) to match project conventions; ensure the ifDefined function from 'prisma-next/utils/defined' is imported (or add it if missing) and remove the old ternary expression in the object literal where graphOutput is being spread (look for the object construction in migrate.ts that currently uses graphOutput).packages/1-framework/3-tooling/cli/src/control-api/operations/migrate.ts (1)
169-169: 💤 Low valuePrefer
ifDefinedfor conditional spreads.The ternary-based spread
...(isAppMember ? { refName } : {})can be replaced with...ifDefined('refName', isAppMember ? refName : undefined)for consistency with the codebase convention. This improves readability and aligns with the pattern used elsewhere (e.g., line 274).♻️ Suggested refactor
const outcome = planMemberPath({ member, aggregate, targetHash: memberTargetHash, refInvariants: memberRefInvariants, liveMarker, - ...(isAppMember ? { refName } : {}), + ...ifDefined('refName', isAppMember ? refName : undefined), });and
const walked = graphWalkStrategy({ aggregateTargetId: aggregate.targetId, member: targetMember, currentMarker: liveMarker, - ...(isAppMember && refName !== undefined ? { refName } : {}), + ...ifDefined('refName', isAppMember ? refName : undefined), });Based on learnings: When using the Prisma Next
ifDefinedhelper fromprisma-next/utils/defined, prefer it for conditional object spreads instead of inline ternary-based spreads.Also applies to: 414-414
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/1-framework/3-tooling/cli/src/control-api/operations/migrate.ts` at line 169, Replace the ternary-based conditional spread ...(isAppMember ? { refName } : {}) with the repository's ifDefined helper: import ifDefined from 'prisma-next/utils/defined' if not already present, then use ...ifDefined('refName', isAppMember ? refName : undefined) so the object only includes refName when defined; apply the same change for the other identical conditional spread in the file that uses refName.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In
`@packages/1-framework/3-tooling/cli/src/utils/formatters/migration-graph-tree-render.ts`:
- Around line 453-497: formatEdgeAnnotationSuffix currently hardcodes a
forward/up glyph for the "will run" suffix, causing mismatches when the preview
path edge is a rollback or self-edge; change the function signature of
formatEdgeAnnotationSuffix to accept the resolved edge kind or the
already-resolved arrow glyph (e.g., add a parameter like edgeKind or edgeGlyph),
update all callers to pass the edge's kind/glyph (respecting opts.glyphMode)
instead of letting the function synthesize '>'/'↑', and use that passed-in glyph
when building the "will run" segment so the suffix matches the rendered gutter
arrow.
---
Nitpick comments:
In `@packages/1-framework/3-tooling/cli/src/commands/migrate.ts`:
- Line 445: Replace the inline ternary spread ...(graphOutput !== undefined ? {
graphOutput } : {}) with the prisma-next helper spread
...ifDefined('graphOutput', graphOutput) to match project conventions; ensure
the ifDefined function from 'prisma-next/utils/defined' is imported (or add it
if missing) and remove the old ternary expression in the object literal where
graphOutput is being spread (look for the object construction in migrate.ts that
currently uses graphOutput).
In `@packages/1-framework/3-tooling/cli/src/control-api/operations/migrate.ts`:
- Line 169: Replace the ternary-based conditional spread ...(isAppMember ? {
refName } : {}) with the repository's ifDefined helper: import ifDefined from
'prisma-next/utils/defined' if not already present, then use
...ifDefined('refName', isAppMember ? refName : undefined) so the object only
includes refName when defined; apply the same change for the other identical
conditional spread in the file that uses refName.
In `@packages/1-framework/3-tooling/cli/test/commands/migrate-show.test.ts`:
- Line 3: Replace the import of join from Node's built-in path with the
project's preferred pathe module: change the import that currently pulls join
from 'node:path' to import join from 'pathe' so the test uses pathe like the
other migration tests (migration-graph.test.ts,
migration-legend-commands.test.ts) and update any usages if necessary to match
pathe semantics.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 0ec87024-74c6-4719-8809-3961e19910a3
⛔ Files ignored due to path filters (4)
projects/migration-graph-rendering/decisions.mdis excluded by!projects/**projects/migration-graph-rendering/slices/migrate-show-preview/code-review.mdis excluded by!projects/**projects/migration-graph-rendering/slices/migrate-show-preview/plan.mdis excluded by!projects/**projects/migration-graph-rendering/slices/migrate-show-preview/spec.mdis excluded by!projects/**
📒 Files selected for processing (12)
packages/1-framework/3-tooling/cli/src/commands/migrate.tspackages/1-framework/3-tooling/cli/src/control-api/operations/migrate.tspackages/1-framework/3-tooling/cli/src/utils/formatters/migration-graph-tree-render.tspackages/1-framework/3-tooling/cli/src/utils/formatters/migration-list-styler.tspackages/1-framework/3-tooling/cli/test/commands/migrate-show.test.tspackages/1-framework/3-tooling/cli/test/commands/migration-graph.test.tspackages/1-framework/3-tooling/cli/test/commands/migration-legend-commands.test.tspackages/1-framework/3-tooling/cli/test/utils/formatters/migration-graph-tree-render.test.tspackages/1-framework/3-tooling/cli/test/utils/formatters/migration-list-styler.test.tspackages/1-framework/3-tooling/migration/src/refs/contract-ref.tspackages/1-framework/3-tooling/migration/src/refs/types.tspackages/1-framework/3-tooling/migration/test/refs/contract-ref.test.ts
… --show (TML-2771) Two correctness bugs from the operator visual review of PR #735: BUG 1: @contract marked the --to target hash instead of the app's working contract. The renderer received contractHash = targetHash; it now receives contractHash = the working contract from buildReadAggregate, independent of --to. BUG 2: @contract leaked into extension spaces when migrate --show rendered a multi-space graph. Extension space renders no longer receive contractHash, so no floating @contract node appears in extension sections. The command now also renders all spaces (app + extensions), matching migrate's real multi-space behaviour. Also wires the spec/decisions doc to record the D-MS3 revised decision. Tests added: - @contract marks the working contract, not the --to target - @contract does not appear in extension spaces (multi-space fixture) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Will Madden <madden@prisma.io>
…green (TML-2771) Visual refinements from the operator review of migrate --show PR #735: REFINEMENT 3: off-path migration rows are now fully drawn in uniform dim grey — name, hash, and lane lines — instead of showing blank spaces where the name would be. Nothing is omitted from the graph. REFINEMENT 4: on-path rows now apply bright green to the lane/gutter cells (direction arrows, branch lines) and the hash column, not just the migration name. The whole row reads as one colour. Both changes are confined to formatEdgeAnnotationSuffix and the edge-row rendering block in renderMigrationGraphTree. No shared graph/status/list rendering paths are affected — the pathHighlight annotation is only set by migrate --show; other commands leave it undefined, taking the unchanged code paths. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Will Madden <madden@prisma.io>
…ng (TML-2771) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Will Madden <madden@prisma.io>
…e (TML-2771) Signed-off-by: Will Madden <madden@prisma.io>
Contributor
Author
|
Addressed the visual-review feedback (3 commits: Bugs fixed
Visual refinements
Reviewed (opus): all six correct on disk; read-only + faithfulness intact; the shared |
…d renderer (TML-2771) `@contract` (the app's working/desired contract) is an app-space concept. `migration graph` and `migration status` both passed the app's `liveContractHash` to every space's render — including extension spaces like `pgvector:` — so any extension space whose working-contract was detached from its graph would show a spurious floating `@contract` node. Fix: add `isAppSpace?: boolean` (default `true`) to `RenderMigrationGraphTreeOptions` and gate the `@contract` marker in `overlayNamesForContract` behind `isAppSpace !== false`. Mirror the gate in `RenderMigrationGraphSpaceTreeInput` so the space-render layer also skips passing `contractHash` to `buildMigrationGraphRows` for extension spaces (suppressing the floating working-contract node, not just its label). Update all callers per space: - `migration-graph.ts`: passes `isAppSpace` derived from `spaceId === aggregate.app.spaceId` - `migration-status.ts`: same derivation in the per-space loop - `migrate.ts` (`--show`): drops the caller-side `spaceContractHash` workaround in favour of the structural gate (`contractHash` always passed to `renderMigrationGraphTree`, `isAppSpace` set per space) `@db` is not app-gated — it is a per-space marker and must render in every space. Add a multi-space renderer test suite (`renderMigrationGraphTree isAppSpace gate`) covering: app-space shows `@contract`; extension-space suppresses it even when `contractHash` matches; extension-space produces no floating working-contract node; `@db` still renders in extension spaces; omitting `isAppSpace` defaults to app-space behaviour. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Will Madden <madden@prisma.io>
…s (TML-2771) Signed-off-by: Will Madden <madden@prisma.io>
Contributor
Author
|
Good catch — the
Reviewed (opus): SATISFIED — marker + floating node both gated, |
…2771) The offline-–from branch in executeMigrateShowCommand was calling markerBySpace.set(member.spaceId, offlineMarker) for every member, including extension spaces. This caused planMemberPath to attempt a graph walk from the app's --from hash in the extension's graph, which has no such node, producing "No migration path from sha256:76c1bd5 to sha256:... in space 'pgvector'". Fix: apply the offline marker only to the app member's space ID. Extension members stay absent from markerBySpace (null / greenfield), so planMemberPath plans each extension from its own marker → own head, exactly as executeMigrate does via readAllMarkers(). Adds a multi-space fixture test that passes --from <app-hash> --to <app-hash> and asserts the extension migration appears in the ordered list (planned from greenfield) and the suppressed app migration does not appear (–from scoping is respected). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Will Madden <madden@prisma.io>
…on (TML-2771) The previous implementation applied dim()/greenBright() as a post-hoc wrapper around the already-rotation-coloured gutter string. This had two problems: 1. Off-path rows rendered in their rotation colour (e.g. red for column 5), because dim(red(text)) still displays red — the inner escape wins. The expected result is uniform dim grey. 2. Node rows (contract markers, ○/∅) received no path-highlight treatment at all — only edge rows had the wrapper applied. Fix: add an optional laneOverride parameter to renderCellPair (and renderNodeMarkerPair) that replaces the laneStylerForColumn call at the source, before any rotation hue is emitted. When laneOverride is provided (dim for off-path, greenBright for on-path), it wins over the rotating lane colour for every cell kind in the row. Also compute contractHighlights before the render loop: a Map from contract hash to path-highlight derived from edgeAnnotationsByHash (on-path wins when a contract appears in both). Node rows look up their contractHash in this map and receive the matching laneOverride, covering node markers and their adjacent lane segments. The laneOverride is undefined for every command that does not supply edgeAnnotationsByHash (graph/status/list), preserving the rotating coloured-lanes behaviour unchanged for those commands. Adds regression-guard tests: node rows for on/off-path contracts are present and contain their hashes; branching-graph test confirms the on-path edge gets 'will run' suffix and the off-path does not. The test env runs NO_COLOR=1 so actual ANSI hues cannot be asserted — the colour routing is confirmed by code inspection and operator visual review. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Will Madden <madden@prisma.io>
…-colour override (TML-2771) Signed-off-by: Will Madden <madden@prisma.io>
Contributor
Author
|
Both fixed (commits
Off-path branches rendered red (the rotating lane colour) and the on-path highlight only covered one segment. The Colour-hue note: the test env runs |
…edule (extensions first) (TML-2771) The "Will run, in order:" list was built by iterating allMembers which puts app first, so extension migrations appeared last. The runner (operations/migrate.ts) applies extensions alphabetically first, then app. The show command now collects orderedMigrations in the same canonicalOrderMembers sequence ([...aggregate.extensions, aggregate.app]) so the listed order matches the actual execution order. Adds a multi-space test asserting the extension migration (pgvector) appears before the app migrations in the ordered list. Signed-off-by: Will Madden <madden@prisma.io> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ering (TML-2771)
Five colour bugs fixed in renderMigrationGraphTree:
1. Node hash text was dim+cyan instead of greenBright/dim.
Now uses rowLaneOverride (forcedGreenBright / forcedDim) instead of
style.sourceHash when a path-highlight is active.
2. On-path edge from→to hashes had the wrong colour. Wrapping an already-styled
hashColumn string in an outer greenBright does not work — inner ANSI codes
(dim+cyan from sourceHash, brightCyan from destHash) override the outer wrapper
at the terminal level. Fixed by passing the override directly to
formatEdgeHashColumn, which now accepts an optional hashOverride that replaces
all sub-stylers (sourceHash, destHash, arrow glyph) uniformly.
3. Off-path destination hash was brightCyan instead of dim. Same root cause as
above; same fix.
4. Branch connector rows rendered off-path lanes in their rotation colour (e.g.
magenta for lane 1). Fixed by building a per-column override map from
edgeAnnotationsByHash + model.edgeColumn and passing it to renderConnectorRow,
which now applies the override per cell column instead of the rotation colour.
5. All path-highlight functions (pathLaneOverride, dirName, suffix, hashOverride)
now use forcedGreenBright / forcedDim from createColors({ useColor: true })
instead of module-level dim / greenBright. The forced variants always emit ANSI
regardless of NO_COLOR=1, which makes the highlighting reachable in unit tests
and removes a class of blind-editing errors.
Adds a colour-forced unit test that asserts on-path rows carry \x1b[92m
(greenBright), off-path rows carry \x1b[2m (dim), and no rotation colour
(\x1b[31m red, \x1b[35m magenta) appears on any highlighted row.
Verification: ran the repro topology (∅ forks to on-path chain ∅→76c1bd5→…→f7a8eb5
and off-path ∅→1375f13) through a wip/verify-colors.mts script with FORCE_COLOR=1
and inspected raw ANSI. Before: node hashes dim+cyan, dest hash brightCyan, branch
connector magenta. After: every on-path cell greenBright, every off-path cell dim,
connector branch dim, no rotation colour anywhere.
Graph/status/list snapshots unchanged (path-highlight code is no-op when
edgeAnnotationsByHash is absent).
Signed-off-by: Will Madden <madden@prisma.io>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ments (TML-2771) Updates D-MS3 in the migrate-show-preview spec to document the precise colour and ordering requirements that were repaired in this session: on-path green on every cell (node markers, contract-hash text, edge from→to hashes, lane segments), off- path dim grey on every cell including the destination hash, and the "Will run, in order:" list ordered by the runner's canonical cross-space schedule (extensions first, then app). Signed-off-by: Will Madden <madden@prisma.io> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…-highlight colours (TML-2771) Signed-off-by: Will Madden <madden@prisma.io>
Contributor
Author
|
Fixed all five, and — importantly — the colours are now verified against the actual ANSI, not asserted blind (the prior misses were because the test env forces
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/1-framework/3-tooling/cli/src/utils/formatters/migration-graph-tree-render.ts`:
- Around line 392-395: The current case block treats both 'branch-tee' and
'merge-tee' the same and when seenTee is true it applies
palette.connectorBranchTeeCo, which flips merge glyphs; update the logic in the
switch handling 'branch-tee'/'merge-tee' so that 'merge-tee' uses
palette.connectorMergeTeeCo when seenTee is true (and palette.connectorMergeTee
when seenTee is false), while 'branch-tee' continues to use
palette.connectorBranchTeeCo/ palette.connectorBranchTee; keep the
override(seenTee ? ... : ...) pattern and preserve the seenTee = true
assignment.
🪄 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: 723289d5-68c3-41d0-b640-028ffb66ee6a
⛔ Files ignored due to path filters (2)
projects/migration-graph-rendering/slices/migrate-show-preview/code-review.mdis excluded by!projects/**projects/migration-graph-rendering/slices/migrate-show-preview/spec.mdis excluded by!projects/**
📒 Files selected for processing (4)
packages/1-framework/3-tooling/cli/src/commands/migrate.tspackages/1-framework/3-tooling/cli/src/utils/formatters/migration-graph-tree-render.tspackages/1-framework/3-tooling/cli/test/commands/migrate-show.test.tspackages/1-framework/3-tooling/cli/test/utils/formatters/migration-graph-tree-render.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/1-framework/3-tooling/cli/test/commands/migrate-show.test.ts
- packages/1-framework/3-tooling/cli/src/commands/migrate.ts
- packages/1-framework/3-tooling/cli/test/utils/formatters/migration-graph-tree-render.test.ts
… green highlight) (TML-2771) On-path migrations now render in ordinary colours (no forced greenBright override). Off-path migrations continue to render dim. This removes the forcedGreenBright from pathLaneOverride, dirName, hashColumnOverride, and the "↑ will run" suffix, replacing each with either forcedDim (off-path) or no override (on-path). Test updated to assert no GREEN_BRIGHT on on-path rows. Co-Authored-By: Will Madden <madden@prisma.io> Signed-off-by: Will Madden <madden@prisma.io>
…2771) renderSpaceTreeBlock in migration-list-render.ts was calling renderMigrationGraphSpaceTree without isAppSpace, so @contract and the floating contract node appeared in every space including extensions. Thread appSpaceId through RenderMigrationListWithStyleOptions → renderSpaceTreeBlock → renderMigrationGraphSpaceTree, pass aggregate.app.spaceId at the call site. Add multi-space test asserting @contract appears only under app:, not pgvector:. Co-Authored-By: Will Madden <madden@prisma.io> Signed-off-by: Will Madden <madden@prisma.io>
…TML-2771) Pre-build all space layouts before rendering so globalMaxEdgeTreePrefixWidth and globalMaxDirNameWidth can be computed once across every space. Pass both to each renderMigrationGraphTree call and use globalMaxDirNameWidth as the run-list name column width, so the name column, from→to hashes, and ops start at the same offset in every section and the "Will run, in order:" list. Remove the now-unused greenBright run-list colouring (consistent with FIX A dropping the green highlight). Add a test asserting the hash column aligns across two spaces with different dirName lengths. Co-Authored-By: Will Madden <madden@prisma.io> Signed-off-by: Will Madden <madden@prisma.io>
…lanes dim correctly (TML-2771) Previously renderMigrationGraphTree applied a single rowLaneOverride to ALL cells in an edge or node row. This caused off-path branch rows to dim the on-path trunk's vertical-pass cells (and vice-versa) because the row-level override didn't distinguish which column each cell belonged to. Fix: in the gutter-building loop, classify vertical-pass cells by columnHighlights (the column's dominant edge annotation) and edge-lane cells by their own cell.migrationHash annotation, rather than using the uniform row-level override. Connector rows already used per-column overrides via columnLaneOverride and are unchanged. Node and other structural cells continue to use the row-level (contractHighlights) classification. Add test: per-segment case confirms that a vertical-pass at an on-path column in an off-path row does NOT carry forced-dim, while the off-path name/hash on the same line still does. Co-Authored-By: Will Madden <madden@prisma.io> Signed-off-by: Will Madden <madden@prisma.io>
…umn alignment (TML-2771) Signed-off-by: Will Madden <madden@prisma.io>
Contributor
Author
|
Addressed all four, force-render-verified against the exact topologies:
Reviewed (opus) with independent force-colour rendering of each case. |
…tree-renderer (TML-2771) Signed-off-by: Will Madden <madden@prisma.io>
…ff-path dim, rotation suppressed; list shares tree renderer (TML-2771) Signed-off-by: Will Madden <madden@prisma.io>
…ur scheme (TML-2771) Signed-off-by: Will Madden <madden@prisma.io>
Contributor
Author
|
Reworked the colour model per your clarification (commit
Reviewed (opus) by independently force-rendering the |
…idated run message (TML-2771) Signed-off-by: Will Madden <madden@prisma.io>
…gnment, consolidated run message (TML-2771) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Will Madden <madden@prisma.io>
… (TML-2771) Signed-off-by: Will Madden <madden@prisma.io>
Contributor
|
Three tweaks (commit
Force-render-verified (green on-path |
…rrow aligned (TML-2771) Signed-off-by: Will Madden <madden@prisma.io>
…t 2-space indent with arrows aligned (TML-2771) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Will Madden <madden@prisma.io>
…ht) — RED pins the lane-bleed bug (TML-2771)
Adds `migration-graph-colour-matrix.test.ts` with:
- `assertRowColors` / `lastCodeBefore` helpers for per-cell ANSI assertions
that bypass NO_COLOR=1 (forced codes still emit \x1b[92m/\x1b[2m).
- Six named fixtures: straightLine, twoBranches, branchPlusRollback, diamond,
rollbackOnPath, loopViaInvariant, showcase.
- Matrix: normal-rotation × path-highlight-trunk × path-highlight-branch per fixture,
plus cross-cutting no-rotation-in-path-highlight-mode suite.
2 tests CURRENTLY FAIL (the stage-3 done condition):
- branchPlusRollback: ∅ landing ╯ carries rotation code (\x1b[35m) in path-highlight
mode — EMPTY_CONTRACT_HASH is excluded from contractHighlights, so the arc-landing
node row falls through to laneStylerForColumn (rotation).
All other 35 tests pass against current src.
The diamond branch/merge connector bleed-guard assertions (╮ and ╯ must be DIM for
off-path col-1) PASS — renderConnectorRow already uses per-column columnHighlights
correctly for forward connectors. The bug is isolated to the ∅ arc-landing row.
Signed-off-by: Will Madden <madden@prisma.io>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…layout (TML-2771) Add optional `migrationHash` to every routing StructuralCell variant so the renderer (stage 3) can classify cells by their owning edge rather than by column index. The field is inert until stage 3 reads it. - `vertical-pass`, `horizontal-pass`, `branch-tee`, `branch-corner`, `merge-tee`, `merge-corner`, `arc-branch-corner`, `arc-branch-tee`, `arc-land-corner`, `arc-land-tee`, `arc-land-bridge` all gain `migrationHash?: string`. - `arc-crossing` gains both `migrationHash` (vertical lane owner) and `arcMigrationHash` (the arc crossing over it). - A `laneEdgeByIndex` map threaded through `layoutComponent` tracks the last-emitted edge hash per lane; `emitEdgeRow` updates it after every emission. - `buildEdgeCells`, `buildNodeCells`, `buildMergeConnectorCells` read from `laneEdgeByIndex` to tag pass-through `vertical-pass` cells. - `buildBranchConnectorCells` uses pre-computed `fanEdgeHashByLane` (first edge per fan group) for tee/corner cells; `trunkEdgeHash` tags the start-lane tee. - `applySkipRollbackRouting` tags every arc cell it writes with `route.edge.migrationHash`; `arc-crossing` cells carry the existing vertical owner's hash in `migrationHash` and the arc's hash in `arcMigrationHash`. - Normal-mode rotation (resolveRowArcLaneColors / laneColorForColumn) reads no new fields; all existing tests + snapshots unchanged. Co-Authored-By: Will Madden <madden@prisma.io> Signed-off-by: Will Madden <madden@prisma.io>
…-path green bleed (TML-2771) Stage-3: threads per-cell edge identity through every structural cell (vertical-pass, branch-tee, merge-corner, arc-land-corner, etc.) so each glyph is coloured by its own edge's on-path/off-path annotation rather than a column-level aggregate. Off-path rollback arc corners and merge corners that share a column with an on-path edge no longer bleed green. The failing test over-asserted: its arcLandingRows filter matched merge- connector rows (├─╯) in addition to true arc-landing node rows (○◂──╯). The ╯ at col-1 in `├─╯ │ │ │` is the merge-corner for bob_avatar, which is on-path — green is correct there. Updated the filter to require ◂ (the arc- landing node marker) so only genuine arc-land-corner cells are checked. Signed-off-by: Will Madden <madden@prisma.io> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
|
Systematic rebuild of the graph colouring (replacing the whack-a-mole). Three commits: Root cause: structural lane cells (vertical-pass, branch/merge tees+corners, rollback arcs, crossings) carried no edge identity, so the renderer classified them by column with "on-path wins". Lanes get reused across edges, so any column touched by an on-path edge was stamped entirely green → green bled across off-path branches, merges, and rollback landing arcs (the showcase mess). Fix (test-first):
Verified by force-rendering the real |
…e continuity in migrate --show (TML-2771) Four residual mis-tagging bugs identified by the operator's X/Y cell annotations: Bug X1 (arc-tee connector): renderNodeMarkerPair applied the node's laneOverride to both the marker (○) and the arc connector (─/◂). When the node is on-path but the back-arc is off-path, the connector was GREEN. Fixed by threading a separate arcLaneOverride derived from the arc cell's migrationHash. Bug X2 (arc-crossing dash in connector): renderConnectorRow applied effectiveOverride to the full ┼─ pair. Fixed by splitting: ┼ takes the glyph column's override, ─ takes the dash column's override independently. Bug Y1 (branch-connector tee hash): emitBranchConnector used laneEdgeByIndex for the trunk lane, which may hold a skip-rollback's hash emitted just before the connector. Fixed by preferring fanEdgeHashByLane for the trunk lane, which holds the downward fanout edge's hash. Bug Y2 (fan-lane pass-through hash): When a node fans out into multiple groups, laneEdgeByIndex was not populated for fan lanes before their edge rows were emitted. Pass-through cells in peer group rows carried no hash, falling through to an incorrect override. Fixed by pre-populating laneEdgeByIndex for every fan lane from fanEdgeHashByLane before any edge rows are emitted. Tests: update branch-connector tee identity assertion (now carries fanout hash, not undefined); add fan-lane pass-through identity test; add arc-tee connector colour, branch-connector tee colour, and arc-crossing colour assertions to colour matrix. Signed-off-by: Will Madden <madden@prisma.io> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…rate --show graph (TML-2771) Two colour-bleed bugs in the path-highlight mode of the migration graph renderer: BUG 1 — arc-crossing bridge cells (rendered as ──) in node/edge rows were coloured by migrationHash (the vertical lane owner) instead of arcMigrationHash (the arc that bridges horizontally). When the vertical lane was on-path and the arc was off-path, the bridge rendered green instead of dim. Fixed by preferring arcMigrationHash for arc-crossing cells in all three per-cell hash selection sites in renderMigrationGraphTree. BUG 2 — The trailing ─ dash after a branch-tee or merge-tee junction in connector rows was coloured by columnLaneOverride?.get(dashColumn) (the next column's annotation) rather than the tee cell's own effectiveOverride. When the tee's edge was off-path but the adjacent column was on-path, the dash bled green. Fixed by using effectiveOverride for both the junction glyph and its trailing dash. New matrix test assertions cover both fixes: off-path tee trailing dash stays dim when adjacent column is on-path; on-path tee trailing dash is green; arc-crossing bridge in node rows is dim when the arc edge is off-path even when the vertical lane is on-path. Signed-off-by: Will Madden <madden@prisma.io> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…-preview-the-path-ordered-migrations-migrate Signed-off-by: Will Madden <madden@prisma.io> # Conflicts: # packages/1-framework/3-tooling/cli/src/commands/migration-graph.ts
…erge (TML-2771) Main renamed the migration-list space-entry field from `spaceId` to `space`. Update the isAppSpace computation in migration-status and the appSpaceId test fixtures (space + name/fromContract/toContract) to match the merged schema. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Will Madden <madden@prisma.io>
wmadden
deleted the
tml-2771-migrate-show-preview-the-path-ordered-migrations-migrate
branch
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.
What & why
Adds
migrate --show— a read-only preview that answers "what willmigratedo when I run it?" before you advance the live database. The migration system is a contract graph (not a linear stack), so "the next migration" isn't obvious; this gives users a trustworthy sanity check at the moment they act, and unifies the way the graph is drawn with the way refs are typed.Reshaped from the original
migration path --from --toidea after a design discussion — a preview belongs on themigrateverb the user already reaches for, not a separate command to learn. Full reasoning:projects/migration-graph-rendering/decisions.md§migrate --show(D-MS1–D-MS7); spec + plan + review log underprojects/migration-graph-rendering/slices/migrate-show-preview/.Refs: TML-2771.
What it does
migrateuses (graphWalkStrategy), via a sharedplanMemberPathhelper that bothexecuteMigrateand the preview call — so the preview cannot show a path different from whatmigratewould run.@db/@contractvocabulary. New reference tokens (@db= the live marker, connection-required;@contract= the working contract, offline) usable in--from/--to. The graph +--legendnow draw reserved markers the same way you type them (@db/@contract), dropping the old<contract, db>angle-bracket form.Changes (commit-per-dispatch)
@db/@contractin the shared overlay and--legend; regengraph/status/listsnapshots.@db/@contractreference-grammar tokens inparseContractRef.migrate --show: read-only path compute (stop beforerunMigration) + ordered execution list + edge cases (no-path / at-target / multi-space /@db-no-connection).edgeAnnotationsByHashannotation hook (no parallel renderer).planMemberPath) so the preview is structurally faithful to apply (review F1).Scope
In: the
migrate --showpreview, the@db/@contracttokens, and the render-vocabulary unification (overlay + legend) — which intentionally changesgraph/status/listoutput (the<db>→@dbrelabel + legend).Out / preserved:
migrate's apply behaviour is unchanged (theplanMemberPathextraction is a pure refactor of its path-compute portion); no behavioural change to plan/verify.Verification
pnpm typecheckgreen; cli suite (1209 tests) + migration-tools (549) green; existingmigrateapply tests unchanged. The two load-bearing properties are tested: read-only (a module-levelrunMigrationmock throws on any call), and faithfulness (the test assertsgraphWalkStrategy's call arguments on an invariant-bearing fixture — it would fail on the pre-fix input divergence). Reviewed across two rounds; the round-1 faithfulness finding (the preview could diverge from apply on invariant-bearing graphs) was fixed by the shared seam.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
migrate --show(with--from/--to) previews the exact ordered migration plan and exits without applying changes; supports offline refs and live-DB markers and emits JSON or human-formatted output.Style / Rendering
@-prefixed tokens; graph/tree renders on-path “will run” indicators, lane/color overrides, and app vs extension space@contractvisibility.Tests