fix(cli): classify converging back-arcs as rollbacks and land them cleanly (TML-2793)#702
Merged
Merged
Conversation
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:
📝 WalkthroughWalkthroughExtends migration-graph tree rendering for converging routed back-arcs: adds ChangesConverging back-arcs with landing tees
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 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 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/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: 2
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/migration-graph-layout.test.ts (1)
1008-1024:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTeach the debug renderer about
arc-land-tee.These new cases validate the cell model directly, but
renderCellPair()above still falls through to blanks forarc-land-tee. That means any snapshot that exercises converging landings can lie about the rendered geometry while the test still passes.Suggested fix
case 'arc-land-corner': return '╯ '; + case 'arc-land-tee': + return '┴─'; case 'arc-crossing': return '┼─';Also applies to: 1033-1088
🤖 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-layout.test.ts` around lines 1008 - 1024, The debug renderer (renderCellPair) doesn't handle the 'arc-land-tee' kind so tests that validate the cell model can still get blank rendering; add handling for 'arc-land-tee' inside renderCellPair (the same debug rendering switch/case that currently handles 'arc-land-corner' and 'arc-land-bridge') and return the appropriate debug geometry/characters for a converging landing (tee) case; ensure the new case uses the same coordinate/character scheme as the other arc-land* cases so snapshots reflect the real rendered geometry.
🧹 Nitpick comments (1)
packages/1-framework/3-tooling/cli/test/utils/formatters/migration-graph-tree-render.test.ts (1)
373-404: ⚡ Quick winAdd ASCII coverage for the new landing tees.
These assertions only exercise
tree(...), but the implementation also addsASCII_PALETTE.arcLandTee. A smalltreeAscii(...)check for the 2-way and 3-way landings would keep the fallback CLI path from drifting.🤖 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-tree-render.test.ts` around lines 373 - 404, Add assertions that exercise the ASCII-rendering path by calling treeAscii(...) (or equivalent ASCII-specific renderer) for the converging skip-rollback cases so the new ASCII_PALETTE.arcLandTee is covered; specifically, for the two-way and three-way landing tests that use tree(convergingEdges()) and tree([...convergingEdges(), edge('n4','n1','rb_c')]) add parallel checks using treeAscii(...) and assert the landing lines start with the ASCII equivalents (matching the expected tee patterns), locating the landing line the same way as the existing tests (find line that includes '◂' and endsWith('n1')) to ensure ASCII_PALETTE.arcLandTee is exercised.
🤖 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 `@docs/reference/migration-graph-rendering.md`:
- Around line 79-81: Add an explicit "arc land tee" row to the "Complete Unicode
↔ ASCII table" that documents the converging-landing glyph pair `┴─` / `+-` (use
that exact label and glyphs to match the earlier description) and fix the
malformed code span for the outer corner by removing the trailing space so the
span reads `/` (not `` `/ ` ``); update any caption or table key references
(e.g., "Converging arcs" explanation) to point to the new "arc land tee" entry
so readers can find the glyph mapping.
In
`@packages/1-framework/3-tooling/cli/src/utils/formatters/migration-list-graph-topology.ts`:
- Around line 65-67: The current seeding picks only global roots (roots) or a
single fallback node, which leaves disjoint rootless components unseeded so
their nodes remain at distance 0 and back-edges can be misclassified; fix by
ensuring each unreached component gets a seed: after building initial seeds
(using roots, compareNodesRootFirst and nodes) and after computing reachability
into forwardDistances, iterate over nodes not yet in forwardDistances, group
them by their forward-connected component (or simply pick the
lexicographically-first node per unreached component using
compareNodesRootFirst), and add that node to seeds so every rootless component
is seeded; apply the same change to the second identical block referenced (uses
inDegree, seeds, forwardDistances).
---
Outside diff comments:
In
`@packages/1-framework/3-tooling/cli/test/utils/formatters/migration-graph-layout.test.ts`:
- Around line 1008-1024: The debug renderer (renderCellPair) doesn't handle the
'arc-land-tee' kind so tests that validate the cell model can still get blank
rendering; add handling for 'arc-land-tee' inside renderCellPair (the same debug
rendering switch/case that currently handles 'arc-land-corner' and
'arc-land-bridge') and return the appropriate debug geometry/characters for a
converging landing (tee) case; ensure the new case uses the same
coordinate/character scheme as the other arc-land* cases so snapshots reflect
the real rendered geometry.
---
Nitpick comments:
In
`@packages/1-framework/3-tooling/cli/test/utils/formatters/migration-graph-tree-render.test.ts`:
- Around line 373-404: Add assertions that exercise the ASCII-rendering path by
calling treeAscii(...) (or equivalent ASCII-specific renderer) for the
converging skip-rollback cases so the new ASCII_PALETTE.arcLandTee is covered;
specifically, for the two-way and three-way landing tests that use
tree(convergingEdges()) and tree([...convergingEdges(), edge('n4','n1','rb_c')])
add parallel checks using treeAscii(...) and assert the landing lines start with
the ASCII equivalents (matching the expected tee patterns), locating the landing
line the same way as the existing tests (find line that includes '◂' and
endsWith('n1')) to ensure ASCII_PALETTE.arcLandTee is exercised.
🪄 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: 3b095a88-3f36-4833-bd47-42f7255b5b3b
📒 Files selected for processing (7)
docs/reference/migration-graph-rendering.mdpackages/1-framework/3-tooling/cli/src/utils/formatters/migration-graph-layout.tspackages/1-framework/3-tooling/cli/src/utils/formatters/migration-graph-tree-render.tspackages/1-framework/3-tooling/cli/src/utils/formatters/migration-list-graph-topology.tspackages/1-framework/3-tooling/cli/test/utils/formatters/migration-graph-layout.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-graph-topology.test.ts
Comment on lines
+79
to
+81
| - **Converging arcs** — when two or more back-arcs land on the *same* node, the inner | ||
| lanes read as a landing tee `┴─` / `+-` (the outer arcs' bridges pass through to the | ||
| node) and only the outermost lane closes the corner `╯` / `/ `. |
Contributor
There was a problem hiding this comment.
Document the landing-tee glyph in the reference table and fix the code span.
This adds the new converging-landing behavior, but the later “Complete Unicode ↔ ASCII table” still has no explicit arc land tee entry for ┴─ / +-. Also, the `/ ` span on Line 81 includes a trailing space and triggers the markdownlint warning.
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 81-81: Spaces inside code span elements
(MD038, no-space-in-code)
🤖 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 `@docs/reference/migration-graph-rendering.md` around lines 79 - 81, Add an
explicit "arc land tee" row to the "Complete Unicode ↔ ASCII table" that
documents the converging-landing glyph pair `┴─` / `+-` (use that exact label
and glyphs to match the earlier description) and fix the malformed code span for
the outer corner by removing the trailing space so the span reads `/` (not `` `/
` ``); update any caption or table key references (e.g., "Converging arcs"
explanation) to point to the new "arc land tee" entry so readers can find the
glyph mapping.
Comment on lines
+65
to
+67
| const roots = [...nodes].filter((node) => (inDegree.get(node) ?? 0) === 0); | ||
| roots.sort(compareNodesRootFirst); | ||
| const seeds = roots.length > 0 ? roots : [...nodes].sort(compareNodesRootFirst).slice(0, 1); |
Contributor
There was a problem hiding this comment.
Seed rootless components independently in forwardDistances.
When the graph has at least one real forward root, seeds only includes those global roots. A separate rootless component then gets no seed, all of its nodes fall through to distance 0, and a node-skipping back-edge in that component can stay misclassified as forward. For example, ∅→R plus a disjoint A→B→C / C→A component will never demote C→A here.
Please seed the lexicographically-first node of each still-unreached component, not only when the entire graph has no roots.
Also applies to: 89-92
🤖 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-list-graph-topology.ts`
around lines 65 - 67, The current seeding picks only global roots (roots) or a
single fallback node, which leaves disjoint rootless components unseeded so
their nodes remain at distance 0 and back-edges can be misclassified; fix by
ensuring each unreached component gets a seed: after building initial seeds
(using roots, compareNodesRootFirst and nodes) and after computing reachability
into forwardDistances, iterate over nodes not yet in forwardDistances, group
them by their forward-connected component (or simply pick the
lexicographically-first node per unreached component using
compareNodesRootFirst), and add that node to seeds so every rootless component
is seeded; apply the same change to the second identical block referenced (uses
inDegree, seeds, forwardDistances).
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/1-framework/3-tooling/cli/test/utils/formatters/migration-graph-tree-render.test.ts (1)
982-999: 💤 Low valueTest fallback may mask styling regressions.
The fallback on lines 994-998 allows the test to pass if
dim('forward')is not found in the output by checking that stripped output matches plain output. This effectively accepts "no styling applied" as valid whencolorize: true, which could mask regressions where styling stops working.Consider removing the fallback to enforce that styling is actually applied when
colorize: true:🔧 Suggested fix
- if (colored.includes(dimForward)) { - expect(firstContent.indexOf(dimForward)).toBe(forwardIdx); - } else { - expect(stripAnsi(colored)).toBe(renderMigrationGraphLegend({ colorize: false })); - } + expect(colored).toContain(dimForward); + expect(firstContent.indexOf(dimForward)).toBe(forwardIdx);🤖 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-tree-render.test.ts` around lines 982 - 999, The test "dims legend label prose when colorize is on, not the heading or glyphs" currently allows a fallback that accepts unstyled output by comparing stripAnsi(colored) to renderMigrationGraphLegend({ colorize: false}); remove that fallback branch and instead assert that the dimmed string produced by createColors().dim (dimForward) is present in the rendered output when calling renderMigrationGraphLegend({ colorize: true }); specifically, in the block where you compute dimForward and forwardIdx, delete the if/else that checks colored.includes(dimForward) and replace it with a strict expect that colored.includes(dimForward) (or that firstContent.indexOf(dimForward) equals forwardIdx), ensuring styling is actually applied rather than tolerated to be absent; keep the existing checks for the heading/glyphs unchanged.
🤖 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.
Nitpick comments:
In
`@packages/1-framework/3-tooling/cli/test/utils/formatters/migration-graph-tree-render.test.ts`:
- Around line 982-999: The test "dims legend label prose when colorize is on,
not the heading or glyphs" currently allows a fallback that accepts unstyled
output by comparing stripAnsi(colored) to renderMigrationGraphLegend({ colorize:
false}); remove that fallback branch and instead assert that the dimmed string
produced by createColors().dim (dimForward) is present in the rendered output
when calling renderMigrationGraphLegend({ colorize: true }); specifically, in
the block where you compute dimForward and forwardIdx, delete the if/else that
checks colored.includes(dimForward) and replace it with a strict expect that
colored.includes(dimForward) (or that firstContent.indexOf(dimForward) equals
forwardIdx), ensuring styling is actually applied rather than tolerated to be
absent; keep the existing checks for the heading/glyphs unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 164229c7-ed69-4af0-81f7-59f1b40b19ce
📒 Files selected for processing (2)
packages/1-framework/3-tooling/cli/src/utils/formatters/migration-graph-tree-render.tspackages/1-framework/3-tooling/cli/test/utils/formatters/migration-graph-tree-render.test.ts
wmadden
approved these changes
Jun 3, 2026
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/1-framework/3-tooling/cli/src/utils/formatters/migration-graph-tree-render.ts (1)
219-222: 💤 Low valueMinor:
dashfallback useslane[column]which was just assigned.Line 221 assigns
dash[column] = lane[column] ?? column, butlane[column]was just set on line 220, so it will never be undefined. The?? columnis defensive but unreachable. This is harmless but could simplify to:case 'horizontal-pass': lane[column] = arcCorner === NEUTRAL_LANE ? column : arcCorner; - dash[column] = lane[column] ?? column; + dash[column] = lane[column]; break;🤖 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 219 - 222, In the 'horizontal-pass' case simplify the unreachable nullish fallback by assigning dash[column] directly; replace "lane[column] = arcCorner === NEUTRAL_LANE ? column : arcCorner; dash[column] = lane[column] ?? column;" with a single consistent assignment (either set dash[column] = lane[column] after lane is set, or set both using the same expression), referencing the 'horizontal-pass' case, variables lane, dash, arcCorner, NEUTRAL_LANE and column.
🤖 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.
Nitpick comments:
In
`@packages/1-framework/3-tooling/cli/src/utils/formatters/migration-graph-tree-render.ts`:
- Around line 219-222: In the 'horizontal-pass' case simplify the unreachable
nullish fallback by assigning dash[column] directly; replace "lane[column] =
arcCorner === NEUTRAL_LANE ? column : arcCorner; dash[column] = lane[column] ??
column;" with a single consistent assignment (either set dash[column] =
lane[column] after lane is set, or set both using the same expression),
referencing the 'horizontal-pass' case, variables lane, dash, arcCorner,
NEUTRAL_LANE and column.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: a2a88d27-980c-4b5e-bc80-9dee15eefd48
📒 Files selected for processing (2)
packages/1-framework/3-tooling/cli/src/utils/formatters/migration-graph-tree-render.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 (1)
- packages/1-framework/3-tooling/cli/test/utils/formatters/migration-graph-tree-render.test.ts
…eanly (TML-2793) The shared forward/rollback/self classifier mis-labelled edges when two or more node-skipping rollbacks converge on the same target: the longest-path / marginal-edge peel heuristic could pick a genuine forward chain edge as the back-edge and leave a real rollback labelled forward, depending on traversal order. Downstream this dropped one converging rollback from the routed back-arc layer and scrambled the vertical (topological) order. Replace the peel heuristic with a principled rule: an edge from -> to is a node-skipping rollback exactly when (a) to is a forward-ancestor of from (it can still reach from over the other forward edges, closing a cycle) and (b) to is strictly shallower than from in shortest-path forward distance (it points back toward the root). Condition (b) is the discriminator that the previous heuristic lacked: in a rollback-induced cycle every edge satisfies (a), but only the rollback runs deep -> shallow, so the forward spine is never peeled however many rollbacks converge. Shortest-path distance is order-independent (backward edges are inert), so the classification is deterministic and the vertical order falls out correct with no special-case ordering path. Add co-landing render handling mirroring the co-sourced tee: when several back-arcs converge on one node, inner lanes read as a landing tee (arc-land-tee / the ┴─ glyph) so the outer arcs' bridges pass through to the node, and only the outermost lane closes the corner ╯ — so every converging arc terminates at the target. Signed-off-by: Will Madden <madden@prisma.io>
…L-2793) Signed-off-by: Will Madden <madden@prisma.io>
…y lead into (TML-2793) Signed-off-by: Will Madden <madden@prisma.io>
…arkers (TML-2773) Signed-off-by: Will Madden <madden@prisma.io>
…t (TML-2773) Signed-off-by: Will Madden <madden@prisma.io>
Signed-off-by: Will Madden <madden@prisma.io>
wmadden-electric
force-pushed
the
tml-2793-converging-back-arcs
branch
from
21cee55 to
7023273
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
docs/reference/migration-graph-rendering.md (1)
79-81:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd the landing-tee entry to the reference table and fix the ASCII code span.
The prose now introduces a landing tee
┴─/+-for converging arc landings, but the glyph table still only documents that glyph asconnector co-merge tee, so the reference no longer matches the behavior it just described. Line 81 also still has the malformed`/ `span, which triggers MD038.Please add an explicit
arc land teerow for┴─/+-and change the corner span to`/`.As per coding guidelines,
**/*.md: Keep docs current (READMEs, rules, links).Also applies to: 116-139
🤖 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 `@docs/reference/migration-graph-rendering.md` around lines 79 - 81, The migration graph rendering docs are out of sync: the “Converging arcs” prose now describes a landing tee that the reference table does not explicitly document, and the ASCII corner example still has a malformed code span. Update the glyph reference table to add an explicit “arc land tee” entry for the `┴─` / `+-` shape, and fix the corner example in the same section so the ASCII span is rendered correctly as `/` rather than a broken code span.packages/1-framework/3-tooling/cli/src/utils/formatters/migration-list-graph-topology.ts (1)
53-95:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSeed every unreached component in
forwardDistances.Right now this only seeds the global in-degree-0 roots, or one fallback node when there are no roots anywhere. A disjoint rootless component then falls through to the
dist = 0backfill, so its real rollback edges never satisfytoDist < fromDistand stay misclassified asforward. For example,∅ → Rplus a separateA → B → C/C → Acomponent still leaves theA/B/Ccycle unseeded.Please add a seed for each still-unreached component before the final zero-fill, rather than zero-filling unreached nodes directly.
🤖 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-list-graph-topology.ts` around lines 53 - 95, The forwardDistances function currently seeds only global in-degree-0 roots or a single fallback node, missing other disconnected components that are rootless, causing them to be misclassified. To fix this, identify each unreached component after the initial seeding and before the final zero-fill loop, then seed one node per such component by setting its distance to zero in dist. This ensures all components have a proper seed for distance calculation rather than defaulting unreached nodes to zero distance directly.
🤖 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.
Duplicate comments:
In `@docs/reference/migration-graph-rendering.md`:
- Around line 79-81: The migration graph rendering docs are out of sync: the
“Converging arcs” prose now describes a landing tee that the reference table
does not explicitly document, and the ASCII corner example still has a malformed
code span. Update the glyph reference table to add an explicit “arc land tee”
entry for the `┴─` / `+-` shape, and fix the corner example in the same section
so the ASCII span is rendered correctly as `/` rather than a broken code span.
In
`@packages/1-framework/3-tooling/cli/src/utils/formatters/migration-list-graph-topology.ts`:
- Around line 53-95: The forwardDistances function currently seeds only global
in-degree-0 roots or a single fallback node, missing other disconnected
components that are rootless, causing them to be misclassified. To fix this,
identify each unreached component after the initial seeding and before the final
zero-fill loop, then seed one node per such component by setting its distance to
zero in dist. This ensures all components have a proper seed for distance
calculation rather than defaulting unreached nodes to zero distance directly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 1610b2d5-faa8-41e6-ae04-3e0ef3c9e3df
📒 Files selected for processing (18)
docs/reference/migration-graph-rendering.mdexamples/prisma-next-demo/fixtures/showcase/migrations/app/20260602T1624_rollback_to_users_from_tip/end-contract.d.tsexamples/prisma-next-demo/fixtures/showcase/migrations/app/20260602T1624_rollback_to_users_from_tip/end-contract.jsonexamples/prisma-next-demo/fixtures/showcase/migrations/app/20260602T1624_rollback_to_users_from_tip/migration.jsonexamples/prisma-next-demo/fixtures/showcase/migrations/app/20260602T1624_rollback_to_users_from_tip/migration.tsexamples/prisma-next-demo/fixtures/showcase/migrations/app/20260602T1624_rollback_to_users_from_tip/ops.jsonexamples/prisma-next-demo/fixtures/showcase/migrations/app/20260602T1626_rollback_to_users_from_bio/end-contract.d.tsexamples/prisma-next-demo/fixtures/showcase/migrations/app/20260602T1626_rollback_to_users_from_bio/end-contract.jsonexamples/prisma-next-demo/fixtures/showcase/migrations/app/20260602T1626_rollback_to_users_from_bio/migration.jsonexamples/prisma-next-demo/fixtures/showcase/migrations/app/20260602T1626_rollback_to_users_from_bio/migration.tsexamples/prisma-next-demo/fixtures/showcase/migrations/app/20260602T1626_rollback_to_users_from_bio/ops.jsonexamples/prisma-next-demo/fixtures/showcase/prisma-next.config.tspackages/1-framework/3-tooling/cli/src/utils/formatters/migration-graph-layout.tspackages/1-framework/3-tooling/cli/src/utils/formatters/migration-graph-tree-render.tspackages/1-framework/3-tooling/cli/src/utils/formatters/migration-list-graph-topology.tspackages/1-framework/3-tooling/cli/test/utils/formatters/migration-graph-layout.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-graph-topology.test.ts
✅ Files skipped from review due to trivial changes (6)
- examples/prisma-next-demo/fixtures/showcase/migrations/app/20260602T1626_rollback_to_users_from_bio/migration.json
- examples/prisma-next-demo/fixtures/showcase/migrations/app/20260602T1624_rollback_to_users_from_tip/end-contract.d.ts
- examples/prisma-next-demo/fixtures/showcase/migrations/app/20260602T1626_rollback_to_users_from_bio/end-contract.d.ts
- examples/prisma-next-demo/fixtures/showcase/migrations/app/20260602T1624_rollback_to_users_from_tip/end-contract.json
- examples/prisma-next-demo/fixtures/showcase/prisma-next.config.ts
- examples/prisma-next-demo/fixtures/showcase/migrations/app/20260602T1626_rollback_to_users_from_bio/end-contract.json
🚧 Files skipped from review as they are similar to previous changes (9)
- examples/prisma-next-demo/fixtures/showcase/migrations/app/20260602T1624_rollback_to_users_from_tip/migration.json
- packages/1-framework/3-tooling/cli/test/utils/formatters/migration-graph-layout.test.ts
- examples/prisma-next-demo/fixtures/showcase/migrations/app/20260602T1624_rollback_to_users_from_tip/ops.json
- examples/prisma-next-demo/fixtures/showcase/migrations/app/20260602T1626_rollback_to_users_from_bio/ops.json
- examples/prisma-next-demo/fixtures/showcase/migrations/app/20260602T1624_rollback_to_users_from_tip/migration.ts
- packages/1-framework/3-tooling/cli/src/utils/formatters/migration-graph-layout.ts
- packages/1-framework/3-tooling/cli/test/utils/formatters/migration-list-graph-topology.test.ts
- packages/1-framework/3-tooling/cli/src/utils/formatters/migration-graph-tree-render.ts
- packages/1-framework/3-tooling/cli/test/utils/formatters/migration-graph-tree-render.test.ts
This was referenced Jun 3, 2026
Merged
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-2793. Follow-up to TML-2773.
Problem
When two or more node-skipping rollbacks converge on the same target node,
migration graph --treecollapsed: only one arc landed, and the vertical (topological) order scrambled (the tip fell to the bottom). A single node-skipping rollback rendered fine — convergence was the trigger.Root cause (fixed at the source)
The shared forward/rollback/self classifier mis-labelled edges under convergence. Its longest-path / "marginal edge" peel heuristic could pick a genuine forward-chain edge as the back-edge and leave a real rollback labelled
forward, depending on traversal order. Consequences, both downstream of that one bug: a converging rollback was dropped from the routed back-arc layer (so it never landed), and the corrupted forward-edge set scrambled vertical order. (It also already mis-classified the non-convergingskipArcEdgesshowcase — the bug was broader than convergence.)The peel heuristic is replaced with a principled, deterministic rule:
from → tois a node-skipping rollback iff (a)tois a forward-ancestor offrom(closes a cycle) and (b)tois strictly shallower thanfromin shortest-path forward distance (points back toward the root). (b) is the discriminator the old code lacked — in a rollback-induced cycle every edge satisfies (a), but only the rollback runs deep→shallow, so the forward spine is never peeled however many rollbacks converge. Shortest-path distance is order-independent, so classification is deterministic.With classification correct, vertical ordering falls out right automatically — no special-case ordering path was needed.
Render
Added co-landing handling mirroring the existing co-sourced tee: inner converging lanes paint a landing tee (new
arc-land-teecell /┴─glyph) so outer arcs' bridges read through to the node, and only the outermost lane closes the corner╯. Two-way lands as○◂┴─╯, three-way as○◂┴─┴─╯.Tests
test/utils/formatters/→ 234 passing (+10 new: classification 2-way/3-way + input-order invariance; layout cell-level ordering + tee/corner; tree-render visual).colors a routed back-arc's whole horizontal run) — it pinned the buggy scrambled render; rewritten to assert the same intent against the now-correct output.Scope
Pure classifier + layout/render; behind unchanged
--json/--dot. Classifier is shared withmigration list --graph(Tier-2) — full formatter suite is green. No on-disk fixture added (rendering paths fully covered by renderer regression tests; avoided runningmigration planagainst the live demo config).Summary by CodeRabbit
Documentation
Improvements
Tests
Examples