fix(mongo): suppress variants, fix validators, add Float#350
Merged
wmadden merged 37 commits intoApr 17, 2026
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:
📝 WalkthroughWalkthroughThis PR centralizes contract-generation metadata into a ControlStack, enriches ContractSourceContext with pslScalarTypeDescriptors/codecLookup/authoringContributions/controlMutationDefaults, updates providers/tests to use stack-derived context, and extends Mongo schema derivation to resolve BSON types via codecLookup and emit polymorphic Changes
Sequence Diagram(s)sequenceDiagram
participant App as CLI / Emitter
participant Stack as ControlStack
participant Ctx as ContractSourceContext
participant Provider as ContractSourceProvider
participant Interpreter as PSL Interpreter
App->>Stack: createControlStack({ family, target, adapter, extensionPacks })
Stack-->>App: assembled stack (pslScalarTypeDescriptors, codecLookup, authoringContributions, controlMutationDefaults, extensionPack ids)
App->>Ctx: build context from stack
App->>Provider: provider.source(Ctx)
Provider->>Interpreter: interpretPsl(document, { codecLookup: Ctx.codecLookup, scalarDescriptors: Ctx.pslScalarTypeDescriptors, ... })
Interpreter->>Interpreter: resolve BSON types via codecLookup
Interpreter->>Interpreter: derive polymorphic `oneOf` validators and merge variant indexes
Interpreter-->>Provider: contract with validators/storage
Provider-->>App: contract
sequenceDiagram
participant PSL as PSL Document
participant Interp as Mongo Interpreter
participant Codec as CodecLookup
participant Schema as JsonSchema Deriver
PSL->>Interp: parse models & polymorphism metadata
Interp->>Schema: derive base schemas (fields, types)
Schema->>Codec: request codec metadata for each field
Codec-->>Schema: codec.targetTypes[0] -> bsonType
Schema-->>Interp: field-level bsonType properties
Interp->>Schema: derivePolymorphicJsonSchema(base, discriminator, variants)
Schema->>Schema: build `oneOf` entries with discriminator enums and variant-only properties
Schema-->>Interp: final validator with `oneOf`
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
@prisma-next/mongo-runtime
@prisma-next/family-mongo
@prisma-next/sql-runtime
@prisma-next/family-sql
@prisma-next/middleware-telemetry
@prisma-next/mongo
@prisma-next/extension-paradedb
@prisma-next/extension-pgvector
@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/contract-authoring
@prisma-next/ids
@prisma-next/psl-parser
@prisma-next/psl-printer
@prisma-next/cli
@prisma-next/emitter
@prisma-next/migration-tools
@prisma-next/vite-plugin-contract-emit
@prisma-next/runtime-executor
@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-pipeline-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: |
![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: 3
🧹 Nitpick comments (5)
packages/1-framework/3-tooling/cli/src/control-api/client.ts (1)
494-499: Use a defensive copy forcomposedExtensionPacks.
stack.extensionIdsis passed by reference; a mutating source provider could leak state across calls. Prefer copying before handing it tosourceProvider.♻️ Suggested tweak
const sourceContext = { - composedExtensionPacks: stack.extensionIds, + composedExtensionPacks: [...stack.extensionIds], pslScalarTypeDescriptors: stack.pslScalarTypeDescriptors, authoringContributions: stack.authoringContributions, codecLookup: stack.codecLookup, controlMutationDefaults: stack.controlMutationDefaults, };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/1-framework/3-tooling/cli/src/control-api/client.ts` around lines 494 - 499, The object passed to sourceProvider should not expose stack.extensionIds by reference; create a defensive copy for composedExtensionPacks instead of assigning stack.extensionIds directly. Update the code that builds the payload (the object containing composedExtensionPacks, pslScalarTypeDescriptors, authoringContributions, codecLookup, controlMutationDefaults) so composedExtensionPacks is set to a shallow copy of stack.extensionIds (e.g., via array copy) before calling sourceProvider, ensuring mutations by the provider cannot leak back into stack.extensionIds.test/integration/test/authoring/cli.emit-parity-fixtures.test.ts (1)
22-27: Keep stack construction fully aligned with CLI by includingdriver.
sourceContextFromConfig()is modeling CLI behavior; passingdriverintocreateControlStackwill keep this helper faithful if driver-level contributions are present.Proposed tweak
const stack = createControlStack({ family: config.family, target: config.target, adapter: config.adapter, + driver: config.driver, extensionPacks: config.extensionPacks ?? [], });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/integration/test/authoring/cli.emit-parity-fixtures.test.ts` around lines 22 - 27, The test constructs createControlStack without the CLI-equivalent driver, so sourceContextFromConfig() diverges from real CLI behavior; update the call to createControlStack to pass the driver from the test config (i.e., include config.driver) so createControlStack(...) receives driver alongside family/target/adapter/extensionPacks, keeping behavior aligned with sourceContextFromConfig and driver-level contributions.packages/3-mongo-target/2-mongo-adapter/test/mongo-planner.test.ts (1)
1043-1089: Please split this test into a dedicated polymorphism test file.This file is already very large; adding FL-09 coverage here increases maintenance cost. Moving this block to something like
mongo-planner.polymorphism.test.tswould keep concerns isolated and navigable.As per coding guidelines:
**/*.test.ts: Keep test files under 500 lines to maintain readability and navigability. Split test files when they exceed this limit or contain distinct concerns.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/3-mongo-target/2-mongo-adapter/test/mongo-planner.test.ts` around lines 1043 - 1089, Extract the entire describe block "polymorphic collections (FL-09)" (the it block that asserts createCollection only for 'tasks' and references planSuccess, planner, makeContract, emptyIR, MongoMigrationPlanOperation and CreateCollectionCommand) into a new dedicated test file (e.g., a polymorphism-focused test file) so the original test file stays under the 500-line guideline; in the new file import the same helpers (planner, planSuccess, makeContract, emptyIR) and types, keep the test logic and assertions unchanged, and run the suite to ensure imports and test names remain correct.test/integration/test/authoring/parity/ts-psl-parity.real-packs.test.ts (1)
26-34: Consider explicit handling when codec is not found.Line 30 falls back to
codecIdwhencodecis undefined or has notargetTypes. While this prevents runtime errors, usingcodecIdasnativeTypemay mask configuration issues where a codec is missing from the stack.For test code this is likely acceptable, but consider whether a missing codec should throw or log a warning to surface misconfiguration earlier.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/integration/test/authoring/parity/ts-psl-parity.real-packs.test.ts` around lines 26 - 34, buildColumnDescriptorMap currently falls back to using codecId as nativeType when stack.codecLookup.get(codecId) returns undefined, which can hide missing codec configuration; update buildColumnDescriptorMap to explicitly handle a missing codec by throwing or logging a clear error instead of silently using codecId — locate buildColumnDescriptorMap and the loop over stack.pslScalarTypeDescriptors, check stack.codecLookup.get(codecId) and if undefined throw an Error (or call test logger) that includes codecId and typeName, otherwise continue to use codec.targetTypes[0] for nativeType.packages/1-framework/1-core/framework-components/src/mutation-default-types.ts (1)
54-89: Collapse the duplicated registry entry types.
DefaultFunctionRegistryEntry/DefaultFunctionRegistryandControlMutationDefaultEntry/ControlMutationDefaultRegistryare identical today. Exporting both gives downstream packages two names for the same API and makes the new control surface harder to use consistently.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/1-framework/1-core/framework-components/src/mutation-default-types.ts` around lines 54 - 89, The file defines two identical pairs DefaultFunctionRegistryEntry/DefaultFunctionRegistry and ControlMutationDefaultEntry/ControlMutationDefaultRegistry; collapse them into a single canonical pair (pick one set of names, e.g., DefaultFunctionRegistryEntry and DefaultFunctionRegistry) and have the other pair be removed or turned into type aliases referencing the canonical types; update ControlMutationDefaults to reference the unified DefaultFunctionRegistry type and replace any usages/imports of ControlMutationDefaultEntry/ControlMutationDefaultRegistry elsewhere in the codebase to use the chosen canonical names (or the new aliases) so there are not two different names for the same API.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/2-mongo-family/2-authoring/contract-psl/src/interpreter.ts`:
- Around line 870-883: The discriminator passed to derivePolymorphicJsonSchema
must be the storage-mapped field name, not the PSL name; locate the mapped key
by scanning modelEntry.fields for the field whose PSL identifier matches
modelEntry.discriminator.field (e.g. check a pslName/name property on each
field) and use that mapped key when calling derivePolymorphicJsonSchema instead
of modelEntry.discriminator.field, falling back to the original value if no
mapped entry is found; update the call site around modelEntry, variantEntries,
valueObjects, codecLookup to pass the mapped discriminator name.
- Around line 279-295: The current logic only merges variantIndexes into
baseCollection when variantCollectionName !== baseCollection, so when a variant
maps to the same collection name the earlier assignment overwrote the base and
indexes get lost; change the block that handles merging to always append
variantIndexes into the base collection's indexes regardless of equality:
compute variantIndexes from collections[variantCollectionName] and baseIndexes
from collections[baseCollection] (fall back to empty arrays if missing), then
set collections[baseCollection] = { ...baseColl, indexes: [...baseIndexes,
...variantIndexes] } (and avoid removing the base entry when names are equal).
Update the handling around filtered/assignment so you do not drop the base when
variantCollectionName === baseCollection and ensure symbols referenced are
variantCollectionName, baseCollection, collections, variantIndexes, and
baseIndexes.
In `@packages/2-sql/2-authoring/contract-psl/src/provider.ts`:
- Around line 18-29: The buildColumnDescriptorMap currently uses
codec?.targetTypes[0] ?? codecId which hides missing codec lookups; change
buildColumnDescriptorMap to fail fast when codecLookup.get(codecId) returns
undefined by throwing a descriptive error (including codecId and typeName)
instead of falling back to codecId for nativeType, so that unresolved
pslScalarTypeDescriptors surface as diagnostics during source loading; reference
symbols: buildColumnDescriptorMap, scalarTypeDescriptors, codecLookup, codec,
codecId, nativeType.
---
Nitpick comments:
In
`@packages/1-framework/1-core/framework-components/src/mutation-default-types.ts`:
- Around line 54-89: The file defines two identical pairs
DefaultFunctionRegistryEntry/DefaultFunctionRegistry and
ControlMutationDefaultEntry/ControlMutationDefaultRegistry; collapse them into a
single canonical pair (pick one set of names, e.g., DefaultFunctionRegistryEntry
and DefaultFunctionRegistry) and have the other pair be removed or turned into
type aliases referencing the canonical types; update ControlMutationDefaults to
reference the unified DefaultFunctionRegistry type and replace any
usages/imports of ControlMutationDefaultEntry/ControlMutationDefaultRegistry
elsewhere in the codebase to use the chosen canonical names (or the new aliases)
so there are not two different names for the same API.
In `@packages/1-framework/3-tooling/cli/src/control-api/client.ts`:
- Around line 494-499: The object passed to sourceProvider should not expose
stack.extensionIds by reference; create a defensive copy for
composedExtensionPacks instead of assigning stack.extensionIds directly. Update
the code that builds the payload (the object containing composedExtensionPacks,
pslScalarTypeDescriptors, authoringContributions, codecLookup,
controlMutationDefaults) so composedExtensionPacks is set to a shallow copy of
stack.extensionIds (e.g., via array copy) before calling sourceProvider,
ensuring mutations by the provider cannot leak back into stack.extensionIds.
In `@packages/3-mongo-target/2-mongo-adapter/test/mongo-planner.test.ts`:
- Around line 1043-1089: Extract the entire describe block "polymorphic
collections (FL-09)" (the it block that asserts createCollection only for
'tasks' and references planSuccess, planner, makeContract, emptyIR,
MongoMigrationPlanOperation and CreateCollectionCommand) into a new dedicated
test file (e.g., a polymorphism-focused test file) so the original test file
stays under the 500-line guideline; in the new file import the same helpers
(planner, planSuccess, makeContract, emptyIR) and types, keep the test logic and
assertions unchanged, and run the suite to ensure imports and test names remain
correct.
In `@test/integration/test/authoring/cli.emit-parity-fixtures.test.ts`:
- Around line 22-27: The test constructs createControlStack without the
CLI-equivalent driver, so sourceContextFromConfig() diverges from real CLI
behavior; update the call to createControlStack to pass the driver from the test
config (i.e., include config.driver) so createControlStack(...) receives driver
alongside family/target/adapter/extensionPacks, keeping behavior aligned with
sourceContextFromConfig and driver-level contributions.
In `@test/integration/test/authoring/parity/ts-psl-parity.real-packs.test.ts`:
- Around line 26-34: buildColumnDescriptorMap currently falls back to using
codecId as nativeType when stack.codecLookup.get(codecId) returns undefined,
which can hide missing codec configuration; update buildColumnDescriptorMap to
explicitly handle a missing codec by throwing or logging a clear error instead
of silently using codecId — locate buildColumnDescriptorMap and the loop over
stack.pslScalarTypeDescriptors, check stack.codecLookup.get(codecId) and if
undefined throw an Error (or call test logger) that includes codecId and
typeName, otherwise continue to use codec.targetTypes[0] for nativeType.
🪄 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: 30495bcb-bd94-4192-9315-19027edde910
⛔ Files ignored due to path filters (3)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlprojects/migration-planner-bugs/plan.mdis excluded by!projects/**projects/migration-planner-bugs/spec.mdis excluded by!projects/**
📒 Files selected for processing (61)
docs/architecture docs/subsystems/10. MongoDB Family.mdexamples/prisma-next-demo/prisma-next.config.tsexamples/retail-store/prisma-next.config.tspackages/1-framework/1-core/config/src/contract-source-types.tspackages/1-framework/1-core/framework-components/src/control-stack.tspackages/1-framework/1-core/framework-components/src/exports/control.tspackages/1-framework/1-core/framework-components/src/framework-components.tspackages/1-framework/1-core/framework-components/src/mutation-default-types.tspackages/1-framework/1-core/framework-components/test/control-stack.test.tspackages/1-framework/3-tooling/cli/src/control-api/client.tspackages/1-framework/3-tooling/cli/src/control-api/operations/contract-emit.tspackages/1-framework/3-tooling/cli/test/config-types.test.tspackages/1-framework/3-tooling/cli/test/control-api/contract-emit.test.tspackages/2-mongo-family/2-authoring/contract-psl/package.jsonpackages/2-mongo-family/2-authoring/contract-psl/src/derive-json-schema.tspackages/2-mongo-family/2-authoring/contract-psl/src/exports/index.tspackages/2-mongo-family/2-authoring/contract-psl/src/interpreter.tspackages/2-mongo-family/2-authoring/contract-psl/src/provider.tspackages/2-mongo-family/2-authoring/contract-psl/src/scalar-type-descriptors.tspackages/2-mongo-family/2-authoring/contract-psl/test/derive-json-schema.test.tspackages/2-mongo-family/2-authoring/contract-psl/test/interpreter.polymorphism.test.tspackages/2-mongo-family/2-authoring/contract-psl/test/interpreter.test.tspackages/2-mongo-family/2-authoring/contract-ts/test/config-types.test.tspackages/2-sql/2-authoring/contract-psl/src/default-function-registry.tspackages/2-sql/2-authoring/contract-psl/src/exports/index.tspackages/2-sql/2-authoring/contract-psl/src/interpreter.tspackages/2-sql/2-authoring/contract-psl/src/provider.tspackages/2-sql/2-authoring/contract-psl/src/psl-column-resolution.tspackages/2-sql/2-authoring/contract-psl/src/psl-field-resolution.tspackages/2-sql/2-authoring/contract-psl/test/composed-mutation-defaults.test.tspackages/2-sql/2-authoring/contract-psl/test/default-function-registry.test.tspackages/2-sql/2-authoring/contract-psl/test/fixtures.tspackages/2-sql/2-authoring/contract-psl/test/provider.test.tspackages/2-sql/2-authoring/contract-ts/test/config-types.test.tspackages/2-sql/9-family/src/core/assembly.tspackages/2-sql/9-family/src/core/control-instance.tspackages/2-sql/9-family/src/core/migrations/types.tspackages/2-sql/9-family/src/exports/control.tspackages/2-sql/9-family/test/mutation-default-assembly.test.tspackages/3-mongo-target/2-mongo-adapter/src/core/codecs.tspackages/3-mongo-target/2-mongo-adapter/src/exports/control.tspackages/3-mongo-target/2-mongo-adapter/test/mongo-planner.test.tspackages/3-targets/6-adapters/postgres/src/core/control-mutation-defaults.tspackages/3-targets/6-adapters/postgres/src/exports/control.tspackages/3-targets/6-adapters/postgres/test/control-mutation-defaults.test.tstest/integration/test/authoring/cli.emit-parity-fixtures.test.tstest/integration/test/authoring/parity/default-pack-slugid/packs.tstest/integration/test/authoring/parity/ts-psl-parity.real-packs.test.tstest/integration/test/authoring/psl.pgvector-dbinit.test.tstest/integration/test/authoring/side-by-side-contracts.test.tstest/integration/test/authoring/templates/prisma-next.config.parity-psl.tstest/integration/test/cli.emit-command.test.tstest/integration/test/cli.emit-contract.test.tstest/integration/test/cli.emit.test.tstest/integration/test/contract-imports.test.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/cli-journeys/prisma-next.config.with-db.psl.tstest/integration/test/fixtures/cli/cli-integration-test-app/fixtures/emit-command/prisma-next.config.parity-psl.tstest/integration/test/mongo/migration-psl-authoring.test.tstest/integration/test/pgvector.test.tstest/integration/test/value-objects/value-objects.integration.test.tstest/integration/utils/framework-components.ts
💤 Files with no reviewable changes (5)
- packages/2-sql/9-family/src/core/control-instance.ts
- examples/retail-store/prisma-next.config.ts
- packages/2-mongo-family/2-authoring/contract-psl/src/exports/index.ts
- test/integration/utils/framework-components.ts
- packages/2-mongo-family/2-authoring/contract-psl/src/scalar-type-descriptors.ts
wmadden
force-pushed
the
tml-2247-migration-planner-bugs-variant-collections-and-float-type-fl
branch
from
ae1276a to
7bf0ff9
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/2-sql/9-family/src/core/assembly.ts (1)
29-35: Minor: Redundant type check beforehasCodecControlHooks.The
typeof descriptor !== 'object' || descriptor === nullcheck on lines 30-32 is already performed insidehasCodecControlHooks(lines 15-17). The outer check is harmless but redundant.♻️ Optional simplification
for (const descriptor of descriptors) { - if (typeof descriptor !== 'object' || descriptor === null) { - continue; - } if (!hasCodecControlHooks(descriptor)) { continue; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-sql/9-family/src/core/assembly.ts` around lines 29 - 35, The loop over descriptors performs a redundant type/null check before calling hasCodecControlHooks(descriptor); remove the outer guard (the typeof descriptor !== 'object' || descriptor === null check) so the code simply iterates descriptors and calls hasCodecControlHooks(descriptor) to decide continuation, relying on hasCodecControlHooks to handle type/null validation.packages/2-mongo-family/2-authoring/contract-psl/src/derive-json-schema.ts (2)
106-110: ConsiderObject.hasOwn()for own-property check.The
inoperator checks the prototype chain, which is unnecessary forRecord<string, ContractField>. UsingObject.hasOwn()is more explicit and aligns with ES2022 conventions.♻️ Suggested change
for (const [name, field] of Object.entries(variant.fields)) { - if (!(name in baseFields)) { + if (!Object.hasOwn(baseFields, name)) { variantOnlyFields[name] = field; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-mongo-family/2-authoring/contract-psl/src/derive-json-schema.ts` around lines 106 - 110, Replace the prototype-checking "name in baseFields" with an own-property check using Object.hasOwn(baseFields, name); specifically, in the loop that iterates Object.entries(variant.fields) and assigns to variantOnlyFields when a field is not present in baseFields, change the condition to use Object.hasOwn(baseFields, name) (or Object.prototype.hasOwnProperty.call(baseFields, name) if you need older runtime support) so the check only considers baseFields' own properties; keep the rest of the loop logic intact.
132-137: Optional: Extract properties to avoid repeated casts.The
entry['properties']cast pattern is a bit awkward. A local variable would be cleaner.♻️ Suggested refactor
+ const entryProperties: Record<string, unknown> = { + [discriminatorField]: { enum: [variant.discriminatorValue] }, + }; const entry: Record<string, unknown> = { - properties: { - [discriminatorField]: { enum: [variant.discriminatorValue] }, - }, + properties: entryProperties, }; const variantProperties: Record<string, unknown> = {}; // ... building variantProperties ... if (Object.keys(variantProperties).length > 0) { - (entry['properties'] as Record<string, unknown>) = { - ...(entry['properties'] as Record<string, unknown>), - ...variantProperties, - }; + Object.assign(entryProperties, variantProperties); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-mongo-family/2-authoring/contract-psl/src/derive-json-schema.ts` around lines 132 - 137, Replace the repeated casts around entry['properties'] by extracting it to a local variable (e.g., let properties = (entry['properties'] as Record<string, unknown>) ?? {}), merge variantProperties into that local properties object, then assign the merged object back to entry['properties']; update the block that currently reads entry['properties'] and variantProperties so you only cast once and perform a single assignment to entry['properties'] to keep types cleaner and avoid repeated casts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/2-sql/9-family/README.md`:
- Line 23: Update the responsibility bullet that currently states that
`@prisma-next/family-sql` "Assembles authoring contributions" to reflect that it
consumes framework-assembled contributions: change the wording to say that the
SQL family accepts/uses the authoringContributions produced by
createControlStack() at the framework layer (e.g., "Consumes framework-assembled
authoringContributions (type constructors, field presets) for PSL
interpretation"), and remove any phrasing that implies assembly responsibility
for `@prisma-next/family-sql`.
---
Nitpick comments:
In `@packages/2-mongo-family/2-authoring/contract-psl/src/derive-json-schema.ts`:
- Around line 106-110: Replace the prototype-checking "name in baseFields" with
an own-property check using Object.hasOwn(baseFields, name); specifically, in
the loop that iterates Object.entries(variant.fields) and assigns to
variantOnlyFields when a field is not present in baseFields, change the
condition to use Object.hasOwn(baseFields, name) (or
Object.prototype.hasOwnProperty.call(baseFields, name) if you need older runtime
support) so the check only considers baseFields' own properties; keep the rest
of the loop logic intact.
- Around line 132-137: Replace the repeated casts around entry['properties'] by
extracting it to a local variable (e.g., let properties = (entry['properties']
as Record<string, unknown>) ?? {}), merge variantProperties into that local
properties object, then assign the merged object back to entry['properties'];
update the block that currently reads entry['properties'] and variantProperties
so you only cast once and perform a single assignment to entry['properties'] to
keep types cleaner and avoid repeated casts.
In `@packages/2-sql/9-family/src/core/assembly.ts`:
- Around line 29-35: The loop over descriptors performs a redundant type/null
check before calling hasCodecControlHooks(descriptor); remove the outer guard
(the typeof descriptor !== 'object' || descriptor === null check) so the code
simply iterates descriptors and calls hasCodecControlHooks(descriptor) to decide
continuation, relying on hasCodecControlHooks to handle type/null validation.
🪄 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: a8e8052c-26a4-4b39-8d13-cb6430ca44ad
⛔ Files ignored due to path filters (3)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlprojects/migration-planner-bugs/plan.mdis excluded by!projects/**projects/migration-planner-bugs/spec.mdis excluded by!projects/**
📒 Files selected for processing (65)
docs/architecture docs/subsystems/10. MongoDB Family.mdexamples/prisma-next-demo/prisma-next.config.tsexamples/retail-store/prisma-next.config.tspackages/1-framework/1-core/config/src/contract-source-types.tspackages/1-framework/1-core/framework-components/src/control-stack.tspackages/1-framework/1-core/framework-components/src/exports/control.tspackages/1-framework/1-core/framework-components/src/framework-components.tspackages/1-framework/1-core/framework-components/src/mutation-default-types.tspackages/1-framework/1-core/framework-components/test/control-stack.test.tspackages/1-framework/3-tooling/cli/src/control-api/client.tspackages/1-framework/3-tooling/cli/src/control-api/operations/contract-emit.tspackages/1-framework/3-tooling/cli/test/config-types.test.tspackages/1-framework/3-tooling/cli/test/control-api/contract-emit.test.tspackages/2-mongo-family/2-authoring/contract-psl/package.jsonpackages/2-mongo-family/2-authoring/contract-psl/src/derive-json-schema.tspackages/2-mongo-family/2-authoring/contract-psl/src/exports/index.tspackages/2-mongo-family/2-authoring/contract-psl/src/interpreter.tspackages/2-mongo-family/2-authoring/contract-psl/src/provider.tspackages/2-mongo-family/2-authoring/contract-psl/src/scalar-type-descriptors.tspackages/2-mongo-family/2-authoring/contract-psl/test/derive-json-schema.test.tspackages/2-mongo-family/2-authoring/contract-psl/test/interpreter.polymorphism.test.tspackages/2-mongo-family/2-authoring/contract-psl/test/interpreter.test.tspackages/2-mongo-family/2-authoring/contract-ts/test/config-types.test.tspackages/2-sql/2-authoring/contract-psl/src/default-function-registry.tspackages/2-sql/2-authoring/contract-psl/src/exports/index.tspackages/2-sql/2-authoring/contract-psl/src/interpreter.tspackages/2-sql/2-authoring/contract-psl/src/provider.tspackages/2-sql/2-authoring/contract-psl/src/psl-column-resolution.tspackages/2-sql/2-authoring/contract-psl/src/psl-field-resolution.tspackages/2-sql/2-authoring/contract-psl/test/composed-mutation-defaults.test.tspackages/2-sql/2-authoring/contract-psl/test/default-function-registry.test.tspackages/2-sql/2-authoring/contract-psl/test/fixtures.tspackages/2-sql/2-authoring/contract-psl/test/provider.test.tspackages/2-sql/2-authoring/contract-ts/test/config-types.test.tspackages/2-sql/9-family/README.mdpackages/2-sql/9-family/src/core/assembly.tspackages/2-sql/9-family/src/core/control-instance.tspackages/2-sql/9-family/src/core/migrations/types.tspackages/2-sql/9-family/src/exports/control.tspackages/2-sql/9-family/test/mutation-default-assembly.test.tspackages/3-extensions/mongo/src/config/define-config.tspackages/3-extensions/postgres/src/config/define-config.tspackages/3-extensions/postgres/test/config/define-config.test.tspackages/3-mongo-target/2-mongo-adapter/src/core/codecs.tspackages/3-mongo-target/2-mongo-adapter/src/exports/control.tspackages/3-mongo-target/2-mongo-adapter/test/mongo-planner.test.tspackages/3-targets/6-adapters/postgres/src/core/control-mutation-defaults.tspackages/3-targets/6-adapters/postgres/src/exports/control.tspackages/3-targets/6-adapters/postgres/test/control-mutation-defaults.test.tstest/integration/test/authoring/cli.emit-parity-fixtures.test.tstest/integration/test/authoring/parity/default-pack-slugid/packs.tstest/integration/test/authoring/parity/ts-psl-parity.real-packs.test.tstest/integration/test/authoring/psl.pgvector-dbinit.test.tstest/integration/test/authoring/side-by-side-contracts.test.tstest/integration/test/authoring/templates/prisma-next.config.parity-psl.tstest/integration/test/cli.emit-command.test.tstest/integration/test/cli.emit-contract.test.tstest/integration/test/cli.emit.test.tstest/integration/test/contract-imports.test.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/cli-journeys/prisma-next.config.with-db.psl.tstest/integration/test/fixtures/cli/cli-integration-test-app/fixtures/emit-command/prisma-next.config.parity-psl.tstest/integration/test/mongo/migration-psl-authoring.test.tstest/integration/test/pgvector.test.tstest/integration/test/value-objects/value-objects.integration.test.tstest/integration/utils/framework-components.ts
💤 Files with no reviewable changes (5)
- examples/retail-store/prisma-next.config.ts
- packages/2-mongo-family/2-authoring/contract-psl/src/exports/index.ts
- packages/2-sql/9-family/src/core/control-instance.ts
- test/integration/utils/framework-components.ts
- packages/2-mongo-family/2-authoring/contract-psl/src/scalar-type-descriptors.ts
✅ Files skipped from review due to trivial changes (16)
- packages/2-mongo-family/2-authoring/contract-psl/package.json
- packages/2-sql/2-authoring/contract-psl/src/psl-field-resolution.ts
- packages/2-sql/2-authoring/contract-psl/test/composed-mutation-defaults.test.ts
- packages/3-targets/6-adapters/postgres/test/control-mutation-defaults.test.ts
- packages/2-sql/2-authoring/contract-psl/src/interpreter.ts
- test/integration/test/pgvector.test.ts
- packages/2-sql/2-authoring/contract-ts/test/config-types.test.ts
- test/integration/test/mongo/migration-psl-authoring.test.ts
- packages/1-framework/3-tooling/cli/test/config-types.test.ts
- packages/1-framework/3-tooling/cli/test/control-api/contract-emit.test.ts
- packages/2-sql/2-authoring/contract-psl/src/exports/index.ts
- test/integration/test/cli.emit-command.test.ts
- packages/3-mongo-target/2-mongo-adapter/test/mongo-planner.test.ts
- test/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/cli-journeys/prisma-next.config.with-db.psl.ts
- packages/3-mongo-target/2-mongo-adapter/src/exports/control.ts
- packages/2-mongo-family/2-authoring/contract-psl/test/interpreter.test.ts
🚧 Files skipped from review as they are similar to previous changes (27)
- packages/2-sql/2-authoring/contract-psl/src/psl-column-resolution.ts
- packages/1-framework/3-tooling/cli/src/control-api/client.ts
- packages/1-framework/1-core/framework-components/src/framework-components.ts
- test/integration/test/contract-imports.test.ts
- packages/2-mongo-family/2-authoring/contract-psl/src/provider.ts
- packages/2-mongo-family/2-authoring/contract-psl/test/derive-json-schema.test.ts
- test/integration/test/authoring/cli.emit-parity-fixtures.test.ts
- packages/3-mongo-target/2-mongo-adapter/src/core/codecs.ts
- packages/2-sql/2-authoring/contract-psl/test/default-function-registry.test.ts
- test/integration/test/fixtures/cli/cli-integration-test-app/fixtures/emit-command/prisma-next.config.parity-psl.ts
- test/integration/test/cli.emit.test.ts
- packages/2-mongo-family/2-authoring/contract-psl/test/interpreter.polymorphism.test.ts
- packages/3-targets/6-adapters/postgres/src/exports/control.ts
- test/integration/test/value-objects/value-objects.integration.test.ts
- test/integration/test/authoring/psl.pgvector-dbinit.test.ts
- test/integration/test/authoring/side-by-side-contracts.test.ts
- packages/2-sql/2-authoring/contract-psl/src/default-function-registry.ts
- packages/1-framework/1-core/framework-components/test/control-stack.test.ts
- docs/architecture docs/subsystems/10. MongoDB Family.md
- packages/2-mongo-family/2-authoring/contract-ts/test/config-types.test.ts
- packages/1-framework/1-core/framework-components/src/exports/control.ts
- packages/2-mongo-family/2-authoring/contract-psl/src/interpreter.ts
- packages/2-sql/2-authoring/contract-psl/test/provider.test.ts
- packages/2-sql/9-family/test/mutation-default-assembly.test.ts
- test/integration/test/authoring/parity/ts-psl-parity.real-packs.test.ts
- packages/1-framework/1-core/framework-components/src/control-stack.ts
- packages/3-targets/6-adapters/postgres/src/core/control-mutation-defaults.ts
Covers three MongoDB migration planner bugs (FL-09 variant collections, FL-10 incomplete validators, FL-11 missing Float) plus an architectural fix to lift PSL contribution assembly (scalar type descriptors, mutation defaults, authoring contributions) to the framework level via ContractSourceContext.
Move ControlMutationDefaults, MutationDefaultGeneratorDescriptor, ParsedDefaultFunctionCall and related types from SQL authoring to the framework layer. The SQL package re-exports from framework-components, preserving the existing public API. This is the foundation for having ComponentMetadata carry mutation default contributions and assembling them in createControlStack.
…ions Add pslScalarTypeDescriptors and controlMutationDefaults fields to ComponentMetadata. Extend createControlStack to assemble them from all component descriptors with duplicate detection. Enrich ContractSourceContext with pslScalarTypeDescriptors, authoringContributions, codecLookup, and controlMutationDefaults so contract source providers can read from context instead of options.
Move createControlStack call ahead of contractConfig.source and populate sourceContext with all assembled contributions from the stack. This lets source providers read pslScalarTypeDescriptors, codecLookup, authoringContributions, and controlMutationDefaults from context.
Replace SqlControlStaticContributions hooks (pslTypeDescriptors, controlMutationDefaults) with direct ComponentMetadata fields (pslScalarTypeDescriptors, controlMutationDefaults). Scalar type descriptors now store only codecId (nativeType derived from codec lookup at use site). Types imported from framework-components.
Remove scalarTypeDescriptors, authoringContributions, and controlMutationDefaults from PrismaContractOptions. The provider now reads these from ContractSourceContext and builds ColumnDescriptor map from codecId-only descriptors + codec lookup at the boundary.
…-09) Add tests for FL-09, FL-10, FL-11 bugs. Fix FL-09 by extending resolvePolymorphism to remove spurious variant collection entries and merge variant indexes into the base collection. Also adds pslScalarTypeDescriptors to Mongo adapter descriptor (including Float mapping) and FL-11 derive-json-schema test.
… (FL-10) Move validator derivation after resolvePolymorphism so discriminator and variant metadata is available. Add derivePolymorphicJsonSchema that generates a $jsonSchema with oneOf entries for variant-specific fields and discriminator enum constraints.
…L-11) Replace CODEC_TO_BSON_TYPE with codec lookup resolution via codecLookup.get(codecId).targetTypes[0]. Fix mongoDoubleCodec targetTypes from "float" to "double" (correct BSON type name). Add codecLookup as optional parameter to interpreter and deriveJsonSchema. Update all test files with mock codec lookup.
…goContract() Update mongoContract() to read pslScalarTypeDescriptors and codecLookup from context instead of options. Remove scalarTypeDescriptors option from MongoContractOptions. Simplify retail-store config to remove manual Float/scalar assembly (now provided by Mongo adapter).
…names Add planner test confirming that a polymorphic contract with only a base collection entry produces createCollection only for the base, not for variant model names like "bug" or "feature".
Remove assembleAuthoringContributions/assemblePslInterpretationContributions from prisma-next-demo config. Contributions now flow through the framework via ContractSourceContext automatically.
…elds Mongo contract-ts and Postgres adapter tests needed updated mocks to match the new ContractSourceContext shape (authoringContributions uses field/type namespaces, controlMutationDefaults is a flat ReadonlyMap, pslScalarTypeDescriptors returns codec IDs directly).
…hism details Add $jsonSchema validation section explaining how BSON types are derived from codec lookup rather than hardcoded maps. Document polymorphic validator oneOf pattern and variant collection suppression.
… duplicate types Delete SqlControlStaticContributions, PslScalarTypeDescriptor, and all SQL-local mutation default types that duplicated the framework-level versions. Inline queryOperations directly on the three SQL descriptor interfaces that still need it.
Remove assemblePslInterpretationContributions, assembleControlMutationDefaultContributions, SqlControlDescriptorWithContributions, and related interfaces. These are replaced by framework-level assembly in createControlStack. Remove corresponding exports from control.ts.
Remove tests for deleted assemblePslInterpretationContributions and assembleControlMutationDefaultContributions (now tested in framework-components control-stack.test.ts). Keep authoring contribution tests using the framework-level assembleAuthoringContributions.
…ompat fallback Delete createMongoScalarTypeDescriptors() and its file. Remove the fallback in mongoContract() that silently used hardcoded descriptors when context was empty — the provider now trusts context unconditionally. Update tests to use an inline descriptor map that includes Float.
…egistry Delete the 12 type re-exports that proxied framework-components/control types through default-function-registry.ts. Update all consumers to import directly from @prisma-next/framework-components/control. The exports/ re-exports are preserved per convention.
Use spread-based immutable object replacement instead of directly mutating baseColl[indexes] in resolvePolymorphism.
…lStack Remove manual assembly calls (assemblePslInterpretationContributions, assembleAuthoringContributions) from integration test configs and test files. Config files now match the demo config pattern. Tests that call interpreters directly use createControlStack to obtain assembled state. Remove SqlControlDescriptorWithContributions from util bundle.
Replace partial { composedExtensionPacks: [] } mocks with full
ContractSourceContext built from createControlStack, providing all
required fields (pslScalarTypeDescriptors, authoringContributions,
codecLookup, controlMutationDefaults).
Remove stale export of deleted createMongoScalarTypeDescriptors, fix SqlControlDescriptorWithContributions reference in control-instance, build full ContractSourceContext in CLI client emit path, and update integration tests to construct descriptor arrays inline instead of using removed SqlDescriptorBundle.descriptors field.
All functional and architectural acceptance criteria have been met. Mark task checkboxes in plan.md and AC checkboxes in spec.md as done.
…ibutions The postgres and mongo defineConfig facades from main still used the old manual assembly pattern. Simplify to match the new architecture: contract source providers receive contributions via ContractSourceContext from the CLI, so facades only need to pass target and output. Also update SQL family README to reflect createControlStack usage instead of removed assemblePslInterpretationContributions.
wmadden
force-pushed
the
tml-2247-migration-planner-bugs-variant-collections-and-float-type-fl
branch
from
7bf0ff9 to
3bd700a
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/1-framework/1-core/framework-components/src/control-stack.ts (1)
352-374:⚠️ Potential issue | 🟠 MajorValidate scalar-descriptor codec ids before returning the stack.
pslScalarTypeDescriptorsis now assembled centrally, but nothing here verifies that each contributedcodecIdexists incodecLookup. A typo in component metadata now poisons every provider: SQL falls back to the raw codec id asnativeType, and Mongo validator derivation silently drops the field schema. This should fail once increateControlStack()instead of leaking a broken context downstream.Proposed fix
export function createControlStack<TFamilyId extends string, TTargetId extends string>( input: CreateControlStackInput<TFamilyId, TTargetId>, ): ControlStack<TFamilyId, TTargetId> { const { family, target, adapter, driver, extensionPacks = [] } = input; const allDescriptors = [family, target, ...(adapter ? [adapter] : []), ...extensionPacks]; + const codecLookup = extractCodecLookup(allDescriptors); + const pslScalarTypeDescriptors = assemblePslScalarTypeDescriptors(allDescriptors); + + for (const [typeName, codecId] of pslScalarTypeDescriptors) { + if (!codecLookup.get(codecId)) { + throw new Error( + `PSL scalar type "${typeName}" references unknown codec "${codecId}".`, + ); + } + } return { family, target, adapter, @@ - codecLookup: extractCodecLookup(allDescriptors), + codecLookup, authoringContributions: assembleAuthoringContributions(allDescriptors), - pslScalarTypeDescriptors: assemblePslScalarTypeDescriptors(allDescriptors), + pslScalarTypeDescriptors, controlMutationDefaults: assembleControlMutationDefaults(allDescriptors), }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/1-framework/1-core/framework-components/src/control-stack.ts` around lines 352 - 374, In createControlStack, validate that every pslScalarTypeDescriptors entry references a known codec by ensuring each descriptor.codecId exists as a key in the codecLookup built just above; after calling assemblePslScalarTypeDescriptors(allDescriptors) and extractCodecLookup(allDescriptors), iterate the resulting pslScalarTypeDescriptors, and if any codecId is missing from codecLookup throw a clear error (or processLogger.error then throw) naming the offending codecId and descriptor (so createControlStack fails fast rather than producing a poisoned stack).
♻️ Duplicate comments (2)
packages/2-mongo-family/2-authoring/contract-psl/src/interpreter.ts (2)
870-883:⚠️ Potential issue | 🟠 MajorUse the discriminator's storage name in the polymorphic validator.
modelEntry.discriminator.fieldis still the PSL identifier here, but Mongo fields and$jsonSchemaproperties are keyed by mapped storage names.type String@Map("_t")with@@discriminator(type)will either trip the earlier field lookup or buildoneOfbranches ontypeinstead of_t, rejecting valid documents.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-mongo-family/2-authoring/contract-psl/src/interpreter.ts` around lines 870 - 883, The polymorphic JSON Schema is using the PSL identifier modelEntry.discriminator.field instead of the mapped Mongo storage name; update the call to derivePolymorphicJsonSchema to pass the discriminator's storage name (lookup the mapped name from the resolved model/field metadata rather than using modelEntry.discriminator.field directly — e.g. resolve via resolvedModels[...] or the field's storageName/property) so the generated $jsonSchema keys use the storage name (e.g., "_t") rather than the PSL name.
279-295:⚠️ Potential issue | 🟠 MajorSame-collection variants still lose one side's indexes.
This merge only runs when
variantCollectionName !== baseCollection. If a variant explicitly maps to the same collection as its base, the earliercollections[collectionName] = ...write has already overwritten one model's indexes, and this block never reconstructs the union. A base@@index([title])plus variant@@index([severity])on"tasks"still emits only one set.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-mongo-family/2-authoring/contract-psl/src/interpreter.ts` around lines 279 - 295, The current conditional skips merging indexes when variantCollectionName === baseCollection, so same-collection variants lose one side's indexes; update the logic around collections, variantCollectionName, baseCollection, variantIndexes and baseIndexes so that whenever a variant maps to the same collection you merge/union the indexes into collections[baseCollection] (i.e., always combine existing base indexes and variantIndexes and assign the merged array back into collections[baseCollection]), and ensure the filtered reconstruction (used when names differ) still preserves that union behavior.
🧹 Nitpick comments (3)
packages/2-sql/2-authoring/contract-psl/test/fixtures.ts (1)
125-135: Consider deriving codec-id descriptors from the existing scalar descriptor map.
postgresCodecIdOnlyDescriptorsrepeats data that already exists inpostgresScalarTypeDescriptors, which can drift over time.♻️ Proposed DRY refactor
-export const postgresCodecIdOnlyDescriptors = new Map<string, string>([ - ['String', 'pg/text@1'], - ['Boolean', 'pg/bool@1'], - ['Int', 'pg/int4@1'], - ['BigInt', 'pg/int8@1'], - ['Float', 'pg/float8@1'], - ['Decimal', 'pg/numeric@1'], - ['DateTime', 'pg/timestamptz@1'], - ['Json', 'pg/jsonb@1'], - ['Bytes', 'pg/bytea@1'], -]); +export const postgresCodecIdOnlyDescriptors = new Map<string, string>( + Array.from(postgresScalarTypeDescriptors, ([scalar, descriptor]) => [scalar, descriptor.codecId]), +);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-sql/2-authoring/contract-psl/test/fixtures.ts` around lines 125 - 135, Replace the hard-coded postgresCodecIdOnlyDescriptors map with one derived from postgresScalarTypeDescriptors: iterate over postgresScalarTypeDescriptors entries and build a new Map<string,string> where each key is the scalar name and each value is the descriptor's codec-id property (e.g. descriptor.codecId or the equivalent field used in postgresScalarTypeDescriptors), then export that Map as postgresCodecIdOnlyDescriptors; this removes duplicated data and keeps codec ids in sync with postgresScalarTypeDescriptors.packages/3-targets/6-adapters/postgres/src/core/control-mutation-defaults.ts (1)
303-323: PreferifDefined()over inline conditional spreads in this descriptor builder.This changed path still uses
...(cond ? { ... } : {})a few times. Swapping those toifDefined()would keep the object assembly consistent with the repo convention.♻️ Suggested cleanup
+import { ifDefined } from '@prisma-next/utils/defined'; ... export function createPostgresMutationDefaultGeneratorDescriptors(): readonly MutationDefaultGeneratorDescriptor[] { return builtinGeneratorRegistryMetadata.map(({ id, applicableCodecIds }) => ({ id, applicableCodecIds, resolveGeneratedColumnDescriptor: ({ generated }) => { if (generated.kind !== 'generator' || generated.id !== id) { return undefined; } const descriptor = resolveBuiltinGeneratedColumnDescriptor({ id, - ...(generated.params ? { params: generated.params } : {}), + ...ifDefined(generated.params, (params) => ({ params })), }); return { codecId: descriptor.type.codecId, nativeType: descriptor.type.nativeType, - ...(descriptor.type.typeRef ? { typeRef: descriptor.type.typeRef } : {}), - ...(descriptor.typeParams ? { typeParams: descriptor.typeParams } : {}), + ...ifDefined(descriptor.type.typeRef, (typeRef) => ({ typeRef })), + ...ifDefined(descriptor.typeParams, (typeParams) => ({ typeParams })), }; }, })); }As per coding guidelines, "Use
ifDefined()from@prisma-next/utils/definedfor conditional object spreads instead of inline conditional spread patterns".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/3-targets/6-adapters/postgres/src/core/control-mutation-defaults.ts` around lines 303 - 323, In createPostgresMutationDefaultGeneratorDescriptors, replace the inline conditional spreads inside resolveGeneratedColumnDescriptor with the repo-standard ifDefined helper: import ifDefined from '@prisma-next/utils/defined' and use it to conditionally add typeRef and typeParams and params (when calling resolveBuiltinGeneratedColumnDescriptor) instead of ...(cond ? {...} : {}); locate resolveGeneratedColumnDescriptor and resolveBuiltinGeneratedColumnDescriptor usages and convert each inline spread to the pattern that wraps the optional value with ifDefined(...) so the returned object is assembled via fixed properties plus ...ifDefined(...) calls.packages/2-sql/9-family/src/core/migrations/types.ts (1)
127-136: ExtractqueryOperationsinto a shared provider type.The same hook signature is now repeated on extension, adapter, and target descriptors. That makes this public surface easy to drift the next time the callback changes. A tiny shared interface or type alias would keep all three in sync.
♻️ Possible cleanup
+export interface SqlQueryOperationsProvider { + readonly queryOperations?: () => ReadonlyArray<SqlOperationDescriptor>; +} + export interface SqlControlExtensionDescriptor<TTargetId extends string> - extends ControlExtensionDescriptor<'sql', TTargetId> { + extends ControlExtensionDescriptor<'sql', TTargetId>, SqlQueryOperationsProvider { readonly databaseDependencies?: ComponentDatabaseDependencies<unknown>; - readonly queryOperations?: () => ReadonlyArray<SqlOperationDescriptor>; } export interface SqlControlAdapterDescriptor<TTargetId extends string> - extends ControlAdapterDescriptor<'sql', TTargetId> { - readonly queryOperations?: () => ReadonlyArray<SqlOperationDescriptor>; -} + extends ControlAdapterDescriptor<'sql', TTargetId>, SqlQueryOperationsProvider {} export interface SqlControlTargetDescriptor<TTargetId extends string, TTargetDetails> - extends MigratableTargetDescriptor<'sql', TTargetId, SqlControlFamilyInstance> { - readonly queryOperations?: () => ReadonlyArray<SqlOperationDescriptor>; + extends MigratableTargetDescriptor<'sql', TTargetId, SqlControlFamilyInstance>, + SqlQueryOperationsProvider { createPlanner(family: SqlControlFamilyInstance): SqlMigrationPlanner<TTargetDetails>; createRunner(family: SqlControlFamilyInstance): SqlMigrationRunner<TTargetDetails>; }Also applies to: 293-295
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-sql/9-family/src/core/migrations/types.ts` around lines 127 - 136, The repeated queryOperations signature should be extracted to a single shared type to avoid drift; define a type alias like QueryOperationsProvider = () => ReadonlyArray<SqlOperationDescriptor> and replace the inline signatures on SqlControlExtensionDescriptor, SqlControlAdapterDescriptor (and the matching target descriptor referenced in the file) with that alias so all three use the same provider type.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/2-mongo-family/2-authoring/contract-psl/src/derive-json-schema.ts`:
- Around line 112-147: The current loop sometimes skips emitting a branch (due
to the "continue" when Object.keys(variantOnlyFields).length === 0 &&
variants.length <= 1) and never marks the discriminator as required; update the
loop that builds oneOf so it always pushes an entry for every variant (do not
continue away branches even if variantOnlyFields is empty) and ensure each entry
(the "entry" object with properties including [discriminatorField]) has the
discriminatorField added to entry['required'] (e.g., push discriminatorField
into variantRequired or otherwise set entry['required'] = [...,
discriminatorField].sort()) so every branch requires the discriminator; keep the
rest of property merging (variantProperties) and required sorting logic intact
and still assign jsonSchema['oneOf'] = oneOf when oneOf is non-empty.
In `@packages/2-sql/2-authoring/contract-psl/test/fixtures.ts`:
- Around line 158-163: The fixture postgresCodecLookup currently returns a
shallow object cast to Codec via CodecLookup.get which omits required Codec
methods and weakens type safety; update postgresCodecLookup.get to return a
complete Codec object (not a blind cast) by adding the missing methods (decode,
encodeJson, decodeJson) alongside id and targetTypes, implementing simple
stubbed behavior appropriate for tests (e.g., throw/unimplemented for decode or
pass-through JSON encode/decode that mirrors expected test behavior) so the
returned value fully satisfies the Codec interface; locate the get
implementation on postgresCodecLookup and targetTypesByCodecId in the same file
to modify the returned object.
---
Outside diff comments:
In `@packages/1-framework/1-core/framework-components/src/control-stack.ts`:
- Around line 352-374: In createControlStack, validate that every
pslScalarTypeDescriptors entry references a known codec by ensuring each
descriptor.codecId exists as a key in the codecLookup built just above; after
calling assemblePslScalarTypeDescriptors(allDescriptors) and
extractCodecLookup(allDescriptors), iterate the resulting
pslScalarTypeDescriptors, and if any codecId is missing from codecLookup throw a
clear error (or processLogger.error then throw) naming the offending codecId and
descriptor (so createControlStack fails fast rather than producing a poisoned
stack).
---
Duplicate comments:
In `@packages/2-mongo-family/2-authoring/contract-psl/src/interpreter.ts`:
- Around line 870-883: The polymorphic JSON Schema is using the PSL identifier
modelEntry.discriminator.field instead of the mapped Mongo storage name; update
the call to derivePolymorphicJsonSchema to pass the discriminator's storage name
(lookup the mapped name from the resolved model/field metadata rather than using
modelEntry.discriminator.field directly — e.g. resolve via resolvedModels[...]
or the field's storageName/property) so the generated $jsonSchema keys use the
storage name (e.g., "_t") rather than the PSL name.
- Around line 279-295: The current conditional skips merging indexes when
variantCollectionName === baseCollection, so same-collection variants lose one
side's indexes; update the logic around collections, variantCollectionName,
baseCollection, variantIndexes and baseIndexes so that whenever a variant maps
to the same collection you merge/union the indexes into
collections[baseCollection] (i.e., always combine existing base indexes and
variantIndexes and assign the merged array back into
collections[baseCollection]), and ensure the filtered reconstruction (used when
names differ) still preserves that union behavior.
---
Nitpick comments:
In `@packages/2-sql/2-authoring/contract-psl/test/fixtures.ts`:
- Around line 125-135: Replace the hard-coded postgresCodecIdOnlyDescriptors map
with one derived from postgresScalarTypeDescriptors: iterate over
postgresScalarTypeDescriptors entries and build a new Map<string,string> where
each key is the scalar name and each value is the descriptor's codec-id property
(e.g. descriptor.codecId or the equivalent field used in
postgresScalarTypeDescriptors), then export that Map as
postgresCodecIdOnlyDescriptors; this removes duplicated data and keeps codec ids
in sync with postgresScalarTypeDescriptors.
In `@packages/2-sql/9-family/src/core/migrations/types.ts`:
- Around line 127-136: The repeated queryOperations signature should be
extracted to a single shared type to avoid drift; define a type alias like
QueryOperationsProvider = () => ReadonlyArray<SqlOperationDescriptor> and
replace the inline signatures on SqlControlExtensionDescriptor,
SqlControlAdapterDescriptor (and the matching target descriptor referenced in
the file) with that alias so all three use the same provider type.
In
`@packages/3-targets/6-adapters/postgres/src/core/control-mutation-defaults.ts`:
- Around line 303-323: In createPostgresMutationDefaultGeneratorDescriptors,
replace the inline conditional spreads inside resolveGeneratedColumnDescriptor
with the repo-standard ifDefined helper: import ifDefined from
'@prisma-next/utils/defined' and use it to conditionally add typeRef and
typeParams and params (when calling resolveBuiltinGeneratedColumnDescriptor)
instead of ...(cond ? {...} : {}); locate resolveGeneratedColumnDescriptor and
resolveBuiltinGeneratedColumnDescriptor usages and convert each inline spread to
the pattern that wraps the optional value with ifDefined(...) so the returned
object is assembled via fixed properties plus ...ifDefined(...) calls.
🪄 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: 000d52d6-f89a-4174-8414-5f1fde19fb0c
⛔ Files ignored due to path filters (3)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlprojects/migration-planner-bugs/plan.mdis excluded by!projects/**projects/migration-planner-bugs/spec.mdis excluded by!projects/**
📒 Files selected for processing (65)
docs/architecture docs/subsystems/10. MongoDB Family.mdexamples/prisma-next-demo/prisma-next.config.tsexamples/retail-store/prisma-next.config.tspackages/1-framework/1-core/config/src/contract-source-types.tspackages/1-framework/1-core/framework-components/src/control-stack.tspackages/1-framework/1-core/framework-components/src/exports/control.tspackages/1-framework/1-core/framework-components/src/framework-components.tspackages/1-framework/1-core/framework-components/src/mutation-default-types.tspackages/1-framework/1-core/framework-components/test/control-stack.test.tspackages/1-framework/3-tooling/cli/src/control-api/client.tspackages/1-framework/3-tooling/cli/src/control-api/operations/contract-emit.tspackages/1-framework/3-tooling/cli/test/config-types.test.tspackages/1-framework/3-tooling/cli/test/control-api/contract-emit.test.tspackages/2-mongo-family/2-authoring/contract-psl/package.jsonpackages/2-mongo-family/2-authoring/contract-psl/src/derive-json-schema.tspackages/2-mongo-family/2-authoring/contract-psl/src/exports/index.tspackages/2-mongo-family/2-authoring/contract-psl/src/interpreter.tspackages/2-mongo-family/2-authoring/contract-psl/src/provider.tspackages/2-mongo-family/2-authoring/contract-psl/src/scalar-type-descriptors.tspackages/2-mongo-family/2-authoring/contract-psl/test/derive-json-schema.test.tspackages/2-mongo-family/2-authoring/contract-psl/test/interpreter.polymorphism.test.tspackages/2-mongo-family/2-authoring/contract-psl/test/interpreter.test.tspackages/2-mongo-family/2-authoring/contract-ts/test/config-types.test.tspackages/2-sql/2-authoring/contract-psl/src/default-function-registry.tspackages/2-sql/2-authoring/contract-psl/src/exports/index.tspackages/2-sql/2-authoring/contract-psl/src/interpreter.tspackages/2-sql/2-authoring/contract-psl/src/provider.tspackages/2-sql/2-authoring/contract-psl/src/psl-column-resolution.tspackages/2-sql/2-authoring/contract-psl/src/psl-field-resolution.tspackages/2-sql/2-authoring/contract-psl/test/composed-mutation-defaults.test.tspackages/2-sql/2-authoring/contract-psl/test/default-function-registry.test.tspackages/2-sql/2-authoring/contract-psl/test/fixtures.tspackages/2-sql/2-authoring/contract-psl/test/provider.test.tspackages/2-sql/2-authoring/contract-ts/test/config-types.test.tspackages/2-sql/9-family/README.mdpackages/2-sql/9-family/src/core/assembly.tspackages/2-sql/9-family/src/core/control-instance.tspackages/2-sql/9-family/src/core/migrations/types.tspackages/2-sql/9-family/src/exports/control.tspackages/2-sql/9-family/test/mutation-default-assembly.test.tspackages/3-extensions/mongo/src/config/define-config.tspackages/3-extensions/postgres/src/config/define-config.tspackages/3-extensions/postgres/test/config/define-config.test.tspackages/3-mongo-target/1-mongo-target/test/mongo-planner.test.tspackages/3-mongo-target/2-mongo-adapter/src/core/codecs.tspackages/3-mongo-target/2-mongo-adapter/src/exports/control.tspackages/3-targets/6-adapters/postgres/src/core/control-mutation-defaults.tspackages/3-targets/6-adapters/postgres/src/exports/control.tspackages/3-targets/6-adapters/postgres/test/control-mutation-defaults.test.tstest/integration/test/authoring/cli.emit-parity-fixtures.test.tstest/integration/test/authoring/parity/default-pack-slugid/packs.tstest/integration/test/authoring/parity/ts-psl-parity.real-packs.test.tstest/integration/test/authoring/psl.pgvector-dbinit.test.tstest/integration/test/authoring/side-by-side-contracts.test.tstest/integration/test/authoring/templates/prisma-next.config.parity-psl.tstest/integration/test/cli.emit-command.test.tstest/integration/test/cli.emit-contract.test.tstest/integration/test/cli.emit.test.tstest/integration/test/contract-imports.test.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/cli-journeys/prisma-next.config.with-db.psl.tstest/integration/test/fixtures/cli/cli-integration-test-app/fixtures/emit-command/prisma-next.config.parity-psl.tstest/integration/test/mongo/migration-psl-authoring.test.tstest/integration/test/pgvector.test.tstest/integration/test/value-objects/value-objects.integration.test.tstest/integration/utils/framework-components.ts
💤 Files with no reviewable changes (5)
- examples/retail-store/prisma-next.config.ts
- packages/2-sql/9-family/src/core/control-instance.ts
- packages/2-mongo-family/2-authoring/contract-psl/src/scalar-type-descriptors.ts
- packages/2-mongo-family/2-authoring/contract-psl/src/exports/index.ts
- test/integration/utils/framework-components.ts
✅ Files skipped from review due to trivial changes (26)
- packages/2-mongo-family/2-authoring/contract-ts/test/config-types.test.ts
- packages/2-sql/2-authoring/contract-psl/test/composed-mutation-defaults.test.ts
- packages/2-mongo-family/2-authoring/contract-psl/package.json
- test/integration/test/cli.emit.test.ts
- packages/2-sql/2-authoring/contract-psl/src/interpreter.ts
- packages/2-sql/2-authoring/contract-psl/src/psl-field-resolution.ts
- packages/2-sql/2-authoring/contract-ts/test/config-types.test.ts
- test/integration/test/mongo/migration-psl-authoring.test.ts
- packages/2-mongo-family/2-authoring/contract-psl/test/derive-json-schema.test.ts
- examples/prisma-next-demo/prisma-next.config.ts
- test/integration/test/authoring/parity/default-pack-slugid/packs.ts
- test/integration/test/fixtures/cli/cli-integration-test-app/fixtures/emit-command/prisma-next.config.parity-psl.ts
- packages/3-mongo-target/2-mongo-adapter/src/core/codecs.ts
- packages/3-mongo-target/1-mongo-target/test/mongo-planner.test.ts
- packages/3-extensions/postgres/test/config/define-config.test.ts
- test/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/cli-journeys/prisma-next.config.with-db.psl.ts
- packages/1-framework/1-core/framework-components/test/control-stack.test.ts
- packages/2-sql/2-authoring/contract-psl/src/exports/index.ts
- packages/1-framework/1-core/framework-components/src/mutation-default-types.ts
- packages/1-framework/1-core/framework-components/src/exports/control.ts
- test/integration/test/authoring/parity/ts-psl-parity.real-packs.test.ts
- test/integration/test/value-objects/value-objects.integration.test.ts
- packages/3-mongo-target/2-mongo-adapter/src/exports/control.ts
- test/integration/test/authoring/psl.pgvector-dbinit.test.ts
- test/integration/test/authoring/side-by-side-contracts.test.ts
- packages/2-sql/9-family/README.md
🚧 Files skipped from review as they are similar to previous changes (13)
- packages/2-sql/2-authoring/contract-psl/src/psl-column-resolution.ts
- packages/2-sql/2-authoring/contract-psl/test/default-function-registry.test.ts
- test/integration/test/pgvector.test.ts
- packages/1-framework/3-tooling/cli/src/control-api/operations/contract-emit.ts
- packages/1-framework/3-tooling/cli/test/control-api/contract-emit.test.ts
- packages/2-mongo-family/2-authoring/contract-psl/src/provider.ts
- packages/1-framework/3-tooling/cli/test/config-types.test.ts
- packages/2-sql/9-family/test/mutation-default-assembly.test.ts
- test/integration/test/authoring/templates/prisma-next.config.parity-psl.ts
- test/integration/test/cli.emit-contract.test.ts
- packages/2-sql/2-authoring/contract-psl/test/provider.test.ts
- packages/3-extensions/postgres/src/config/define-config.ts
- packages/2-sql/9-family/src/exports/control.ts
The source context was passing stack.extensionIds (which includes family, target, and adapter IDs) as composedExtensionPacks. This caused the PSL interpreter to emit spurious extensionPacks entries for sql/postgres/adapter in the contract, breaking parity with TS contracts and causing verify mismatches. Use stack.extensionPacks.map(p => p.id) to match the original main behavior of only including user-configured extension packs.
The deriveJsonSchema function requires a CodecLookup to resolve BSON types from codec IDs. Without it, $jsonSchema validators have no field-level type properties, causing test assertions to fail. Also regenerates contract fixtures to reflect updated type declarations from the rebase.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/integration/test/cli.emit-command.test.ts (1)
540-625: Split this diagnostics scenario into its own test file.
cli.emit-command.test.tsis already well past the repo’s size limit, and this adds another distinct concern on top of the existing emit-command coverage. Moving this case into something likecli.emit-command.diagnostics.test.tswould keep the suite easier to navigate and maintain.As per coding guidelines, "
**/*.test.ts: Keep test files under 500 lines to maintain readability and navigability. If a test file exceeds this limit, it should be split into multiple files.`"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/integration/test/cli.emit-command.test.ts` around lines 540 - 625, This test should be extracted into a new file to keep cli.emit-command.test.ts under the 500-line guideline: create a new test file (e.g., cli.emit-command.diagnostics.test.ts) and move the entire "renders provider diagnostics when psl provider fails" it block along with its helper references (createContractEmitCommand, setupIntegrationTestDirectoryFromFixtures, loadConfig, createControlStack, executeCommand, and the consoleErrors/cleanup usage) into that file; ensure you import or require any shared helpers/fixtures used by the test, preserve the same async structure and timeouts, remove the moved test from cli.emit-command.test.ts, and run the test suite to verify the new file runs and all references (like prisma-next.config.parity-psl.ts and schema.prisma writes) resolve correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@test/integration/test/cli.emit-command.test.ts`:
- Around line 540-625: This test should be extracted into a new file to keep
cli.emit-command.test.ts under the 500-line guideline: create a new test file
(e.g., cli.emit-command.diagnostics.test.ts) and move the entire "renders
provider diagnostics when psl provider fails" it block along with its helper
references (createContractEmitCommand,
setupIntegrationTestDirectoryFromFixtures, loadConfig, createControlStack,
executeCommand, and the consoleErrors/cleanup usage) into that file; ensure you
import or require any shared helpers/fixtures used by the test, preserve the
same async structure and timeouts, remove the moved test from
cli.emit-command.test.ts, and run the test suite to verify the new file runs and
all references (like prisma-next.config.parity-psl.ts and schema.prisma writes)
resolve correctly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: d0f9ccf3-7f4f-44f8-8d43-9dd332cf469d
⛔ Files ignored due to path filters (4)
test/integration/test/mongo/fixtures/generated/contract.d.tsis excluded by!**/generated/**test/integration/test/mongo/fixtures/generated/contract.jsonis excluded by!**/generated/**test/integration/test/sql-builder/fixtures/generated/contract.d.tsis excluded by!**/generated/**test/integration/test/sql-builder/fixtures/generated/contract.jsonis excluded by!**/generated/**
📒 Files selected for processing (6)
packages/1-framework/3-tooling/cli/src/control-api/client.tspackages/1-framework/3-tooling/cli/src/control-api/operations/contract-emit.tstest/integration/test/authoring/cli.emit-parity-fixtures.test.tstest/integration/test/cli.emit-command.test.tstest/integration/test/cli.emit-contract.test.tstest/integration/test/mongo/migration-psl-authoring.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/1-framework/3-tooling/cli/src/control-api/client.ts
- packages/1-framework/3-tooling/cli/src/control-api/operations/contract-emit.ts
- test/integration/test/cli.emit-contract.test.ts
…hic validators The early continue at L112 skipped branch emission when a single variant had no extra fields, silently dropping discriminator validation. The discriminatorField was also missing from the required array of each branch. Now every variant always produces a oneOf branch with the discriminator in both properties and required, ensuring Mongo rejects documents with missing or invalid discriminator values.
…ollection name When a variant @@maps to the same collection as its base, the collection entry was being overwritten during initial interpretation, losing the base indexes. Now indexes are merged at interpretation time when the collection already exists, and a deduplication pass in applyPolymorphism prevents duplicates when same-collection variants are processed.
The discriminator field was stored using its PSL name, but model.fields uses storage-mapped names (via @Map). When a discriminator field has @Map (e.g. type String @Map("_type")), the $jsonSchema validator would reference the wrong property name. Now resolveFieldMappings is used to map the discriminator name at applyPolymorphism time.
wmadden
commented
Apr 16, 2026
The fallback of using the raw codecId as nativeType silently masked broken control-stack configurations. Now buildColumnDescriptorMap throws a descriptive error naming the codecId and PSL typeName when codecLookup.get() returns undefined.
Provides a reusable validation function that checks all PSL scalar type descriptor codec IDs against a codecLookup. Not called automatically in createControlStack because some adapters have legitimate gaps in codec coverage (e.g. pg/bytea@1 not yet implemented). Callers can opt into validation by calling validatePslScalarTypeCodecIds explicitly.
The throw introduced in the previous commit was too aggressive: it fired for all registered PSL types including those not used by the schema (e.g. Bytes/pg/bytea@1 which has no codec instance yet). Now unresolvable codecs are skipped, and the schema interpreter reports diagnostics when a field actually uses an unsupported type.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/2-sql/2-authoring/contract-psl/src/provider.ts (1)
70-72: Unnecessary array copy.Both
context.composedExtensionPacksand the target parameter are typed asreadonly string[], so the spread is redundant. Passing the array directly avoids allocation.♻️ Suggested simplification
...ifDefined( 'composedExtensionPacks', - context.composedExtensionPacks.length > 0 - ? [...context.composedExtensionPacks] - : undefined, + context.composedExtensionPacks.length > 0 ? context.composedExtensionPacks : undefined, ),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-sql/2-authoring/contract-psl/src/provider.ts` around lines 70 - 72, The spread of context.composedExtensionPacks is unnecessary and causes an extra allocation; where the code builds the parameter (currently using [...context.composedExtensionPacks] or undefined), pass context.composedExtensionPacks directly instead (i.e., use context.composedExtensionPacks) because both the source and the target parameter are typed readonly string[]; update the call site in provider.ts to remove the spread and keep the conditional that yields undefined when length is 0.packages/2-mongo-family/2-authoring/contract-psl/test/interpreter.polymorphism.test.ts (1)
1-565: Consider splitting this test file to stay under the 500-line guideline.The file is 566 lines. Per coding guidelines, test files should be kept under 500 lines. This could be split into:
interpreter.polymorphism.basic.test.ts— happy paths and diagnostic tests (lines 53-346)interpreter.polymorphism.storage.test.ts— FL-09/FL-10 collection, index, and validator tests (lines 347-565)The test implementations themselves look solid — good coverage of variant collection suppression, index merging, and polymorphic validator generation with
oneOfbranches.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-mongo-family/2-authoring/contract-psl/test/interpreter.polymorphism.test.ts` around lines 1 - 565, This test file exceeds the 500-line guideline; split it into two files: move the "FL-09: variant collection suppression" and "FL-10: polymorphic validators" describe blocks (the tests starting at the "FL-09: variant collection suppression" and "FL-10: polymorphic validators" headings) into a new file named e.g. interpreter.polymorphism.storage.test.ts, leaving the remaining "happy paths" and "diagnostics" describe blocks in the original interpreter.polymorphism.test.ts; ensure both new files retain the helper functions interpret, interpretOk, mongoScalarTypeDescriptors, and mongoCodecLookup and import statements (so tests run independently) and update test runner/exports if needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@packages/2-mongo-family/2-authoring/contract-psl/test/interpreter.polymorphism.test.ts`:
- Around line 1-565: This test file exceeds the 500-line guideline; split it
into two files: move the "FL-09: variant collection suppression" and "FL-10:
polymorphic validators" describe blocks (the tests starting at the "FL-09:
variant collection suppression" and "FL-10: polymorphic validators" headings)
into a new file named e.g. interpreter.polymorphism.storage.test.ts, leaving the
remaining "happy paths" and "diagnostics" describe blocks in the original
interpreter.polymorphism.test.ts; ensure both new files retain the helper
functions interpret, interpretOk, mongoScalarTypeDescriptors, and
mongoCodecLookup and import statements (so tests run independently) and update
test runner/exports if needed.
In `@packages/2-sql/2-authoring/contract-psl/src/provider.ts`:
- Around line 70-72: The spread of context.composedExtensionPacks is unnecessary
and causes an extra allocation; where the code builds the parameter (currently
using [...context.composedExtensionPacks] or undefined), pass
context.composedExtensionPacks directly instead (i.e., use
context.composedExtensionPacks) because both the source and the target parameter
are typed readonly string[]; update the call site in provider.ts to remove the
spread and keep the conditional that yields undefined when length is 0.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 45efd16f-6e0e-4248-a25f-be9b070b0b51
📒 Files selected for processing (8)
packages/1-framework/1-core/framework-components/src/control-stack.tspackages/1-framework/1-core/framework-components/test/control-stack.test.tspackages/2-mongo-family/2-authoring/contract-psl/src/derive-json-schema.tspackages/2-mongo-family/2-authoring/contract-psl/src/interpreter.tspackages/2-mongo-family/2-authoring/contract-psl/test/derive-json-schema.test.tspackages/2-mongo-family/2-authoring/contract-psl/test/interpreter.polymorphism.test.tspackages/2-sql/2-authoring/contract-psl/src/provider.tspackages/2-sql/2-authoring/contract-psl/test/provider.test.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/1-framework/1-core/framework-components/test/control-stack.test.ts
- packages/2-mongo-family/2-authoring/contract-psl/test/derive-json-schema.test.ts
- packages/2-sql/2-authoring/contract-psl/test/provider.test.ts
- packages/1-framework/1-core/framework-components/src/control-stack.ts
The psl prefix implied this map is specific to the PSL authoring format, but it is a general-purpose scalar type name to codec ID mapping consumed by both SQL and Mongo interpreters. Drop the prefix to reflect this. Renames: assemblePslScalarTypeDescriptors -> assembleScalarTypeDescriptors, validatePslScalarTypeCodecIds -> validateScalarTypeCodecIds, createPostgresPslScalarTypeDescriptors -> createPostgresScalarTypeDescriptors.
Add undefined guard for optional-chained oneOf array access to satisfy strict null checks in CI.
When skipping unresolvable codecs was introduced, the narrowing from a non-null codec lookup no longer guaranteed targetTypes[0] was defined under noUncheckedIndexedAccess. Skip descriptors whose codec has no target types instead of assigning undefined to nativeType.
wmadden
deleted the
tml-2247-migration-planner-bugs-variant-collections-and-float-type-fl
branch
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.
closes TML-2247
Intent
Fix three MongoDB migration planner bugs and eliminate their root cause by lifting PSL contribution assembly to the framework level. After this work,
ContractSourceContextcarries all assembled contributions (scalar type descriptors, authoring contributions, mutation defaults, codec lookup), providers read from context instead of manual options, and user configs no longer perform manual assembly.Key snippet
New —
ContractSourceContextnow carries all assembled contributionsChange map
ComponentMetadatagainspslScalarTypeDescriptorsandcontrolMutationDefaultsassemblePslScalarTypeDescriptors,assembleControlMutationDefaultsContractSourceContextoneOfpslScalarTypeDescriptorsincluding FloattargetTypes: ['double']The story
Lift mutation default types to the framework. Move
DefaultFunctionRegistryEntry,MutationDefaultGeneratorDescriptor,ControlMutationDefaultsand related types from the SQL authoring layer topackages/1-framework/1-core/framework-components/src/mutation-default-types.ts. These types are family-agnostic (they reference codec IDs, not SQL-specific constructs).Enrich
ComponentMetadataandContractSourceContext. Add optionalpslScalarTypeDescriptors(Map<string, string>— PSL name → codec ID) andcontrolMutationDefaultstoComponentMetadata. AddpslScalarTypeDescriptors,authoringContributions,codecLookup, andcontrolMutationDefaultstoContractSourceContext. This makes the framework carry everything a contract source provider needs.Assemble contributions in
createControlStack. AddassemblePslScalarTypeDescriptorsandassembleControlMutationDefaultswith duplicate detection (same pattern as existingassembleAuthoringContributions).createControlStacknow populates all contribution fields.Reorder CLI to build stack first. Move
createControlStack(config)before the source provider invocation inexecuteContractEmit, and pass the assembled contributions to the provider viaContractSourceContext. Previously the stack was built after the source provider returned.Migrate Postgres adapter to framework-level fields. Replace
pslTypeDescriptors()andcontrolMutationDefaults()method hooks (fromSqlControlStaticContributions) with directpslScalarTypeDescriptorsandcontrolMutationDefaultsdata fields on the adapter descriptor. Scalar type descriptors simplify from{ codecId, nativeType }to just codec ID —nativeTypeis derived fromcodecLookup.get(codecId).targetTypes[0].Update SQL provider to read from context.
prismaContract()readspslScalarTypeDescriptors,authoringContributions,controlMutationDefaults, andcodecLookupfromContractSourceContextinstead ofPrismaContractOptions. DerivesnativeTypefrom codec lookup via abuildColumnDescriptorMaphelper.Fix FL-09: suppress variant collections. In
resolvePolymorphism, filter out collection entries for variant models that share their base's collection. Merge variant-specific indexes into the base collection's index list.Fix FL-10: compose polymorphic validators. Move validator generation to after polymorphism resolution. For discriminated base models, call the new
derivePolymorphicJsonSchemawhich builds aoneOf-based validator with base fields at the top level and variant-specific fields in discriminated branches.Fix FL-11: derive BSON types from codec lookup. Replace the hardcoded
CODEC_TO_BSON_TYPEmap withresolveBsonType— resolves the codec viacodecLookup.get(codecId)and readstargetTypes[0]. Fix the Mongo double codec'stargetTypesfrom['float']to['double']. AddFloat → mongo/double@1to the Mongo adapter'spslScalarTypeDescriptors.Wire Mongo provider to context.
mongoContract()readspslScalarTypeDescriptorsandcodecLookupfromContractSourceContext. ThescalarTypeDescriptorsoption is removed.Eliminate old SQL infrastructure. Delete
SqlControlStaticContributions,PslScalarTypeDescriptor,ControlMutationDefaultFunctionEntry, andControlMutationDefaultGeneratorDescriptorfromtypes.ts. DeleteassemblePslInterpretationContributions()andassembleControlMutationDefaultContributions()fromassembly.ts. Remove their exports from@prisma-next/family-sql/control. Rewritemutation-default-assembly.test.tsto test via framework-levelassembleControlMutationDefaults.Delete hardcoded Mongo descriptors. Delete
scalar-type-descriptors.tsandcreateMongoScalarTypeDescriptors(). Remove the backward-compat fallback frommongoContract()— the provider now trustsContractSourceContextunconditionally.Simplify example configs. Remove all manual assembly (
assembleAuthoringContributions,assemblePslInterpretationContributions,createMongoScalarTypeDescriptors) fromprisma-next-demoandretail-storeconfigs.Update MongoDB Family subsystem doc. Document schema validation (
$jsonSchema), polymorphic validator structure, BSON type derivation from codec metadata, and variant collection suppression.Behavior changes & evidence
FL-09 — Variant models sharing a base collection no longer produce separate
storage.collectionsentries: Before, the interpreter created a collection entry for every model regardless of polymorphism. After, variant models with@@baseare suppressed and their indexes merge into the base collection.createCollectionoperations for variant models that share the base's collection.resolvePolymorphism(L279–292)FL-10 — Polymorphic collection validators use
oneOfdiscriminator pattern: Before, the validator was overwritten per-model (last-writer-wins). After, base fields appear at the top level, and variant-specific fields are grouped intooneOfbranches keyed by discriminator value.derivePolymorphicJsonSchema(L94–155)FL-11 — Float fields produce
{ bsonType: "double" }via codec lookup: Before, BSON types were resolved from a hardcoded map that didn't include Float. After, BSON types are resolved fromcodecLookup.get(codecId).targetTypes[0], and the Mongo adapter declaresFloat → mongo/double@1.resolveBsonType(L5–11)targetTypes: ['double'](L31)['Float', 'mongo/double@1'](L43)Contributions flow through
ContractSourceContextinstead of manual options (no behavior change externally): The CLI buildsControlStackbefore calling the source provider and passes assembled contributions via context. Providers read from context. Example configs no longer call assembly functions.assemblePslScalarTypeDescriptors,assembleControlMutationDefaultsOld SQL contribution infrastructure eliminated (refactor, no behavior change):
SqlControlStaticContributions,PslScalarTypeDescriptor,assemblePslInterpretationContributions(),assembleControlMutationDefaultContributions(), and SQL-specific mutation default types are all deleted. The framework-level assembly increateControlStackis now the single path.Mongo double codec
targetTypescorrected:['float']→['double']. This is a BSON type name, anddoubleis the correct BSON type.Compatibility / migration / risk
storageHashvalues because variant collection entries are no longer present. This is expected per the spec's NFR.pslTypeDescriptors()andcontrolMutationDefaults()methods replaced bypslScalarTypeDescriptorsandcontrolMutationDefaultsdata fields. Any code that called these as methods will break. This is intentional.PrismaContractOptionsno longer acceptsscalarTypeDescriptors,authoringContributions,controlMutationDefaults, orcomposedExtensionPacks— these are read from context.MongoContractOptionsno longer acceptsscalarTypeDescriptors— read from context.SqlControlStaticContributions,PslScalarTypeDescriptor,assemblePslInterpretationContributions(),assembleControlMutationDefaultContributions()are deleted — consumers must use framework-level assembly viacreateControlStack.Follow-ups / open questions
SourceSpan/SourceDiagnostictype duplication — The newmutation-default-types.tsdefinesSourceSpanandSourceDiagnosticthat are structurally equivalent toPslSpanandContractSourceDiagnostic. Consider unifying.ColumnDescriptorretainsnativeType— The outputColumnDescriptortype keepsnativeType,typeRef, andtypeParamsas output fields from resolution paths (enums,@db.*, type constructors, generators). Only the input scalar type descriptor is simplified.Non-goals / intentionally out of scope
Long,Decimal,Bytes)Summary by CodeRabbit
New Features
Refactor
Tests
Documentation