feat(cli): migration-graph convergence, geometry, layout + colour fixes#767
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (12)
📒 Files selected for processing (13)
💤 Files with no reviewable changes (13)
📝 WalkthroughWalkthroughRefactors migration-graph lane allocation to run per disconnected component with per-edge lane overrides, rewrites back-arc routing to group rollbacks by target (shared geom lanes) and compute separate geom/plane/colour lanes, introduces DEFAULT_COLS_PER_LANE and separator cells, and adds/updates scenarios and tests validating convergence and colouring invariants. ChangesBack-arc convergence routing and layout refactor
🎯 4 (Complex) | ⏱️ ~45 minutes Possibly Related PRs
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 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/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.
Actionable comments posted: 1
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/test/utils/formatters/gallery-goldens-backlink.ts (1)
8-13:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate the
rollback-mergecoverage header to match converged behavior.Line 11 still says “separate back-lanes”, but this file now models a shared converged back-lane (and adds
rollback-merge-3with the same convergence model).🤖 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/utils/formatters/gallery-goldens-backlink.ts` around lines 8 - 13, Update the coverage header for the `rollback-merge` scenario in gallery-goldens-backlink.ts to describe the converged/shared back-lane model (instead of saying “separate back-lanes”), and adjust the phrasing to indicate that `rollback-merge` (and the newly added `rollback-merge-3`) model two rollback arcs converging onto the same back-lane/target; locate the comment block that starts with “Backlink scenarios” and replace the line describing `rollback-merge` so it reflects the shared/converged back-lane behavior.
🤖 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/test/utils/formatters/migration-graph-gallery-snapshots.test.ts`:
- Around line 7-9: Update the outdated comment wording that refers to
“it.fails”/RED in the convergence invariant comments: locate the comment block
describing the convergence invariant (the one that mentions "skipping rollbacks
sharing a target occupy one back-lane, not one per arc") and replace references
to "it.fails" or "RED" with language indicating these are now normal passing
assertions (e.g., "now asserted" or "passing test"); make the same change in the
other comment block later in the file that covers the same invariant (previously
at the second block covering lines 136–151) so both comments accurately reflect
that the test is a regular passing assertion rather than an it.fails/RED
expectation.
---
Outside diff comments:
In
`@packages/1-framework/3-tooling/cli/test/utils/formatters/gallery-goldens-backlink.ts`:
- Around line 8-13: Update the coverage header for the `rollback-merge` scenario
in gallery-goldens-backlink.ts to describe the converged/shared back-lane model
(instead of saying “separate back-lanes”), and adjust the phrasing to indicate
that `rollback-merge` (and the newly added `rollback-merge-3`) model two
rollback arcs converging onto the same back-lane/target; locate the comment
block that starts with “Backlink scenarios” and replace the line describing
`rollback-merge` so it reflects the shared/converged back-lane behavior.
🪄 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: 329c6c41-a1e5-43ef-99a4-ad4f43942ab2
⛔ Files ignored due to path filters (4)
packages/1-framework/3-tooling/cli/test/utils/formatters/__snapshots__/migration-graph-gallery-snapshots.test.ts.snapis excluded by!**/*.snapprojects/migration-graph-rendering/slices/converging-back-arcs/spec.mdis excluded by!projects/**projects/migration-graph-rendering/slices/render-redesign-geometry/plan.mdis excluded by!projects/**projects/migration-graph-rendering/trace.jsonlis excluded by!projects/**
📒 Files selected for processing (8)
packages/1-framework/3-tooling/cli/src/utils/formatters/migration-graph-grid-layout.tspackages/1-framework/3-tooling/cli/src/utils/formatters/migration-graph-model.tspackages/1-framework/3-tooling/cli/src/utils/formatters/migration-graph-occlusion-render.tspackages/1-framework/3-tooling/cli/test/utils/formatters/gallery-goldens-backlink.tspackages/1-framework/3-tooling/cli/test/utils/formatters/golden-pipeline.test.tspackages/1-framework/3-tooling/cli/test/utils/formatters/migration-graph-gallery-snapshots.test.tspackages/1-framework/3-tooling/cli/test/utils/formatters/migration-graph-geometry.test.tspackages/1-framework/3-tooling/cli/test/utils/formatters/migration-graph-scenario-gallery.ts
Comment on lines
+7
to
+9
| * A convergence invariant (marked it.fails until convergence is implemented) asserts | ||
| * that skipping rollbacks sharing a target occupy one back-lane, not one per arc. | ||
| * |
Contributor
There was a problem hiding this comment.
Remove outdated “RED/it.fails” wording from convergence comments.
Line 7 and Lines 136-151 still describe these as it.fails/RED checks, but the tests now run as normal passing assertions.
Also applies to: 136-151
🤖 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/utils/formatters/migration-graph-gallery-snapshots.test.ts`
around lines 7 - 9, Update the outdated comment wording that refers to
“it.fails”/RED in the convergence invariant comments: locate the comment block
describing the convergence invariant (the one that mentions "skipping rollbacks
sharing a target occupy one back-lane, not one per arc") and replace references
to "it.fails" or "RED" with language indicating these are now normal passing
assertions (e.g., "now asserted" or "passing test"); make the same change in the
other comment block later in the file that covers the same invariant (previously
at the second block covering lines 136–151) so both comments accurately reflect
that the test is a regular passing assertion rather than an it.fails/RED
expectation.
Decompose back-arc convergence + configurable geometry into three test-first dispatches. Mark converging-back-arcs superseded (it targeted the deleted old layout/tee renderer; its outcome moves here). Signed-off-by: Will Madden <madden@prisma.io>
Correct D1/D2 to the vitest snapshot harness (structural convergence assertion, not hand-drawn golden); pin the structural definition of 'converged' (one back-lane per target group). Signed-off-by: Will Madden <madden@prisma.io>
…assertion Adds two scenario fixtures (rollback-converge-2, rollback-converge-3) where multiple skipping rollback arcs all land on the same target node. The structural assertion block (convergence structural assertions) uses it.fails to lock in the expected converged grid width (4 columns) while today's per-arc layout allocates one back-lane per arc (6 and 8 columns respectively). The it.fails tests are intentionally RED until the layout algorithm is updated to merge same-target back-arcs into a single lane; at that point the it.fails wrappers are removed. Signed-off-by: Will Madden <madden@prisma.io>
…-lane Skipping rollbacks that land on the same target node now share a single geometric lane (rail column) instead of getting one lane each. numBackLanes now equals the count of distinct target nodes among skipping rollbacks, not the arc count — reducing grid width from (lanes+arcs)*2 to (lanes+groups)*2. Each arc keeps its own colourLane so colour differentiation is preserved; occlusion arbitrates the shared column per the existing plane/z-order model. Flip both convergence structural assertions from it.fails to it, strengthen the tip-topmost check to assert the highest-rank tip node's contractHash (not just that some node leads grid[0]), and re-record the five rollback-converge-2/3 snapshot keys to the converged narrower output. Signed-off-by: Will Madden <madden@prisma.io>
…geometry plan Signed-off-by: Will Madden <madden@prisma.io>
… scaling proof Replaces the bare literal `2` in both `buildGrid` and `renderGridRow` with `DEFAULT_COLS_PER_LANE`, defined in `migration-graph-model.ts` alongside `GridOptions`. No other tunable geometry literals were found in the four audited files — the remaining numeric offsets (`railCol + 1`, etc.) are all structurally derived from `colsPerLane` and are not independent constants. Adds `migration-graph-geometry.test.ts` which proves the scaling invariant: passing `colsPerLane: 3` to `buildGrid` widens the fork-2 grid from 4 to 6 columns (numLanes × colsPerLane) with no renderer code change, the no-tee alphabet holds at colsPerLane=3, and the default output is byte-identical to colsPerLane=2. Signed-off-by: Will Madden <madden@prisma.io>
…m D3 review Signed-off-by: Will Madden <madden@prisma.io>
Signed-off-by: Will Madden <madden@prisma.io>
… add 3-arc golden The stale two-lane rollback-merge goldens broke when D2 introduced back-arc convergence. Replace the three existing rollback-merge goldens (flat, focus:via-A, focus:via-B) with the converged one-lane form derived from the design: one shared back-lane column, each source tees with its own corner (lane colour follows arc priority / migration-list order), single ○◂╯ landing. Add rollback-merge-3 (∅→rm3_a→…→rm3_e, three arcs all landing on rm3_a) with a flat golden to cover the 3-arc converging case. Drop the stale "RED baseline — expected to fail" framing from the golden-pipeline describe block; the oracle is now a real passing suite (54/54). No src/ changes. No snapshot changes. Signed-off-by: Will Madden <madden@prisma.io>
Signed-off-by: Will Madden <madden@prisma.io>
…oritative plan The line/plane/occlusion rewrite (#762) made several fine-grained glyph-bug slices moot — they targeted the deleted tee-based renderer. Delete converging-back-arcs, connector-crossing-glyph, node-merge-landing-marker (dead by construction in the new model), plus showcase-multilane-merge and diamond-fixture-regeneration (low-value examples/ chores, captured as optional follow-ups). Add plan.md as the single source of truth for what's done vs left. Signed-off-by: Will Madden <madden@prisma.io>
Bug 1 (disconnected components interleave): sort within each component separately in computeDisplayOrder; reset nextLane to 0 per component in buildLaneAssignment; emit separator rows between components that renderGrid preserves as blank lines. Bug 2 (asymmetric diamond merge node stranded on non-trunk lane): track per-edge lanes in edgeLane map during BFS; reconcile merge nodes to the highest-rank parent's lane after BFS; use edgeLane throughout fork connector and inEdges sort so branch lines render in their allocated columns. Adds two-component:flat and asymmetric-diamond:flat goldens; clears the known-broken list; updates coloured-output and gallery snapshots. Signed-off-by: Will Madden <madden@prisma.io>
The showcase golden candidate was generated against the pre-fix renderer and is now stale; its file is untracked. Remove the gallery import so the tree is self-consistent. The candidate will be regenerated after the remaining showcase layout bugs are fixed. Signed-off-by: Will Madden <madden@prisma.io>
When an asymmetric-diamond merge node is reconciled to the trunk lane after BFS, its forward continuation was already assigned the old branch lane and never updated. This caused the trunk to jump lanes at the merge point. The fix: after moving a merge node to its trunk lane, BFS forward through its outbound edges and move any descendant still on the old lane to the new trunk lane. The branch edges pointing INTO the merge are left on their branch lanes so the branch column still renders correctly. Moves continued-merge golden from KNOWN_BROKEN_GOLDENS to GOLDENS; updates showcase snapshots (improvement: trunk stays on lane 0 through the merge instead of indenting into lane 1). Signed-off-by: Will Madden <madden@prisma.io>
Back-arc colours were assigned by a simple counter (numLanes + i) which wraps modulo 6 — causing arcs in graphs with 4+ forward lanes to reuse white (palette index 0, the trunk colour) and green (index 5, reserved for focus on-path). Replace the counter with a greedy bottom-up walk: for each back-arc at its target node, pick the lowest palette index not already held by any concurrently-active forward lane or back-arc, additionally excluding the arc's own origin lane's colour and green. This satisfies two rules: 1. No two concurrently-active lanes share a palette colour. 2. No back-arc reuses its origin lane's colour or green. Z-order (planeLane) is now based on source position (bottom-most source draws on top) rather than colour index, decoupling colour from occlusion. Includes a property test (migration-graph-lane-colours.test.ts) that builds a minimal 4-forward-lane + 3-arc graph to assert both rules and confirm the pre-fix violation. Snapshot goldens for rollback-converge-2, rollback-converge-3, and showcase:rotating updated to reflect the new (correct) colour assignments. Signed-off-by: Will Madden <madden@prisma.io>
Flat mode tints each migration name with its lane hue (not bold); focus mode tints on-path names green (matching the route) and dims off-path — neither bolded. Regenerate the graph + coloured-output snapshots. Signed-off-by: Will Madden <madden@prisma.io>
Signed-off-by: Will Madden <madden@prisma.io>
wmadden-electric
force-pushed
the
render-redesign-geometry
branch
from
b911317 to
5ea3318
Compare
wmadden-electric
changed the title
wmadden
approved these changes
Jun 8, 2026
Signed-off-by: Will Madden <madden@prisma.io>
wmadden-electric
added a commit
that referenced
this pull request
Jun 9, 2026
## Close-out: migration-graph-rendering Closes the `migration-graph-rendering` project (TML-2746). It began as a redesign of `migration graph`'s renderer and broadened into a revamp of the whole interrogative migration **read-command family** (`list` / `graph` / `status` / `log`) on a shared renderer + ledger foundation. All work has shipped; this PR migrates the durable knowledge into `docs/` and deletes the transient project scaffolding. ### Definition of Done — verified | Outcome | Evidence | |---|---| | Tier-3 renderer rebuilt (line/plane/occlusion) | #762 | | Back-arc convergence, configurable geometry, greedy lane colouring, layout fixes | #767 | | Ledger foundation (per-migration journal) | #665 | | `list`/`status`/`log` revamped on the shared renderer; dagre + `list --graph` retired | shipped; verified — `migration-list/status/log/graph.ts` use the shared renderer; no `dagre`/`tree-render`/`layout` renderers remain | | Read-command consistency (TML-2801) | re-validated this PR: 4/7 findings resolved, 2 partial, 1 open (4 small follow-ups noted below) | | Showcase real-world golden | on `main` | No unmet acceptance criteria. External-reference scan for `projects/migration-graph-rendering/` is empty (reference-strip step was a no-op). ### Durable knowledge migrated to `docs/` - **ADR 227** — Migration read commands share one graphical renderer with command-specific annotations. - **ADR 228** — The migration apply ledger is a per-migration journal. - **ADR 229** — The migration graph renderer uses a line/plane/occlusion model (the renderer's internal design — lines as the primitive, single-owner cells, occlusion over blended glyphs). All three verified against shipped code. - **`docs/reference/Migration Graph Visual Language.md`** — the glyph/layout vocabulary the renderer draws from (was the project's `mockups.md`). The read-command consistency audit was **re-validated** against current code (verdict: largely accomplished — 4/7 findings resolved, 2 partial, 1 open) and captured as a Linear follow-up ticket (**TML-2877**, related to TML-2801) rather than a committed doc, since what remains is actionable backlog, not long-lived reference. Transient artefacts (spec, plan, slice specs/plans/reviews, `decisions.md` — now ADR'd, `learnings.md`, the followups draft, `trace.jsonl`, prototype, the audit doc) deleted with the folder / moved to Linear. ### Follow-ups (tracked, not in this PR) - **TML-2877** — the four remaining read-command consistency items (show `--space` policy, log unscoped-semantics doc, check see-also, show/check decoration flags, + a parity-test extension). - **PR #773** — the demo fixture no-op self-edge fix + offline integrity guard. ### Retro — lessons - **A wholesale rewrite obsoletes fine-grained bug-slices.** Three glyph-bug slice specs (tee/marker bugs) were made moot by the line/plane/occlusion rewrite — they targeted deleted code. Re-triage the backlog after a rewrite; don't carry dead slices. - **Hand-authored goldens beat auto-snapshots for correctness.** `toMatchSnapshot()` self-certifies whatever the renderer emits; the hand-authored `golden-pipeline` oracle caught a convergence regression the snapshots happily recorded as "correct." - **Real-world fixtures expose layout bugs unit fixtures miss.** Validating against the `showcase` graph surfaced four distinct layout/colour bugs (disconnected-component interleave, asymmetric-diamond merge lane, trunk-continuation, greedy-colour wraparound) that the simple scenarios never hit. - **`@prisma-next/cli` runs vitest with `isolate: false`** — "passes locally" ≠ passes in CI (parallel state pollution). Candidate for a durable testing note. - **`fixtures:emit` can emit an integrity-failing fixture** — the emitter and `migration check` disagreed (a hash-collapse produced a no-op self-edge). Landed an offline demo integrity guard (#773). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: Will Madden <madden@prisma.io> Co-authored-by: Will Madden <madden@prisma.io>
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
Continues the Tier-3 migration-graph renderer redesign (built on the line/plane/occlusion core merged in #762). Started as back-arc convergence + configurable geometry; while validating against the real-world
showcasefixture it surfaced and fixed a series of layout and colour bugs. All flat-mode behaviour, no change to focus-mode path semantics beyond colour.Changes
Convergence + geometry
colsPerLanedefault lifted to a namedDEFAULT_COLS_PER_LANEconstant + a scaling test (proves a one-line change rescales the grid with no renderer edit).Layout bug fixes (each with a permanent golden / property-test guard)
computeDisplayOrderhonours the row model's per-component grouping; lanes reset per component; one blank separator between components.Colour
Housekeeping
projects/migration-graph-rendering/plan.md.Tests
golden-pipeline(the hand-authored structure+colour oracle) green; focus-mode goldens unchanged where intended.pnpm --filter @prisma-next/cli typecheck,pnpm lint:depsclean.Not included (follow-up)
The real-world
showcasegolden is intentionally not in this PR — its candidate was generated against the pre-fix renderer and is stale. It will be regenerated against the now-correct renderer and reviewed separately. The on-diskexamples/showcase demo fixture is untouched.Summary by CodeRabbit
Release Notes
Bug Fixes
Tests