Add Prisma Next Migrations view with visual contract diffs#1533
Open
sorenbs wants to merge 11 commits into
Open
Add Prisma Next Migrations view with visual contract diffs#1533sorenbs wants to merge 11 commits into
sorenbs wants to merge 11 commits into
Conversation
Studio now detects a Prisma Next migration ledger (prisma_contract.ledger) via introspection and, only then, shows a Migrations item in the navigation. The view lists every applied migration newest-first (name, apply time, op count, destructive marker, and compact diff-stat chips) and renders the selected migration as a visual, UML-style diff on a React Flow canvas with ELK layout, built from the contract snapshots stored in the ledger: green cards for added models, red strikethrough for removed ones, amber cards with per-field before → after pills for changed models, enum cards, and relation edges, with untouched neighbors dimmed for context. A collapsible SQL panel shows each operation class and the executed statements, and the selection is URL-addressable. The diff engine (contract-diff.ts) is pure and tested against the Prisma Next contract JSON shape; ledger parsing repairs missing before-snapshots from each predecessor edge. The demo seed replays a 20-migration history captured from the prisma-next migrations-showcase example so pnpm demo:ppg exercises the whole feature. The FigJam-style canvas cards are documented as an approved non-standard UI exception in Architecture/non-standard-ui.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ema diff, morphing canvas Follow-ups from design review of the Migrations view: - A model now turns amber only when its own table changed (fields or indexes). Back-relations whose foreign key lives in the other table no longer mark the untouched model as updated — the new relation shows through the emphasized edge instead, and the list chips follow the same rule. - An "All models" toggle (persisted UI state) expands the canvas from touched-plus-neighbors to the migration's full schema. - A Schema detail panel joins SQL: contract snapshots are projected into Prisma-schema-style text and diffed line-by-line (jsdiff) with color-coded added/removed rows and collapsed unchanged runs. jsdiff over @pierre/diffs to keep shiki out of the published bundle; the renderer is isolated so it can be swapped. - Switching migrations morphs the canvas instead of rebuilding it: the React Flow instance persists, stable node ids let surviving cards glide to their new ELK positions over ~500ms, entering nodes fade in, and the camera refits smoothly. This also removes the AnimatePresence remount path entirely. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The "All models" toggle expanded models but the canvas still filtered enums to touched ones, so an enum added in an earlier migration (e.g. Priority from #5) silently disappeared from later migrations even with the toggle on. The graph builder now widens both models and enums when showing everything, rendering unchanged enums in the dimmed context style that already existed for them. buildDiffGraph moves from MigrationsView.tsx into diff-layout.ts so the scoping rules (touched-plus-neighbors default, all-models expansion, touched-enum inclusion, relation-only touched scoping, no-op fallback) are pinned by direct unit tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… resizable Two layout refinements to the Migrations view: - The per-migration header (title, hash edge, stat chips, All models / SQL / Schema controls) is now a slim translucent bar floating over the top edge of the canvas, so the diagram owns the full height of the content pane instead of losing a header strip. - The SQL/Schema detail panel gained a drag handle on its top edge: the split is resizable by pointer (with row-resize cursor and text-select suppression during the drag) and by ArrowUp/ArrowDown on the focused handle, clamped to 120-640px and persisted in UI state. Both panels now share one scrollable container sized by that height. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The migrations seed read its fixture with readFileSync relative to import.meta.url, which breaks in bundled artifacts (the Compute preview deploy and demo:ppg:build bundle server.ts into a single file with no sibling fixtures/ directory). A static JSON import lets every bundling path inline the fixture, so PR preview deploys seed the full 20-migration history. Verified by building the deploy artifact and booting a fresh demo. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
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:
WalkthroughChangesThis PR adds a Prisma Next Migrations view to Studio, with ledger detection and parsing hooks, contract snapshot diffing, Prisma-schema rendering, React Flow/ELK graph layout, navigation and URL state wiring, demo seeding, documentation, and tests. It also updates preview deploy URL resolution to use a fallback lookup path and filters null GitHub output values. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
Contributor
|
Compute preview deployed. Branch: |
Open
5 tasks
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Warning
CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.
Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.
Actionable comments posted: 14
🤖 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 `@demo/ppg-dev/seed-migrations.ts`:
- Around line 105-152: The fixture replay in the seeding flow is leaving partial
`prisma_contract.ledger` and side-effect SQL committed before completion, so
wrap the entire replay path in a single transaction. Update the seeding logic
around the `count` guard and the `for` loops in `seed-migrations.ts` to run
inside `sql.begin(...)` (or equivalent transactional wrapper), including the
nested `operation.execute` steps, the `ledger` inserts, the `marker` inserts,
and `seedShowcaseRows`. This keeps the `seed` function atomic and prevents the
`count > 0` idempotency check from permanently skipping a half-failed run.
In `@package.json`:
- Line 200: The runtime import of diffLines from
ui/studio/views/migrations/psl-schema.ts means diff must be available in
production, but it is currently only listed as a dev dependency. Move diff from
devDependencies to dependencies in package.json so the migration code can
resolve it in production and packaged workspace installs.
In `@ui/hooks/use-migrations.ts`:
- Around line 6-14: The ledger fetch in LEDGER_QUERY is loading the full
history, including large contract_json_before/contract_json_after payloads, on
every Migrations view refresh. Update use-migrations.ts so the initial query
returns only paginated ledger metadata or a limited page of rows, and defer
snapshot JSON loading until a specific migration is selected. Adjust the related
query path in the 211-230 section to support this lazy/split loading pattern
using the existing LEDGER_QUERY flow.
- Around line 47-57: The parseJsonish helper is swallowing JSON.parse failures
and returning null without any diagnostics, so corrupted migration payloads are
hidden. Update parseJsonish in use-migrations to report the parse error when
JSON parsing fails, and make sure the callers that process contract_json_before,
contract_json_after, and operations can surface or log that failure instead of
quietly treating it as empty data.
- Around line 93-101: The fallback in use-migrations.ts currently treats any
missing or invalid operationClass as "additive", which can hide destructive
migrations. Update the normalization logic in useMigrations so
operation.operationClass is only accepted when it matches a known valid value;
otherwise surface it as unknown/invalid instead of defaulting to "additive".
Then adjust the downstream destructive badge and isDestructive handling in the
same hook so unrecognized classes are treated conservatively and do not get
classified as non-destructive.
- Around line 76-105: The parsed migration operations in use-migrations.ts
currently default missing operation.id values to an empty string, which can
produce duplicate React keys in MigrationSqlPanel. Update the normalization
logic that builds the StudioMigrationOperation array so every operation gets a
stable unique id when the source id is missing or invalid, rather than "". Make
sure the fix is applied in the parser that feeds migration.operations so
MigrationsView’s map keying stays unique and deterministic.
In `@ui/index.css`:
- Around line 286-288: The `.react-flow__node.dragging` selector in the
migrations diff canvas stylesheet appears unreachable because `nodesDraggable`
is disabled, so either remove the dead rule or add a brief comment near the
relevant React Flow config/style block explaining that it is intentionally kept
for defensive or future draggable behavior. Use the `.react-flow__node.dragging`
rule and the `nodesDraggable={false}` setting to locate the affected styling.
In `@ui/studio/views/migrations/contract-diff.ts`:
- Around line 48-59: `diffFields` is missing primary-key detection, so PK-only
edits can still be treated as unchanged; update the field comparison logic in
`diffFields` to compare `isPrimaryKey` and add a corresponding
`FieldChangeDetail` when it differs so the `FieldDiff.status` becomes changed.
Also align `FieldChangeDetail.aspect` in `FieldChangeDetail`/`FieldDiff` with
the actual emitted details by either adding a real enum-diff branch or removing
the unused `"enum"` union member.
In `@ui/studio/views/migrations/diff-layout.ts`:
- Around line 239-276: The catch block in the ELK layout routine is swallowing
layout failures, so add error logging in the same try/catch around elk.layout
before falling back to the grid. Use a clear message that includes the caught
error details and keep the existing fallback behavior in diff-layout.ts so
issues in the layout path can be diagnosed without changing the returned
positions.
- Around line 174-226: The edge-deduping in the relation rendering logic is too
coarse because `pairKey` only keys by model pair, which collapses distinct
relations between the same two models. Update the edge identity in
`diff-layout.ts` so the `seenEdges` check and `id` use a relation-level key from
`relation.name`, `relation.toModel`, and `relation.cardinality`, matching the
distinctness used by `contract-diff.ts`. Keep the `added`/`removed` styling
logic in the same loop, but ensure each unique relation produces its own edge
instead of being skipped by the model-pair key.
In `@ui/studio/views/migrations/MigrationsView.tsx`:
- Around line 560-567: In MigrationsView, replace the array index keys used in
operation.statements.map and lines.map with stable identifiers. Update the <pre>
and related rendered items to derive a key from a persistent value such as the
statement/line content plus an operation-specific identifier (for example
operation.id or line type) so future reordering or insertions do not reuse the
wrong React element state.
- Around line 374-456: `MigrationDiffCanvas` is re-rendering on every
`MigrationsView` resize tick even when `migration` and `showAllModels` haven’t
changed. Wrap `MigrationDiffCanvas` with `React.memo` so the ReactFlow subtree
is skipped during `isPanelResizing`/`draftPanelHeight` updates, and keep its
existing `migration` and `showAllModels` props as the memo boundary.
- Around line 880-917: The SQL and Schema controls in MigrationsView are toggle
buttons but only expose state via data-active, so add aria-pressed to both
Button components and bind it to the same detailsPanel checks used for
variant/data-active. Update the SQL toggle and the Schema toggle together so
assistive tech can announce their pressed/unpressed state consistently.
In `@ui/studio/views/migrations/psl-schema.ts`:
- Around line 36-54: The pslDefault helper is emitting invalid PSL for
parameterized SQL defaults because only zero-arg calls are routed through
dbgenerated; update pslDefault to detect any function-like default expression
that is not one of the explicit special cases (such as now() and
gen_random_uuid()) and wrap it with `@default`(dbgenerated(...)) instead of
returning `@default`(...) directly.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e334b34e-846f-47e7-abba-09906179f7bd
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (25)
.changeset/migrations-view.mdArchitecture/migrations-view.mdArchitecture/non-standard-ui.mdFEATURES.mddemo/ppg-dev/fixtures/prisma-contract-migrations.jsondemo/ppg-dev/seed-database.tsdemo/ppg-dev/seed-migrations.tspackage.jsonui/hooks/nuqs.tsui/hooks/react-query.tsui/hooks/use-migrations.test.tsui/hooks/use-migrations.tsui/hooks/use-navigation.tsxui/index.cssui/studio/Navigation.test.tsxui/studio/Navigation.tsxui/studio/Studio.tsxui/studio/views/migrations/MigrationsView.test.tsxui/studio/views/migrations/MigrationsView.tsxui/studio/views/migrations/contract-diff.test.tsui/studio/views/migrations/contract-diff.tsui/studio/views/migrations/diff-layout.test.tsui/studio/views/migrations/diff-layout.tsui/studio/views/migrations/psl-schema.test.tsui/studio/views/migrations/psl-schema.ts
Comment on lines
+105
to
+152
| const [{ count }] = (await sql.unsafe( | ||
| `select count(*)::int as count from prisma_contract.ledger`, | ||
| )) as unknown as [{ count: number }]; | ||
|
|
||
| if (count > 0) { | ||
| return; | ||
| } | ||
|
|
||
| for (const row of fixture.ledger) { | ||
| for (const operation of row.operations) { | ||
| for (const step of operation.execute ?? []) { | ||
| await sql.unsafe(step.sql, (step.params ?? []) as never[]); | ||
| } | ||
| } | ||
|
|
||
| await sql` | ||
| insert into prisma_contract.ledger ( | ||
| created_at, space, migration_name, migration_hash, | ||
| origin_core_hash, destination_core_hash, | ||
| contract_json_before, contract_json_after, operations | ||
| ) values ( | ||
| ${row.created_at}, ${row.space}, ${row.migration_name}, | ||
| ${row.migration_hash}, ${row.origin_core_hash}, | ||
| ${row.destination_core_hash}, | ||
| ${row.contract_json_before === null ? null : sql.json(row.contract_json_before as never)}, | ||
| ${row.contract_json_after === null ? null : sql.json(row.contract_json_after as never)}, | ||
| ${sql.json(row.operations as never)} | ||
| ) | ||
| `; | ||
| } | ||
|
|
||
| for (const row of fixture.marker) { | ||
| await sql` | ||
| insert into prisma_contract.marker ( | ||
| space, core_hash, profile_hash, contract_json, canonical_version, | ||
| updated_at, app_tag, meta, invariants | ||
| ) values ( | ||
| ${row.space}, ${row.core_hash}, ${row.profile_hash}, | ||
| ${row.contract_json === null ? null : sql.json(row.contract_json as never)}, | ||
| ${row.canonical_version}, ${row.updated_at}, ${row.app_tag}, | ||
| ${sql.json((row.meta ?? {}) as never)}, ${row.invariants} | ||
| ) | ||
| on conflict (space) do nothing | ||
| `; | ||
| } | ||
|
|
||
| await seedShowcaseRows(sql); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Wrap fixture replay in a transaction to avoid a permanently stuck partial seed.
The idempotency guard only checks count > 0 before replaying. If the process fails partway through the loop (e.g., row 10 of 20 fails to execute, or the process is killed), some ledger rows and their SQL side-effects will already be committed. On the next run, count > 0 is now true, so the function returns immediately — leaving the demo database in a permanently half-migrated, unrecoverable state. This is inconsistent with seed-database.ts, which wraps its own seeding in sql.begin(...) for exactly this reason.
🔧 Proposed fix: wrap the replay + inserts in a transaction
- for (const row of fixture.ledger) {
- for (const operation of row.operations) {
- for (const step of operation.execute ?? []) {
- await sql.unsafe(step.sql, (step.params ?? []) as never[]);
- }
- }
-
- await sql`
- insert into prisma_contract.ledger (
- ...
- )
- `;
- }
-
- for (const row of fixture.marker) {
- await sql`
- insert into prisma_contract.marker (...)
- on conflict (space) do nothing
- `;
- }
-
- await seedShowcaseRows(sql);
+ await sql.begin(async (tx) => {
+ for (const row of fixture.ledger) {
+ for (const operation of row.operations) {
+ for (const step of operation.execute ?? []) {
+ await tx.unsafe(step.sql, (step.params ?? []) as never[]);
+ }
+ }
+
+ await tx`
+ insert into prisma_contract.ledger (...)
+ `;
+ }
+
+ for (const row of fixture.marker) {
+ await tx`
+ insert into prisma_contract.marker (...)
+ on conflict (space) do nothing
+ `;
+ }
+
+ await seedShowcaseRows(tx as unknown as postgres.Sql);
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const [{ count }] = (await sql.unsafe( | |
| `select count(*)::int as count from prisma_contract.ledger`, | |
| )) as unknown as [{ count: number }]; | |
| if (count > 0) { | |
| return; | |
| } | |
| for (const row of fixture.ledger) { | |
| for (const operation of row.operations) { | |
| for (const step of operation.execute ?? []) { | |
| await sql.unsafe(step.sql, (step.params ?? []) as never[]); | |
| } | |
| } | |
| await sql` | |
| insert into prisma_contract.ledger ( | |
| created_at, space, migration_name, migration_hash, | |
| origin_core_hash, destination_core_hash, | |
| contract_json_before, contract_json_after, operations | |
| ) values ( | |
| ${row.created_at}, ${row.space}, ${row.migration_name}, | |
| ${row.migration_hash}, ${row.origin_core_hash}, | |
| ${row.destination_core_hash}, | |
| ${row.contract_json_before === null ? null : sql.json(row.contract_json_before as never)}, | |
| ${row.contract_json_after === null ? null : sql.json(row.contract_json_after as never)}, | |
| ${sql.json(row.operations as never)} | |
| ) | |
| `; | |
| } | |
| for (const row of fixture.marker) { | |
| await sql` | |
| insert into prisma_contract.marker ( | |
| space, core_hash, profile_hash, contract_json, canonical_version, | |
| updated_at, app_tag, meta, invariants | |
| ) values ( | |
| ${row.space}, ${row.core_hash}, ${row.profile_hash}, | |
| ${row.contract_json === null ? null : sql.json(row.contract_json as never)}, | |
| ${row.canonical_version}, ${row.updated_at}, ${row.app_tag}, | |
| ${sql.json((row.meta ?? {}) as never)}, ${row.invariants} | |
| ) | |
| on conflict (space) do nothing | |
| `; | |
| } | |
| await seedShowcaseRows(sql); | |
| } | |
| const [{ count }] = (await sql.unsafe( | |
| `select count(*)::int as count from prisma_contract.ledger`, | |
| )) as unknown as [{ count: number }]; | |
| if (count > 0) { | |
| return; | |
| } | |
| await sql.begin(async (tx) => { | |
| for (const row of fixture.ledger) { | |
| for (const operation of row.operations) { | |
| for (const step of operation.execute ?? []) { | |
| await tx.unsafe(step.sql, (step.params ?? []) as never[]); | |
| } | |
| } | |
| await tx` | |
| insert into prisma_contract.ledger ( | |
| created_at, space, migration_name, migration_hash, | |
| origin_core_hash, destination_core_hash, | |
| contract_json_before, contract_json_after, operations | |
| ) values ( | |
| ${row.created_at}, ${row.space}, ${row.migration_name}, | |
| ${row.migration_hash}, ${row.origin_core_hash}, | |
| ${row.destination_core_hash}, | |
| ${row.contract_json_before === null ? null : sql.json(row.contract_json_before as never)}, | |
| ${row.contract_json_after === null ? null : sql.json(row.contract_json_after as never)}, | |
| ${sql.json(row.operations as never)} | |
| ) | |
| `; | |
| } | |
| for (const row of fixture.marker) { | |
| await tx` | |
| insert into prisma_contract.marker ( | |
| space, core_hash, profile_hash, contract_json, canonical_version, | |
| updated_at, app_tag, meta, invariants | |
| ) values ( | |
| ${row.space}, ${row.core_hash}, ${row.profile_hash}, | |
| ${row.contract_json === null ? null : sql.json(row.contract_json as never)}, | |
| ${row.canonical_version}, ${row.updated_at}, ${row.app_tag}, | |
| ${sql.json((row.meta ?? {}) as never)}, ${row.invariants} | |
| ) | |
| on conflict (space) do nothing | |
| `; | |
| } | |
| await seedShowcaseRows(tx as unknown as postgres.Sql); | |
| }); | |
| } |
🤖 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 `@demo/ppg-dev/seed-migrations.ts` around lines 105 - 152, The fixture replay
in the seeding flow is leaving partial `prisma_contract.ledger` and side-effect
SQL committed before completion, so wrap the entire replay path in a single
transaction. Update the seeding logic around the `count` guard and the `for`
loops in `seed-migrations.ts` to run inside `sql.begin(...)` (or equivalent
transactional wrapper), including the nested `operation.execute` steps, the
`ledger` inserts, the `marker` inserts, and `seedShowcaseRows`. This keeps the
`seed` function atomic and prevents the `count > 0` idempotency check from
permanently skipping a half-failed run.
| "clsx": "2.1.1", | ||
| "cmdk": "1.1.1", | ||
| "dayjs": "1.11.19", | ||
| "diff": "8.0.2", |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how other runtime-only packages (e.g. react, dayjs) are classified in package.json,
# and whether any build/deploy step installs with production-only deps.
jq '.dependencies // "no dependencies field", (.devDependencies | has("dayjs")), (.devDependencies | has("react"))' package.json
rg -n '"dependencies"' package.json
rg -n 'npm ci|--omit=dev|--production' . -g '*.yml' -g '*.yaml' -g 'Dockerfile*'Repository: prisma/studio
Length of output: 536
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- package.json around diff dependency ---'
nl -ba package.json | sed -n '180,215p'
echo
echo '--- runtime import usage of diff ---'
rg -n 'from ["'\'']diff["'\'']|require\(["'\'']diff["'\'']\)' .
echo
echo '--- build/deploy/package install references ---'
rg -n 'npm ci|npm install|pnpm install|yarn install|--omit=dev|--production|NODE_ENV=production|production-only|deploy/demo' . -g '*.yml' -g '*.yaml' -g 'Dockerfile*' -g '*.sh' -g '*.ts' -g '*.js' -g '*.mjs' -g '*.cjs' -g '*.json'Repository: prisma/studio
Length of output: 235
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- package.json around diff dependency ---'
awk 'NR>=180 && NR<=215 { printf "%5d %s\n", NR, $0 }' package.json
echo
echo '--- location of diff in package.json ---'
rg -n '"diff"\s*:' package.json
echo
echo '--- runtime import usage of diff ---'
rg -n 'from ["'\'']diff["'\'']|require\(["'\'']diff["'\'']\)' .
echo
echo '--- install/deploy references ---'
rg -n 'npm ci|npm install|pnpm install|yarn install|--omit=dev|--production|NODE_ENV=production|production-only|deploy/demo|bundle' . -g '*.yml' -g '*.yaml' -g 'Dockerfile*' -g '*.sh' -g '*.ts' -g '*.js' -g '*.mjs' -g '*.cjs' -g '*.json'Repository: prisma/studio
Length of output: 7629
Move diff to dependencies
ui/studio/views/migrations/psl-schema.ts imports diffLines at runtime, so keeping diff in devDependencies can break production installs or isolated workspace packaging.
🤖 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 `@package.json` at line 200, The runtime import of diffLines from
ui/studio/views/migrations/psl-schema.ts means diff must be available in
production, but it is currently only listed as a dev dependency. Move diff from
devDependencies to dependencies in package.json so the migration code can
resolve it in production and packaged workspace installs.
Comment on lines
+6
to
+14
| const LEDGER_QUERY = ` | ||
| select | ||
| "id", "space", "migration_name", "migration_hash", | ||
| "origin_core_hash", "destination_core_hash", | ||
| "contract_json_before", "contract_json_after", | ||
| "operations", "created_at" | ||
| from "prisma_contract"."ledger" | ||
| order by "id" asc | ||
| `; |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy lift
No pagination/limit on the ledger query.
LEDGER_QUERY fetches every ledger row—including both full contract_json_before and contract_json_after snapshots—on every load (subject to the 30s staleTime). For databases with long migration histories, this could mean transferring and parsing many large JSON schema snapshots on each Migrations view visit/refetch.
Consider paginating or lazily loading contract snapshots (e.g., load only ledger metadata initially, fetch snapshot JSON for the selected migration on demand).
Also applies to: 211-230
🤖 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 `@ui/hooks/use-migrations.ts` around lines 6 - 14, The ledger fetch in
LEDGER_QUERY is loading the full history, including large
contract_json_before/contract_json_after payloads, on every Migrations view
refresh. Update use-migrations.ts so the initial query returns only paginated
ledger metadata or a limited page of rows, and defer snapshot JSON loading until
a specific migration is selected. Adjust the related query path in the 211-230
section to support this lazy/split loading pattern using the existing
LEDGER_QUERY flow.
Comment on lines
+47
to
+57
| function parseJsonish(value: unknown): unknown { | ||
| if (typeof value !== "string") { | ||
| return value; | ||
| } | ||
|
|
||
| try { | ||
| return JSON.parse(value); | ||
| } catch { | ||
| return null; | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
Silent JSON parse failures with no diagnostics.
parseJsonish swallows JSON.parse errors and returns null (Line 54-56) with no logging. A corrupted contract_json_before/after or operations payload will silently degrade to an empty/blank diff rather than surfacing an actionable error.
🤖 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 `@ui/hooks/use-migrations.ts` around lines 47 - 57, The parseJsonish helper is
swallowing JSON.parse failures and returning null without any diagnostics, so
corrupted migration payloads are hidden. Update parseJsonish in use-migrations
to report the parse error when JSON parsing fails, and make sure the callers
that process contract_json_before, contract_json_after, and operations can
surface or log that failure instead of quietly treating it as empty data.
Comment on lines
+76
to
+105
| const operations: StudioMigrationOperation[] = []; | ||
|
|
||
| for (const operation of parsed) { | ||
| if (!isRecord(operation)) { | ||
| continue; | ||
| } | ||
|
|
||
| const statements: string[] = []; | ||
|
|
||
| if (Array.isArray(operation.execute)) { | ||
| for (const step of operation.execute) { | ||
| if (isRecord(step) && typeof step.sql === "string") { | ||
| statements.push(step.sql); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| operations.push({ | ||
| id: typeof operation.id === "string" ? operation.id : "", | ||
| label: typeof operation.label === "string" ? operation.label : "", | ||
| operationClass: | ||
| typeof operation.operationClass === "string" | ||
| ? operation.operationClass | ||
| : "additive", | ||
| statements, | ||
| }); | ||
| } | ||
|
|
||
| return operations; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
operation.id defaults to "", risking duplicate React keys downstream.
When operation.id is missing/non-string it falls back to "" (Line 94). MigrationSqlPanel (ui/studio/views/migrations/MigrationsView.tsx:537-573) renders migration.operations.map(...) keyed directly by operation.id. If a migration has multiple operations without ids, they'll all share the key "", causing React reconciliation bugs/dev warnings in the SQL detail panel.
💡 Proposed fix
- operations.push({
- id: typeof operation.id === "string" ? operation.id : "",
+ operations.push({
+ id:
+ typeof operation.id === "string" && operation.id.length > 0
+ ? operation.id
+ : `op-${operations.length}`,
label: typeof operation.label === "string" ? operation.label : "",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const operations: StudioMigrationOperation[] = []; | |
| for (const operation of parsed) { | |
| if (!isRecord(operation)) { | |
| continue; | |
| } | |
| const statements: string[] = []; | |
| if (Array.isArray(operation.execute)) { | |
| for (const step of operation.execute) { | |
| if (isRecord(step) && typeof step.sql === "string") { | |
| statements.push(step.sql); | |
| } | |
| } | |
| } | |
| operations.push({ | |
| id: typeof operation.id === "string" ? operation.id : "", | |
| label: typeof operation.label === "string" ? operation.label : "", | |
| operationClass: | |
| typeof operation.operationClass === "string" | |
| ? operation.operationClass | |
| : "additive", | |
| statements, | |
| }); | |
| } | |
| return operations; | |
| } | |
| const operations: StudioMigrationOperation[] = []; | |
| for (const operation of parsed) { | |
| if (!isRecord(operation)) { | |
| continue; | |
| } | |
| const statements: string[] = []; | |
| if (Array.isArray(operation.execute)) { | |
| for (const step of operation.execute) { | |
| if (isRecord(step) && typeof step.sql === "string") { | |
| statements.push(step.sql); | |
| } | |
| } | |
| } | |
| operations.push({ | |
| id: | |
| typeof operation.id === "string" && operation.id.length > 0 | |
| ? operation.id | |
| : `op-${operations.length}`, | |
| label: typeof operation.label === "string" ? operation.label : "", | |
| operationClass: | |
| typeof operation.operationClass === "string" | |
| ? operation.operationClass | |
| : "additive", | |
| statements, | |
| }); | |
| } | |
| return operations; | |
| } |
🤖 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 `@ui/hooks/use-migrations.ts` around lines 76 - 105, The parsed migration
operations in use-migrations.ts currently default missing operation.id values to
an empty string, which can produce duplicate React keys in MigrationSqlPanel.
Update the normalization logic that builds the StudioMigrationOperation array so
every operation gets a stable unique id when the source id is missing or
invalid, rather than "". Make sure the fix is applied in the parser that feeds
migration.operations so MigrationsView’s map keying stays unique and
deterministic.
Comment on lines
+239
to
+276
| try { | ||
| const layouted = await elk.layout({ | ||
| id: "root", | ||
| layoutOptions: { ...ELK_LAYOUT_OPTIONS }, | ||
| children: nodes.map((node) => ({ | ||
| id: node.id, | ||
| ...estimateNodeSize(node), | ||
| })), | ||
| edges: edges.map((edge) => ({ | ||
| id: edge.id, | ||
| sources: [edge.source], | ||
| targets: [edge.target], | ||
| })), | ||
| }); | ||
|
|
||
| const positions = new Map( | ||
| (layouted.children ?? []).map((child) => [ | ||
| child.id, | ||
| { x: child.x ?? 0, y: child.y ?? 0 }, | ||
| ]), | ||
| ); | ||
|
|
||
| return nodes.map((node) => ({ | ||
| ...node, | ||
| position: positions.get(node.id) ?? node.position, | ||
| })); | ||
| } catch { | ||
| const columns = Math.max(1, Math.ceil(Math.sqrt(nodes.length))); | ||
|
|
||
| return nodes.map((node, index) => ({ | ||
| ...node, | ||
| position: { | ||
| x: (index % columns) * (NODE_WIDTH + 90), | ||
| y: Math.floor(index / columns) * 360, | ||
| }, | ||
| })); | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔵 Trivial | 💤 Low value
Silent catch hides ELK layout failures.
If elk.layout throws, the error is discarded and the grid fallback kicks in with no trace, making degraded layouts hard to diagnose in production.
♻️ Suggested fix
} catch {
+ console.error("Migration diff layout failed, falling back to grid", error);
const columns = Math.max(1, Math.ceil(Math.sqrt(nodes.length)));🤖 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 `@ui/studio/views/migrations/diff-layout.ts` around lines 239 - 276, The catch
block in the ELK layout routine is swallowing layout failures, so add error
logging in the same try/catch around elk.layout before falling back to the grid.
Use a clear message that includes the caught error details and keep the existing
fallback behavior in diff-layout.ts so issues in the layout path can be
diagnosed without changing the returned positions.
Comment on lines
+374
to
+456
| function MigrationDiffCanvas(props: { | ||
| migration: StudioMigration; | ||
| showAllModels: boolean; | ||
| }) { | ||
| const { migration, showAllModels } = props; | ||
| const diff = useMemo( | ||
| () => diffContracts(migration.contractBefore, migration.contractAfter), | ||
| [migration.contractBefore, migration.contractAfter], | ||
| ); | ||
| const graph = useMemo( | ||
| () => buildDiffGraph(diff, showAllModels), | ||
| [diff, showAllModels], | ||
| ); | ||
| const [layoutedGraph, setLayoutedGraph] = useState<{ | ||
| nodes: MigrationDiffNode[]; | ||
| edges: Edge[]; | ||
| }>({ nodes: [], edges: [] }); | ||
| const reactFlowInstanceRef = useRef<ReactFlowInstance | null>(null); | ||
|
|
||
| useEffect(() => { | ||
| let cancelled = false; | ||
|
|
||
| void layoutMigrationDiffNodes(graph.nodes, graph.edges).then((nodes) => { | ||
| if (cancelled) { | ||
| return; | ||
| } | ||
|
|
||
| // Nodes and edges swap together so the morph transition animates | ||
| // shared nodes (stable `model:<name>` ids) to their new positions | ||
| // instead of tearing the whole canvas down. | ||
| setLayoutedGraph({ nodes, edges: graph.edges }); | ||
|
|
||
| if (typeof window !== "undefined") { | ||
| window.requestAnimationFrame(() => { | ||
| window.requestAnimationFrame(() => { | ||
| void reactFlowInstanceRef.current?.fitView({ | ||
| duration: 500, | ||
| padding: 0.18, | ||
| }); | ||
| }); | ||
| }); | ||
| } | ||
| }); | ||
|
|
||
| return () => { | ||
| cancelled = true; | ||
| }; | ||
| }, [graph]); | ||
|
|
||
| if (migration.contractAfter == null && migration.contractBefore == null) { | ||
| return ( | ||
| <div className="flex h-full items-center justify-center px-6 text-center text-sm text-muted-foreground"> | ||
| This migration was applied before contract snapshots were recorded, so | ||
| there is no visual diff to show. | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| return ( | ||
| <ReactFlow | ||
| className="migrations-diff-canvas" | ||
| edges={layoutedGraph.edges} | ||
| fitView | ||
| fitViewOptions={{ padding: 0.18 }} | ||
| maxZoom={1.4} | ||
| minZoom={0.2} | ||
| nodes={layoutedGraph.nodes} | ||
| nodesConnectable={false} | ||
| nodesDraggable={false} | ||
| nodeTypes={nodeTypes} | ||
| onInit={(instance) => { | ||
| reactFlowInstanceRef.current = instance; | ||
| }} | ||
| proOptions={{ hideAttribution: true }} | ||
| > | ||
| <Background className="bg-muted/40" gap={20} size={1.5} /> | ||
| <Controls | ||
| className="shadow-sm [&_button]:border [&_button]:border-input [&_button]:bg-background [&_button_>_svg]:fill-foreground" | ||
| showInteractive={false} | ||
| /> | ||
| </ReactFlow> | ||
| ); | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
Memoize MigrationDiffCanvas to avoid re-render during panel resize.
isPanelResizing/draftPanelHeight update on every pointermove while dragging the details-panel handle (lines 696-708), re-rendering MigrationsView and therefore MigrationDiffCanvas even though its migration/showAllModels props are unchanged. Wrapping it in React.memo avoids reconciling the ReactFlow subtree on every drag tick.
♻️ Suggested fix
-function MigrationDiffCanvas(props: {
+const MigrationDiffCanvas = memo(function MigrationDiffCanvas(props: {
migration: StudioMigration;
showAllModels: boolean;
}) {
...
-}
+});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function MigrationDiffCanvas(props: { | |
| migration: StudioMigration; | |
| showAllModels: boolean; | |
| }) { | |
| const { migration, showAllModels } = props; | |
| const diff = useMemo( | |
| () => diffContracts(migration.contractBefore, migration.contractAfter), | |
| [migration.contractBefore, migration.contractAfter], | |
| ); | |
| const graph = useMemo( | |
| () => buildDiffGraph(diff, showAllModels), | |
| [diff, showAllModels], | |
| ); | |
| const [layoutedGraph, setLayoutedGraph] = useState<{ | |
| nodes: MigrationDiffNode[]; | |
| edges: Edge[]; | |
| }>({ nodes: [], edges: [] }); | |
| const reactFlowInstanceRef = useRef<ReactFlowInstance | null>(null); | |
| useEffect(() => { | |
| let cancelled = false; | |
| void layoutMigrationDiffNodes(graph.nodes, graph.edges).then((nodes) => { | |
| if (cancelled) { | |
| return; | |
| } | |
| // Nodes and edges swap together so the morph transition animates | |
| // shared nodes (stable `model:<name>` ids) to their new positions | |
| // instead of tearing the whole canvas down. | |
| setLayoutedGraph({ nodes, edges: graph.edges }); | |
| if (typeof window !== "undefined") { | |
| window.requestAnimationFrame(() => { | |
| window.requestAnimationFrame(() => { | |
| void reactFlowInstanceRef.current?.fitView({ | |
| duration: 500, | |
| padding: 0.18, | |
| }); | |
| }); | |
| }); | |
| } | |
| }); | |
| return () => { | |
| cancelled = true; | |
| }; | |
| }, [graph]); | |
| if (migration.contractAfter == null && migration.contractBefore == null) { | |
| return ( | |
| <div className="flex h-full items-center justify-center px-6 text-center text-sm text-muted-foreground"> | |
| This migration was applied before contract snapshots were recorded, so | |
| there is no visual diff to show. | |
| </div> | |
| ); | |
| } | |
| return ( | |
| <ReactFlow | |
| className="migrations-diff-canvas" | |
| edges={layoutedGraph.edges} | |
| fitView | |
| fitViewOptions={{ padding: 0.18 }} | |
| maxZoom={1.4} | |
| minZoom={0.2} | |
| nodes={layoutedGraph.nodes} | |
| nodesConnectable={false} | |
| nodesDraggable={false} | |
| nodeTypes={nodeTypes} | |
| onInit={(instance) => { | |
| reactFlowInstanceRef.current = instance; | |
| }} | |
| proOptions={{ hideAttribution: true }} | |
| > | |
| <Background className="bg-muted/40" gap={20} size={1.5} /> | |
| <Controls | |
| className="shadow-sm [&_button]:border [&_button]:border-input [&_button]:bg-background [&_button_>_svg]:fill-foreground" | |
| showInteractive={false} | |
| /> | |
| </ReactFlow> | |
| ); | |
| } | |
| const MigrationDiffCanvas = memo(function MigrationDiffCanvas(props: { | |
| migration: StudioMigration; | |
| showAllModels: boolean; | |
| }) { | |
| const { migration, showAllModels } = props; | |
| const diff = useMemo( | |
| () => diffContracts(migration.contractBefore, migration.contractAfter), | |
| [migration.contractBefore, migration.contractAfter], | |
| ); | |
| const graph = useMemo( | |
| () => buildDiffGraph(diff, showAllModels), | |
| [diff, showAllModels], | |
| ); | |
| const [layoutedGraph, setLayoutedGraph] = useState<{ | |
| nodes: MigrationDiffNode[]; | |
| edges: Edge[]; | |
| }>({ nodes: [], edges: [] }); | |
| const reactFlowInstanceRef = useRef<ReactFlowInstance | null>(null); | |
| useEffect(() => { | |
| let cancelled = false; | |
| void layoutMigrationDiffNodes(graph.nodes, graph.edges).then((nodes) => { | |
| if (cancelled) { | |
| return; | |
| } | |
| // Nodes and edges swap together so the morph transition animates | |
| // shared nodes (stable `model:<name>` ids) to their new positions | |
| // instead of tearing the whole canvas down. | |
| setLayoutedGraph({ nodes, edges: graph.edges }); | |
| if (typeof window !== "undefined") { | |
| window.requestAnimationFrame(() => { | |
| window.requestAnimationFrame(() => { | |
| void reactFlowInstanceRef.current?.fitView({ | |
| duration: 500, | |
| padding: 0.18, | |
| }); | |
| }); | |
| }); | |
| } | |
| }); | |
| return () => { | |
| cancelled = true; | |
| }; | |
| }, [graph]); | |
| if (migration.contractAfter == null && migration.contractBefore == null) { | |
| return ( | |
| <div className="flex h-full items-center justify-center px-6 text-center text-sm text-muted-foreground"> | |
| This migration was applied before contract snapshots were recorded, so | |
| there is no visual diff to show. | |
| </div> | |
| ); | |
| } | |
| return ( | |
| <ReactFlow | |
| className="migrations-diff-canvas" | |
| edges={layoutedGraph.edges} | |
| fitView | |
| fitViewOptions={{ padding: 0.18 }} | |
| maxZoom={1.4} | |
| minZoom={0.2} | |
| nodes={layoutedGraph.nodes} | |
| nodesConnectable={false} | |
| nodesDraggable={false} | |
| nodeTypes={nodeTypes} | |
| onInit={(instance) => { | |
| reactFlowInstanceRef.current = instance; | |
| }} | |
| proOptions={{ hideAttribution: true }} | |
| > | |
| <Background className="bg-muted/40" gap={20} size={1.5} /> | |
| <Controls | |
| className="shadow-sm [&_button]:border [&_button]:border-input [&_button]:bg-background [&_button_>_svg]:fill-foreground" | |
| showInteractive={false} | |
| /> | |
| </ReactFlow> | |
| ); | |
| }); |
🤖 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 `@ui/studio/views/migrations/MigrationsView.tsx` around lines 374 - 456,
`MigrationDiffCanvas` is re-rendering on every `MigrationsView` resize tick even
when `migration` and `showAllModels` haven’t changed. Wrap `MigrationDiffCanvas`
with `React.memo` so the ReactFlow subtree is skipped during
`isPanelResizing`/`draftPanelHeight` updates, and keep its existing `migration`
and `showAllModels` props as the memo boundary.
Comment on lines
+560
to
+567
| {operation.statements.map((statement, index) => ( | ||
| <pre | ||
| key={index} | ||
| className="overflow-x-auto rounded-md border border-border/70 bg-muted/40 px-2.5 py-1.5 font-mono text-[11px] leading-relaxed text-foreground" | ||
| > | ||
| {statement} | ||
| </pre> | ||
| ))} |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Array index used as React key.
operation.statements.map((statement, index) => <pre key={index}>...) and lines.map((line, index) => ... key={index} ...) both use the array index as key. Risk here is limited since these lists are freshly recomputed per migration/operation and hold stateless text, but a stable key (e.g. combining operation.id/line kind with index, or the content itself) would be more resilient to future reordering.
Also applies to: 616-647
🧰 Tools
🪛 React Doctor (0.5.8)
[warning] 562-562: Your users can see & submit the wrong data when this list reorders or filters, so use a stable id like key={item.id}, not the array index "index".
Use a stable id from the item, like key={item.id} or key={item.slug}. Index keys break when the list reorders or filters.
(no-array-index-as-key)
[warning] 562-562: Your users can see & submit the wrong data when this list reorders.
Use a stable key from your data so reordered items keep the right state and DOM.
(no-array-index-key)
🤖 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 `@ui/studio/views/migrations/MigrationsView.tsx` around lines 560 - 567, In
MigrationsView, replace the array index keys used in operation.statements.map
and lines.map with stable identifiers. Update the <pre> and related rendered
items to derive a key from a persistent value such as the statement/line content
plus an operation-specific identifier (for example operation.id or line type) so
future reordering or insertions do not reuse the wrong React element state.
Source: Linters/SAST tools
Comment on lines
+880
to
+917
| <div className="flex items-center gap-1"> | ||
| <Button | ||
| className="h-7 shadow-none" | ||
| data-active={detailsPanel === "sql"} | ||
| data-testid="migration-panel-sql" | ||
| onClick={() => | ||
| setDetailsPanel((panel) => | ||
| panel === "sql" ? null : "sql", | ||
| ) | ||
| } | ||
| size="xs" | ||
| type="button" | ||
| variant={ | ||
| detailsPanel === "sql" ? "secondary" : "outline" | ||
| } | ||
| > | ||
| <Terminal data-icon="inline-start" /> | ||
| SQL | ||
| </Button> | ||
| <Button | ||
| className="h-7 shadow-none" | ||
| data-active={detailsPanel === "schema"} | ||
| data-testid="migration-panel-schema" | ||
| onClick={() => | ||
| setDetailsPanel((panel) => | ||
| panel === "schema" ? null : "schema", | ||
| ) | ||
| } | ||
| size="xs" | ||
| type="button" | ||
| variant={ | ||
| detailsPanel === "schema" ? "secondary" : "outline" | ||
| } | ||
| > | ||
| <FileDiff data-icon="inline-start" /> | ||
| Schema | ||
| </Button> | ||
| </div> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Add aria-pressed to SQL/Schema toggle buttons.
data-active conveys state visually but assistive tech has no way to know these are pressed/unpressed toggles.
♻️ Suggested fix
<Button
className="h-7 shadow-none"
data-active={detailsPanel === "sql"}
data-testid="migration-panel-sql"
+ aria-pressed={detailsPanel === "sql"}🤖 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 `@ui/studio/views/migrations/MigrationsView.tsx` around lines 880 - 917, The
SQL and Schema controls in MigrationsView are toggle buttons but only expose
state via data-active, so add aria-pressed to both Button components and bind it
to the same detailsPanel checks used for variant/data-active. Update the SQL
toggle and the Schema toggle together so assistive tech can announce their
pressed/unpressed state consistently.
Comment on lines
+36
to
+54
| function pslDefault(defaultValue: string | null): string | null { | ||
| if (defaultValue === null) { | ||
| return null; | ||
| } | ||
|
|
||
| if (defaultValue === "now()") { | ||
| return "@default(now())"; | ||
| } | ||
|
|
||
| if (defaultValue === "gen_random_uuid()") { | ||
| return "@default(uuid())"; | ||
| } | ||
|
|
||
| if (defaultValue.endsWith("()")) { | ||
| return `@default(dbgenerated("${defaultValue}"))`; | ||
| } | ||
|
|
||
| return `@default(${defaultValue})`; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file outline =="
ast-grep outline ui/studio/views/migrations/psl-schema.ts --view expanded
echo
echo "== relevant file section =="
sed -n '1,140p' ui/studio/views/migrations/psl-schema.ts | cat -n
echo
echo "== usages of pslDefault =="
rg -n "pslDefault|dbgenerated\\(|now\\(|gen_random_uuid\\(" ui/studio/views/migrations -SRepository: prisma/studio
Length of output: 6686
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== contract-diff default parsing =="
ast-grep outline ui/studio/views/migrations/contract-diff.ts --view expanded
echo
echo "== default-related sections =="
rg -n "defaultValue|default:" ui/studio/views/migrations/contract-diff.ts ui/studio/views/migrations/psl-schema.test.ts ui/studio/views/migrations/contract-diff.test.ts -S
echo
echo "== relevant ranges in contract-diff.ts =="
sed -n '1,260p' ui/studio/views/migrations/contract-diff.ts | cat -n
echo
echo "== relevant ranges in psl-schema.test.ts =="
sed -n '1,220p' ui/studio/views/migrations/psl-schema.test.ts | cat -nRepository: prisma/studio
Length of output: 19420
Wrap parameterized defaults in dbgenerated(...)
pslDefault only special-cases now(), gen_random_uuid(), and other zero-arg calls. A default like nextval('table_id_seq'::regclass) still falls through to @default(nextval(...)), which is not valid PSL; route any other function expression through dbgenerated(...) instead.
🤖 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 `@ui/studio/views/migrations/psl-schema.ts` around lines 36 - 54, The
pslDefault helper is emitting invalid PSL for parameterized SQL defaults because
only zero-arg calls are routed through dbgenerated; update pslDefault to detect
any function-like default expression that is not one of the explicit special
cases (such as now() and gen_random_uuid()) and wrap it with
`@default`(dbgenerated(...)) instead of returning `@default`(...) directly.
- migrations-view-demo-v2.mp4: the 30s feature demo linked from the PR description (renders in GitHub via the blob page player). - compute-preview-deploy.mjs: newer Compute CLI payloads expose the endpoint as appEndpointDomain rather than serviceEndpointDomain, so the deploy step emitted an undefined URL and the PR comment step skipped. Fall back through the known payload shapes (deploy result, then services show) so the preview URL always lands on the PR. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The deploy CLI payload does not currently include a version endpoint,
so the literal string "undefined" flowed through GITHUB_OUTPUT into the
PR comment ("Version: undefined"). Skip null/undefined values when
writing outputs; the comment script already treats them as optional.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Warning
CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.
Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.
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 `@scripts/compute-preview/compute-preview-deploy.mjs`:
- Around line 44-59: The serviceUrl fallback chain in compute-preview-deploy.mjs
can still resolve to undefined, which then propagates into the outputs and the
preview comment flow. Add explicit validation right after deriving serviceUrl
(using deployResult.serviceEndpointDomain, deployResult.appEndpointDomain, and
showService(service.id)) and fail fast with a clear error if none of those
values are present. Keep the check close to the existing serviceUrl/result
construction so writeOutputs and compute-preview-comment.mjs never receive an
unset preview URL.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7e371a8e-26ba-4b89-9d22-26c0ea95939a
⛔ Files ignored due to path filters (1)
migrations-view-demo-v2.mp4is excluded by!**/*.mp4
📒 Files selected for processing (1)
scripts/compute-preview/compute-preview-deploy.mjs
Comment on lines
+44
to
+59
| // Newer Compute CLI payloads surface the endpoint as `appEndpointDomain` | ||
| // (on both the deploy result and `services show`) instead of the older | ||
| // `serviceEndpointDomain`; fall back through the known shapes so the PR | ||
| // comment step always receives a URL. | ||
| const serviceUrl = | ||
| deployResult.serviceEndpointDomain ?? | ||
| deployResult.appEndpointDomain ?? | ||
| (await showService(service.id)).appEndpointDomain; | ||
|
|
||
| const result = { | ||
| branchName, | ||
| projectId: project.id, | ||
| region: project.defaultRegion ?? "eu-west-3", | ||
| serviceId: service.id, | ||
| serviceName, | ||
| serviceUrl: deployResult.serviceEndpointDomain, | ||
| serviceUrl, |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Add explicit validation when all endpoint fallbacks are exhausted.
If deployResult.serviceEndpointDomain, deployResult.appEndpointDomain, and the showService result are all nullish, serviceUrl silently becomes undefined. This flows into result and writeOutputs, and downstream compute-preview-comment.mjs only checks that PREVIEW_SERVICE_URL is set (via getRequiredEnv), not that it's a valid URL — risking a silently broken "Preview: undefined" comment posted to the PR instead of a clear CI failure.
🛡️ Proposed fix
const serviceUrl =
deployResult.serviceEndpointDomain ??
deployResult.appEndpointDomain ??
(await showService(service.id)).appEndpointDomain;
+
+ if (!serviceUrl) {
+ throw new Error(
+ "Unable to determine the preview service URL from Compute CLI output.",
+ );
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Newer Compute CLI payloads surface the endpoint as `appEndpointDomain` | |
| // (on both the deploy result and `services show`) instead of the older | |
| // `serviceEndpointDomain`; fall back through the known shapes so the PR | |
| // comment step always receives a URL. | |
| const serviceUrl = | |
| deployResult.serviceEndpointDomain ?? | |
| deployResult.appEndpointDomain ?? | |
| (await showService(service.id)).appEndpointDomain; | |
| const result = { | |
| branchName, | |
| projectId: project.id, | |
| region: project.defaultRegion ?? "eu-west-3", | |
| serviceId: service.id, | |
| serviceName, | |
| serviceUrl: deployResult.serviceEndpointDomain, | |
| serviceUrl, | |
| // Newer Compute CLI payloads surface the endpoint as `appEndpointDomain` | |
| // (on both the deploy result and `services show`) instead of the older | |
| // `serviceEndpointDomain`; fall back through the known shapes so the PR | |
| // comment step always receives a URL. | |
| const serviceUrl = | |
| deployResult.serviceEndpointDomain ?? | |
| deployResult.appEndpointDomain ?? | |
| (await showService(service.id)).appEndpointDomain; | |
| if (!serviceUrl) { | |
| throw new Error( | |
| "Unable to determine the preview service URL from Compute CLI output.", | |
| ); | |
| } | |
| const result = { | |
| branchName, | |
| projectId: project.id, | |
| region: project.defaultRegion ?? "eu-west-3", | |
| serviceId: service.id, | |
| serviceName, | |
| serviceUrl, |
🤖 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 `@scripts/compute-preview/compute-preview-deploy.mjs` around lines 44 - 59, The
serviceUrl fallback chain in compute-preview-deploy.mjs can still resolve to
undefined, which then propagates into the outputs and the preview comment flow.
Add explicit validation right after deriving serviceUrl (using
deployResult.serviceEndpointDomain, deployResult.appEndpointDomain, and
showService(service.id)) and fail fast with a clear error if none of those
values are present. Keep the check close to the existing serviceUrl/result
construction so writeOutputs and compute-preview-comment.mjs never receive an
unset preview URL.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
scripts/compute-preview/compute-preview-deploy.mjs (1)
44-59: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winFallback chain can still resolve to
undefinedwithout explicit failure.The
serviceUrlfallback chain remains unchanged from the previous review — ifdeployResult.serviceEndpointDomain,deployResult.appEndpointDomain, andshowService(service.id).appEndpointDomainare all nullish,serviceUrlsilently becomesundefined. The newwriteOutputsfilter (Line 156) now omits the key entirely rather than writingundefined, which will eventually surface as agetRequiredEnvfailure incompute-preview-comment.mjs, but that's an indirect, less actionable failure mode compared to throwing here with a clear message at the actual point of failure.🛡️ Proposed fix
const serviceUrl = deployResult.serviceEndpointDomain ?? deployResult.appEndpointDomain ?? (await showService(service.id)).appEndpointDomain; + + if (!serviceUrl) { + throw new Error( + "Unable to determine the preview service URL from Compute CLI output.", + ); + }🤖 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 `@scripts/compute-preview/compute-preview-deploy.mjs` around lines 44 - 59, The service URL resolution in compute-preview-deploy.mjs can still end up undefined, so add an explicit failure after the deployResult.serviceEndpointDomain/appEndpointDomain/showService(service.id).appEndpointDomain fallback chain in the deploy flow. Update the logic around serviceUrl and the result object so it throws a clear error at the point of resolution if no endpoint domain is available, using the existing deployResult, showService, and serviceUrl symbols to keep the failure actionable.
🤖 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 `@scripts/compute-preview/compute-preview-deploy.mjs`:
- Around line 44-59: The service URL resolution in compute-preview-deploy.mjs
can still end up undefined, so add an explicit failure after the
deployResult.serviceEndpointDomain/appEndpointDomain/showService(service.id).appEndpointDomain
fallback chain in the deploy flow. Update the logic around serviceUrl and the
result object so it throws a clear error at the point of resolution if no
endpoint domain is available, using the existing deployResult, showService, and
serviceUrl symbols to keep the failure actionable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d5138d10-11bc-43ef-9f96-d2cc65d94e57
📒 Files selected for processing (1)
scripts/compute-preview/compute-preview-deploy.mjs
sorenbs
added a commit
to prisma/prisma-next
that referenced
this pull request
Jul 6, 2026
…ation isDefaultValue() treats false and [] as omittable shape-defaults, but a column default literal payload is data: dropping it turned `Boolean @default(false)` schemas into contracts that failed their own structural validation on the next read (surfaced as PN-CLI-4003 when planning a migration). Preserve `value` whenever its parent key is `default`. Only previously-broken contracts change hashes: any contract carrying a literal false/[] default was unloadable before this fix, so no working contract identity moves. Found while generating the 20-migration demo history that seeds the Prisma Studio Migrations view (prisma/studio#1533). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Søren Bramer Schmidt <sorenbs@gmail.com>
prisma/prisma-next#908 moved per-migration contract snapshots off the ledger's contract_json_before/after columns into a 1:1 contract table (ledger_id PK/FK, after-state only). The ledger query now LEFT JOINs that table, and each migration's before-state is derived from its predecessor's snapshot per contract space, guarded by hash continuity (origin must equal the predecessor's destination) so a broken chain renders an unknown baseline instead of a wrong one. Databases without the contract table fall back to a join-less query via introspection detection. The demo seeder creates the new table, inserts ledger rows RETURNING id with one contract row per snapshot-carrying migration, and upgrades a legacy-seeded volume in place (backfill + drop of the old columns). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Each '⋯ N unchanged lines' separator in the Migrations schema panel is now a button carrying a discrete '· expand' hint; clicking it reveals the folded lines in place. diffSchemas carries the folded run's lines on the collapsed entry so expansion needs no re-diff, and the panel is keyed by migration id so folds reset when switching migrations. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ct data The Migrations navigation item now requires actual history, not just table presence: useHasMigrationHistory combines the introspection check with a one-row EXISTS probe, so a missing prisma_contract schema, a missing ledger table, or an empty ledger all hide the item (probing beats fetching the full ledger, whose snapshots can be megabytes of jsonb). When the ledger has rows but none joins to a contract snapshot (the contract table is missing or empty — a database written by an older prisma-next), the view keeps the migration list and SQL panel, hides the diff-dependent All models toggle and Schema button, and replaces the canvas with a notice asking to update Prisma Next. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
prisma/prisma-next#908 re-keyed prisma_contract.contract by storage hash (review feedback: a contract's identity is its hash, which the ledger already carries in origin_core_hash / destination_core_hash). The ledger query now LEFT JOINs the store twice — once per edge endpoint — so both before- and after-states are direct lookups and the client-side predecessor-chain derivation with its hash guard is deleted. An unresolved origin hash joins to nothing and renders as an unknown baseline, same as before. The demo seeder creates the hash-keyed store, upserts each after-state under its destination hash (ON CONFLICT DO NOTHING), and upgrades both legacy volume shapes in place (ledger_id-keyed store re-keyed by hash; two-column ledger backfilled and dropped). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
1 participant
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
A new Migrations view for Studio that turns the Prisma Next migration ledger into a visual story of your schema's history.
When the connected database carries
prisma_contract.ledger, a Migrations item appears in the navigation (databases without the ledger never see it). The view shows:+/−/~chips summarizing what each migration did to models, fields, enums, and indexes.before → afterdetail pills, enum cards, and relation edges. Untouched neighbors render dimmed for context, and an All models toggle expands to the migration's full schema (including unchanged enums).Design decisions
prisma_contract.ledgerthrough the standardAdapter.rawsurface (works on every executor) and never touches migration bundles on disk or the Prisma Next CLI. Detection is derived purely from introspection data — no probe query.contract_json_beforemay be null (synthesised applies); the parser fills it from the predecessor edge's after-snapshot, which is exact by the ledger's chain invariant.@pierre/diffsfor the schema diff: the latter hard-depends on shiki, which is too heavy for the published bundle. The renderer is isolated so it can be swapped.Architecture/migrations-view.md.Try it — live preview & demo video
Live preview (deployed from this PR): https://cmr485hrg05jfzvdiywd68wat.cdg.prisma.build — open Migrations in the left nav. The demo seeds the full 20-migration history.
Demo video (30s): migrations-view-demo-v2.mp4 — GitHub renders an inline player on that page. Committed on this branch alongside the feature.
Demo / PR preview
pnpm demo:ppgseeds a real 20-migration history (models, fields, enums, uniques, indexes, a destructive column drop, and a retired model), captured from the actual Prisma Nextemit → migration plan → migrateflow via the newexamples/migrations-showcasein the prisma-next repo (companion PR: prisma/prisma-next — "Persist per-migration contract snapshots into the ledger"). The fixture is statically imported so the Compute preview deploy inlines it — the PR preview seeds the full history; open Migrations in the nav to try the view.Companion PR
Requires nothing at runtime, but the data model comes from the prisma-next branch that populates
contract_json_before/afterin the ledger (see companion PR in prisma/prisma-next).Testing
pnpm build+check:exportspass with the newdiffdependency.build:deployartifact check confirming the fixture is inlined.🤖 Generated with Claude Code