TML-2812: unified migration tree rendering, column alignment, colored lanes, ledger op-count tests#713
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:
📝 WalkthroughWalkthroughRefactors migration CLI rendering to produce per-space condensed tree outputs, adds a ChangesMigration Graph Space-Scoped Rendering
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 unit tests (beta)
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 (2)
packages/1-framework/3-tooling/cli/src/utils/cli-errors.ts (1)
111-120:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMove the displaced
errorInvalidSpaceIdJSDoc back above its function.Lines 111-116 now describe
errorInvalidSpaceId, but the next symbol iserrorMigrationGraphLegendHumanOnly. That leaves the space-id contract orphaned and makes the hover/docs here misleading. Move that block down above Line 134, or delete it if the comment is no longer needed.🤖 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/cli-errors.ts` around lines 111 - 120, The JSDoc block describing the space-id contract currently sits above errorMigrationGraphLegendHumanOnly and should be moved so it documents errorInvalidSpaceId instead; locate the errorInvalidSpaceId function and place the displaced comment block immediately above it (or remove the comment entirely if it's no longer relevant), ensuring the JSDoc text matches the symbol errorInvalidSpaceId so hover/docs show the correct description.docs/reference/migration-graph-rendering.md (1)
1-7:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove the stale
--treeframing from this reference.This page still documents
migration graph --treeas a distinct mode and says plainmigration graphuses the dagre renderer, butcreateMigrationGraphCommand()no longer defines a--treeflag and the human path always renders the tree unless--jsonor--dotis selected. As written, the reference tells users to use a mode that no longer exists and describes the default behavior backwards.As per coding guidelines, "Keep docs current (READMEs, rules, links)."
Also applies to: 11-21, 156-183, 213-218
🤖 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 1 - 7, The docs incorrectly describe a non-existent "migration graph --tree" flag and state that plain "migration graph" uses the dagre renderer; update the document so it removes the stale `--tree` framing and instead documents the current behavior: the human renderer outputs the tree by default and `createMigrationGraphCommand()` no longer defines a `--tree` flag, with only `--json` and `--dot` as alternate outputs; revise all affected sections (including the intro and the sections called out) to reflect that default tree rendering, adjust examples and terminology to drop `--tree`, and ensure references to `--json`/`--dot` and the tree rendering are accurate and consistent.
🧹 Nitpick comments (2)
packages/1-framework/3-tooling/migration/test/fixtures/ledger-operation-count-parity.ts (1)
25-49: ⚡ Quick winDerive
buildMultiEdgeRefs()fromMULTI_EDGE_OPERATION_COUNTS.This helper hardcodes
1, 2, 1a second time, so the exported expected vector and the built edge refs can drift independently. Building the refs fromMULTI_EDGE_OPERATION_COUNTSkeeps the fixture internally consistent.♻️ Suggested refactor
export function buildMultiEdgeRefs(destHash: string): readonly AggregateMigrationEdgeRef[] { const hashA = 'sha256:parity-mid-a'; const hashB = 'sha256:parity-mid-b'; return [ { migrationHash: 'sha256:parity-mig-a', dirName: '001_a', from: EMPTY_CONTRACT_HASH, to: hashA, - operationCount: 1, + operationCount: MULTI_EDGE_OPERATION_COUNTS[0], }, { migrationHash: 'sha256:parity-mig-b', dirName: '002_b', from: hashA, to: hashB, - operationCount: 2, + operationCount: MULTI_EDGE_OPERATION_COUNTS[1], }, { migrationHash: 'sha256:parity-mig-c', dirName: '003_c', from: hashB, to: destHash, - operationCount: 1, + operationCount: MULTI_EDGE_OPERATION_COUNTS[2], }, ]; }🤖 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/migration/test/fixtures/ledger-operation-count-parity.ts` around lines 25 - 49, The buildMultiEdgeRefs function currently hardcodes operation counts (1,2,1) which duplicates MULTI_EDGE_OPERATION_COUNTS and can drift; change buildMultiEdgeRefs to derive its operationCount values from the MULTI_EDGE_OPERATION_COUNTS constant (e.g., iterate over MULTI_EDGE_OPERATION_COUNTS or destructure it) when constructing each AggregateMigrationEdgeRef so the fixture stays consistent with the exported expected vector; update the entries for migrationHash/dirName/from/to to set operationCount = MULTI_EDGE_OPERATION_COUNTS[index] (using the same index order as the exported vector).packages/3-targets/6-adapters/postgres/test/migrations/runner.ledger.integration.test.ts (1)
178-228: ⚡ Quick winAlso assert that the failed run leaves the ledger empty.
This test locks down the thrown invariant, but not the side effect it is protecting. Adding a
readLedger/readLedgerRowsassertion after the rejection would catch regressions where rows are partially written before the error is surfaced.✅ Suggested assertion
await expect( runner.execute({ driver: driver!, perSpaceOptions: [ { @@ ], }), ).rejects.toThrow(/does not match sum of migrationEdges operationCount/); + + expect(await readLedgerRows(driver!)).toEqual([]); });🤖 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/3-targets/6-adapters/postgres/test/migrations/runner.ledger.integration.test.ts` around lines 178 - 228, After the existing expect(...).rejects.toThrow(...) add an assertion that the ledger remained empty by calling the ledger read helper (e.g., readLedger or readLedgerRows) with the test driver and LEDGER_TEST_SPACE_ID and asserting it returns no rows; this should run after the rejected runner.execute call (references: runner.execute, driver, LEDGER_TEST_SPACE_ID, readLedger/readLedgerRows).
🤖 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-list-render.ts`:
- Around line 143-151: The change bypasses MigrationListStyler for non-empty
tree rows by calling renderMigrationGraphSpaceTree with only graph/migrations;
restore usage of the styler by passing the active MigrationListStyler (or its
computed style object) into renderMigrationGraphSpaceTree so that style.kind,
style.dirName, style.sourceHash, style.destHash, style.glyph, style.lane,
style.invariants and style.refs are applied per-row; update
renderMigrationListWithStyle to forward the styler (or apply it to the tree rows
before rendering), and ensure buildRefsByHashFromListEntries remains used for
refs resolution.
In
`@packages/1-framework/3-tooling/cli/test/commands/migration-read-commands-parity.test.ts`:
- Around line 253-260: The test hard-codes HASH_804e018 as targetHash (which is
the app-space tip) so the postgis branch doesn't exercise real per-space target
resolution; replace that hard-coded use of HASH_804e018 with a target derived
from the current test space before calling deriveStatusEdgeAnnotations (i.e.,
compute targetHash from the graph/space tip used in this fixture instead of
using HASH_804e018). Locate the test that calls deriveStatusEdgeAnnotations and
set targetHash by calling the same helper or graph API used elsewhere in the
suite to get the tip for the current space (the per-space tip resolver used in
migration status), then pass that derived targetHash into
deriveStatusEdgeAnnotations.
---
Outside diff comments:
In `@docs/reference/migration-graph-rendering.md`:
- Around line 1-7: The docs incorrectly describe a non-existent "migration graph
--tree" flag and state that plain "migration graph" uses the dagre renderer;
update the document so it removes the stale `--tree` framing and instead
documents the current behavior: the human renderer outputs the tree by default
and `createMigrationGraphCommand()` no longer defines a `--tree` flag, with only
`--json` and `--dot` as alternate outputs; revise all affected sections
(including the intro and the sections called out) to reflect that default tree
rendering, adjust examples and terminology to drop `--tree`, and ensure
references to `--json`/`--dot` and the tree rendering are accurate and
consistent.
In `@packages/1-framework/3-tooling/cli/src/utils/cli-errors.ts`:
- Around line 111-120: The JSDoc block describing the space-id contract
currently sits above errorMigrationGraphLegendHumanOnly and should be moved so
it documents errorInvalidSpaceId instead; locate the errorInvalidSpaceId
function and place the displaced comment block immediately above it (or remove
the comment entirely if it's no longer relevant), ensuring the JSDoc text
matches the symbol errorInvalidSpaceId so hover/docs show the correct
description.
---
Nitpick comments:
In
`@packages/1-framework/3-tooling/migration/test/fixtures/ledger-operation-count-parity.ts`:
- Around line 25-49: The buildMultiEdgeRefs function currently hardcodes
operation counts (1,2,1) which duplicates MULTI_EDGE_OPERATION_COUNTS and can
drift; change buildMultiEdgeRefs to derive its operationCount values from the
MULTI_EDGE_OPERATION_COUNTS constant (e.g., iterate over
MULTI_EDGE_OPERATION_COUNTS or destructure it) when constructing each
AggregateMigrationEdgeRef so the fixture stays consistent with the exported
expected vector; update the entries for migrationHash/dirName/from/to to set
operationCount = MULTI_EDGE_OPERATION_COUNTS[index] (using the same index order
as the exported vector).
In
`@packages/3-targets/6-adapters/postgres/test/migrations/runner.ledger.integration.test.ts`:
- Around line 178-228: After the existing expect(...).rejects.toThrow(...) add
an assertion that the ledger remained empty by calling the ledger read helper
(e.g., readLedger or readLedgerRows) with the test driver and
LEDGER_TEST_SPACE_ID and asserting it returns no rows; this should run after the
rejected runner.execute call (references: runner.execute, driver,
LEDGER_TEST_SPACE_ID, readLedger/readLedgerRows).
🪄 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: 62ff4887-c34c-406e-915f-1f485c1c69a5
⛔ Files ignored due to path filters (5)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlprojects/migration-graph-rendering/README.mdis excluded by!projects/**projects/migration-graph-rendering/slices/lane-colors-and-legend/spec.mdis excluded by!projects/**projects/migration-graph-rendering/slices/render-polish-and-ledger-tests/spec.mdis excluded by!projects/**projects/migration-graph-rendering/slices/unify-pretty-rendering/spec.mdis excluded by!projects/**
📒 Files selected for processing (19)
docs/reference/migration-graph-rendering.mdpackages/1-framework/3-tooling/cli/src/commands/migration-graph.tspackages/1-framework/3-tooling/cli/src/commands/migration-list.tspackages/1-framework/3-tooling/cli/src/commands/migration-status.tspackages/1-framework/3-tooling/cli/src/utils/cli-errors.tspackages/1-framework/3-tooling/cli/src/utils/formatters/migration-graph-lane-colors.tspackages/1-framework/3-tooling/cli/src/utils/formatters/migration-graph-space-render.tspackages/1-framework/3-tooling/cli/src/utils/formatters/migration-graph-tree-render.tspackages/1-framework/3-tooling/cli/src/utils/formatters/migration-list-render.tspackages/1-framework/3-tooling/cli/test/commands/migration-graph.test.tspackages/1-framework/3-tooling/cli/test/commands/migration-list-json-golden.test.tspackages/1-framework/3-tooling/cli/test/commands/migration-list.test.tspackages/1-framework/3-tooling/cli/test/commands/migration-read-commands-parity.test.tspackages/1-framework/3-tooling/cli/test/commands/migration-status-overlay.test.tspackages/1-framework/3-tooling/cli/test/utils/formatters/migration-graph-tree-render.test.tspackages/1-framework/3-tooling/migration/test/fixtures/ledger-operation-count-parity.tspackages/3-targets/6-adapters/postgres/test/migrations/runner.ledger.integration.test.tstest/integration/package.jsontest/integration/test/migration-ledger/ledger-operation-count-parity.test.ts
wmadden-electric
pushed a commit
that referenced
this pull request
Jun 3, 2026
Pass MigrationListStyler through the shared tree renderer, fix integration parity test types (mongo family + migrationEdges), derive multi-edge fixture counts from one source, assert empty ledger on operationCount mismatch, use per-space target hash in status parity test, and refresh the graph rendering reference doc for the unified tree default. Signed-off-by: Will Madden <madden@prisma.io>
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 (2)
packages/1-framework/3-tooling/cli/src/commands/migration-status.ts (2)
490-493:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPass the current space’s contract hash into the shared tree renderer.
renderMigrationGraphSpaceTree()renders againstmember.graph(), but this call still feeds it the app-levelcontractHash. In multi-space repos that will mark the wrong live node/trunk for every non-app space, becausespaceContractHashis already computed just above for the graph you’re rendering.Suggested fix
const tree = renderSpaceTree({ member, - liveContractHash: contractHash, + liveContractHash: spaceContractHash, migrations: spaceEntry.migrations, markerHash, showDbMarker, statusOverlay: annotations,🤖 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/migration-status.ts` around lines 490 - 493, The call to renderSpaceTree for a space is using the app-level contractHash (contractHash) but should use the space-specific hash (spaceContractHash); update the renderSpaceTree invocation (the one building tree for spaceEntry/migrations) to pass liveContractHash: spaceContractHash so renderMigrationGraphSpaceTree which renders against member.graph() highlights the correct live node/trunk for that space.
455-464:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep
markerCannotReachTargetscoped to the app space.The final summary always talks about
appMarkerHash/headlineTargetHash, but this flag is currently set by any space. In a multi-space aggregate, one disconnected secondary space can therefore flip the top-level summary to “No migration path …” even when the app space is reachable.Suggested fix
- if ( + if ( + spaceEntry.spaceId === aggregate.app.spaceId && connected && !usingFromOverride && markerHash !== undefined && markerInGraph && markerHash !== targetHash &&🤖 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/migration-status.ts` around lines 455 - 464, The flag markerCannotReachTarget is being set for any space but should only reflect reachability for the app space; update the condition that sets markerCannotReachTarget (the if that currently checks connected, !usingFromOverride, markerHash, markerInGraph, markerHash !== targetHash, and findPath(...)=null) to also require that this evaluation is for the app space — e.g. add a check like space === appSpace or markerHash === appMarkerHash/headlineTargetHash (whichever identifies the app space in this scope) so only the app's disconnected state flips markerCannotReachTarget.
🤖 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/commands/migration-status.ts`:
- Around line 490-493: The call to renderSpaceTree for a space is using the
app-level contractHash (contractHash) but should use the space-specific hash
(spaceContractHash); update the renderSpaceTree invocation (the one building
tree for spaceEntry/migrations) to pass liveContractHash: spaceContractHash so
renderMigrationGraphSpaceTree which renders against member.graph() highlights
the correct live node/trunk for that space.
- Around line 455-464: The flag markerCannotReachTarget is being set for any
space but should only reflect reachability for the app space; update the
condition that sets markerCannotReachTarget (the if that currently checks
connected, !usingFromOverride, markerHash, markerInGraph, markerHash !==
targetHash, and findPath(...)=null) to also require that this evaluation is for
the app space — e.g. add a check like space === appSpace or markerHash ===
appMarkerHash/headlineTargetHash (whichever identifies the app space in this
scope) so only the app's disconnected state flips markerCannotReachTarget.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: a7ece171-95b8-4ee2-9a3e-4b3fb7643bf8
⛔ Files ignored due to path filters (1)
projects/migration-graph-rendering/slices/render-polish-and-ledger-tests/spec.mdis excluded by!projects/**
📒 Files selected for processing (3)
packages/1-framework/3-tooling/cli/src/commands/migration-status.tspackages/1-framework/3-tooling/cli/test/commands/format-status-summary.test.tstest/integration/test/cli-journeys/migration-status-diagnostics.e2e.test.ts
wmadden-electric
approved these changes
Jun 3, 2026
wmadden-electric
left a comment
wmadden-electric
left a comment
Contributor
There was a problem hiding this comment.
LGTM — render-polish-and-ledger-tests slice (D1–D12). Trunk choice now anchors on the live contract; cross-space columns align; --legend is universal across list/graph/status with the bracket-convention key.
… spec + plan Single combined slice covering TML-2812 (unify pretty rendering across list / status / graph), TML-2811 (column alignment), TML-2773 (colored lanes + --legend), and TML-2774 (the two open op-count test-coverage items). Subsumes the unify-pretty-rendering and lane-colors-and-legend slice drafts. TML-2767 is superseded by D16 and closed. Five dispatches, all decisions locked (D14-D20). The deferred edges-on-plan and empty-origin-as-null drafts stay deferred per operator (the empty-origin work has non-trivial blast radius into the MigrationGraph node-keying and was already ruled out of scope in the TML-2769 review). Signed-off-by: Will Madden <madden@prisma.io>
Route list, status, and graph through the shared space-tree renderer with live-contract trunk choice, list-derived edge annotations, and all-space iteration (including --space on graph). Status composes the status overlay onto the list overlay. Add cross-command parity tests. Signed-off-by: Will Madden <madden@prisma.io>
Right-pad each migration row's tree prefix to the widest prefix in the render block so dirName and hash columns line up on branched graphs. Signed-off-by: Will Madden <madden@prisma.io>
Add integration harness asserting per-edge LedgerEntryRecord.operationCount matches across Postgres, SQLite, Mongo, and synth edge metadata; add the Postgres runner guard test for migrationEdges operationCount sum mismatch. Signed-off-by: Will Madden <madden@prisma.io>
Centralize arc and connector colour resolution in migration-graph-lane-colors for legend reuse, keep crossings dim while dashes take the served lane hue, and add a kitchen-sink colorized test over diamond, back-arc, fan, and sink fixtures asserting all six lane-color rules. Signed-off-by: Will Madden <madden@prisma.io>
Print a palette- and color-aware legend to stderr when --legend is set, including gutter lane swatches from LANE_COLOR_CYCLE when colorize is on. Reject --legend combined with --json or --dot. Snapshot the four unicode/ascii × color on/off legend variants and document the flag. Signed-off-by: Will Madden <madden@prisma.io>
Pass MigrationListStyler through the shared tree renderer, fix integration parity test types (mongo family + migrationEdges), derive multi-edge fixture counts from one source, assert empty ledger on operationCount mismatch, use per-space target hash in status parity test, and refresh the graph rendering reference doc for the unified tree default. Signed-off-by: Will Madden <madden@prisma.io>
…get + no-path wording (in-scope D6) Signed-off-by: Will Madden <madden@prisma.io>
…h ?? contractHash` Signed-off-by: Will Madden <madden@prisma.io>
…mary (D21) Collapse target resolution to activeRefHash ?? contractHash, remove the unreachable MIGRATION.DIVERGED path, and replace the no-path headline with context-aware buildNoPathSummary wording. Signed-off-by: Will Madden <madden@prisma.io>
…n `()`, fresh-DB no-path guard Signed-off-by: Will Madden <madden@prisma.io>
…DB no-path guard (D22) Signed-off-by: Will Madden <madden@prisma.io>
Signed-off-by: Will Madden <madden@prisma.io>
…t-node refs adjacent to hash, right-justified from-hash Signed-off-by: Will Madden <madden@prisma.io>
…cent to hash; right-justify ∅ (D23) Compute a single global maxEdgeTreePrefixWidth across all per-space blocks so dirName columns align in multi-space renderings; place contract-node markers/refs two spaces after the hash; right-justify the from-hash column so ∅ sits flush against →. Signed-off-by: Will Madden <madden@prisma.io>
…+ capitalised "Up to date" Signed-off-by: Will Madden <madden@prisma.io>
Pad migration dirName to a global max across per-space blocks so the from-hash column and arrow align with D8 tree-prefix alignment. Capitalise the no-pending status headline. Signed-off-by: Will Madden <madden@prisma.io>
…actHash for connected components Signed-off-by: Will Madden <madden@prisma.io>
…ch list/graph (D1 parity) Signed-off-by: Will Madden <madden@prisma.io>
Bias row-model layering toward the live-contract leaf and reorder sibling forward edges so the layout module visits that chain first. Covers the demo two-leaf-from-∅ case without touching migration-graph-layout.ts. Signed-off-by: Will Madden <madden@prisma.io>
… convention + universal --legend Signed-off-by: Will Madden <madden@prisma.io>
Replace the collapsed marker/ref legend line with two D22-aligned example lines, hoist legend gating helpers into utils/legend.ts, and wire --legend into migration list and migration status with the renamed human-only error. Signed-off-by: Will Madden <madden@prisma.io>
…us commands Signed-off-by: Will Madden <madden@prisma.io>
Signed-off-by: Will Madden <madden@prisma.io>
vi.mock on config-loader stops applying after other suites call vi.doUnmock with isolate disabled; migration-graph then hits real loadConfig with a contract.json path and fails validation. Spy on the real export survives that ordering. Signed-off-by: Will Madden <madden@prisma.io>
wmadden-electric
force-pushed
the
tml-2812-render-polish-and-ledger-tests
branch
from
0b3ee14 to
ef94189
Compare
Cast the graph JSON golden loadConfig stub to LoadedConfig and type aggregatePlannerWarnings against OrderedResolution so PerSpacePlan warnings are visible after the control-policy merge. Signed-off-by: Will Madden <madden@prisma.io>
Contributor
|
Babysit: merge-ready (CI green, no conflicts, no open review threads) Fixes pushed on this pass:
Branch is rebased on current |
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.
Linked issue
Refs TML-2812, TML-2811, TML-2773, TML-2774. Closes TML-2767 (superseded — multi-space policy lands here as D16).
At a glance
Same DB, same on-disk migrations — three commands now draw the same tree:
Migration data columns align at a fixed offset regardless of branch depth; the gutter is per-column colored (
git log --graphstyle);migration graph --legendprints a stderr key.Summary
After the read-command-family slices merged (
list→tree,statusoverlay,log→ledger), the three tree commands still diverged in trunk choice, per-row data, and space iteration — plus two presentation gaps (column alignment, colored lanes) and two open ledger test holes from the TML-2769 review. This PR closes all of that in one slice: one shared renderer path with locked rules (live-contract trunk, full per-row data everywhere, all spaces by default), aligned data columns, colored gutter +--legend, and a cross-target op-count parity harness.What changed
Rendering unification (TML-2812 / D14–D16). New shared module
migration-graph-space-render.tsdriveslist,status, andgraphhuman output through the samebuildMigrationGraphRows→ layout → tree renderer pipeline. Live-contract hash is the trunk in all three commands;graphiterates every on-disk space (fixes the silentpgvectorelision);statuscomposes list overlay (ops,{invariants}) with its applied/pending column instead of replacing it. Parity test:migration-read-commands-parity.test.ts.Column alignment (TML-2811 / D17). Per-space block, pad the tree prefix to max visible width so
dirNamestarts at the same column on every migration row — branches and rollback peels included.Colored lanes + legend (TML-2773 / D18–D19). Six locked lane-color rules in
migration-graph-lane-colors.ts(column-0 neutral, per-arc single hue, bright arrows, etc.).--legendonmigration graphprints glyph + lane-color key to stderr; rejected on--json/--dot.Ledger test coverage (TML-2774 items 3 + 5 / D20). Integration harness asserts Postgres, SQLite, and Mongo agree on per-edge
operationCountfor multi-edge and idempotency-skip applies. Postgres op-count-mismatch throw test added (SQLite + Mongo already had it).Project docs. Combined slice spec at
projects/migration-graph-rendering/slices/render-polish-and-ledger-tests/spec.md; supersedesunify-pretty-renderingandlane-colors-and-legenddrafts; README updated; TML-2767 marked superseded.Why
Project decision D1 promised one shared graphical renderer with command-specific annotations only. On disk the commands fed the same renderer different inputs (trunk hash, overlay shape, space list). Fixing that plus the alignment and coloring gaps makes the migration graph the coherent demo surface it was designed to be. The ledger tests pin a cross-target invariant the TML-2769 review left open.
Testing performed
@prisma-next/cli:pnpm typecheckpass;pnpm test1108/1110 (2 pre-existing flakes:db-update-read-aggregate-json-golden,removed-verb-redirects— not touched by this branch)pnpm lint:depspassmigration-read-commands-parity,migration-graph-tree-render(alignment + colorized kitchen-sink + legend snapshots),ledger-operation-count-parityintegration harness, Postgres ledger integration throw testSkill update
n/a — CLI flags and output formatting only; no new public API surface beyond
--legendon an existing command (help text updated).Checklist
git commit -s)Notes for the reviewer
graph{nodes,edges}vslist/statusper-space arrays) — only human pretty output converges.migration graphremains a separate top-level command now that pretty output matcheslistis explicitly deferred.edges-on-planandempty-origin-as-nullledger cleanups remain deferred (not in TML-2774's open items).Summary by CodeRabbit
New Features
Documentation
Bug Fixes
Tests