Conversation
Adds phase-specific execution defaults to SQL TS authoring so generated IDs stay on-create-only while updatedAt can use both create and update phases. Wires Prisma-compatible @updatedat lowering for Postgres and SQLite PSL, plus field.updatedAt() helpers backed by target-owned timestampNow generators. Empty update payloads skip onUpdate defaults so no-op updates do not advance updatedAt.
Reject @updatedat before relation fields are discarded so invalid PSL cannot be accepted silently. Keep the adapter mutation default descriptors contextually typed and narrow optional runtime generator hooks in tests so package typechecks remain green.
Resolves todos/056 (P1) and todos/057 (P1) from the PR #422 review. The phased mutation-default shape `executionDefaults: { onCreate?, onUpdate? }` was added in 682714e alongside the legacy single-slot `executionDefault?`. The legacy slot is `{ onCreate: X }` in disguise, so carrying both forces a runtime guard, a dynamic property-name label, and duplicated default+nullable validation across every layer (FieldNode, ValueObjectFieldNode, AuthoringFieldPresetOutput, ScalarFieldState, ResolvedField). It also sets a precedent that adding onDelete later requires another conditional in the reconciler. Changes: - Drop `executionDefault` from FieldNode, ValueObjectFieldNode, AuthoringFieldPresetOutput, ScalarFieldState, AnyScalarFieldState, and ResolvedField. - Every site that produced the singular form now wraps as `executionDefaults: { onCreate: X }`. Affects field.generated, the family-sql id helper presets (uuidv4, uuidv7, ulid, nanoid, cuid2, ksuid), and PSL `lowerDefaultForField`. - Delete `resolveExecutionDefaultPhases` and the dynamic `executionDefaultProperty` branch from build-contract.ts. The loop body checks `field.executionDefaults` directly. - Loosen the nullable check (todo 057): only fire when `onCreate` is set. An `onUpdate`-only mutation default is fine on a nullable field — it can legitimately start as NULL until first update. New regression test: `allows nullable fields with onUpdate-only executionDefaults`. - PSL `psl-field-resolution.ts` now combines `loweredDefault.executionDefaults` (on-create from generated `@default(...)`) and `updatedAtExecutionDefaults` (both phases from `@updatedAt`) via `??`. The two are mutually exclusive — `lowerUpdatedAtAttribute` rejects `@updatedAt @default(...)`. Net change: -33 lines across 13 files. One validator function gone, one runtime guard gone, one dynamic property-name label branch gone. Validation: - pnpm typecheck (123/123 tasks) - pnpm lint:deps (clean) - @prisma-next/framework-components test: 131/131 - @prisma-next/sql-contract-ts test: 218/218 - @prisma-next/sql-contract-psl test: 115/115 - @prisma-next/family-sql test: 285/285 - @prisma-next/adapter-postgres test: 615/615 - @prisma-next/adapter-sqlite test: 110/110
…agnostic (todos 061, 066) - Extract `rejectUpdatedAtOnNonScalar` helper in `psl-field-resolution.ts` and call it from the two upstream relation/list-field early-return sites instead of duplicating the inline diagnostic. Scalar-field rules continue to live in `lowerUpdatedAtAttribute` as the authoritative codec gate, while the new helper is the single owner of the relation/list rejection wording. - Pass `targetId` into `lowerUpdatedAtAttribute` and rewrite the `PSL_INVALID_DEFAULT_APPLICABILITY` message raised when the `timestampNow` generator is absent. The message now points users at the responsible target adapter rather than surfacing the SPI-level concept of a mutation default generator. The diagnostic code is unchanged so existing matchers continue to work.
…ext creation (todo 063) Surface RUNTIME.MISSING_MUTATION_DEFAULT_GENERATOR at createExecutionContext time when the contract references generator ids the assembled stack does not provide. Walks contract.execution.mutations.defaults across both onCreate and onUpdate phases, deduplicates missing ids, and throws a single structured error. Aligns with ADR 031/065's startup-time capability negotiation: contracts naming a generator id whose runtime implementation isn't shipped now fail fast at context construction rather than at first applyMutationDefaults call. The existing lazy fallback in computeExecutionDefaultValue (RUNTIME.MUTATION_DEFAULT_GENERATOR_MISSING) is retained as defense-in-depth — the new contract-time check supplements it, it does not replace it.
…(todos 058, 059, 060, 064) - 064: export `TIMESTAMP_NOW_GENERATOR_ID` constant and `ExecutionMutationDefaultPhases` type from canonical sources; replace duplicated literals/aliases across PSL/TS authoring and adapters. - 059: add `MutationDefaultGeneratorDescriptor.buildPhases?`; PSL `lowerUpdatedAtAttribute` now delegates phase construction to the generator descriptor (no inline literal). PSL/TS authoring stay byte-equivalent for any future params-bearing generator. - 058: centralize `timestampNow` wiring in `@prisma-next/family-sql` via `timestampNowControlDescriptor(applicableCodecIds)` and `timestampNowRuntimeGenerator()`. Postgres + SQLite adapters opt in rather than re-declare the body. - 060: add `RuntimeMutationDefaultGenerator.applyOnEmptyUpdate?` and switch `applyMutationDefaults` from a global empty-update short-circuit to a per-generator opt-out. Default `false` preserves @updatedat RD2; future onUpdate generators can opt in to sentinel-write semantics. Test counts (post-change): - @prisma-next/framework-components: 133 tests - @prisma-next/sql-contract-ts: 218 tests - @prisma-next/sql-contract-psl: 115 tests (incl. ts-psl-parity unchanged) - @prisma-next/family-sql: 285 tests - @prisma-next/sql-runtime: 186 tests + 1 skipped (incl. new applyOnEmptyUpdate=true regression) - @prisma-next/adapter-postgres: 615 tests - @prisma-next/adapter-sqlite: 110 tests
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughDerive model primary keys from single inline ChangesPrimary Key Support for ID-less SQL Tables
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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-arktype-json
@prisma-next/middleware-telemetry
@prisma-next/mongo
@prisma-next/extension-paradedb
@prisma-next/extension-pgvector
@prisma-next/postgres
@prisma-next/sql-orm-client
@prisma-next/sqlite
@prisma-next/target-mongo
@prisma-next/adapter-mongo
@prisma-next/driver-mongo
@prisma-next/contract
@prisma-next/utils
@prisma-next/config
@prisma-next/errors
@prisma-next/framework-components
@prisma-next/operations
@prisma-next/ts-render
@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
@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: |
…69, 070) Reject the four silent-acceptance gaps surfaced by the PR #424 review: duplicate @@id blocks, duplicate fields inside an @@id list, and optional fields used as inline @id or in an @@id list. Adds the missing diagnostic tests for each new branch and for the previously-untested inline-vs-block conflict.
Lift the duplicate-field-name check that PR #424 added to the @@id branch to @@unique and @@index, where the same gap existed: a list like @@unique([email, email]) previously emitted columns: ['email', 'email'] and only failed at DDL time (CREATE UNIQUE INDEX rejects duplicates), while @@index([email, email]) silently produced a wasteful index. Extracted findDuplicateFieldName into psl-attribute-parsing alongside the other field-list helpers; called from all three model-attribute branches with the unified entityLabel diagnostic shape.
… rows The runtime contract already encoded @updatedat as a both-phase execution mutation default (timestampNow generator on onCreate and onUpdate), but only the create path threaded it through applyMutationDefaults. ORM update, updateAll, updateCount, the upsert update branch, and the nested update branch in updateFirstGraph all skipped the call, so a non-empty update never advanced updatedAt. Wire applyMutationDefaults into every update path. Empty payloads still short-circuit before the runtime call, preserving the no-write-no-advance rule. Bulk inserts (createAll/createMany) called applyMutationDefaults per row, which made timestampNow generate a fresh Date per row and drift within a single ORM operation. Add a stableAcrossRows generator flag plus an acrossRowsCache parameter to applyMutationDefaults so bulk operations reuse one generated value across every row that needs the default. Mark timestampNow as stableAcrossRows: true. Per-row generators (cuid, uuidv4, etc.) stay independent because the cache only honors stableAcrossRows generators. Adds runtime-side tests for the cache contract and orm-client tests covering create, createAll, updateAll, updateCount, upsert (both branches), and explicit-value preservation.
…ability Add Milestone 4 to the created-updated-at-authoring plan covering the two compatibility gaps surfaced by comparing against Prisma 6's @updatedat semantics: ORM update paths skipped applyMutationDefaults, and bulk inserts drifted timestampNow per row. Also record the naming choice (stableAcrossRows / acrossRowsCache) under Resolved Decisions so future readers see why it didn't end up as bulkStable.
…69, 070) Reject the four silent-acceptance gaps surfaced by the PR #424 review: duplicate @@id blocks, duplicate fields inside an @@id list, and optional fields used as inline @id or in an @@id list. Adds the missing diagnostic tests for each new branch and for the previously-untested inline-vs-block conflict.
Lift the duplicate-field-name check that PR #424 added to the @@id branch to @@unique and @@index, where the same gap existed: a list like @@unique([email, email]) previously emitted columns: ['email', 'email'] and only failed at DDL time (CREATE UNIQUE INDEX rejects duplicates), while @@index([email, email]) silently produced a wasteful index. Extracted findDuplicateFieldName into psl-attribute-parsing alongside the other field-list helpers; called from all three model-attribute branches with the unified entityLabel diagnostic shape.
76aacd8 to
8ff2127
Compare
Reframe the project around a generic PSL field-preset dispatch path (mirroring the existing TS field-preset registry), exposed as temporal.createdAt() / temporal.updatedAt() in PSL and field.temporal.createdAt() / field.temporal.updatedAt() in TS. The Prisma-flavored @updatedat attribute is removed; codec applicability stops being a user-facing diagnostic and becomes a registry-coherence concern. Contract IR, runtime, and ORM-client work from prior milestones survive untouched (consolidated into "Inherited Foundation"). All open design questions resolved inline; reviewer feedback applied where it didn't conflict with prior decisions.
Adds a generic field-preset dispatch path to PSL, symmetric with the existing type-constructor dispatch. The path resolves namespaced calls in field-type position against authoringContributions.field, instantiates the preset via the same instantiateAuthoringFieldPreset machinery TS uses, and produces default / executionDefaults / id / unique / nullable contributions on the resolved field. Field presets resolve before type constructors (presets carry richer semantics; the more complete answer wins). The temporal namespace is added to the curated-namespaces exemption list, alongside db/family/ target — chosen for forward-compatibility with the JS/TS Temporal API. Preset usage is constrained per spec FR7: optional preset (?), list preset ([]), preset + @default, preset + @id, and preset + @updatedat are all hard errors with stable diagnostic codes. PSL → typed-args coercion happens in instantiatePslFieldPreset via mapPslHelperArgs; instantiateAuthoringFieldPreset itself stays typed-input-only. Test fixture proves genericness via a synthetic temporal.exampleField() preset (not a temporal-specific code path).
…sion check Adds defense-in-depth and user-facing diagnostics that close out Phase A of the field-preset pivot: PSL preset-misuse diagnostics: - PSL_PRESET_NOT_OPTIONAL — `temporal.X()?` rejected (preset is a complete declaration; system owns the value, optional contradicts that). - PSL_PRESET_NOT_LIST — `temporal.X()[]` rejected (presets aren't list elements). - PSL_PRESET_AND_DEFAULT_CONFLICT — `temporal.X() @default(...)` rejected (preset already specifies its own defaults). - PSL_PRESET_AND_ID_CONFLICT — `temporal.X() @id` rejected when the preset doesn't itself contribute id semantics. - PSL_PRESET_AND_UPDATED_AT_CONFLICT — `temporal.X() @updatedAt` rejected (preset already specifies execution defaults). - PSL_UNKNOWN_FIELD_PRESET — typo in a curated namespace surfaces a spelling-targeted hint instead of falling through to the generic PSL_UNSUPPORTED_FIELD_TYPE. Compose-time registry collision check: - createComposedAuthoringHelpers now rejects any path registered as both a field preset and a type constructor with a clear "Ambiguous authoring registry path" error. Belt-and-suspenders to RD9's runtime field-preset- first ordering — the error surfaces at composition, not at PSL resolution. Tests: - Five preset-misuse cases in interpreter.defaults.test.ts. - Cross-registry collision case in authoring-helper-runtime.test.ts. Validation gate (all green): - @prisma-next/sql-contract-psl: 121 tests - @prisma-next/sql-contract-ts: 219 tests - @prisma-next/framework-components: 133 tests - pnpm lint:deps: 716 modules, 0 violations
The original RD11 said to rename PSL_INVALID_TYPE_CONSTRUCTOR_ARITY → PSL_INVALID_NAMESPACED_CALL_ARITY. Phase A revealed the assumption was wrong: that code name doesn't exist. Type-constructor arity/arg errors today emit PSL_INVALID_ATTRIBUTE_ARGUMENT (a code shared with genuine attribute-arg errors like @id(badarg)). The honest rename to a PSL_INVALID_NAMESPACED_CALL_ARGUMENT code would need to disambiguate those cases too, which is a wider refactor than this project warrants. Decision: field-preset arity/arg errors emit PSL_INVALID_ATTRIBUTE_ARGUMENT for now, matching the existing type-constructor pattern. Honest rename deferred to a follow-up. This commit: - Updates FR7 to enumerate diagnostic codes that match what's actually implemented, with notes on which cases are tested in Phase A vs deferred. - Marks AC5 sub-items by implementation status (a, b, d, e, h, j shipped with tests; c, f, g, i deferred or covered indirectly). - Adds AC5j for the transient PSL_PRESET_AND_UPDATED_AT_CONFLICT code that disappears in Phase C alongside the @updatedat attribute path. - Rewrites RD11 to reflect the discovery and the deferral decision. - Updates plan.md task list and Resolved Decisions consistently.
Moves the createdAt/updatedAt field-preset registrations from flat
target.authoring.field.{createdAt,updatedAt} to nested
target.authoring.field.temporal.{createdAt,updatedAt} for both Postgres
and SQLite targets. The TS surface follows: field.createdAt() /
field.updatedAt() → field.temporal.createdAt() / field.temporal.updatedAt().
PSL was already wired in Phase A — temporal.createdAt() /
temporal.updatedAt() now resolve through the field-preset dispatch path
once the registry move lands here.
Updates to keep parity with the new namespace:
- Test fixtures: contract-builder.dsl.helpers.test.ts,
ts-psl-parity.test.ts (sqliteTimestampTargetPack,
postgresTimestampTargetPack, sqlFamilyPack).
- Examples: react-router-demo and prisma-next-demo TS contracts.
- CLI init templates (TS template) + snapshot.
- Integration type tests.
- Doc comments in mutation-default-types.ts and timestamp-now-generator.ts.
PSL examples (.prisma) and the @updatedat attribute path are unchanged
in Phase B — they ship intact through Phase C, which deletes the
attribute and updates PSL examples to use temporal.updatedAt().
Validation gate (all green):
- pnpm test:packages: 110/110 turbo tasks
- pnpm lint:deps: 716 modules, 0 violations
Deletes the Prisma-flavored @updatedat attribute path that landed on this branch in commit 682714e. The codec-applicability validation that motivated the pivot is gone; users now write temporal.updatedAt() in PSL and field.temporal.updatedAt() in TS, both consuming the same field-preset registry shipped in Phases A+B. What's deleted from psl-field-resolution.ts (~110 lines): - 'updatedAt' entry in BUILTIN_FIELD_ATTRIBUTE_NAMES - reportInvalidUpdatedAt, rejectUpdatedAtOnNonScalar (helpers) - lowerUpdatedAtAttribute (the codec-applicability path) - The relation-field @updatedat rejection branches in collectResolvedFields - The @updatedat lowering merge with @default(generator()) - The PSL_PRESET_AND_UPDATED_AT_CONFLICT branch (no longer reachable once the attribute is gone) Migration hint: - New helper getRemovedAttributeHint() returns a per-attribute suggestion. Currently has one entry: 'updatedAt' → 'Use `temporal.updatedAt()` as a field-preset call instead.' - The hint is appended to the standard PSL_UNSUPPORTED_FIELD_ATTRIBUTE message when the attribute name matches and the field doesn't already declare a temporal.* preset (suppression check). - Implementation is the requested hardcoded if-branch — no map. Adding more migrations later means adding another branch. Tests: - Replaced four @updatedAt-attribute tests in interpreter.defaults with temporal.updatedAt()-preset tests (Postgres + SQLite, plus inline preset registrations matching the production target shape). - Added two new tests: migration-hint emission, and hint suppression for already-migrated fields (the `temporal.updatedAt() @updatedAt` half-migrated case). - Removed the obsolete PSL_PRESET_AND_UPDATED_AT_CONFLICT test. - Updated the PSL parity test (sqlite + postgres) to use temporal.updatedAt() PSL syntax. Diagnostic-code inventory (per RD11): grep confirmed PSL_INVALID_DEFAULT_APPLICABILITY remains in use elsewhere (storage default lowering); PSL_PRESET_AND_UPDATED_AT_CONFLICT is now orphaned and removed. Doc-comment updates in: - packages/1-framework/1-core/framework-components/src/shared/mutation-default-types.ts - packages/3-extensions/sql-orm-client/src/collection.ts Spec/plan updates: - AC4 marked complete; references PSL_UNSUPPORTED_FIELD_ATTRIBUTE (the actual code) instead of the spec's earlier PSL_UNKNOWN_ATTRIBUTE (which doesn't exist). - AC5j superseded note: half-migrated fields now use the suppression path instead of a dedicated diagnostic. Validation gate: 120 PSL tests pass, full lint:deps clean (716 modules, 0 violations). adapter-postgres (615) and target-mongo (447) pass standalone; turbo-aggregate flake unrelated to this commit. BREAKING CHANGE: PSL @updatedat is no longer a recognized attribute. Migrate to `temporal.updatedAt()` field-preset syntax. The unknown- attribute diagnostic includes the migration hint inline.
Update existing references to the renamed/removed surfaces: - docs/products/psl/README.md — timestamp section now lists temporal.createdAt() / temporal.updatedAt() as the preset surface alongside @default(now()), and notes that @updatedat is no longer recognized (with the migration-hint diagnostic). - packages/2-sql/2-authoring/contract-psl/README.md — Responsibilities bullet and "Supported timestamp authoring surface" section. - packages/2-sql/2-authoring/contract-ts/{README.md,API.md} — bulk rename field.createdAt() / field.updatedAt() → field.temporal.createdAt() / field.temporal.updatedAt() across helper-vocabulary lists, code examples, and timestamp-helper notes. Scope-bounded: only updates pre-existing references — no restructuring, no new sections, no docs that weren't already in the inventory. Validation: 110/110 turbo tasks (adapter-postgres flake confirmed unrelated by standalone re-run, 615 tests pass), lint:deps 716 modules / 0 violations.
…gets
Hoist the `temporal.{createdAt,updatedAt}` field-preset descriptors into a
single `temporalAuthoringPresets({ codecId, nativeType })` helper exported
from `@prisma-next/family-sql/control`. Postgres and SQLite each pass only
their target-specific codec/native-type pair; the rest of the descriptor
(default expression, generator id, both phases) is owned by the helper, so
PSL `temporal.updatedAt()` and TS `field.temporal.updatedAt()` lower to
byte-identical contracts across targets by construction.
Also drops the unused flat `dateTime` preset from SQLite authoring — it had
no callers in `packages/`, `examples/`, or tests, and was a parity wart
relative to Postgres' pre-existing flat `dateTime`. The Postgres flat
`dateTime` is left untouched since it predates this branch.
The `applyOnEmptyUpdate?: boolean` flag on `RuntimeMutationDefaultGenerator`
was added as a forward-compat hook for hypothetical future generators that
want sentinel-write semantics on empty update payloads. No production
generator opts in; the only caller was a self-test of the flag itself.
Per CLAUDE.md ("Don't design for hypothetical future requirements"), drop
the flag, the runtime branch in `applyMutationDefaults`, and the self-test.
Empty-update skip is now unconditional. Re-add when a real generator needs
sentinel semantics, with a test exercising real production code.
RD2 ("empty update payloads skip onUpdate defaults") is still enforced —
just simpler now.
`build-contract.ts` previously allowed nullable fields when only
`executionDefaults.onUpdate` was present (no `onCreate`), reasoning that
the column could start NULL until first update. That branch was a
forward-compat hook for hypothetical onUpdate-only presets — dead code with
respect to `temporal.updatedAt()`, which always sets both phases and is
also rejected at the PSL layer via `PSL_PRESET_NOT_OPTIONAL`.
Per CLAUDE.md ("Don't design for hypothetical future requirements"),
collapse the check to "nullable + any executionDefaults = error" and
update the error message accordingly. Drop the dedicated test that
asserted the permissive branch. Re-add the allowance when a real preset
(e.g. `temporal.lastModifiedAt()` with onUpdate-only and a nullable
column) needs it, alongside that preset's own tests.
- Replace Phase B parity-note for SQLite's flat `dateTime` preset with a consolidation note pointing at the new `temporalAuthoringPresets()` helper (the flat preset was dropped; the consolidation makes per-target drift structurally impossible). - Update the "temporal.updatedAt() semantics" RD to reflect that the `nullable + onUpdate-only` allowance is no longer speculatively built in — a future onUpdate-only preset will introduce it alongside its own tests. - Add a "YAGNI cuts at PR-close" RD recording the two forward-compat hooks removed before merge: `applyOnEmptyUpdate` opt-in and `nullable + onUpdate-only` allowance.
Resolves a single modify/delete conflict on `docs/products/psl/README.md`. The file was deleted on main in commit b5c3381 ("elevate PSL authoring to a top-level May workstream") for being "significantly stale, unreferenced, and actively misleading." This branch had previously made small edits to the same file as part of Phase D (spec AC9 / FR10 named it as the canonical PSL doc). Resolution: accept main's deletion. The temporal-preset writeup survives in the package READMEs (`packages/2-sql/2-authoring/contract-psl/README.md`, `packages/2-sql/2-authoring/contract-ts/{README.md,API.md}`), none of which link back to the deleted file. A fresh canonical PSL doc is owned by the May WS5 workstream and is out of scope for this PR. `plan.md` records the AC9/FR10 adjustment. Validation: full package test suite green post-merge (sql-contract-psl, sql-contract-ts, sql-runtime, sql-orm-client) plus `pnpm lint:deps`.
…sting Per CodeRabbit review on PR #422: `resolveAuthoringColumnDefaultTemplate` in `framework-authoring.ts` was casting `value as ColumnDefaultLiteralInputValue` without runtime validation. Since `resolveAuthoringTemplateValue` returns `unknown` (it dereferences caller-provided `AuthoringArgRef` indices into arbitrary user args), the cast would silently emit class instances or non-JSON-serializable types into the contract. Add an `isColumnDefaultLiteralInputValue` predicate to `contract/types.ts` that recursively validates JSON primitives, plain arrays/objects of JSON values, and `Date` instances. Use it in `framework-authoring.ts` to throw a clear error when a resolved literal default doesn't match the input shape — mirrors the pattern already used by `resolveExecutionMutationDefaultPhase` with `isExecutionMutationDefaultValue`. Drops the cast entirely; the predicate narrows `value` to the exact type.
…dule Per CodeRabbit review on PR #422: `src/core/timestamp-now-generator.ts` mixed control-plane (`timestampNowControlDescriptor`, `temporalAuthoringPresets`) and runtime-plane (`timestampNowRuntimeGenerator`) implementations in a single file. The runtime function pulled in `RuntimeMutationDefaultGenerator` from `@prisma-next/sql-runtime`, dragging a runtime-plane import into a file otherwise consumed only by control-plane callers. Extract `timestampNowRuntimeGenerator` into a dedicated `src/core/timestamp-now-runtime-generator.ts`. The control-plane file no longer imports from `@prisma-next/sql-runtime`. `src/exports/runtime.ts` re-exports from the new location. No behavior change; `temporal.updatedAt()` and the per-target adapter wiring continue to resolve the same descriptor + generator pair.
Per CodeRabbit review on PR #422: the `stableAcrossRows` test compared `.getTime()` across the three Dates observed in a 3-row bulk insert. That assertion would still pass if the generator produced three distinct `Date` instances within the same millisecond, defeating the point of the test. Tighten to identity (`toBe`), so the test fails unless the cached `Date` instance is reused across rows — which is exactly what `stableAcrossRows` + `acrossRowsCache` claim to do.
…69, 070) Reject the four silent-acceptance gaps surfaced by the PR #424 review: duplicate @@id blocks, duplicate fields inside an @@id list, and optional fields used as inline @id or in an @@id list. Adds the missing diagnostic tests for each new branch and for the previously-untested inline-vs-block conflict.
…K invariant (todos 071, 072, 073, 074, 075) Replace the three-variable (inlinePrimaryKey/modelPrimaryKey/span) plus post-loop merge with a single primaryKey local; co-locate the inline-vs-block and duplicate-@@id conflict checks inside the @@id branch. Reject inline @id on multiple fields (PSL composite identity must use @@id), drop the two avoidable as-SqlStorage casts in the new tests, add round-trip companion tests that exercise the printer's snapshot output through the interpreter, and document the implicit ORM single-PK invariant in docs/architecture docs/subsystems/3. Query Lanes.md.
…el (todo 078) Rename messagePrefix and contextLabel parameters on parseAttributeFieldList and mapFieldNamesToColumns to entityLabel, matching the dominant naming convention across psl-attribute-parsing, psl-column-resolution, psl-authoring-arguments, and framework-components/control. Hoist the "Model X @@<attr>" label to a single per-iteration local in the model attribute loop instead of recomputing it 6 times across the @@id / @@unique / @@index branches. No behavior change; diagnostic message text unchanged.
Lift the duplicate-field-name check that PR #424 added to the @@id branch to @@unique and @@index, where the same gap existed: a list like @@unique([email, email]) previously emitted columns: ['email', 'email'] and only failed at DDL time (CREATE UNIQUE INDEX rejects duplicates), while @@index([email, email]) silently produced a wasteful index. Extracted findDuplicateFieldName into psl-attribute-parsing alongside the other field-list helpers; called from all three model-attribute branches with the unified entityLabel diagnostic shape.
8ff2127 to
405ea06
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
saevarb
left a comment
There was a problem hiding this comment.
This looks good to me. 👍 I also ran it through my agent and it didn't really come up with anything substantive either.
Preserve literal temporal preset codec and native types so the integration type assertions keep exact target strings. Stabilize the package tests by matching the nullable emitter round-trip timeout to the other TypeScript round-trip cases, isolating the CLI emitter mock baseline, and disabling CLI file parallelism under non-isolated Vitest. Invalidate Vite generated contract artifacts after each successful emit so framework SSR modules reload fresh contract JSON during dev re-emits.
wmadden
left a comment
There was a problem hiding this comment.
Looks good barring two small comments 👍🏻
There was a problem hiding this comment.
There are no corresponding changes to the Mongo PSL interpreter - should there be?
There was a problem hiding this comment.
That is on purpose! It would've bloated out the scope of this PR. I called it out in the PR description as a non-goal :)
SevInf
left a comment
There was a problem hiding this comment.
Nice job, just a few comments.
I think we'll need a follow up to this to properly assess and gate all the places where ORM actually expects PK
# Conflicts: # packages/1-framework/3-tooling/cli/src/commands/init/templates/code-templates.ts # packages/1-framework/3-tooling/cli/test/commands/init/__snapshots__/templates.test.ts.snap # packages/2-sql/2-authoring/contract-psl/src/psl-column-resolution.ts # packages/2-sql/2-authoring/contract-psl/src/psl-field-resolution.ts # packages/2-sql/2-authoring/contract-psl/test/interpreter.defaults.test.ts # packages/2-sql/2-authoring/contract-psl/test/ts-psl-parity.test.ts # packages/2-sql/2-authoring/contract-ts/src/composed-authoring-helpers.ts # packages/2-sql/4-lanes/relational-core/src/query-lane-context.ts # packages/2-sql/5-runtime/src/sql-context.ts # packages/2-sql/5-runtime/test/sql-context.test.ts # packages/2-sql/9-family/src/core/timestamp-now-generator.ts # packages/2-sql/9-family/src/core/timestamp-now-runtime-generator.ts # packages/3-extensions/sql-orm-client/src/collection.ts # packages/3-extensions/sql-orm-client/test/collection-mutation-defaults.test.ts # packages/3-targets/6-adapters/postgres/src/core/control-mutation-defaults.ts # packages/3-targets/6-adapters/postgres/src/exports/runtime.ts # packages/3-targets/6-adapters/postgres/test/control-mutation-defaults.test.ts # packages/3-targets/6-adapters/sqlite/src/core/control-mutation-defaults.ts # packages/3-targets/6-adapters/sqlite/src/core/runtime-adapter.ts # packages/3-targets/6-adapters/sqlite/test/control-mutation-defaults.test.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/1-framework/3-tooling/emitter/test/emitter.roundtrip.test.ts (1)
236-238: ⚡ Quick winUse one object matcher for related nullable assertions.
These three assertions validate one logical shape and can be expressed as a single
toMatchObjectfor consistency with repo test style.💡 Suggested change
- expect(id['nullable']).toBe(false); - expect(email['nullable']).toBe(true); - expect(name['nullable']).toBe(false); + expect({ + id: id['nullable'], + email: email['nullable'], + name: name['nullable'], + }).toMatchObject({ + id: false, + email: true, + name: false, + });As per coding guidelines, "
**/*.test.ts: Prefer object matchers (toMatchObject) over multiple individual expect().toBe() calls when checking 2 or more related values in tests".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/1-framework/3-tooling/emitter/test/emitter.roundtrip.test.ts` around lines 236 - 238, Replace the three separate nullable assertions for id, email, and name with a single object matcher assertion using toMatchObject: build an object mapping each field to its nullable value (referencing the existing id, email, name symbols) and assert that the container/object being tested matches that shape with toMatchObject, keeping the same expected boolean values for nullable on id (false), email (true) and name (false).
🤖 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/3-tooling/emitter/test/emitter.roundtrip.test.ts`:
- Around line 236-238: Replace the three separate nullable assertions for id,
email, and name with a single object matcher assertion using toMatchObject:
build an object mapping each field to its nullable value (referencing the
existing id, email, name symbols) and assert that the container/object being
tested matches that shape with toMatchObject, keeping the same expected boolean
values for nullable on id (false), email (true) and name (false).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: b1699a2d-e5ea-4e37-ae62-b7227cab2d1f
📒 Files selected for processing (7)
packages/1-framework/3-tooling/cli/test/control-api/client.test.tspackages/1-framework/3-tooling/cli/vitest.config.tspackages/1-framework/3-tooling/emitter/test/emitter.roundtrip.test.tspackages/1-framework/3-tooling/vite-plugin-contract-emit/src/plugin.tspackages/1-framework/3-tooling/vite-plugin-contract-emit/test/plugin.test.tspackages/2-sql/2-authoring/contract-psl/src/interpreter.tspackages/2-sql/2-authoring/contract-psl/src/psl-field-resolution.ts
✅ Files skipped from review due to trivial changes (2)
- packages/1-framework/3-tooling/cli/vitest.config.ts
- packages/1-framework/3-tooling/cli/test/control-api/client.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/2-sql/2-authoring/contract-psl/src/psl-field-resolution.ts
- packages/2-sql/2-authoring/contract-psl/src/interpreter.ts
|
Follow-up from the review summary: agreed that the ORM primary-key assumptions need a separate pass. I kept this PR scoped to the id-less contract/authoring work plus docs clarification. The remaining ORM work is to audit and gate primary-key fallback paths, not to block predicate-based find/update/delete on id-less tables. |
Source content for the Linear ticket that this PR should land under. Follow-up to TML-2380 (PR #424); covers the ORM-lane completion work that builds on the authoring/emission support TML-2380 introduced. Includes scope, out-of-scope, acceptance criteria, and references. Maintainer will create the Linear ticket from this content and either replace this file with the ticket URL or delete it post-creation.
…tion Linear ticket created: https://linear.app/prisma-company/issue/TML-2432 The Linear: line is now at the top of the PR description, matching the convention used by the parent PR #424 (TML-2380).
Linear: TML-2380 — sql: add support for id-less models
Stacked on #422.
Intent
Make SQL PSL authoring round-trip database tables that do not have primary keys, while also lowering model-level composite
@@iddeclarations into SQL storage primary keys. This keeps introspected SQL PSL emit-able without changing document-family behavior or adding runtime ORM APIs for id-less models.The PR also closes five pre-existing silent-failure paths in the identity / constraint validation surface that the multi-agent review surfaced; see "Identity & constraint validation tightening" below.
Change map
@@idlowering, id-less acceptance, composite identity validation, and the inline-vs-block / duplicate-@@idconflict policy@idon optional fieldsentityLabelparameter andfindDuplicateFieldNamehelperprimaryKeyThe story
primaryKeyproperty instead of reportingPSL_MISSING_PRIMARY_KEY.@@idis parsed with the existing model-attribute helpers, maps field names through@map, preserves the optionalmapconstraint name, and feeds the same SQL primary-key node used by inline@id.primaryKeywhile retaining validation for primary-key column references when a primary key is present.@@idblocks, duplicate columns in@@id/@@unique/@@index, nullable@id/@@idcolumns, multi-field inline@id) now produce explicit diagnostics rather than emit semantically broken contracts.Identity & constraint validation tightening
In addition to enabling id-less models, the PR adds explicit
PSL_INVALID_ATTRIBUTE_ARGUMENTdiagnostics for five pre-existing gaps where the interpreter previously accepted invalid input and emitted a structurally or semantically broken contract:@@idblocks on the same model: previously the second silently overwrote the first; now the duplicate is rejected.@@id,@@unique, or@@index(e.g.@@id([email, email])): previously emittedcolumns: ['email', 'email']— invalid SQL that fails at DDL time for@@id/@@uniqueand produces a wasteful no-op index for@@index. Now rejected at authoring time via the sharedfindDuplicateFieldNamehelper.@idor in@@id: previously accepted, producing a NULLABLE primary-key column that the database would reject at DDL time; now rejected at authoring time with an explanatory diagnostic.@idon multiple fields (e.g.id1 Int @idtogether withid2 Int @id): previously silently lowered into a composite primary key whosenamewas dropped; now rejected with a hint to use@@id([...])instead.The conflict between inline
@idand@@idon the same model also gains a single-clause diagnostic in the established voice ("cannot declare both field-level @id and model-level @@id").Behavior changes & evidence
@idor@@idproduces a table with noprimaryKeywhile preserving columns, uniques, and model field mappings.@@idlowers to SQL primary keys:@@id([email, token])emitsprimaryKey.columnsin field order, and mapped fields plusmap: "..."are reflected in the contract.primaryKeyare valid, but existing validation still checksprimaryKey.columnswhen present.printPsl(sqlSchemaIrToPslAst(ir))for id-less and composite-PK tables; a newdescribe('round-trips printer output', ...)block ininterpreter.test.tsparses those exact snapshot strings, so a drift on either side breaks one of the two suites.Compatibility / migration / risk
@idmodels continue to emit the same SQL primary-key shape — no change tostorageHashorprofileHashfor any pre-existing valid schema (verified per ADR 010 canonicalization rules:idis still emitted viaifDefined('id', primaryKey), so absent values remain absent).storage.tables.<t>.primaryKeywas already optional per ADR 172).PSL_MISSING_PRIMARY_KEYis removed from the diagnostic set. It has no production references outside the test that was updated; this is aContractSourceDiagnosticcode (authoring time), not aRuntimeErrorenvelope code, so ADR 027 stability rules do not apply.model.idortable.primaryKey. The 9-family migrations / schema-verify / control-instance code paths that do referencetable.primaryKeyalready optional-chain.pnpm lint:depsreports zero violations.docs/architecture docs/subsystems/3. Query Lanes.mdas an implicit invariant pending a future per-model capability gate (e.g.sql.ormRequiresPrimaryKey). Today the ORM lane has no code that depends onmodel.id, so id-less models remain compatible with composition; the doc records the constraint for future ORM-op authors.Validation
pnpm --filter @prisma-next/sql-contract-psl test— 132 tests pass (was 118; +14)pnpm --filter @prisma-next/sql-contract-emitter test— 103 tests passpnpm --filter @prisma-next/sql-contract-psl typecheckpnpm --filter @prisma-next/sql-contract-emitter typecheckpnpm test:packages— 110 tasks, all greenpnpm lint:deps— 0 dependency violationsFollow-ups / open questions
sql.ormRequiresPrimaryKeycapability flag and ORM-lane composition gate (currently doc-only).projects/id-less-sql-models/during project close-out perprojects/README.md.Non-goals / intentionally out of scope
Summary by CodeRabbit
Documentation
Bug Fixes
Tests
Tooling