tml-2912: PSL native scalar lists (author, migrate, infer end-to-end)#870
Open
SevInf wants to merge 11 commits into
Open
tml-2912: PSL native scalar lists (author, migrate, infer end-to-end)#870SevInf wants to merge 11 commits into
SevInf wants to merge 11 commits into
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 threads a ChangesCapability-gated native scalar-list support
CHECK constraint polymorphism (valueSet vs expression)
Telemetry-backend contract regeneration
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant ControlStack
participant CliClient
participant PslInterpreter
participant FieldResolution
ControlStack->>ControlStack: mergeCapabilityMatrices(target, adapter, extensionPacks)
CliClient->>PslInterpreter: source.load({ capabilities: stack.capabilities })
PslInterpreter->>FieldResolution: collectResolvedFields({ capabilities })
FieldResolution-->>PslInterpreter: ok (native array field) or PSL_SCALAR_LIST_UNSUPPORTED_TARGET
sequenceDiagram
participant PlannerStrategies
participant AddCheckConstraintCall
participant ConstraintsOp
participant Postgres
PlannerStrategies->>PlannerStrategies: projectContractChecks + checkPayload
PlannerStrategies->>AddCheckConstraintCall: new AddCheckConstraintCall(payload)
AddCheckConstraintCall->>ConstraintsOp: addCheckConstraint(payload)
ConstraintsOp->>Postgres: ALTER TABLE ... ADD CONSTRAINT ... CHECK(predicate)
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
@prisma-next/extension-author-tools
@prisma-next/mongo-runtime
@prisma-next/family-mongo
@prisma-next/sql-runtime
@prisma-next/family-sql
@prisma-next/extension-arktype-json
@prisma-next/middleware-cache
@prisma-next/mongo
@prisma-next/extension-paradedb
@prisma-next/extension-pgvector
@prisma-next/extension-postgis
@prisma-next/postgres
@prisma-next/sql-orm-client
@prisma-next/sqlite
@prisma-next/extension-supabase
@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/config-loader
@prisma-next/emitter
@prisma-next/language-server
@prisma-next/migration-tools
prisma-next
@prisma-next/vite-plugin-contract-emit
@prisma-next/mongo-codec
@prisma-next/mongo-contract
@prisma-next/mongo-value
@prisma-next/mongo-contract-psl
@prisma-next/mongo-contract-ts
@prisma-next/mongo-emitter
@prisma-next/mongo-schema-ir
@prisma-next/mongo-query-ast
@prisma-next/mongo-orm
@prisma-next/mongo-query-builder
@prisma-next/mongo-lowering
@prisma-next/mongo-wire
@prisma-next/sql-contract
@prisma-next/sql-errors
@prisma-next/sql-operations
@prisma-next/sql-schema-ir
@prisma-next/sql-contract-psl
@prisma-next/sql-contract-ts
@prisma-next/sql-contract-emitter
@prisma-next/sql-lane-query-builder
@prisma-next/sql-relational-core
@prisma-next/sql-builder
@prisma-next/target-postgres
@prisma-next/target-sqlite
@prisma-next/adapter-postgres
@prisma-next/adapter-sqlite
@prisma-next/driver-postgres
@prisma-next/driver-sqlite
commit: |
size-limit report 📦
|
SevInf
commented
Jul 1, 2026
| ...ifDefined( | ||
| 'many', | ||
| resolvedField.many && resolvedField.valueObjectTypeName === undefined | ||
| ? (true as const) |
Contributor
Author
There was a problem hiding this comment.
Wtf is this? Why not just many: Boolean(resolvedField.many && resolvedField.valueObjectTypeName === undefined)?
SevInf
commented
Jul 1, 2026
SevInf
force-pushed
the
scalar-lists-slice2
branch
from
80a4be0 to
0760919
Compare
SevInf
commented
Jul 1, 2026
SevInf
commented
Jul 1, 2026
SevInf
commented
Jul 1, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
test/integration/test/scalar-lists/psl-list-roundtrip.integration.test.ts (2)
1-15: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMilestone acceptance-criteria codes (
AC1,AC2/NFR1,NFR1) in comments/test titles.Same doc-maintenance concern as the Mongo-parity test: these acceptance-criteria labels are transient and will become stale after the milestone closes.
Based on learnings, the doc-maintenance rule "violations are limited to transient artifacts such as... milestone acceptance-criteria codes like
AM12/AC-13" —AC1/AC2/NFR1match this pattern and should be flagged.Also applies to: 116-116, 185-185
🤖 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 `@test/integration/test/scalar-lists/psl-list-roundtrip.integration.test.ts` around lines 1 - 15, Remove the transient acceptance-criteria labels from the `psl-list-roundtrip.integration.test.ts` documentation and test titles, including the `AC1`, `AC2/NFR1`, and `NFR1` references in the top comment. Keep the description focused on the scalar-list roundtrip behavior itself, and update any related comments or titles in this test file so they no longer mention milestone-specific codes.Source: Learnings
131-136: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer separate
expect()calls over multi-fieldtoMatchObject().Several assertions bundle multiple fields into a single
toMatchObject()(e.g.codecId/nativeType/manyat 131-135,name/typeName/list/optionalat 171-176, and the three list-column checks at 209-220). A failure only reports the object diff rather than pinpointing which field broke. Line 136's separatenativeTypecheck is also redundant once line 133 already assertsnativeType: 'text'.Based on learnings, "prefer separate expect() assertions for each field instead of combining checks with toMatchObject() when validating multiple fields" for clearer, more actionable failure messages.
♻️ Example for lines 131-136
- expect(tagsColumn).toMatchObject({ - codecId: 'pg/text@1', - nativeType: 'text', - many: true, - }); - expect(tagsColumn?.['nativeType']).not.toBe('jsonb'); + expect(tagsColumn?.['codecId']).toBe('pg/text@1'); + expect(tagsColumn?.['nativeType']).toBe('text'); + expect(tagsColumn?.['many']).toBe(true);Also applies to: 171-176, 209-220
🤖 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 `@test/integration/test/scalar-lists/psl-list-roundtrip.integration.test.ts` around lines 131 - 136, The scalar list roundtrip test uses multi-field toMatchObject() assertions in the affected blocks, which makes failures harder to pinpoint; split these into separate expect() calls for each field in the relevant test cases. Update the assertions around tagsColumn and the other list-column checks to validate codecId, nativeType, many, name, typeName, list, and optional individually, and remove the redundant nativeType-only check where the same field is already asserted in the object match.Source: Learnings
test/integration/test/scalar-lists/psl-list-mongo-parity.integration.test.ts (1)
1-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMilestone acceptance-criteria codes (
NFR4,AC2) in comments.These are transient acceptance-criteria references rather than stable identifiers (like
TML-*tickets or ADR references).Based on learnings, the doc-maintenance rule flags "milestone acceptance-criteria codes like
AM12/AC-13" while exempting stableTML-*/ADR references —NFR4/AC2fall into the flagged category.🤖 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 `@test/integration/test/scalar-lists/psl-list-mongo-parity.integration.test.ts` around lines 1 - 18, The top-of-file comment in psl-list-mongo-parity.integration.test.ts uses transient milestone acceptance-criteria codes (`NFR4`, `AC2`) that should not appear in docs/comments. Update the comment in this test file to remove those codes and replace them with stable references only, keeping the rest of the parity explanation intact.Source: Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@test/integration/test/scalar-lists/psl-list-mongo-parity.integration.test.ts`:
- Around line 1-18: The top-of-file comment in
psl-list-mongo-parity.integration.test.ts uses transient milestone
acceptance-criteria codes (`NFR4`, `AC2`) that should not appear in
docs/comments. Update the comment in this test file to remove those codes and
replace them with stable references only, keeping the rest of the parity
explanation intact.
In `@test/integration/test/scalar-lists/psl-list-roundtrip.integration.test.ts`:
- Around line 1-15: Remove the transient acceptance-criteria labels from the
`psl-list-roundtrip.integration.test.ts` documentation and test titles,
including the `AC1`, `AC2/NFR1`, and `NFR1` references in the top comment. Keep
the description focused on the scalar-list roundtrip behavior itself, and update
any related comments or titles in this test file so they no longer mention
milestone-specific codes.
- Around line 131-136: The scalar list roundtrip test uses multi-field
toMatchObject() assertions in the affected blocks, which makes failures harder
to pinpoint; split these into separate expect() calls for each field in the
relevant test cases. Update the assertions around tagsColumn and the other
list-column checks to validate codecId, nativeType, many, name, typeName, list,
and optional individually, and remove the redundant nativeType-only check where
the same field is already asserted in the object match.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: eccfbb64-0cee-4b45-9564-37d381af9a13
📒 Files selected for processing (34)
apps/telemetry-backend/src/prisma/contract.d.tsapps/telemetry-backend/src/prisma/contract.jsonpackages/1-framework/1-core/config/src/contract-source-types.tspackages/1-framework/1-core/framework-components/src/control/control-stack.tspackages/1-framework/1-core/framework-components/src/exports/components.tspackages/1-framework/1-core/framework-components/src/shared/capabilities.tspackages/1-framework/3-tooling/cli/src/control-api/client.tspackages/1-framework/3-tooling/cli/src/control-api/operations/contract-emit.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/interpreter.capability-gating.test.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.diagnostics.test.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.value-objects.test.tspackages/2-sql/2-authoring/contract-ts/src/build-contract.tspackages/2-sql/2-authoring/contract-ts/test/scalar-list-output-types.test-d.tspackages/2-sql/3-tooling/emitter/src/index.tspackages/2-sql/3-tooling/emitter/test/emitter-hook.storage-column-types.test.tspackages/2-sql/4-lanes/relational-core/src/ast/ddl-types.tspackages/2-sql/4-lanes/relational-core/src/contract-free/column.tspackages/2-sql/4-lanes/relational-core/src/exports/contract-free.tspackages/3-targets/3-targets/postgres/src/core/default-normalizer.tspackages/3-targets/3-targets/postgres/src/core/migrations/issue-planner.tspackages/3-targets/3-targets/postgres/src/core/migrations/op-factory-call.tspackages/3-targets/3-targets/postgres/src/exports/migration.tspackages/3-targets/3-targets/postgres/test/default-normalizer.test.tspackages/3-targets/3-targets/sqlite/src/core/migrations/op-factory-call.tspackages/3-targets/6-adapters/postgres/src/core/control-adapter.tspackages/3-targets/6-adapters/postgres/test/migrations/native-array-columns.integration.test.tspackages/3-targets/6-adapters/sqlite/src/core/control-adapter.tstest/integration/test/scalar-lists/psl-list-authoring.tstest/integration/test/scalar-lists/psl-list-mongo-parity.integration.test.tstest/integration/test/scalar-lists/psl-list-roundtrip.integration.test.ts
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/2-sql/2-authoring/contract-psl/test/interpreter.relations.test.ts (1)
15-21: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider centralizing the
capabilitiesliteral.
capabilities: { sql: { scalarList: true } }is now duplicated verbatim across a large number of test files in this package (per cross-file evidence:interpreter.extensions.test.ts,interpreter.enum.test.ts,interpreter.types.test.ts,interpreter.namespaces.test.ts,ts-psl-parity.test.ts,psl-ts-namespace-parity.test.ts,interpreter.control-policy.test.ts, etc.). Sincecontract-psl/test/fixtures.tsalready exists as a shared test fixtures module, extracting a single exported constant (e.g.,DEFAULT_SCALAR_LIST_CAPABILITIES) there and importing it would reduce copy/paste risk if the capability shape changes again.🤖 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/2-sql/2-authoring/contract-psl/test/interpreter.relations.test.ts` around lines 15 - 21, Centralize the repeated sql scalar-list capability literal used in the contract-psl tests. Move the shared object from the local test setup in interpreter.relations.test.ts into the existing shared fixtures module contract-psl/test/fixtures.ts as a named exported constant such as DEFAULT_SCALAR_LIST_CAPABILITIES, then import and reuse it from baseInput and the other affected interpreter/parity tests. Keep the shape identical so callers still pass the same capabilities value, but remove the duplicate inline literal everywhere.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/2-sql/2-authoring/contract-psl/test/interpreter.relations.test.ts`:
- Around line 15-21: Centralize the repeated sql scalar-list capability literal
used in the contract-psl tests. Move the shared object from the local test setup
in interpreter.relations.test.ts into the existing shared fixtures module
contract-psl/test/fixtures.ts as a named exported constant such as
DEFAULT_SCALAR_LIST_CAPABILITIES, then import and reuse it from baseInput and
the other affected interpreter/parity tests. Keep the shape identical so callers
still pass the same capabilities value, but remove the duplicate inline literal
everywhere.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: c97aceb4-5845-4be6-9105-85894eab5086
📒 Files selected for processing (46)
packages/1-framework/1-core/config/src/contract-source-types.tspackages/1-framework/2-authoring/psl-parser/src/exports/index.tspackages/1-framework/2-authoring/psl-parser/src/resolve.tspackages/1-framework/2-authoring/psl-parser/src/symbol-table.tspackages/1-framework/3-tooling/cli/test/config-types.test.tspackages/2-mongo-family/2-authoring/contract-psl/test/provider.test.tspackages/2-mongo-family/2-authoring/contract-ts/test/config-types.test.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-attribute-parsing.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/fixtures.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.capability-gating.test.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.control-policy.test.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.defaults.test.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.diagnostics.test.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.enum.test.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.extensions.test.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.namespaces.test.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.polymorphism.test.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.relations.many-to-many.test.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.relations.test.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.test.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.types.test.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.value-objects.test.tspackages/2-sql/2-authoring/contract-psl/test/psl-ts-namespace-parity.test.tspackages/2-sql/2-authoring/contract-psl/test/ts-psl-parity.test.tspackages/2-sql/2-authoring/contract-ts/test/config-types.test.tspackages/3-extensions/postgres/test/psl-namespace-qualifier-routing.test.tspackages/3-targets/3-targets/postgres/test/psl-policy-authoring.test.tspackages/3-targets/6-adapters/postgres/test/migrations/rls-lifecycle-e2e.integration.test.tspackages/3-targets/6-adapters/postgres/test/migrations/rls-migration-plan.integration.test.tspackages/3-targets/6-adapters/postgres/test/migrations/rls-walking-skeleton-psl.integration.test.tstest/integration/test/authoring/cli.emit-parity-fixtures.test.tstest/integration/test/authoring/parity/ts-psl-parity.real-packs.test.tstest/integration/test/authoring/psl-index-type-options.integration.test.tstest/integration/test/authoring/psl.pgvector-dbinit.test.tstest/integration/test/authoring/side-by-side-contracts.test.tstest/integration/test/cli.emit-command.additional.test.tstest/integration/test/cli.emit-contract.test.tstest/integration/test/scalar-lists/psl-list-authoring.tstest/integration/test/scalar-lists/psl-list-mongo-parity.integration.test.tstest/integration/test/scalar-lists/psl-list-roundtrip.integration.test.tstest/integration/test/value-objects/value-objects.integration.test.ts
✅ Files skipped from review due to trivial changes (3)
- packages/2-mongo-family/2-authoring/contract-ts/test/config-types.test.ts
- packages/2-sql/2-authoring/contract-psl/test/interpreter.relations.many-to-many.test.ts
- packages/3-extensions/postgres/test/psl-namespace-qualifier-routing.test.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/2-sql/2-authoring/contract-psl/src/provider.ts
- test/integration/test/scalar-lists/psl-list-mongo-parity.integration.test.ts
- packages/2-sql/2-authoring/contract-psl/test/interpreter.capability-gating.test.ts
- packages/2-sql/2-authoring/contract-psl/test/interpreter.value-objects.test.ts
- test/integration/test/scalar-lists/psl-list-roundtrip.integration.test.ts
- test/integration/test/scalar-lists/psl-list-authoring.ts
SevInf
commented
Jul 1, 2026
SevInf
commented
Jul 1, 2026
SevInf
added a commit
that referenced
this pull request
Jul 1, 2026
…#870 The PR touches one packages/3-extensions/ test file (threading the now- required capabilities matrix into a test ContractSourceContext), which trips the check:upgrade-coverage per-pr-declaration gate. Record it as an incidental substrate diff: the affected types are framework-internal contract-emission seams the control stack always populates; no extension-author API changed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/1-framework/2-authoring/psl-parser/src/resolve.ts (1)
73-80: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer
ifDefinedover ternary-based conditional spreads.Both
...(name !== undefined ? { name } : {})and the new...(expression !== undefined ? { expression } : {})match the ternary-spread anti-pattern the codebase has previously preferred to avoid in favor ofifDefined.Based on learnings, "prefer
ifDefinedfromprisma-next/utils/definedfor conditional object spreads instead of inline ternary-based spreads... to avoid '{...cond ? {a: b} : {}}'-style patterns."♻️ Proposed refactor
+import { ifDefined } from '`@prisma-next/utils/defined`'; ... args.push({ kind: name !== undefined ? 'named' : 'positional', - ...(name !== undefined ? { name } : {}), + ...ifDefined('name', name), value: renderExpression(expression), - ...(expression !== undefined ? { expression } : {}), + ...ifDefined('expression', expression), span: nodePslSpan(arg.syntax, sourceFile), });🤖 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/2-authoring/psl-parser/src/resolve.ts` around lines 73 - 80, Replace the ternary-based conditional spreads in resolve.ts with the shared ifDefined helper from prisma-next/utils/defined. In the args.push object inside the expression handling logic, keep the existing kind/value/span fields but use ifDefined for the optional name and expression properties instead of inline `...(cond ? { ... } : {})` spreads. This should preserve the same behavior while matching the codebase’s preferred pattern.Source: Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/1-framework/2-authoring/psl-parser/src/resolve.ts`:
- Around line 73-80: Replace the ternary-based conditional spreads in resolve.ts
with the shared ifDefined helper from prisma-next/utils/defined. In the
args.push object inside the expression handling logic, keep the existing
kind/value/span fields but use ifDefined for the optional name and expression
properties instead of inline `...(cond ? { ... } : {})` spreads. This should
preserve the same behavior while matching the codebase’s preferred pattern.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 6cc50a32-f062-4cbd-aa0c-ff92ce98b17e
📒 Files selected for processing (3)
packages/1-framework/2-authoring/psl-parser/src/resolve.tspackages/2-sql/2-authoring/contract-psl/src/psl-attribute-parsing.tspackages/2-sql/2-authoring/contract-psl/src/psl-column-resolution.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/2-sql/2-authoring/contract-psl/src/psl-attribute-parsing.ts
SevInf
commented
Jul 1, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
packages/3-targets/6-adapters/postgres/src/core/control-adapter.ts (2)
893-900: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStale comment: scope note no longer matches the parser.
This comment still says only
= ANY (ARRAY[...])andIN (...)shapes are recognized, butparseCheckConstraintDefnow also recognizes thearray_position(col, NULL) IS NULLelement-non-null expression shape.✏️ Suggested comment update
- // Scope: only parses the `= ANY (ARRAY[...])` and `IN (...)` shapes - // that this slice emits. Arbitrary SQL predicates are left as-is - // and will not produce check IR entries (they are silently skipped). + // Scope: only parses the `= ANY (ARRAY[...])`, `IN (...)`, and + // `array_position(col, NULL) IS NULL` shapes that this slice emits. + // Arbitrary SQL predicates are left as-is and will not produce check + // IR entries (they are silently skipped).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/3-targets/6-adapters/postgres/src/core/control-adapter.ts` around lines 893 - 900, Update the stale scope comment near parseCheckConstraintDef to reflect all supported CHECK predicate shapes, not just = ANY (ARRAY[...]) and IN (...). Add the array_position(col, NULL) IS NULL form to the parser scope note so the comment matches the current behavior in the control-adapter logic.
1119-1141: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStale comment: "two shapes" / "column + value set" no longer accurate.
Same drift as the query comment above — the loop now branches on
valueSetvsexpressionkinds.✏️ Suggested comment update
- // Process check constraints — parse each predicate into column + value set. - // Only the two shapes emitted by this slice are recognised; free-form - // predicates are silently skipped (they won't produce check IR entries). + // Process check constraints — parse each predicate into a valueSet or + // expression check. Only the shapes emitted by this slice are + // recognised; free-form predicates are silently skipped (they won't + // produce check IR entries).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/3-targets/6-adapters/postgres/src/core/control-adapter.ts` around lines 1119 - 1141, Update the stale inline comment in control-adapter.ts around checksForTable parsing so it matches the current logic in parseCheckConstraintDef and the valueSet/expression branches. Replace the outdated “two shapes” / “column + value set” wording with a comment that reflects both supported parsed kinds, and keep the wording aligned with the existing identifiers checksForTable, parseCheckConstraintDef, valueSet, and expression.packages/2-sql/9-family/src/core/migrations/contract-to-schema-ir.ts (1)
233-266: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffPostgres-specific SQL syntax hardcoded in the shared "9-family" layer.
elementNonNullCheckExpression'sarray_position(...) IS NULLpredicate is a Postgres physical detail (as the docstring itself admits), but it's synthesized unconditionally insideprojectContractChecks/convertTable, which lives in the SQL-family-shared layer, not a target-specific adapter. The sameconvertTablesignature already acceptsexpandNativeType/renderDefaultadapter hooks for exactly this kind of target-specific customization — the new check-expression synthesis doesn't follow that pattern and would need to be re-derived if a non-Postgres SQL target is ever added to this family.Is another SQL dialect target planned for the "2-sql" family in the near term, or is Postgres expected to remain the sole SQL target? If the former, consider threading an injectable check-expression renderer analogous to
NativeTypeExpander/DefaultRenderer.
As per coding guidelines, "Don't branch on target; use adapters:.agents/rules/no-target-branches.mdc."🤖 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/2-sql/9-family/src/core/migrations/contract-to-schema-ir.ts` around lines 233 - 266, The check-constraint projection in projectContractChecks is hardcoding a Postgres-only expression inside the shared SQL-family layer. Move the element-non-null check generation behind an injectable adapter, similar to NativeTypeExpander and DefaultRenderer in convertTable, and thread it through projectContractChecks/convertTable so target-specific renderers can supply the expression instead of synthesizing array_position(...) unconditionally. Keep the existing check name wiring via elementNonNullCheckName, but make elementNonNullCheckExpression target-provided rather than shared-layer fixed.packages/2-sql/9-family/src/core/schema-verify/verify-helpers.ts (1)
698-702: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSilent fallback on unrecognized check kind.
liveCheckLabelreturns''ifcheckis neitherSqlValueSetCheckIRnorSqlExpressionCheckIR. With only two subclasses currently, this is unreachable today, but if a third check kind is added later without updating this helper, mismatch/missing messages would silently show an empty label instead of surfacing the gap.🛡️ Optional: fail loudly instead of returning an empty label
function liveCheckLabel(check: SqlCheckConstraintIR): string { if (check instanceof SqlValueSetCheckIR) return check.permittedValues.join(', '); if (check instanceof SqlExpressionCheckIR) return check.expression; - return ''; + throw new Error(`Unrecognized check constraint kind for "${check.name}"`); }🤖 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/2-sql/9-family/src/core/schema-verify/verify-helpers.ts` around lines 698 - 702, The liveCheckLabel helper currently falls back to an empty string for unknown SqlCheckConstraintIR variants, which can hide missing support when new check types are added. Update liveCheckLabel in verify-helpers.ts to fail loudly on unrecognized kinds instead of returning a silent default, so any future SqlValueSetCheckIR/SqlExpressionCheckIR mismatch is surfaced during development.
🤖 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/2-sql/9-family/src/core/migrations/contract-to-schema-ir.ts`:
- Around line 214-216: The generated check name from elementNonNullCheckName can
exceed PostgreSQL’s 63-byte identifier limit, causing truncation and drift
between projection and introspection. Update this helper to deterministically
bound the returned name to Postgres-safe length, using a stable hash/truncation
scheme so the emitted constraint name always matches what introspection will
see. Keep the fix localized to elementNonNullCheckName and ensure any callers
relying on the name continue to get a stable, reproducible result.
In
`@packages/3-targets/3-targets/postgres/test/migrations/planner.check-constraints.test.ts`:
- Around line 447-459: In the planner check-constraints test for
checkConstraintPlanCallStrategy, make the assertion require a successful match
before verifying there are zero calls. Update the test around the existing
result handling so it explicitly fails on a no_match outcome and only checks
result.calls after confirming the strategy matched; this ensures regressions in
element-check matching are caught. Use the checkConstraintPlanCallStrategy and
result.kind branches in the current test to locate the change.
---
Nitpick comments:
In `@packages/2-sql/9-family/src/core/migrations/contract-to-schema-ir.ts`:
- Around line 233-266: The check-constraint projection in projectContractChecks
is hardcoding a Postgres-only expression inside the shared SQL-family layer.
Move the element-non-null check generation behind an injectable adapter, similar
to NativeTypeExpander and DefaultRenderer in convertTable, and thread it through
projectContractChecks/convertTable so target-specific renderers can supply the
expression instead of synthesizing array_position(...) unconditionally. Keep the
existing check name wiring via elementNonNullCheckName, but make
elementNonNullCheckExpression target-provided rather than shared-layer fixed.
In `@packages/2-sql/9-family/src/core/schema-verify/verify-helpers.ts`:
- Around line 698-702: The liveCheckLabel helper currently falls back to an
empty string for unknown SqlCheckConstraintIR variants, which can hide missing
support when new check types are added. Update liveCheckLabel in
verify-helpers.ts to fail loudly on unrecognized kinds instead of returning a
silent default, so any future SqlValueSetCheckIR/SqlExpressionCheckIR mismatch
is surfaced during development.
In `@packages/3-targets/6-adapters/postgres/src/core/control-adapter.ts`:
- Around line 893-900: Update the stale scope comment near
parseCheckConstraintDef to reflect all supported CHECK predicate shapes, not
just = ANY (ARRAY[...]) and IN (...). Add the array_position(col, NULL) IS NULL
form to the parser scope note so the comment matches the current behavior in the
control-adapter logic.
- Around line 1119-1141: Update the stale inline comment in control-adapter.ts
around checksForTable parsing so it matches the current logic in
parseCheckConstraintDef and the valueSet/expression branches. Replace the
outdated “two shapes” / “column + value set” wording with a comment that
reflects both supported parsed kinds, and keep the wording aligned with the
existing identifiers checksForTable, parseCheckConstraintDef, valueSet, and
expression.
🪄 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: 75719089-bc75-4683-95e4-a53cc7d09976
📒 Files selected for processing (23)
examples/prisma-next-demo/migrations/app/20260422T0720_initial/migration.tspackages/2-sql/1-core/schema-ir/src/exports/types.tspackages/2-sql/1-core/schema-ir/src/ir/sql-check-constraint-ir.tspackages/2-sql/1-core/schema-ir/src/ir/sql-table-ir.tspackages/2-sql/1-core/schema-ir/src/types.tspackages/2-sql/9-family/src/core/migrations/contract-to-schema-ir.tspackages/2-sql/9-family/src/core/schema-verify/verify-helpers.tspackages/2-sql/9-family/src/core/schema-verify/verify-sql-schema.tspackages/2-sql/9-family/src/exports/control.tspackages/2-sql/9-family/src/exports/schema-verify.tspackages/2-sql/9-family/test/schema-verify.check-constraints.test.tspackages/3-targets/3-targets/postgres/src/core/migrations/issue-planner.tspackages/3-targets/3-targets/postgres/src/core/migrations/op-factory-call.tspackages/3-targets/3-targets/postgres/src/core/migrations/operations/constraints.tspackages/3-targets/3-targets/postgres/src/core/migrations/planner-strategies.tspackages/3-targets/3-targets/postgres/src/core/migrations/postgres-migration.tspackages/3-targets/3-targets/postgres/src/core/schema-ir/postgres-table-ir.tspackages/3-targets/3-targets/postgres/test/migrations/op-factory-call.test.tspackages/3-targets/3-targets/postgres/test/migrations/planner.check-constraints.test.tspackages/3-targets/6-adapters/postgres/src/core/control-adapter.tspackages/3-targets/6-adapters/postgres/test/control-adapter.check-constraints.test.tspackages/3-targets/6-adapters/postgres/test/migrations/native-array-columns.integration.test.tsskills/upgrade/prisma-next-upgrade/upgrades/0.14-to-0.15/instructions.md
✅ Files skipped from review due to trivial changes (1)
- packages/2-sql/9-family/src/exports/control.ts
Comment on lines
+214
to
+216
| export function elementNonNullCheckName(tableName: string, columnName: string): string { | ||
| return `${tableName}_${columnName}_elem_not_null`; | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for existing length-truncation handling in sibling name generators
rg -n -A5 'function defaultIndexName' packages/2-sql/9-family/src/core/migrations/contract-to-schema-ir.ts
rg -n 'NAMEDATALEN|63' packages/2-sql packages/3-targets/3-targets/postgres --type=tsRepository: prisma/prisma-next
Length of output: 713
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the target file and inspect the relevant section.
ast-grep outline packages/2-sql/9-family/src/core/migrations/contract-to-schema-ir.ts --view expanded || true
sed -n '180,245p' packages/2-sql/9-family/src/core/migrations/contract-to-schema-ir.ts
echo '--- sql-utils ---'
sed -n '1,140p' packages/3-targets/3-targets/postgres/src/core/sql-utils.ts
echo '--- tests ---'
sed -n '1,120p' packages/3-targets/3-targets/postgres/test/sql-utils.test.tsRepository: prisma/prisma-next
Length of output: 12622
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find all name generators in the migration IR file and nearby checks.
rg -n 'function .*Name|checkName|indexName|constraint' packages/2-sql/9-family/src/core/migrations/contract-to-schema-ir.tsRepository: prisma/prisma-next
Length of output: 1106
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for integration tests around long identifiers and generated constraint names.
rg -n -A3 -B3 'elementNonNullCheckName|defaultIndexName|exceeds PostgreSQL' packages -g '*test*' -g '*spec*'Repository: prisma/prisma-next
Length of output: 840
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the Postgres-specific identifier helpers for exact handling.
sed -n '1,180p' packages/3-targets/3-targets/postgres/src/core/sql-utils.tsRepository: prisma/prisma-next
Length of output: 3669
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the exact lines around the elementNonNullCheckName implementation.
nl -ba packages/2-sql/9-family/src/core/migrations/contract-to-schema-ir.ts | sed -n '206,222p'Repository: prisma/prisma-next
Length of output: 196
Bound generated check names to PostgreSQL’s 63-byte limit. elementNonNullCheckName can produce names Postgres truncates, so the stored constraint name can differ from the expected one and trigger false drift. Hash/truncate the name here to keep projection and introspection aligned.
🤖 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/2-sql/9-family/src/core/migrations/contract-to-schema-ir.ts` around
lines 214 - 216, The generated check name from elementNonNullCheckName can
exceed PostgreSQL’s 63-byte identifier limit, causing truncation and drift
between projection and introspection. Update this helper to deterministically
bound the returned name to Postgres-safe length, using a stable hash/truncation
scheme so the emitted constraint name always matches what introspection will
see. Keep the fix localized to elementNonNullCheckName and ensure any callers
relying on the name continue to get a stable, reproducible result.
2ab97be to
283c5ba
Compare
…slice 2 D1) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
…ce 2 D2) Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
…pability (slice 2 D3) Thread a merged CapabilityMatrix (built via mergeCapabilityMatrices over [target, adapter, ...extensionPacks], the same merge enrichContract performs) from the ControlStack through ContractSourceContext into the PSL interpreter, and reject a scalar-list field whose target does not report sql.scalarList with PSL_SCALAR_LIST_UNSUPPORTED_TARGET. An absent matrix means do not gate, so non-adapter authoring paths stay valid; the CLI emit path always populates it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
… D1b) Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
…r PSL lists (slice 2 D4) Part A (FR13, AC8 PSL half): every native scalar-array column gets a deterministic CHECK (array_position(col, NULL) IS NULL) constraint named <table>_<column>_elem_not_null, emitted at the migration/DDL layer in the postgres issue-planner createTable path via a new CheckExpressionConstraint DDL node. Introspection-based verify skips non-enum check predicates, so the constraint round-trips without false drift and is invisible to infer. Part B (FR14, AC9): PSL array-typed @default lowering. @default([]) -> empty literal array (DEFAULT *{}*); @default(["a","b"]) -> literal array encoded element-wise against the element codec (build-contract encodeColumnDefault is now many-aware). A scalar literal default on a list field is rejected at authoring time with PSL_LIST_DEFAULT_NOT_ARRAY (closes slice-1 D2 carry-in). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
…ice 2 D5) AC1: a `posts.tags String[]` schema authored in PSL emits a native array storage column (pg/text@1, many:true, not jsonb), migrates onto a fresh Postgres database as text[] with no manual edits, and round-trips through `contract infer` back to a `tags String[]` PSL field. AC2/NFR1: DateTime[]/Bytes[]/Decimal[] list fields authored in PSL (not hand-built typed contracts) insert and select rows whose decoded element values deep-equal the originals, proving per-element codec application through the whole authored path. NFR4: the same list schema yields matching observable semantics on SQL and Mongo — both author cleanly, both generate a ReadonlyArray<...> domain type over the string element codec, and list element values round-trip. The live Mongo decode runs on mongodb-memory-server (CI; nixos binary download is local env-noise). Also hardens the introspection-side array-literal parser (parseArrayLiteralBody in default-normalizer.ts) so a quoted element containing a comma or an escaped quote round-trips through the migration-diff layer without spurious drift (slice-1 carry-in). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
…ing test (rebase on #864) Main #864 made SqlNamespace abstract and createNamespace required on InterpretPslDocumentToSqlContractInput. Thread createTestSqlNamespace through the three D3 capability-gating interpret calls, matching the sibling interpreter tests updated by that refactor. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
Respond to reviewer feedback on the scalar-list authoring path:
- Simplify the `many` emission to the file's own
`...(rf.many ? { many: true as const } : {})` idiom; `many` stays
omitted (never `false`) for non-list fields, keeping canonical
emission byte-stable.
- Consume the parser's structured expression AST for list-field
`@default([...])` instead of re-parsing a stringified form. The
PSL parser now decodes each attribute-arg expression into a
`ResolvedExpr` discriminated union (string/number/boolean/array/
object/call/identifier) exposed on `ResolvedAttributeArg`, and the
SQL column resolver reads it directly. Deletes the hand-rolled
`splitTopLevelArrayElements` tokenizer and the array-literal string
parsing; array/scalar/function-default behavior is preserved,
including quoted elements containing commas.
- Make the adapter capability matrix required end-to-end so the
scalar-list gate has no test-only undefined branch and fails closed:
an empty matrix rejects scalar lists. `ContractSourceContext`,
`InterpretPslDocumentToSqlContractInput`, and the internal
resolution inputs now require `capabilities`; production already
threads it via the control stack. Test call sites pass an explicit
matrix; the former "does not gate when no matrix is threaded" case
becomes a fail-closed empty-matrix rejection.
- Drop transient milestone codes from the scalar-list integration
test comments and titles.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
…#870 The PR touches one packages/3-extensions/ test file (threading the now- required capabilities matrix into a test ContractSourceContext), which trips the check:upgrade-coverage per-pr-declaration gate. Record it as an incidental substrate diff: the affected types are framework-internal contract-emission seams the control stack always populates; no extension-author API changed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
…teArg (slice 2 N1) Replace the decoded ResolvedExpr union with the parser ExpressionAst node directly on ResolvedAttributeArg, per code-owner review. List-default parsing narrows via the AST classes (ArrayLiteralAst/StringLiteralExprAst/...) instead of a bespoke decoded shape. The stringified value is kept, as it is still consumed by scalar/function-call/relation-name/authoring-arg parsing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
…nd ORM client Add a sibling to the DateTime[]/Bytes[]/Decimal[] roundtrip covering the plain scalar element types: author `tags String[]`/`scores Int[]` in PSL, migrate onto real Postgres, insert, and SELECT back, asserting element-wise decode for pg/text@1 and pg/int4@1. Add an ORM-client read-back test: over the same PSL-authored contract, read the native array columns through orm().<ns>.<model>.select(...).all(), proving the ORM projects and decodes scalar many:true columns as JS arrays (not just to-many relations). The PSL path yields a generic Contract<SqlStorage>, so accessors are index signatures; the narrow row-type array inference is covered by the emitted contract path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
SevInf
force-pushed
the
scalar-lists-slice2
branch
from
283c5ba to
d4c5290
Compare
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.
tml-2912 — PSL native scalar lists (slice 2 of native-scalar-arrays)
Flips the implicit gate slice 1 built behind: a user can now author SQL scalar lists in PSL (
tags String[]) and get them end to end — native Postgres array columns (text[]), element-wise codec round-trip, authoring diagnostics, capability gating, and the headlineposts.tags String[]milestone checkpoint (project AC1).Stacked on slice 1 (#846, merged). The load-bearing change is one line in the interpreter — replacing the JSONB fallback with the element descriptor +
many: true; the rest is making the authoring layer correct about lists.Dispatches
StorageColumnTypes/StorageColumnInputTypesmany-aware (the insert/select builders consume the storage maps; only the domain maps were wrapped before).@id/autoincrementon a list) with actionable diagnostics.scalarList(SQLite) is rejected; threads a mergedCapabilityMatrixto the interpreter via the existingmergeCapabilityMatrices.CHECK (array_position(col, NULL) IS NULL)(enforce-on-all) + array-typed@default([])/@default([...])validation; rejects a scalar default on a list.DateTime[]/Bytes[]/Decimal[]) + Mongo parity (NFR4).Acceptance criteria
AC1 (milestone round-trip), AC6 (FR3 diagnostics), AC8 PSL-half (non-null-element CHECK), AC9 (array defaults), AC10 (capability gating), AC2/NFR1 (element fidelity), NFR4 (Mongo parity), NFR2 (no scalar regression —
fixtures:checkbyte-clean). 9/9 PASS.Notes for reviewers
T?[](element-nullable) syntax, so every PSL-authored list column carries the non-null-element CHECK; noelementNullableIR flag was needed.IN(...)-only and can't expressarray_position(...), so D4 added a smallCheckExpressionConstraintDDL node (<table>_<col>_elem_not_null).--strictverify would false-flag the element-non-null CHECK as drift, because introspection doesn't parse it back. Default verify is non-strict and unaffected; the AC1 infer round-trip is clean (text[]→String[]).🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes