TML-2727: migrate migration aggregate to elementCoordinates#629
Conversation
The granular S1.D ticket (TML-2625) was canceled and archived in the 2026-05-20 ticket cleanup; recreated as TML-2727 (the single live S1.D ticket). Also: the subsumed cleanup tickets (TML-2579/2580/2582/2545/ 2563) are already Canceled, so PDoD10 needs no close-out dispatch \u2014 mark it satisfied in the coverage map. Signed-off-by: Will Madden <madden@prisma.io>
…efer structural items A 2026-05-29 inventory falsified S1.D as one deletions-only slice: three of the eight subsumed surfaces carry structural prerequisites (a contract.json- shape coordinate change, a hash-computation change, a query-builder type rewrite). Per the narrow-and-defer decision, S1.D ships the three clean deletes now as independent parallel slices (construction-discipline shims, canonicalizer family hook, migration -> elementCoordinates) and defers the three structural items to follow-ups recorded in deferred.md. Amends PDoD5/PDoD10 to scope the deferred items out; restructures the plan composition into parallel groups (no stack); adds the three lean slice specs. Signed-off-by: Will Madden <madden@prisma.io>
Replace extractStorageElementNames with a migration-internal storageElementNames helper that delegates to elementCoordinates. Close the StorageBase/Storage gap at the helper boundary with a runtime namespace guard and blindCast — no exported type widening. Refs: TML-2727 Closes: TML-2580 Signed-off-by: Will Madden <madden@prisma.io>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR removes the ChangesStorage Namespace Topology Refactoring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate 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/extension-cipherstash
@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: |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/migration/src/aggregate/storage-element-names.ts`:
- Line 16: The code uses unsafe "as" casts for (contract as Contract).storage
and (storage as { readonly namespaces?: unknown }).namespaces; replace these
with proper type-narrowing: add or use a type guard (e.g., isContract or
predicate) to assert contract is a Contract before accessing storage, and
check/storage-narrow the storage shape (e.g., typeof/Array/isObject checks or an
isStorageWithNamespaces predicate) before reading namespaces so you can avoid
casting; update the references around the storage variable and the namespaces
access to use the guard-validated types instead of bare "as" casts.
- Around line 27-33: The type guard hasNamespaceMap currently only verifies that
storage.namespaces is a non-null non-array object, but elementCoordinates uses
Object.entries(ns) and will crash if any namespace value is null; update
hasNamespaceMap to also validate that every value in (storage as { readonly
namespaces?: unknown }).namespaces is itself a non-null object and not an array
(e.g., iterate Object.values(namespaces) and ensure each value passes typeof ===
'object' && value !== null && !Array.isArray(value)) so the guard guarantees
safe iteration in elementCoordinates and related code paths.
🪄 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: f1203d71-e757-4d62-bf8b-1448f1c3dc5b
⛔ Files ignored due to path filters (6)
projects/contract-ir-planes/deferred.mdis excluded by!projects/**projects/contract-ir-planes/plan.mdis excluded by!projects/**projects/contract-ir-planes/slices/canonicalizer-family-hook/spec.mdis excluded by!projects/**projects/contract-ir-planes/slices/construction-discipline-shims/spec.mdis excluded by!projects/**projects/contract-ir-planes/slices/migration-element-coordinates/spec.mdis excluded by!projects/**projects/contract-ir-planes/spec.mdis excluded by!projects/**
📒 Files selected for processing (6)
packages/1-framework/3-tooling/migration/src/aggregate/extract-storage-element-names.tspackages/1-framework/3-tooling/migration/src/aggregate/loader.tspackages/1-framework/3-tooling/migration/src/aggregate/project-schema-to-space.tspackages/1-framework/3-tooling/migration/src/aggregate/storage-element-names.tspackages/1-framework/3-tooling/migration/src/aggregate/verifier.tspackages/1-framework/3-tooling/migration/test/aggregate/project-schema-to-space.test.ts
💤 Files with no reviewable changes (1)
- packages/1-framework/3-tooling/migration/src/aggregate/extract-storage-element-names.ts
The lint:no-contract-cast guardrail flags `as Contract` as a serializer-seam bypass. Type the helper parameter as Contract (all callers pass member.contract) and narrow storage to Storage via an in-narrowing type predicate, removing both the as Contract cast and the blindCast at the walk boundary. Zero casts; behaviour unchanged. Refs: TML-2727 Signed-off-by: Will Madden <madden@prisma.io>
elementCoordinates walks each namespace via Object.entries(ns), which throws on a null value. The deleted name-only helper tolerated such malformed input by skipping it. Restore that no-throw guarantee: hasNamespaceMap now requires every namespace entry to be a non-null object. Add a regression case to the malformed-storage projection test. Refs: TML-2727 Signed-off-by: Will Madden <madden@prisma.io>
size-limit report 📦
|
…ype-bridge Reviewer questioned the defensiveness in storage-element-names.ts. Record the two actions: drop the per-entry null sweep (impossible input for a typed Contract; elementCoordinates already skips null slots) and reframe the guard as a foundation/core layering type-bridge rather than malformed-input defence. Signed-off-by: Will Madden <madden@prisma.io>
Resolve add/add conflicts in contract-ir-planes slice specs by keeping this branch's migration-element-coordinates Round 2 review section and main's construction-discipline-shims Round 2 section. Signed-off-by: wmadden-electric <286902546+wmadden-electric@users.noreply.github.com>
Drop the per-entry null sweep and reframe the guard as a foundation/ core layering type-bridge. Remove the impossible-input test case for null namespace values. Signed-off-by: wmadden-electric <286902546+wmadden-electric@users.noreply.github.com>
Operator pulled the structural fix into the slice: the duck-typing bridge is a known layering violation (foundation lacks a typed primitive for storage topology). Supersede the Round-2 bridge with a topology lift — StorageBase carries a plain namespaces shape, core Storage/Namespace refine it, the migration walk goes through contract.storage with no narrowing. Decompose into D1 (substrate, green workspace) and D2 (delete the duck-typing). Signed-off-by: Will Madden <madden@prisma.io>
Declare plain-data StorageTopology on StorageBase so hydrated contracts statically carry namespaces; core Storage/Namespace refine the shape and elementCoordinates walks StorageTopology without casts. Align test/fixture construction sites with required namespaces; migration duck-typing unchanged. Signed-off-by: Will Madden <madden@prisma.io>
Take main's tolerant aggregate loader and member.contract() API; keep D1 storageElementNames bridge in check-integrity, verifier, and project-schema-to-space until D2 removes duck-typing. Signed-off-by: Will Madden <madden@prisma.io>
…ft breaks downstream source The StorageBase topology lift is a public-type change; if keeping the workspace green forces edits to examples/**/src or packages/3-extensions/**/src, that is a consumer-facing breaking change that needs a record-upgrade-instructions entry. Frames it as a conditional done-condition + D1 watch note, and is explicit that framework/family-only fan-out needs no entry (no no-op changes:[] placeholders). Signed-off-by: Will Madden <madden@prisma.io>
Delete the StorageBase→Storage bridge and call elementCoordinates on contract.storage directly in aggregate disjointness, verifier, and schema projection. Output-preserving for hydrated contracts. Signed-off-by: Will Madden <madden@prisma.io>
…n-coordinates Signed-off-by: Will Madden <madden@prisma.io>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/1-framework/3-tooling/migration/src/aggregate/check-integrity.ts (1)
219-223:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDedup
entityNameper member when buildingelementClaimedByto avoid spuriousdisjointnessviolations
contractViolationsiterateselementCoordinates(contract.storage), which yields one coordinate per(namespaceId, entityKind, entityName), butelementClaimedByis keyed only byentityNameand blindlypushesmember.spaceIdfor every coordinate. If a single member’s storage contains the sameentityNamein multiple namespaces / entityKinds,claimedBy.lengthbecomes> 1even though only one spaceId should be counted, producing a false-positivedisjointnessviolation.This is a behavior change from the removed
extractStorageElementNameshelper, which returned a deduplicatedSet<string>of names.🐛 Proposed fix: collapse names per member before recording claimers
- for (const { entityName: elementName } of elementCoordinates(contract.storage)) { - const claimers = elementClaimedBy.get(elementName); - if (claimers) claimers.push(member.spaceId); - else elementClaimedBy.set(elementName, [member.spaceId]); - } + const memberElementNames = new Set<string>(); + for (const { entityName } of elementCoordinates(contract.storage)) { + memberElementNames.add(entityName); + } + for (const elementName of memberElementNames) { + const claimers = elementClaimedBy.get(elementName); + if (claimers) claimers.push(member.spaceId); + else elementClaimedBy.set(elementName, [member.spaceId]); + }🤖 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/src/aggregate/check-integrity.ts` around lines 219 - 223, The code that builds elementClaimedBy inside the loop over elementCoordinates(contract.storage) pushes member.spaceId for every coordinate, causing duplicate claims when the same entityName appears multiple times per member; change the logic in the block that iterates elementCoordinates(contract.storage) (used by contractViolations) to first deduplicate entityName values per member (e.g., collect entityName into a Set for this member or track seen names) and only record member.spaceId once per unique entityName when updating elementClaimedBy, preserving the map key (entityName) and values (array of spaceIds) semantics from the removed extractStorageElementNames helper.
🤖 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/migration/src/aggregate/check-integrity.ts`:
- Around line 219-223: The code that builds elementClaimedBy inside the loop
over elementCoordinates(contract.storage) pushes member.spaceId for every
coordinate, causing duplicate claims when the same entityName appears multiple
times per member; change the logic in the block that iterates
elementCoordinates(contract.storage) (used by contractViolations) to first
deduplicate entityName values per member (e.g., collect entityName into a Set
for this member or track seen names) and only record member.spaceId once per
unique entityName when updating elementClaimedBy, preserving the map key
(entityName) and values (array of spaceIds) semantics from the removed
extractStorageElementNames helper.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 660fe619-c0cb-46b4-bc82-7262dc309955
⛔ Files ignored due to path filters (1)
projects/contract-ir-planes/slices/migration-element-coordinates/spec.mdis excluded by!projects/**
📒 Files selected for processing (4)
packages/1-framework/3-tooling/migration/src/aggregate/check-integrity.tspackages/1-framework/3-tooling/migration/src/aggregate/project-schema-to-space.tspackages/1-framework/3-tooling/migration/src/aggregate/verifier.tspackages/1-framework/3-tooling/migration/test/aggregate/project-schema-to-space.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/1-framework/3-tooling/migration/test/aggregate/project-schema-to-space.test.ts
Removing the hasNamespaceMap guard makes elementCoordinates require
storage.namespaces. CLI test mocks built minimal storage without it;
add namespaces: {} so they model a hydrated contract honestly and
yield zero coordinates (behaviour-preserving).
Signed-off-by: Will Madden <madden@prisma.io>
…torage types Topology is an emergent property of these shapes, not the shapes themselves. Rename StorageNamespaceTopology -> StorageNamespace and fold StorageTopology's namespaces member into StorageBase directly. elementCoordinates accepts Pick<StorageBase, 'namespaces'>. Pure type-only rename; no envelope movement. Signed-off-by: Will Madden <madden@prisma.io>
…istory Replace the three synthetic normal-shape golden cases with cases drawn from real merged PRs, so the corpus measures Drive runs against work the team actually shipped rather than synthesised tasks: - direct-change-example-emit-outputpath (TML-2722 / #618) - slice-dedupe-generated-imports (TML-2714 / #614) - project-reap-subsumed-ir-surfaces (TML-2727 / #630, #631, #629) — a three-slice parallel fan-out that exercises planner parallelisation and scope discipline. Each real case carries the task as posed (Linear ticket, solution-scrubbed so the run still does the design/planning), a base_sha to run against, and a reference.md describing the known-good output by commit SHA (the output itself is fetchable via git diff <base_sha> <merge_sha>). case.json gains source + base_sha; the loader ignores the extra fields until the experiment-engine slice wires base_sha into a checkout. The two pathological cases (i12-halt, spike-first) stay synthetic: no clean merged PR exhibits a halted or spiked run. Update harness tests, SKILL.md examples, and the corpus README for the renamed slugs. validate-parser fixtures are left as-is — they are synthetic parser fixtures with tuned event counts, not corpus members. Signed-off-by: wmadden-electric <286902546+wmadden-electric@users.noreply.github.com>
Linked issue
Refs TML-2727 — closes TML-2580.
Parallel to S1.D-1/-2 + S1.E — disjoint files.
Summary
The migration aggregate still walked storage entities via the name-only
extractStorageElementNamesduck-typing helper. This PR completes PDoD6's migration consumer by routing every caller throughelementCoordinates(storage)via a migration-internalstorageElementNameswrapper. Output-preserving: no on-disk contract shape change, no migration-behaviour change.Spec:
projects/contract-ir-planes/slices/migration-element-coordinates/spec.mdAt a glance
Call sites in
loader.ts,verifier.ts, andproject-schema-to-space.tsnow importstorageElementNamesinstead of the deleted helper.Type-gap resolution
Contract.storageis typed asStorageBase(hash only);elementCoordinatesexpects frameworkStorage. Closed inside the migration package with a runtimehasNamespaceMapguard plus a singleblindCastat the helper boundary — no exported public type was widened.Grep gate
Testing performed
pnpm --filter @prisma-next/framework-components build && pnpm --filter @prisma-next/migration-tools buildpnpm typecheckpnpm --filter @prisma-next/migration-tools testpnpm test:packages@prisma-next/migration-toolsfailures)pnpm test:integrationpnpm fixtures:checkpnpm lint:depsSkill update
n/a — internal only.
Checklist
git commit -s).memberWithCollectionsnow uses namespace-scoped storage, matching on-disk shape).TML-2727:prefix.Notes for the reviewer
Refusal trigger did not fire — the
StorageBase/Storagegap closed without loosening any exported type beyond the migration package.Summary by CodeRabbit
Refactor
New Features
StorageEntitySlot,StorageNamespaceTopology, andStorageTopologyfor enhanced storage schema type coverage.