feat: close VP1 authoring parity with constructor-first PSL surface#325
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:
📝 WalkthroughWalkthroughPSL authoring shifts from attribute-style extension syntax (e.g., Changes
Sequence Diagram(s)sequenceDiagram
participant PSLParser as PSL Parser
participant Interpreter as PSL Interpreter
participant Authoring as Authoring Contributions
participant ContractBuilder as Contract Builder
PSLParser->>PSLParser: parse document, detect constructor calls
PSLParser->>Interpreter: emit AST with PslTypeConstructorCall
Interpreter->>Authoring: resolvePslTypeConstructorDescriptor(call, composed)
Authoring-->>Interpreter: descriptor or undefined (uncomposed)
Interpreter->>Authoring: instantiatePslTypeConstructor(call, descriptor)
Authoring-->>Interpreter: { codecId, nativeType, typeParams } or diagnostics
Interpreter->>ContractBuilder: populate namedTypeDescriptors / columns
ContractBuilder-->>Interpreter: assembled contract fragment
sequenceDiagram
participant Parser as Parser
participant Legacy as Legacy Attr Flow
participant New as Constructor Flow
participant Resolver as Resolution Pipeline
Legacy->>Resolver: `@pgvector.column(length:1536)` (attribute)
Resolver->>Resolver: legacy special-case path removed
New->>Parser: `pgvector.Vector(1536)` (constructor)
Parser->>Resolver: AST with typeConstructor
Resolver->>Authoring: lookup constructor in authoringContributions
Authoring-->>Resolver: validate args & instantiate descriptor
Resolver-->>Resolver: produce unified column/storage descriptor
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
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)
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/mongo-runtime
@prisma-next/family-mongo
@prisma-next/sql-runtime
@prisma-next/family-sql
@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: 7
🧹 Nitpick comments (2)
packages/2-sql/9-family/test/ts-psl-parity.real-packs.test.ts (1)
1-14: Move this parity spec to the integration test package.This file exercises multiple packages and reaches into sibling packages’
src/exports/*internals via relative imports. Keeping it here makes the suite depend on workspace layout instead of the integrated package surface.As per coding guidelines,
packages/**/test/**/*.ts: “Integration tests that depend on multiple packages should be placed in@prisma-next/integration-teststo avoid cyclic dependencies.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-sql/9-family/test/ts-psl-parity.real-packs.test.ts` around lines 1 - 14, Move this parity spec out of the package unit tests into the integration test package (e.g. `@prisma-next/integration-tests`) and update its imports so it only consumes public package entry points instead of reaching into sibling src/exports/* via relative paths; replace relative imports like ../../../3-extensions/pgvector/src/exports/control and ../../../3-targets/.../src/exports/pack with the published package exports (the packages that expose pgvectorControl/pgvectorPack, postgresControl/postgresPack, postgresAdapter, sqlFamilyControl/sqlFamilyPack), keep using symbols referenced in the file such as parsePslDocument, interpretPslDocumentToSqlContract, defineContract, assembleAuthoringContributions, and assemblePslInterpretationContributions, and adjust the integration-tests runner config (test globs) so this file runs from the integration-tests package.packages/1-framework/2-authoring/psl-parser/test/parser.test.ts (1)
115-193: Split constructor parsing coverage into its own spec file.
parser.test.tsis already well past the 500-line limit, and these constructor-expression cases are a clean standalone concern. Please move them into something likeparser.constructors.test.tsto keep the parser suite navigable.As per coding guidelines,
**/*.test.ts: “Keep test files under 500 lines... Split test files when they exceed 500 lines, contain multiple distinct concerns that can be logically separated...”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/1-framework/2-authoring/psl-parser/test/parser.test.ts` around lines 115 - 193, Move the two test cases titled "parses named type constructor expressions in types blocks" and "parses inline field constructor expressions" out of the large parser.test.ts into a new, focused test file (e.g., parser.constructors.test.ts); copy the two it(...) blocks along with any necessary imports/setup (including parsePslDocument usage) into the new file, remove them from the original file, and ensure the new file exports/runs under the test runner so assertions still execute. Ensure you keep the assertions and references to symbols like parsePslDocument, ShortName, Embedding1536, and the Document model intact and run the test suite to confirm no failures after the move.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/architecture` docs/adrs/ADR 112 - Target Extension Packs.md:
- Around line 66-67: The ADR uses constructor-first PSL wording in some bullets
but still refers to "attributes" later, causing inconsistency; update the later
bullets (the lines mentioning "attributes" in "ADR 112 - Target Extension Packs"
— e.g., the bullets that currently read like “Maps supported PSL extension
syntax...” and “Validates supported syntax usage...”) to use constructor-first
terminology (replace "attributes" with "constructors" or "constructor syntax"
and ensure examples use constructor form such as `pgvector.Vector(length:
1536)`), and make the wording consistent across the ADR so all references align
with constructor-first PSL terminology.
In `@docs/architecture` docs/adrs/ADR 170 - Pack-provided type constructors and
field presets.md:
- Around line 68-72: The rationale currently contradicts the ADR’s
recommendations by listing pgvector.Vector(1536) as an example of the current
“Today” problem while the ADR advocates for constructor-first syntax; revise the
paragraph that begins “Today, PSL handles…” to remove or rephrase the
pgvector.Vector(1536) example (or move it to the future/target examples) so it
only lists true current anti-patterns like String with extra attributes and
`@default`(uuid()), and make an explicit call-out in ADR 170 to treat
pgvector.Vector(...) as the constructor-first direction rather than a current
problem.
In `@docs/reference/capabilities.md`:
- Line 230: Replace the inconsistent slash-separated capability key
"pgvector/cosine" with the dot-separated namespaced format used elsewhere (e.g.,
change "pgvector/cosine" to "pgvector.cosine") so it matches canonical keys like
"sql.lateral" and "pgvector.ivfflat"; update any references in the document to
use "pgvector.cosine" to ensure capability checks and user configs remain
consistent.
In `@packages/1-framework/2-authoring/psl-parser/src/parser.ts`:
- Around line 439-490: parseTypeConstructorCall currently emits a hardcoded
PSL_INVALID_TYPES_MEMBER diagnostic and message, causing field-level constructor
errors to be misclassified; update parseTypeConstructorCall to accept (or
derive) context-specific diagnostic info and pass that through to pushDiagnostic
and to its call to parseArgumentList (i.e., replace the hardcoded
'PSL_INVALID_TYPES_MEMBER' and its "Invalid types declaration" / other messages
with parameters such as invalidCode and invalid* message templates), then update
callers (e.g., parseField) to pass the appropriate invalidCode/message strings
so pushDiagnostic and parseArgumentList produce model-field diagnostics instead
of types-member ones; use the existing identifiers parseTypeConstructorCall,
parseArgumentList, pushDiagnostic, and createInlineSpan to locate and change the
relevant spots.
In `@packages/2-sql/2-authoring/contract-psl/src/interpreter.ts`:
- Around line 1184-1195: The call to buildValueObjects can append errors to
diagnostics but those are ignored because the earlier diagnostics check already
ran; after calling buildValueObjects (the valueObjects assignment) re-check
diagnostics and return the same error path used earlier (i.e., mirror the
existing failure gate behavior that returns the error/failed Result instead of
continuing to produce ok(patchedContract)), so that any
composite-type/constructor/value-object failures cause the function to fail
rather than produce a partial patchedContract.
In `@packages/2-sql/2-authoring/contract-psl/src/psl-column-resolution.ts`:
- Around line 121-138: parsePslAuthoringArgumentValue currently ignores
descriptor.kind === 'object' and always returns INVALID_AUTHORING_ARGUMENT for
non-string/number/stringArray inputs; add a branch in
parsePslAuthoringArgumentValue to handle descriptor.kind === 'object' by parsing
the rawValue into a structured object (e.g., unquote the literal then safely
JSON.parse or use a small, robust parser), validate/normalize the resulting
object according to the expected descriptor shape, and return
INVALID_AUTHORING_ARGUMENT on parse/validation failure; if you need immutability
or deep-normalization after parsing, apply a vetted utility (e.g., Lodash's
cloneDeep/merge or a native lightweight approach) rather than ad hoc code.
- Around line 346-359: The logic in psl-column-resolution.ts currently treats
any dotted constructor namespace as an uncomposed extension and emits
PSL_EXTENSION_NAMESPACE_NOT_COMPOSED (see the block referencing input.call.path
and input.composedExtensions); update this to reuse checkUncomposedNamespace()
or explicitly ignore the reserved "db" namespace before pushing that diagnostic
so constructors like db.VarChar(10) do not suggest adding an extension pack but
instead fall through to the normal unsupported-constructor diagnostic (modify
the branch that checks namespace !== input.familyId && namespace !==
input.targetId && !input.composedExtensions.has(namespace) to call
checkUncomposedNamespace(namespace) or add a namespace === 'db' exclusion).
---
Nitpick comments:
In `@packages/1-framework/2-authoring/psl-parser/test/parser.test.ts`:
- Around line 115-193: Move the two test cases titled "parses named type
constructor expressions in types blocks" and "parses inline field constructor
expressions" out of the large parser.test.ts into a new, focused test file
(e.g., parser.constructors.test.ts); copy the two it(...) blocks along with any
necessary imports/setup (including parsePslDocument usage) into the new file,
remove them from the original file, and ensure the new file exports/runs under
the test runner so assertions still execute. Ensure you keep the assertions and
references to symbols like parsePslDocument, ShortName, Embedding1536, and the
Document model intact and run the test suite to confirm no failures after the
move.
In `@packages/2-sql/9-family/test/ts-psl-parity.real-packs.test.ts`:
- Around line 1-14: Move this parity spec out of the package unit tests into the
integration test package (e.g. `@prisma-next/integration-tests`) and update its
imports so it only consumes public package entry points instead of reaching into
sibling src/exports/* via relative paths; replace relative imports like
../../../3-extensions/pgvector/src/exports/control and
../../../3-targets/.../src/exports/pack with the published package exports (the
packages that expose pgvectorControl/pgvectorPack, postgresControl/postgresPack,
postgresAdapter, sqlFamilyControl/sqlFamilyPack), keep using symbols referenced
in the file such as parsePslDocument, interpretPslDocumentToSqlContract,
defineContract, assembleAuthoringContributions, and
assemblePslInterpretationContributions, and adjust the integration-tests runner
config (test globs) so this file runs from the integration-tests package.
🪄 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: d436120a-5d92-4c08-8579-41362cec02ee
📒 Files selected for processing (38)
docs/architecture docs/adrs/ADR 104 - PSL extension namespacing & syntax.mddocs/architecture docs/adrs/ADR 112 - Target Extension Packs.mddocs/architecture docs/adrs/ADR 170 - Pack-provided type constructors and field presets.mddocs/architecture docs/subsystems/1. Data Contract.mddocs/glossary.mddocs/products/psl/README.mddocs/reference/capabilities.mddocs/reference/extensions-glossary.mdpackages/1-framework/1-core/framework-components/src/framework-authoring.tspackages/1-framework/2-authoring/psl-parser/README.mdpackages/1-framework/2-authoring/psl-parser/src/exports/index.tspackages/1-framework/2-authoring/psl-parser/src/parser.tspackages/1-framework/2-authoring/psl-parser/src/types.tspackages/1-framework/2-authoring/psl-parser/test/parser.test.tspackages/2-sql/2-authoring/contract-psl/README.mdpackages/2-sql/2-authoring/contract-psl/src/interpreter.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/src/psl-relation-resolution.tspackages/2-sql/2-authoring/contract-psl/test/fixtures.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.extensions.test.tspackages/2-sql/2-authoring/contract-psl/test/provider.test.tspackages/2-sql/2-authoring/contract-psl/test/ts-psl-parity.test.tspackages/2-sql/2-authoring/contract-ts/README.mdpackages/2-sql/2-authoring/contract-ts/package.jsonpackages/2-sql/2-authoring/contract-ts/test/authoring-helper-runtime.test.tspackages/2-sql/2-authoring/contract-ts/test/contract-builder.dsl.helpers.test.tspackages/2-sql/2-authoring/contract-ts/test/package-exports.test.tspackages/2-sql/9-family/README.mdpackages/2-sql/9-family/src/core/authoring-type-constructors.tspackages/2-sql/9-family/src/core/control-descriptor.tspackages/2-sql/9-family/src/exports/control.tspackages/2-sql/9-family/src/exports/pack.tspackages/2-sql/9-family/test/mutation-default-assembly.test.tspackages/2-sql/9-family/test/pack.authoring.test.tspackages/2-sql/9-family/test/ts-psl-parity.real-packs.test.tspackages/3-extensions/pgvector/src/core/authoring.tspackages/3-extensions/pgvector/test/pack.authoring.test.ts
💤 Files with no reviewable changes (2)
- packages/2-sql/9-family/src/exports/control.ts
- packages/2-sql/2-authoring/contract-ts/package.json
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
docs/architecture docs/adrs/ADR 112 - Target Extension Packs.md (1)
66-72:⚠️ Potential issue | 🟡 MinorThe
PslSPIexample still advertises an attribute-shaped API.The surrounding prose is constructor-first now, but the sample interface still exposes
attributes: ..., which makes the ADR internally inconsistent and suggests the legacy surface is still the SPI contract.As per coding guidelines:
docs/**/*.{md,mdc}: Keep docs, READMEs, and rules up-to-date.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/architecture` docs/adrs/ADR 112 - Target Extension Packs.md around lines 66 - 72, The ADR's PslSPI example still exposes the legacy attributes property; update the sample to the constructor-first SPI shape by removing the attributes: Record... field and replacing it with a constructor/factory or method-oriented signature that directly maps an attribute invocation to Decorations (e.g., a constructor or factory function on PslSPI that accepts AttrArgs and returns Decoration[] or a method like getDecorations(name: string, args: AttrArgs): Decoration[]); ensure the example uses the PslSPI type name and AttrArgs/Decoration symbols so readers can locate and reconcile the surrounding prose with the updated SPI surface.
🤖 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/2-authoring/contract-psl/src/interpreter.ts`:
- Around line 213-250: The branch handling constructor-based named types returns
early and never validates declaration.attributes, so attributes like `@db`.* or
legacy extensions are ignored; before the early continue after
instantiatePslTypeConstructor (and/or before resolving the constructor) run the
same attribute validation/diagnostic logic used for non-constructor named types
(i.e., inspect declaration.attributes and call the attribute validation/emission
helper that emits unsupported/uncomposed diagnostics), ensuring
resolvePslTypeConstructorDescriptor, instantiatePslTypeConstructor, and the
namedTypeDescriptors/storageTypes population still occur only if attributes pass
validation.
In `@packages/2-sql/2-authoring/contract-psl/src/psl-column-resolution.ts`:
- Around line 121-140: parsePslObjectLiteral currently relies solely on
JSON.parse so unquoted keys or single-quoted strings (e.g. { length: 35 }) are
rejected; change parsePslObjectLiteral to first attempt JSON.parse as-is and if
that fails, attempt a JS-like object literal evaluation (e.g. wrap trimmed in
parentheses and use Function('return ' + wrapped)()) to accept unquoted
keys/single quotes, then validate the result is a non-null object and not an
array before returning; keep the same INVALID_AUTHORING_ARGUMENT returns on any
failure and ensure you only update the parsing branch inside
parsePslObjectLiteral.
---
Duplicate comments:
In `@docs/architecture` docs/adrs/ADR 112 - Target Extension Packs.md:
- Around line 66-72: The ADR's PslSPI example still exposes the legacy
attributes property; update the sample to the constructor-first SPI shape by
removing the attributes: Record... field and replacing it with a
constructor/factory or method-oriented signature that directly maps an attribute
invocation to Decorations (e.g., a constructor or factory function on PslSPI
that accepts AttrArgs and returns Decoration[] or a method like
getDecorations(name: string, args: AttrArgs): Decoration[]); ensure the example
uses the PslSPI type name and AttrArgs/Decoration symbols so readers can locate
and reconcile the surrounding prose with the updated SPI surface.
🪄 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: e48ce7f5-3a74-40fa-8347-2361d9c5d0f1
⛔ Files ignored due to path filters (6)
packages/2-sql/4-lanes/sql-builder/test/fixtures/generated/contract.d.tsis excluded by!**/generated/**packages/2-sql/4-lanes/sql-builder/test/fixtures/generated/contract.jsonis excluded by!**/generated/**packages/3-extensions/sql-orm-client/test/fixtures/generated/contract.d.tsis excluded by!**/generated/**packages/3-extensions/sql-orm-client/test/fixtures/generated/contract.jsonis excluded by!**/generated/**test/e2e/framework/test/fixtures/generated/contract.d.tsis excluded by!**/generated/**test/e2e/framework/test/fixtures/generated/contract.jsonis excluded by!**/generated/**
📒 Files selected for processing (25)
docs/architecture docs/adrs/ADR 112 - Target Extension Packs.mddocs/architecture docs/adrs/ADR 170 - Pack-provided type constructors and field presets.mddocs/reference/capabilities.mdexamples/prisma-next-demo/migration-fixtures/kitchen-sink/20260326T1428_migration/migration.jsonexamples/prisma-next-demo/migration-fixtures/multi-branch/20260326T1430_migration/migration.jsonexamples/prisma-next-demo/migration-fixtures/multi-branch/20260326T1431_foobar/migration.jsonexamples/prisma-next-demo/migration-fixtures/multi-branch/20260326T1431_migration/migration.jsonexamples/prisma-next-demo/migration-fixtures/multi-branch/20260326T1432_foobar/migration.jsonexamples/prisma-next-demo/prisma/contract.tsexamples/prisma-next-demo/prisma/schema.prismaexamples/prisma-next-demo/src/prisma/contract.d.tsexamples/prisma-next-demo/src/prisma/contract.jsonpackages/1-framework/2-authoring/psl-parser/src/parser.tspackages/1-framework/3-tooling/cli/test/control-api/contract-enrichment.test.tspackages/2-sql/2-authoring/contract-psl/src/interpreter.tspackages/2-sql/2-authoring/contract-psl/src/psl-column-resolution.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.extensions.test.tspackages/3-extensions/pgvector/README.mdpackages/3-extensions/pgvector/src/core/descriptor-meta.tspackages/3-extensions/pgvector/test/manifest.test.tstest/integration/test/authoring/diagnostics/namespace-without-pack/schema.prismatest/integration/test/authoring/parity/pgvector-named-type/expected.contract.jsontest/integration/test/authoring/parity/pgvector-named-type/schema.prismatest/integration/test/authoring/parity/ts-psl-parity.real-packs.test.tstest/integration/test/authoring/psl.pgvector-dbinit.test.ts
✅ Files skipped from review due to trivial changes (11)
- packages/3-extensions/pgvector/README.md
- packages/3-extensions/pgvector/test/manifest.test.ts
- examples/prisma-next-demo/migration-fixtures/kitchen-sink/20260326T1428_migration/migration.json
- examples/prisma-next-demo/migration-fixtures/multi-branch/20260326T1430_migration/migration.json
- examples/prisma-next-demo/prisma/schema.prisma
- test/integration/test/authoring/parity/pgvector-named-type/expected.contract.json
- test/integration/test/authoring/diagnostics/namespace-without-pack/schema.prisma
- examples/prisma-next-demo/src/prisma/contract.json
- test/integration/test/authoring/parity/pgvector-named-type/schema.prisma
- docs/architecture docs/adrs/ADR 170 - Pack-provided type constructors and field presets.md
- packages/1-framework/2-authoring/psl-parser/src/parser.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/reference/capabilities.md
The ./schema-sql export was removed from contract-ts package.json but the test and README still referenced it, causing test failure.
The aliases field was speculative infrastructure inherited from the old parsePgvectorLength length/dim aliasing. No descriptor provides aliases, so remove the type and the findIndex check.
The pgvectorAuthoringContributions fixture uses a controlled maximum different from the real VECTOR_MAX_DIM to prove the interpreter works generically. Add a comment pointing to the real-pack parity test.
…space validation The same parse→check→diagnose pattern for unrecognized extension namespaces was repeated across psl-field-resolution, psl-relation-resolution, and interpreter. Extract a shared predicate in psl-column-resolution.
…ConstructorCall The three-valued return (T | undefined | false) was a readability hazard. Replace false with the self-documenting 'malformed' literal, remove the redundant || undefined at call sites, and simplify the IIFE for typeName extraction.
…t objects Replace 15 and 10 positional parameters with named input objects to prevent accidental argument-order swaps and align with the rest of the module. Also extract shared resolveInput in collectResolvedFields to deduplicate two identical resolveFieldTypeDescriptor call-site param bags.
…rface
- Add sql.String(length) as a real SQL family-owned type constructor
in shared authoring composition
- Extend PSL parsing to support constructor expressions in both
types {} declarations and inline field positions
- Prove TS/PSL parity with cohesive tests exercising both family-owned
(sql.String) and extension-owned (pgvector.Vector) constructors,
including real-pack parity coverage
- Remove legacy @pgvector.column lowering; constructor expressions are
now the only supported extension-owned PSL surface
- Make contract-psl lower pack-owned constructors generically via
AuthoringContributions and emit precise diagnostics for unrecognized
namespaces
- Rename pgvector.vector → pgvector.Vector (PascalCase) to match
type constructor naming convention
- Add name field to AuthoringArgumentDescriptor for PSL named-argument
matching
- Update architecture docs, product docs, and references to reflect
constructor-first PSL surface
- Remove unused resolveColumnDescriptor import in interpreter.ts (left over from the collectResolvedFields refactor that now delegates via resolveFieldTypeDescriptor). - Cast storage to access .types in interpreter.extensions.test.ts for the inline constructor test, mirroring the pattern already used in interpreter.polymorphism.test.ts.
The prisma-next-demo schema and integration test fixtures still used the removed @pgvector.column attribute form, breaking fixture emission and integration tests. Switch them to pgvector.Vector(...) constructor expressions to match the supported PSL surface.
Addresses CodeRabbit review: the later bullets in ADR 112 still said "attributes" even after the PSL SPI section moved to constructor-first language. Update them to mention pack-owned constructor syntax.
Addresses CodeRabbit review: the "Today ... we end up with" framing in the rationale listed `pgvector.Vector(1536)` as a pain point, which contradicts the direction the ADR proposes. Rephrase as "extension-specific attribute forms that push storage meaning outside the type position."
Addresses CodeRabbit review: the pgvector pack declared its capability as "pgvector/cosine" while every other capability key in the codebase uses dot-separated namespace (sql.lateral, postgres.partialIndex, postgis.geometry, etc.). Switch to "pgvector.cosine" for consistency. Updates the pack descriptor-meta source of truth, regenerates all affected contract fixtures (json + d.ts), and updates the demo contract, integration fixtures, migration fixtures, docs, manifest test, and CLI enrichment test accordingly.
…calls
Addresses CodeRabbit review: parseTypeConstructorCall was used from both
parseTypesBlock and parseField but hardcoded PSL_INVALID_TYPES_MEMBER,
so a bad inline field constructor reported a types {} error instead of
a model-field error. Thread invalidCode + invalidMessage through the
helper and set them at the two call sites.
…ject errors Addresses CodeRabbit review: buildValueObjects can append diagnostics when composite-type field lowering fails (unsupported field types, constructor errors), but the previous failure gate ran before that call. Unsupported composite-type fields were therefore silently producing ok(patchedContract) with missing value objects. Add a second diagnostics check immediately after buildValueObjects so value-object lowering failures cause the interpreter to fail instead of producing a partial contract.
Addresses CodeRabbit review: parsePslAuthoringArgumentValue previously
returned INVALID_AUTHORING_ARGUMENT for every descriptor with
kind: 'object', so any pack-owned constructor expecting structured
options would always fail lowering.
Add a parsePslObjectLiteral branch that accepts JSON object literals
(e.g. {"length": 1536}) and returns them as Record<string, unknown>.
The downstream validateAuthoringHelperArguments call enforces the
descriptor's expected property shape.
Addresses CodeRabbit review: resolvePslTypeConstructorDescriptor treated every dotted constructor path as an extension namespace, so invalid input like db.VarChar(10) reported PSL_EXTENSION_NAMESPACE_NOT_COMPOSED with "Add extension pack 'db'" even though db is a reserved native-type namespace elsewhere in this module. Special-case 'db' so invalid db.* constructors fall through to the generic unsupported-constructor diagnostic instead.
The ts-psl-parity.real-packs.test.ts lived in @prisma-next/family-sql but imported from @prisma-next/extension-pgvector and the postgres target/adapter packs, which depend back on family-sql. Adding those as devDependencies creates cyclic workspace dependencies; importing via relative paths breaks TypeScript rootDir. Move the test to @prisma-next/integration-tests, which already has all the required workspace dependencies and is the designated home for cross-layer integration tests.
jkomyno
force-pushed
the
vp1-family-type-constructor-parity
branch
from
3899061 to
c8429b4
Compare
An unbounded integer like sql.String(999999999999999999999) silently flows through parse and Number coercion to 1e+21, then into contract.json and emitted DDL where Postgres finally rejects it — but artifacts on disk already contain the truncated exponent-form value. Add maximum: 10485760 to match the Postgres VARCHAR limit. This mirrors the pgvector.Vector bound on VECTOR_MAX_DIM.
The test was moved from packages/2-sql/9-family/test/ to test/integration/test/authoring/parity/ (commit 13854cf).
pack.ts is imported from src/core and exported to pack consumers. lint:deps already inherited rules via the parent 9-family glob, but the explicit mapping documents the shared plane for future maintainers.
new Map<string, never>() reads as a type hack. Annotate the const with the real type alias and construct an empty Map — same runtime semantics, expresses intent.
resolveFieldTypeDescriptor and resolveColumnDescriptor never mutate the enum/named type descriptor maps. Accepting ReadonlyMap removes two type casts in buildValueObjects and matches intent.
Previously there were two diagnostics gates around buildValueObjects: the first deduped and returned, the second (added by 414769a as a fix to surface value-object errors) did not dedupe. Since buildValueObjects doesn't depend on the built contract, hoist it above the first gate. One gate now covers every producer, including value-object lowering, and dedup applies uniformly.
The filter(findIndex(...)) pattern is quadratic: for 10k diagnostics on a failing schema it performs 100M comparisons. Replace with a Map keyed by (code, sourceId, span-offsets, message). Drops unused hasSameSpan helper and PslSpan import.
Intended a discriminated union, but TypeScript cannot cleanly narrow the intersection or either-or union variants of this shape when the declaration flows through helpers. Land the invariant as a doc comment on the type and tighten the runtime guard so it binds a local and also documents that the fallthrough is unreachable under a correct parser.
Was building an inline pgvectorPack with a hand-rolled lowercase vector key. After the pgvector.Vector PascalCase rename, the inline pack drifted from the real @prisma-next/extension-pgvector/pack — the tests still typechecked because the inline pack was self-contained, but they were testing a fake. Replace with the real pack import so type expectations are pinned to production shape.
checkUncomposedNamespace deduplicates detection but the diagnostic payload was still copy-pasted at five sites (interpreter named-type attrs, interpreter model attrs, field attrs, relation attrs, and type constructor paths). Centralize the push so the code, message template, and suggestion live once.
…ostic
Agents and IDE tooling consuming diagnostics had to parse English
prose to extract the missing namespace. Add an optional `data`
slot on ContractSourceDiagnostic and populate it at the
PSL_EXTENSION_NAMESPACE_NOT_COMPOSED emission site with
{ namespace, suggestedPack }. Lock the payload with a test.
… codes Four recent behavioral commits landed without tests: - f9a1dc4 db.* not suggested as extension pack - 414769a re-check diagnostics after buildValueObjects - 02d1cdd context-specific diagnostics for malformed constructor calls - 52fd3f1 object constructor arguments in PSL (covered in 004) Add three regression cases pinning: 1. db.VarChar passes through without PSL_EXTENSION_NAMESPACE_NOT_COMPOSED 2. A composite type field referencing an unknown type surfaces PSL_UNSUPPORTED_FIELD_TYPE through the single diagnostics gate 3. Malformed sql.String(...) does NOT emit a namespace-not-composed code, and pgvector.Vector(...) without the pack DOES, with structured data
Add six cases covering strict-JSON vs JS-like unquoted keys, missing/ unknown/wrong-typed property validation, malformed syntax, and a non-object top-level literal. Also annotate the remaining structural cast in parsePslObjectLiteral — 'typeof !== object' narrows to 'object', not Record<string, unknown>, and downstream key-validation in framework-authoring enforces the record shape.
Was building the same { authoringContributions, pslInputs } + spreading
four controls twice per case. Extract REAL_PACK_CONTROLS + a shared
interpretWithRealPacks(schema) helper so cases read as pure schema +
expected contract. Adding a third parity case is now two lines.
Callers previously had to guard with `if (!field.typeConstructor)`
before pushing a generic PSL_UNSUPPORTED_FIELD_TYPE diagnostic, because
the resolver silently pushes its own diagnostic on the constructor
failure path. Leaked callee internals to two callers in three places.
Return ResolveFieldTypeResult { ok, descriptor | alreadyReported } so
the caller only has to check alreadyReported. Three identical guards
become three tiny branches with no knowledge of field.typeConstructor.
collectResolvedFields was mixing attribute validation, relation
filtering, type resolution, default lowering, and id/unique constraint
extraction in one ~190-line body. Extract:
- validateFieldAttributes: walks field.attributes once, flags uncomposed
namespaces and unsupported attributes
- extractFieldConstraintNames: returns { idAttribute, uniqueAttribute,
idName, uniqueName } in one call
- BUILTIN_FIELD_ATTRIBUTE_NAMES constant set (replaces the chained ORs)
The orchestrator reads as a straightforward pipeline per field.
Three small improvements identified during review: - mapPslHelperArgs: drop redundant explicit type-guards; TS 5.5+ narrows .filter(arg => arg.kind === 'x') automatically. - parsePslAuthoringArgumentValue: convert if-chain to a switch with an exhaustive never check so a future AuthoringArgumentDescriptor variant is a compile-time error instead of silently falling through to INVALID_AUTHORING_ARGUMENT. - family-sql pack.ts: replace the double cast 'typeof sqlFamilyPack & FamilyPackRef<"sql">' with a plain 'as const satisfies FamilyPackRef<"sql">'.
…ional?, drop dead branch
- psl-column-resolution: inline parsePslAttributeName + drop the
exported PslAttributeNameParts type; checkUncomposedNamespace is
the only caller and fits in six lines without it.
- framework-authoring: lift name? and optional? into a shared
AuthoringArgumentDescriptorCommon interface and intersect it with
the kind-specific variants. Removes eight duplicated lines and
means a new variant doesn't need to remember to re-add the two
common fields.
- psl-parser: drop the unreachable 'openParen < 0' branch in
parseTypeConstructorCall; the preceding regex already required
a literal '('.
Pin the existing escape behavior with negative-input tests so a future refactor can't silently regress into a d.ts injection vulnerability: - quote-terminator attempt - backslash-terminated strings (including double-backslash) - control chars and line separators (U+2028/U+2029) pass through - object keys containing escape sequences are quoted safely
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/2-sql/2-authoring/contract-psl/src/psl-column-resolution.ts (1)
369-387: Comma-split limitation in string array parsing.The simple
split(',')approach doesn't handle quoted strings containing commas. For example,["hello, world", "test"]would split incorrectly into['"hello', ' world"', ' "test"'].If PSL syntax guarantees no commas within string elements, this is acceptable. Otherwise, consider reusing
parseJsLikeLiteralfor consistency (it already handles arrays correctly).♻️ Optional: Use parseJsLikeLiteral for consistency
function parseStringArrayLiteral( value: string, ): readonly string[] | typeof INVALID_AUTHORING_ARGUMENT { - const trimmed = value.trim(); - if (!trimmed.startsWith('[') || !trimmed.endsWith(']')) { - return INVALID_AUTHORING_ARGUMENT; - } - - const body = trimmed.slice(1, -1).trim(); - if (body.length === 0) { - return []; - } - - return body - .split(',') - .map((entry) => entry.trim()) - .filter((entry) => entry.length > 0) - .map((entry) => unquoteStringLiteral(entry)); + const parsed = parseJsLikeLiteral(value); + if (parsed === INVALID_AUTHORING_ARGUMENT || !Array.isArray(parsed)) { + return INVALID_AUTHORING_ARGUMENT; + } + if (!parsed.every((item): item is string => typeof item === 'string')) { + return INVALID_AUTHORING_ARGUMENT; + } + return parsed; }🤖 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/psl-column-resolution.ts` around lines 369 - 387, The parseStringArrayLiteral function incorrectly splits on commas which breaks quoted elements (e.g., '["hello, world","test"]'). Fix by replacing the naive split with a robust parse: either call and reuse the existing parseJsLikeLiteral (which already handles quoted strings and arrays) from parseStringArrayLiteral, or implement a small stateful parser that respects quotes/escapes when tokenizing the array body; ensure you still validate bracket presence and return INVALID_AUTHORING_ARGUMENT on parse failures and map results through unquoteStringLiteral where appropriate.
🤖 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/2-authoring/contract-psl/src/interpreter.ts`:
- Around line 802-829: buildValueObjects currently drops field.list for
scalar/enum/named types: after resolveFieldTypeDescriptor succeeds in the branch
that assigns fields[field.name] = { nullable: field.optional, type: { kind:
'scalar', codecId: resolved.descriptor.codecId } }, preserve the list/many
information by checking field.list (or field.many) and, if true, wrap the
resolved type in a list type (e.g. type: { kind: 'list', item: { kind: 'scalar'
| 'named', ... } }) instead of emitting a singular scalar/named type; update the
assignment site (the block using resolveFieldTypeDescriptor, fields[field.name],
compositeType.name, field.name, field.optional) so list fields like String[] are
emitted as lists, not singular values.
---
Nitpick comments:
In `@packages/2-sql/2-authoring/contract-psl/src/psl-column-resolution.ts`:
- Around line 369-387: The parseStringArrayLiteral function incorrectly splits
on commas which breaks quoted elements (e.g., '["hello, world","test"]'). Fix by
replacing the naive split with a robust parse: either call and reuse the
existing parseJsLikeLiteral (which already handles quoted strings and arrays)
from parseStringArrayLiteral, or implement a small stateful parser that respects
quotes/escapes when tokenizing the array body; ensure you still validate bracket
presence and return INVALID_AUTHORING_ARGUMENT on parse failures and map results
through unquoteStringLiteral where appropriate.
🪄 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: 59bab155-734a-4960-8b39-af25dda51571
⛔ Files ignored due to path filters (6)
packages/2-sql/4-lanes/sql-builder/test/fixtures/generated/contract.d.tsis excluded by!**/generated/**packages/2-sql/4-lanes/sql-builder/test/fixtures/generated/contract.jsonis excluded by!**/generated/**packages/3-extensions/sql-orm-client/test/fixtures/generated/contract.d.tsis excluded by!**/generated/**packages/3-extensions/sql-orm-client/test/fixtures/generated/contract.jsonis excluded by!**/generated/**test/e2e/framework/test/fixtures/generated/contract.d.tsis excluded by!**/generated/**test/e2e/framework/test/fixtures/generated/contract.jsonis excluded by!**/generated/**
📒 Files selected for processing (60)
architecture.config.jsondocs/architecture docs/adrs/ADR 104 - PSL extension namespacing & syntax.mddocs/architecture docs/adrs/ADR 112 - Target Extension Packs.mddocs/architecture docs/adrs/ADR 170 - Pack-provided type constructors and field presets.mddocs/architecture docs/subsystems/1. Data Contract.mddocs/glossary.mddocs/products/psl/README.mddocs/reference/capabilities.mddocs/reference/extensions-glossary.mdexamples/prisma-next-demo/migration-fixtures/kitchen-sink/20260326T1428_migration/migration.jsonexamples/prisma-next-demo/migration-fixtures/multi-branch/20260326T1430_migration/migration.jsonexamples/prisma-next-demo/migration-fixtures/multi-branch/20260326T1431_foobar/migration.jsonexamples/prisma-next-demo/migration-fixtures/multi-branch/20260326T1431_migration/migration.jsonexamples/prisma-next-demo/migration-fixtures/multi-branch/20260326T1432_foobar/migration.jsonexamples/prisma-next-demo/prisma/contract.tsexamples/prisma-next-demo/prisma/schema.prismaexamples/prisma-next-demo/src/prisma/contract.d.tsexamples/prisma-next-demo/src/prisma/contract.jsonpackages/1-framework/1-core/config/src/contract-source-types.tspackages/1-framework/1-core/framework-components/src/framework-authoring.tspackages/1-framework/2-authoring/psl-parser/README.mdpackages/1-framework/2-authoring/psl-parser/src/exports/index.tspackages/1-framework/2-authoring/psl-parser/src/parser.tspackages/1-framework/2-authoring/psl-parser/src/types.tspackages/1-framework/2-authoring/psl-parser/test/parser.test.tspackages/1-framework/3-tooling/cli/test/control-api/contract-enrichment.test.tspackages/1-framework/3-tooling/emitter/test/domain-type-generation.test.tspackages/2-sql/2-authoring/contract-psl/README.mdpackages/2-sql/2-authoring/contract-psl/src/interpreter.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/src/psl-relation-resolution.tspackages/2-sql/2-authoring/contract-psl/test/fixtures.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.diagnostics.test.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.extensions.test.tspackages/2-sql/2-authoring/contract-psl/test/provider.test.tspackages/2-sql/2-authoring/contract-psl/test/ts-psl-parity.test.tspackages/2-sql/2-authoring/contract-ts/README.mdpackages/2-sql/2-authoring/contract-ts/package.jsonpackages/2-sql/2-authoring/contract-ts/test/authoring-helper-runtime.test.tspackages/2-sql/2-authoring/contract-ts/test/contract-builder.dsl.helpers.test.tspackages/2-sql/2-authoring/contract-ts/test/package-exports.test.tspackages/2-sql/9-family/README.mdpackages/2-sql/9-family/src/core/authoring-type-constructors.tspackages/2-sql/9-family/src/core/control-descriptor.tspackages/2-sql/9-family/src/exports/control.tspackages/2-sql/9-family/src/exports/pack.tspackages/2-sql/9-family/test/mutation-default-assembly.test.tspackages/2-sql/9-family/test/pack.authoring.test.tspackages/3-extensions/pgvector/README.mdpackages/3-extensions/pgvector/src/core/authoring.tspackages/3-extensions/pgvector/src/core/descriptor-meta.tspackages/3-extensions/pgvector/test/manifest.test.tspackages/3-extensions/pgvector/test/pack.authoring.test.tstest/integration/test/authoring/diagnostics/namespace-without-pack/schema.prismatest/integration/test/authoring/parity/pgvector-named-type/expected.contract.jsontest/integration/test/authoring/parity/pgvector-named-type/schema.prismatest/integration/test/authoring/parity/ts-psl-parity.real-packs.test.tstest/integration/test/authoring/psl.pgvector-dbinit.test.tstest/integration/test/contract-builder.types.test-d.ts
💤 Files with no reviewable changes (2)
- packages/2-sql/9-family/src/exports/control.ts
- packages/2-sql/2-authoring/contract-ts/package.json
✅ Files skipped from review due to trivial changes (25)
- docs/glossary.md
- packages/1-framework/2-authoring/psl-parser/README.md
- packages/3-extensions/pgvector/README.md
- examples/prisma-next-demo/migration-fixtures/multi-branch/20260326T1430_migration/migration.json
- architecture.config.json
- examples/prisma-next-demo/migration-fixtures/multi-branch/20260326T1431_migration/migration.json
- packages/1-framework/1-core/config/src/contract-source-types.ts
- packages/3-extensions/pgvector/test/manifest.test.ts
- examples/prisma-next-demo/migration-fixtures/multi-branch/20260326T1432_foobar/migration.json
- examples/prisma-next-demo/src/prisma/contract.json
- examples/prisma-next-demo/src/prisma/contract.d.ts
- test/integration/test/authoring/parity/pgvector-named-type/expected.contract.json
- packages/1-framework/2-authoring/psl-parser/src/exports/index.ts
- examples/prisma-next-demo/migration-fixtures/kitchen-sink/20260326T1428_migration/migration.json
- examples/prisma-next-demo/migration-fixtures/multi-branch/20260326T1431_foobar/migration.json
- packages/2-sql/2-authoring/contract-ts/test/authoring-helper-runtime.test.ts
- test/integration/test/authoring/psl.pgvector-dbinit.test.ts
- packages/2-sql/2-authoring/contract-ts/test/contract-builder.dsl.helpers.test.ts
- docs/architecture docs/subsystems/1. Data Contract.md
- packages/2-sql/9-family/src/core/authoring-type-constructors.ts
- packages/2-sql/9-family/test/pack.authoring.test.ts
- packages/1-framework/3-tooling/emitter/test/domain-type-generation.test.ts
- docs/products/psl/README.md
- packages/3-extensions/pgvector/src/core/authoring.ts
- test/integration/test/authoring/parity/ts-psl-parity.real-packs.test.ts
🚧 Files skipped from review as they are similar to previous changes (22)
- examples/prisma-next-demo/prisma/contract.ts
- docs/reference/capabilities.md
- packages/2-sql/9-family/test/mutation-default-assembly.test.ts
- test/integration/test/authoring/diagnostics/namespace-without-pack/schema.prisma
- examples/prisma-next-demo/prisma/schema.prisma
- packages/3-extensions/pgvector/src/core/descriptor-meta.ts
- docs/architecture docs/adrs/ADR 104 - PSL extension namespacing & syntax.md
- packages/2-sql/9-family/README.md
- test/integration/test/authoring/parity/pgvector-named-type/schema.prisma
- packages/2-sql/2-authoring/contract-psl/test/ts-psl-parity.test.ts
- packages/2-sql/2-authoring/contract-ts/test/package-exports.test.ts
- docs/reference/extensions-glossary.md
- packages/3-extensions/pgvector/test/pack.authoring.test.ts
- docs/architecture docs/adrs/ADR 112 - Target Extension Packs.md
- packages/2-sql/9-family/src/core/control-descriptor.ts
- packages/2-sql/2-authoring/contract-ts/README.md
- packages/1-framework/1-core/framework-components/src/framework-authoring.ts
- packages/2-sql/9-family/src/exports/pack.ts
- packages/2-sql/2-authoring/contract-psl/test/provider.test.ts
- packages/1-framework/2-authoring/psl-parser/test/parser.test.ts
- docs/architecture docs/adrs/ADR 170 - Pack-provided type constructors and field presets.md
- packages/2-sql/2-authoring/contract-psl/README.md
… types
buildValueObjects wrapped value-object composite fields with
{ ...result, many: true } when field.list was true, but the
scalar/enum/named-type fallthrough emitted a singular descriptor
regardless of field.list — a String[] inside a composite type
was silently lowered as a single String.
Mirror the value-object branch: build the scalar ContractField, then
spread { many: true } when field.list. Mongo's buildValueObjects
already does this for both branches.
Spotted by CodeRabbit on PR #325.
The naive split(',') implementation broke string elements that
contain commas (e.g. ['hello, world', 'a,b,c']) — they'd be split
into 'hello' + ' world' + 'a' + 'b' + 'c'. Route through the
existing parseJsLikeLiteral, which is already quote- and
escape-aware, then assert the result is a string array.
Spotted by CodeRabbit on PR #325.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/2-sql/2-authoring/contract-psl/test/interpreter.test.ts (1)
392-422: Move this regression into a split value-object test file.This case fits neatly with the adjacent composite/value-object assertions, but
interpreter.test.tsis already well past the 500-line limit. Adding more here makes the file harder to navigate than necessary.As per coding guidelines "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 `@packages/2-sql/2-authoring/contract-psl/test/interpreter.test.ts` around lines 392 - 422, The test "preserves the many marker for scalar list fields inside composite types" should be moved out of the oversized interpreter.test.ts into a new split test file focused on value-object/composite tests to keep files under 500 lines; create a new test file (e.g., value-object.test.ts), copy the test block including the use of parsePslDocument and interpretPslDocumentToSqlContract and required test helpers (builtinControlMutationDefaults), update imports so the test runs standalone, and remove the original block from interpreter.test.ts so both files stay under the line limit.
🤖 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/2-authoring/contract-psl/src/psl-column-resolution.ts`:
- Around line 71-84: checkUncomposedNamespace currently only exempts the 'db'
namespace and composedExtensions, causing family/target namespaces (e.g.,
language/target ids) to be mis-reported as extension packs; update the function
to also treat active family and target ids as exempt by returning undefined when
namespace is in the known family/target id sets (e.g., use existing constants
like ACTIVE_FAMILY_IDS and ACTIVE_TARGET_IDS or add small lookup sets) so that
family/target namespaces fall through to the proper unsupported-attribute
diagnostics rather than PS L_EXTENSION_NAMESPACE_NOT_COMPOSED.
---
Nitpick comments:
In `@packages/2-sql/2-authoring/contract-psl/test/interpreter.test.ts`:
- Around line 392-422: The test "preserves the many marker for scalar list
fields inside composite types" should be moved out of the oversized
interpreter.test.ts into a new split test file focused on value-object/composite
tests to keep files under 500 lines; create a new test file (e.g.,
value-object.test.ts), copy the test block including the use of parsePslDocument
and interpretPslDocumentToSqlContract and required test helpers
(builtinControlMutationDefaults), update imports so the test runs standalone,
and remove the original block from interpreter.test.ts so both files stay under
the line limit.
🪄 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: c9f69fcd-da7c-48c2-ba84-d840cf47b243
📒 Files selected for processing (4)
packages/2-sql/2-authoring/contract-psl/src/interpreter.tspackages/2-sql/2-authoring/contract-psl/src/psl-column-resolution.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.extensions.test.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/2-sql/2-authoring/contract-psl/test/interpreter.extensions.test.ts
…agnostic
checkUncomposedNamespace only exempted 'db' and composed extension
packs, so attributes like @sql.foo or @postgres.bar were reported as
PSL_EXTENSION_NAMESPACE_NOT_COMPOSED instead of the correct
PSL_UNSUPPORTED_*_ATTRIBUTE — the constructor path (at
resolvePslTypeConstructorDescriptor) already did this exemption but
the attribute path did not.
Accept an optional { familyId, targetId } context and thread it
through all five call sites (named-type attrs, model attrs, field
attrs, navigation-list field attrs, uncomposed constructor). The
constructor branch keeps its inline check since it still needs to
cover the no-dot path-length case.
Regression test asserts @sql.foo / @postgres.bar / @@sql.qux surface
as PSL_UNSUPPORTED_*_ATTRIBUTE, not PSL_EXTENSION_NAMESPACE_NOT_COMPOSED.
Spotted by CodeRabbit on PR #325.
interpreter.test.ts had grown to 627 lines, exceeding the 500-line guideline. Move the seven value-object / composite-type / scalar-list tests into interpreter.value-objects.test.ts. Both files are now under 360 lines. Spotted by CodeRabbit on PR #325.
The three anonymous error-message builders in parseTypesBlock, parseField, and parseTypeConstructorCall were untested, dropping psl-parser function coverage to 94.73% (below the 95% threshold). Add three targeted regression tests: - trailing junk after a types-block constructor hits the invalidMessage callback in parseTypesBlock - trailing junk after an inline field constructor hits the invalidMessage callback in parseField - an empty named-argument (sql.String(length:)) hits the invalidNamedArgumentMessage callback in parseArgumentList Function coverage climbs to 100%.
validateNamedTypeAttributes grabbed the first @db.* attribute with .find() and skipped every subsequent one in the attribute loop, so 'Email = String @db.VarChar(10) @db.Char(2)' silently dropped the second attribute instead of rejecting invalid PSL. Filter all @db.* attributes; if more than one is present, emit a PSL_INVALID_ATTRIBUTE_ARGUMENT diagnostic at each extra attribute's span and mark the named type as unsupported so the contract isn't emitted. Spotted by CodeRabbit on PR #325.
wmadden
approved these changes
Apr 10, 2026
Contributor
wmadden
left a comment
wmadden
left a comment
There was a problem hiding this comment.
Nice work — the generic lowering through AuthoringContributions is the right architectural move, and the test coverage across unit/integration/parity layers is thorough. The refactoring (input objects, helper extraction, O(D) dedup) leaves the codebase cleaner than before.
This review is preemptively approving. Please address or respond to the inline comments before merging — no re-review needed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
sql.String(length)) to shared authoring compositiontypes {}declarations and inline field positions (e.g.,ShortName = sql.String(length: 35),embedding pgvector.Vector(1536)?)@pgvector.columnlowering — constructor expressions are now the only supported extension-owned PSL surfacecontract-psllower pack-owned constructors generically viaAuthoringContributionsand emit precise diagnostics for unrecognized namespacespgvector.vector→pgvector.Vector(PascalCase) to match type naming conventionsCode quality improvements (from review)
checkUncomposedNamespacehelper to deduplicate namespace validation across 4 call sitesfalsesentinel with self-documenting'malformed'string literal inparseTypeConstructorCallcollectResolvedFields(15 params) andbuildValueObjects(10 params) to named input objectsaliasesfield fromPslHelperArgumentDescriptor./schema-sqlexport test and README references incontract-tsTest plan
pnpm test --filter=@prisma-next/psl-parser— 68 tests pass (parser constructor expression parsing)pnpm test --filter=@prisma-next/sql-contract-psl— 91 tests pass (interpreter lowering, extension diagnostics, parity)pnpm test --filter=@prisma-next/family-sql— 138 tests pass (pack authoring, real-pack TS/PSL parity)pnpm test --filter=@prisma-next/extension-pgvector— 31 tests pass (pgvector pack contributions)pnpm test --filter=@prisma-next/sql-contract-ts— package export test updated for removed./schema-sqlSummary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests