TML-3037: contract infer output round-trips through contract emit#998
Open
wmadden-electric wants to merge 25 commits into
Open
TML-3037: contract infer output round-trips through contract emit#998wmadden-electric wants to merge 25 commits into
wmadden-electric wants to merge 25 commits into
Conversation
Spec and plan for TML-3037. An external user's post-processing script, audited claim-by-claim, surfaced eight defects — all instances of one class: contract infer writes PSL that contract emit rejects, or that db verify then reports as drift. Our own extension-supabase generate-contract.ts is independently the same workaround script, which is the evidence this is worth fixing at the source rather than papering over per-consumer. No test anywhere does introspect → infer → emit. That is why all eight shipped, and building that instrument is the first dispatch. Refs: TML-3037 Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
…against a live database `contract infer` output does not survive `contract emit`, and no test anywhere drives introspect -> infer -> emit, which is why eight defects shipped. This adds the instrument that reproduces all of them, and the failure record it produced. Two files, because the CLI path cannot observe runtime decode: - `cli-journeys/infer-roundtrip-fidelity.e2e.test.ts` drives infer -> emit -> `db verify --schema-only` over a schema carrying already-plural table names, a 1:1 and a 1:N FK, both `GENERATED ... AS IDENTITY` variants plus a `serial` control, bounded and unbounded `numeric`, a `date`, array/jsonb defaults, GIN and `USING hash` indexes, and an FK pointing out of the introspected scope. - `infer-roundtrip-runtime.integration.test.ts` builds a real `ExecutionContext` against an emitted contract and reads a `date` through `.include()` — the only way to see the numeric-connect and date-decode defects. All ten assertions fail today; each asserts the fixed behaviour, so each flips green as its fix lands. Each `it()` isolates one defect, repairing the unrelated emit-blockers so a single run reports every defect independently. `projects/infer-emit-roundtrip/reproduction.md` records, per defect, the command that surfaces it and the verbatim error. Two findings correct the spec: the unbounded-numeric crash arrives via the bare `Decimal` base scalar, not via `@db.Numeric` with no args (infer never prints that attribute), so the affected surface is every `Decimal` field on postgres; and only the to-many side of a relation double-pluralizes, so this fixture reproduces `sessionses` but not `identitieses`. Refs: TML-3037 Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
…gres D1 reproduced the numeric crash and showed the spec had the mechanism wrong. It does not arrive via @db.Numeric with no arguments; infer never prints that. It arrives via the base-scalar path, where control-mutation-defaults maps Decimal to pg/numeric@1 with no typeParams — so a plain `amount Decimal` field crashes at connect regardless of any attribute. This is not an infer defect that happens to touch Decimal. Decimal has never worked on postgres; infer is just the first thing that generates one. No .prisma file in the repo declares a Decimal field, which is why it went unnoticed. The chosen fix is unchanged, but D4 must test the bare scalar — an attribute-only test would pass while the defect ships. Refs: TML-3037 Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
`pluralize()` appended `es` to any word ending in s/x/z/ch/sh, so an already-plural table name like `sessions` doubled to `sessionses` in inferred back-relation field names. Replaced the hand-rolled suffix rule with the `pluralize` package (real inflection: irregular/uncountable word lists plus ordered regex rules, not a trailing-character heuristic), per TML-3024 and the spec's preference for a maintained library over a hand-rolled table. Verified it satisfies every row of the acceptance table, including the two cases that pull in opposite directions: already plural (sessions, identities, mfaAmrClaims unchanged) and singular-but-s-ending (status -> statuses, bus -> buses, class -> classes). Added `@types/pluralize` as a dev dependency since the library ships untyped. Import as a default import (`import pluralizeLib from 'pluralize'`), not a namespace import: pluralize assigns its whole CJS module.exports in one statement, so cjs-module-lexer-based interop (Node's native ESM loader, used when the CLI runs the built dist/psl-infer.mjs) cannot statically detect `.plural` as a named export and leaves the namespace with only `default`. A default import always resolves to the full module.exports value, so `pluralizeLib.plural` works reliably. Vitest hides this because Vite pre-bundles CJS deps with esbuild, which copies the CJS exports' own properties onto the namespace at runtime; confirmed the failure and the fix against a real Node ESM import, not just the test runner. Refs: TML-3037 Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
…ults infer prints The interpreter only collected back-relation candidates for list-typed fields, so a bare, `@relation`-less model-typed field (the back side of a 1:1) fell through to scalar resolution and died with PSL_UNSUPPORTED_FIELD_TYPE. A model-typed, non-list field with no `@relation` can only be the back side — the owning side always carries `@relation(fields, references)` to declare its FK columns — so this is never ambiguous. ModelBackrelationCandidate now carries `isList`, and applyBackrelationCandidates resolves cardinality to `1:1` for a singular candidate (`1:N` unchanged for list) and skips many-to-many junction matching for singular candidates, since that concept only applies to lists. Separately, the list-default check conflated a storage-level SQL default (`dbgenerated(...)`, `now()`, lowered as `defaultValue.kind: "function"`) with a genuine per-element execution default (`executionDefaults.onCreate`, e.g. `uuid()`), rejecting both on list fields. Only the latter has no list semantics; Postgres accepts `DEFAULT (expression)` on an array column regardless of `many`, and buildColumnDefaultSql already renders it that way. The check now only rejects `executionDefaults.onCreate` on a list field. Refs: TML-3037 Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
Permitting storage-level function defaults on lists (the defect-7 fix) also let `Int[] @default(autoincrement())` through, which the postgres DDL builder miscompiles: buildColumnTypeSql keys on the `autoincrement()` sentinel to emit SERIAL and drops the array suffix entirely. The old broad `kind === "function"` rule blocked that shape, but for the wrong reason — it called every storage default an execution default. Restoring it would undo defect 7, so this adds a narrow guard instead: a sequence yields one scalar per row, which an array column cannot be. The check keys on the lowered `autoincrement()` sentinel rather than the authored call name. Every target lowers the call to that exact expression and keys its DDL rendering off it, so rejecting the sentinel keeps the shape out of the DDL builders however it was spelled. Both sides of the line are tested: Int[] @default(autoincrement()) errors, String[] @default(dbgenerated("{}::text[]")) still lowers to a storage function default. Scalar @default(autoincrement()) is untouched. Also regenerates the print-psl inline snapshot invalidated by the pluralize fix: `child` now pluralizes to `children`, the correct irregular plural, where the hand-rolled rule produced `childs`. Refs: TML-3037 Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
… date `NumericParams.precision` was required while every sibling temporal codec (`PrecisionParams`) declares its param optional. A bare `amount Decimal` field — the base-scalar path `control-mutation-defaults.ts` maps every postgres `Decimal` field through, with or without `@db.Numeric` — produced a `pg/numeric@1` ref with no `typeParams` and crashed `createExecutionContext` with `RUNTIME.CODEC_PARAMETERIZATION_MISMATCH`. Make `precision` optional on `NumericParams`/`numericParamsSchema`, matching the sibling pattern; `assertColumnCodecIntegrity` is untouched. A bounded `numeric(10,2)` still carries and enforces its params. No `pg/date@1` codec existed: `@db.Date` inherited `DateTime`s `pg/timestamptz@1`. Top-level `SELECT` survived because `decode()` was a passthrough over the driver-parsed `Date`, but `.include()` (`json_agg` -> `decodeJson()`) rejected a bare `YYYY-MM-DD` string. Add a real `pg/date@1` codec and point `@db.Date` at it. A Postgres `date` has no timezone, so the codec canonicalizes its JS-level value as a `Date` at UTC midnight: `decode()` re-derives the calendar date from the pg drivers local-midnight `Date` via local getters and reconstructs it via `Date.UTC`; `encode()` formats via UTC getters directly to `YYYY-MM-DD`, bypassing the pg drivers own local-getter-based `Date` serialization (which would shift the day near midnight under a negative UTC offset). `decodeJson`/`encodeJson` share the same `YYYY-MM-DD` representation. BREAKING: `@db.Date` columns now emit `codecId: "pg/date@1"` instead of inheriting `"pg/timestamptz@1"`, which changes the contract hash and therefore signed markers for any existing `@db.Date` column. Upgrade instructions are dispatch D9s job, not covered here. RT.01 (unbounded numeric via infer) and RT.02 (date via .include()) in test/integration/test/infer-roundtrip-runtime.integration.test.ts now pass. RT.02s top-level assertion is tightened from toBeInstanceOf(Date) to the exact UTC instant, now that pg/date@1 owns the conversion and the round trip no longer depends on the process timezone. Added RT.03, authoring "amount Decimal" directly (no @db.Numeric, bypassing infer) so the base-scalar fix is proven independently of infers own behavior. Refs: TML-3037 Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
…dangling FKs The postgres target registered zero index types, so validateIndexTypes rejected any @@index(..., type: "gin"/"hash") contract infer produced for a non-default access method. Register the six access methods Postgres ships (btree, hash, gin, gist, spgist, brin) via IndexTypeRegistry, following the same wiring ParadeDB already uses for its own bm25 type. Options schemas stay permissive; per-method option validation is a later slice. btree is Postgres default access method already normalized to `undefined` by introspection, so registering it does not change what gets emitted for an ordinary index. Separately, resolveForeignKeys correctly drops a foreign key whose target is outside the introspected schema, but did so silently: the scalar column survived with no comment and no warning, so a user loses every such relationship (e.g. every auth.users FK in a Supabase database) with no indication. Track dropped FKs per table and emit an explanatory comment on the model, matching the existing missing-primary-key comment mechanism and voice. Refs: TML-3037 Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
… both sides `GENERATED ALWAYS AS IDENTITY` and `GENERATED BY DEFAULT AS IDENTITY` columns report no `column_default` at all — Postgres tracks generation via `pg_attribute.attidentity`, not a default expression — so infer emitted a bare column with no default, and even once printed, the introspected side had nothing to compare a `@default(autoincrement())` contract against. Fixed symmetrically, on both sides: - `SqlColumnIR` gains an `identity` boolean (introspection-only; a contract-derived column never sets it, since PSL has no identity vocabulary). - The postgres columns query selects `a.attidentity` and threads it onto the column; the control adapter computes `resolvedDefault` from the identity flag when set, since there is no raw expression to parse. - `parsePostgresDefault` takes an `identity` option that resolves straight to `autoincrement()`, independent of `rawDefault`. - Postgres infer (`buildScalarField`) prints `@default(autoincrement())` for an identity column, the same as it already does for `serial`. Deliberate boundary: at the contract's altitude, `autoincrement()` means "the database generates this value" — identity and `serial` are both that, and PSL has no syntax to distinguish `GENERATED ALWAYS` from `GENERATED BY DEFAULT`. This maps both identity variants onto `autoincrement()` rather than modelling the distinction. Consequence: a fresh `db init` from such a contract creates a `serial` column, not an identity one — a pre-existing gap (identity has never been authorable), unchanged by this fix. Verified against a live database: a new introspection integration test proves both identity variants thread through as `identity: true` with `resolvedDefault: autoincrement()`, that `serial` is unaffected, and that a plain column gets no default. The TML-3037 D1 instrument's IF.06 assertion (both `GENERATED` variants print `@default(autoincrement())`; `serial` keeps working) now passes, and a manual reduction of the same journey (stripping only the two known non-identity default mismatches) shows `db verify --schema-only` reports zero drift for the identity columns. Refs: TML-3037 Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
D6 exposed a second instance: dbgenerated("'{}'::text[]") drifts
identically to the jsonb case. D1 could not reproduce it because emit
failed before verify ever ran; clearing the emit blockers uncovered it.
The defect is any literal default the authoring and introspection paths
resolve differently — kind:function on one side, kind:literal on the
other, and resolvedDefaultsEqual compares kind first. jsonb and text[]
are two instances; enumerate the rest from parsePostgresDefault rather
than from a list.
The chosen fix already covers the class, because reusing
parsePostgresDefault (rather than copying it) means a form it learns to
parse later is consistent on both sides automatically.
Refs: TML-3037
Signed-off-by: willbot <w.a.madden+machine@gmail.com>
Signed-off-by: Will Madden <madden@prisma.io>
…aults, not just function text
Emit kept every `@default(dbgenerated("..."))` as `kind: 'function'`, even when the
raw SQL text was actually a literal (`'{}'::jsonb`, `'{}'::text[]`, NULL, true/false,
a bare number, a plain string). Introspection parses the same text through
`parsePostgresDefault` and gets `kind: 'literal'` for all of those forms.
`resolvedDefaultsEqual` compares `kind` before content, so any dbgenerated
literal drifted forever in `db verify`, even when the database matched the
contract exactly. jsonb and text[] were the two instances TML-3037 caught,
but the mismatch is generic to every literal form the normalizer recognizes.
Fix the class: `lowerDbgenerated` (adapter-postgres) now resolves the raw
text through the same `parsePostgresDefault` introspection already uses,
instead of keeping it as opaque text. A form the normalizer does not
recognize as a literal falls through to its own function fallback, so
genuine functions (`gen_random_uuid()`, `nextval(...)`, arbitrary
expressions) are unaffected.
`parsePostgresDefault` needs the column's native type to parse array and
jsonb literals correctly, so `DefaultFunctionLoweringContext` gains an
optional `nativeType` field, populated by the SQL-family authoring layer
(`lowerDefaultForField`) with the `[]`-suffixed form for list columns —
matching the format introspection's `resolvedNativeType` already uses.
Covers every literal form `parsePostgresDefault` handles (array, null,
boolean, numeric, string, jsonb-parsed-object, out-of-range bigint) and
proves three genuine functions stay functions, including a dbgenerated
nextval(...) and an enum-cast default whose qualified type name defeats
the string-literal pattern.
No fixture moved: the only existing dbgenerated usages in the repo
(gen_random_uuid(), enum casts, an interval expression) all already
resolve to `kind: 'function'` under the new path, unchanged.
Refs: TML-3037
Signed-off-by: willbot <w.a.madden+machine@gmail.com>
Signed-off-by: Will Madden <madden@prisma.io>
…ate contract D2-D7 fixed the eight infer/emit round-trip defects TML-3037 set out to fix. This dispatch is the proof: delete the workarounds from generate-contract.ts and confirm the regenerated contract carries the fix instead of the patch. DOUBLE_PLURALIZED_FIELD_NAMES + applyDoublePluralizationFix: deleted. pluralize() (D2) is now correct for already-plural table names, so the override and the fix cancel out exactly - no back-relation name changed in the regenerated contract.prisma. INDEX_OMISSIONS + applyIndexOmissions + indexOmissionNameOf: deleted. The postgres target now registers hash as a built-in index type (D5), so auth.one_time_tokens' two USING hash indexes come back declared as @@index(..., type: "hash") instead of silently dropped. DEFAULT_OMISSIONS: reduced from 7 entries to 1. The jsonb/array dbgenerated(...) literal-default disagreement (D7) is fixed generically - authoring and introspection now resolve the same literal to the same `kind: 'literal'` shape via parsePostgresDefault - so the six custom_oauth_providers/webauthn_credentials/iceberg_namespaces entries are gone and those columns' defaults are declared again. Only auth.users.phone remains: `DEFAULT NULL` on a nullable column round-trips to an explicit `@default(null)`, which the interpreter rejects (PSL_INVALID_DEFAULT_VALUE) - a different, still-open defect this slice did not fix. COLUMN_OMISSIONS is untouched: nullable text[] columns still have no PSL form. CONTRACT-FIDELITY.md's audit summary is updated to match. Regenerated packages/3-extensions/supabase/src/contract/contract.prisma via `contract:generate`. The diff is exactly the two classes above (jsonb/array defaults restored, two hash indexes declared) plus the resulting contract.json/contract.d.ts changes - nothing else moved. reference-fixture-verify.integration.test.ts passes against the regenerated, workaround-free contract. Refs: TML-3037 Closes: TML-3024 Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
Four consumer-visible breaking changes from this slice need an upgrade entry, in both the user-skill and the extension-author-skill packages (both substrates hit the same postgres target/interpreter machinery): - pg/date@1: an existing @db.Date column's codecId changes on the next contract emit, changing storageHash, with no source change required. - dbgenerated literal defaults (jsonb/array/null/bool/numeric/string) now resolve to kind: 'literal' on both the authoring and introspection side; an existing dbgenerated(...) literal default's resolved shape changes on the next emit, changing storageHash. - Identity columns (GENERATED ... AS IDENTITY) now resolve to autoincrement() on introspection too; db verify --strict newly reports drift for an identity column whose contract predates this fix and declares no default. Non-strict verify is unaffected (undeclared auxiliary defaults are tolerated below --strict). - pluralize no longer double-pluralizes an already-plural table name in contract infer output; only affects a future infer run, not an already-generated .prisma file, but a consumer who re-infers and gets a renamed back-relation field must update the application/pack code that references it. The extension-author entry additionally explains the packages/3-extensions/supabase diff itself (the D8 workaround-deletion regeneration): it demonstrates the dbgenerated-literal-defaults fix directly, plus two additive/non-breaking changes (postgres index-type registration, the Decimal base-scalar fix) that need no entry. pnpm check:upgrade-coverage passes with these entries committed. Refs: TML-3037 Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
…-blind D7's dbgenerated fix added a nativeType field directly to the shared framework DefaultFunctionLoweringContext type, which lint:framework-vocabulary correctly rejected: nativeType is explicit family vocabulary the framework domain must not carry (per no-family-vocabulary-in-framework.mdc, which names nativeType as the paradigm example). Replace the concrete field with an opaque fieldContext?: unknown, the same pattern ControlMutationDefaultEntry.signature already uses one property over: the SQL authoring layer (contract-psl's lowerDefaultForField) populates it with a plain object, and the postgres adapter's lowerDbgenerated narrows it back via blindCast against a locally-declared shape, since core cannot name a postgres-specific type and the authoring layer has no reason to export one. Guards the case where fieldContext is entirely absent so no other lower handler that never reads it breaks. lint:framework-vocabulary is back at the committed threshold (836). Refs: TML-3037 Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
…d comments Per no-transient-project-ids-in-code.mdc, source and tests must never carry transient project-plan IDs — only durable TML-#### references. This slice's own reproduction.md and plan.md track defects/dispatches by D1-D9 and each integration test by its own IF.NN / RT.NN row; those IDs leaked into the actual test suite: - Dispatch IDs (D1, D4, D5, D6) in doc-comment headers across index-types.test.ts, identity-column-introspection.integration.test.ts, infer-roundtrip-fidelity.e2e.test.ts, and infer-roundtrip-runtime.integration.test.ts. - IF.01-IF.08 / RT.01-RT.03 test-case IDs prefixing every it() description and expect() message in the two new infer-roundtrip integration suites. Removed all of them, keeping the durable TML-3037 references and rewording the two prose mid-sentence D4 references in infer-roundtrip-runtime.integration.test.ts so the paragraph still reads correctly without the dispatch letter. Test behavior is unchanged - only names/comments moved; both files re-run green. Refs: TML-3037 Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
…efault rejection
D3's relaxation of the list-default check rejected only
executionDefaults.onCreate on a list field, permitting any storage-level
function default through. With D7 landed, the case that motivated the
relaxation - dbgenerated("'{}'::text[]") - already resolves to
kind: 'literal' before this check runs (parsePostgresDefault
recognizes the array-literal pattern), so the relaxation's only live
effect was a hole: a genuine function default on a list
(DateTime[] @default(now()), an array-returning dbgenerated(...)) now
emits a contract whose DDL Postgres rejects at apply time instead of
being caught at authoring time.
Restores the original check: isListField && (loweredOnCreate ||
loweredFunctionDefault). This also makes the narrow
PSL_LIST_AUTOINCREMENT_UNSUPPORTED guard D3 added afterward redundant -
it existed only to re-plug the hole for one function (autoincrement());
the restored broad check covers it along with every other function-kind
default, so the guard and its error code are deleted. Restores the
now()-on-a-list rejection test D3 deleted and replaces the two
D3-added "accepts" tests (dbgenerated array literal, now()) and the
narrow autoincrement rejection test with the original three
rejects-an-execution-default tests (now, uuid, autoincrement).
Verified empirically via the CLI journey (infer -> emit -> db verify):
String[] @default(dbgenerated("'{}'::text[]")) still emits and
round-trips clean, confirming D7's literal resolution is what makes
this case work, not the now-reverted relaxation.
A genuine array-returning function default
(dbgenerated("ARRAY[gen_random_uuid()]")) stays rejected - pre-existing
behaviour, not a regression, and a candidate for a future authoring
feature rather than something to fix here.
Refs: TML-3037
Signed-off-by: willbot <w.a.madden+machine@gmail.com>
Signed-off-by: Will Madden <madden@prisma.io>
…n too The 1:1 back-relation guard added by D3 skipped every singular model-typed field carrying @relation, under a comment claiming "it declares fields/references" - which it never actually checked. Infer prints a named singular back relation (@relation(name: "..."), no fields/references) whenever two FKs connect the same table pair, or a unique FK self-references its own table - relation-inference.ts sets relationName whenever needsRelationName is true, independent of whether the candidate is a list. That shape still hit PSL_INVALID_RELATION_ATTRIBUTE ("requires fields and references arguments"), because both the backrelation-candidate collector and the owning-side FK-building loop keyed off "has @relation", not off "declares fields/references". Adds relationAttributeDeclaresOwningSide(), which checks for the fields/references named arguments the owning side actually declares, and uses it in both places: the collector now only skips a genuine owning-side field, and the FK-building loop now skips a back-relation field (named or bare) instead of trying to validate fields/references it never has. The single-FK-per-pair fixture couldn't see this - it never produced a relationName. Extends the round-trip fixture with `accounts` (two FKs to `users`, one unique) and a self-referencing `categories.parent_id unique`, and adds two isolated tests proving both shapes now emit, alongside the existing tests proving the genuine owning side (fields + references) is still treated as such. Refs: TML-3037 Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
…t function rewrites
D7 made lowerDbgenerated() reuse parsePostgresDefault() so a literal
default (e.g. '{}'::jsonb) resolves the same way on both the
authoring and introspection sides. But parsePostgresDefault() also
rewrites some function forms for introspection's benefit - notably
nextval(...) -> autoincrement() - and the authoring side was adopting
those rewrites wholesale. dbgenerated("nextval('my_seq')") silently
lowered to autoincrement(), whose DDL renders SERIAL - creating a
fresh table_col_seq sequence and discarding the user's named one.
Authoring now adopts the parser's result only when it resolves to
kind: 'literal'; a kind: 'function' resolution (or no resolution)
keeps the original raw expression text unchanged. This keeps the class
fix intact for the case it was meant for - both sides still agree on
literals - while authoring no longer rewrites function expressions it
was only ever meant to read.
Corrects the "dbgenerated keeps genuine functions as functions" test:
it previously asserted the nextval() rewrite as the *expected*
behaviour ("normalized to autoincrement(), not demoted to a literal"),
which is exactly the bug. It now asserts the expression is preserved
verbatim. Also drops the transient "F13" dispatch-finding label from
that describe block's name, missed by the commit that swept the
others (23cb5d7).
A contract authored with dbgenerated("nextval('my_seq')") will still
drift against a live column under `db verify`, since introspection
maps a live nextval default to autoincrement() regardless - that
mismatch pre-dates this slice and is not chased here.
Corrects the upgrade note in both 0.15-to-0.16/instructions.md copies
(user-facing and extension-author): "nextval(...) is unaffected" was
true of `kind` but not of the expression text before this fix: the
note now explains authoring only adopts literal resolutions, so a
named sequence is preserved rather than silently becoming
autoincrement().
Refs: TML-3037
Signed-off-by: willbot <w.a.madden+machine@gmail.com>
Signed-off-by: Will Madden <madden@prisma.io>
…), not just no-throw The include() date-decode test only checked that nothing threw and discarded the row, so a decode that produced a *wrong* date would still pass. That assertion is the only coverage for the defect pg/date@1 was built for - the top-level select() path never had the bug (decode() there is a passthrough over an already-parsed Date), so this is the one path that actually exercises decodeJson()'s YYYY-MM-DD handling. Captures the include() result and asserts it equals the exact expected row, matching the top-level assertion's style immediately above it. Refs: TML-3037 Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
Review caught that the prescribed remedy was unsound. Relaxing the list check to permit storage function defaults admits DateTime[] @default(now()), whose DDL postgres refuses — trading an authoring-time error for a DDL-apply-time one. Nothing in the authoring layer can tell whether a function returns an array, so the relaxation cannot be made safe. The tell was in the implementation: it needed a special case for autoincrement() to plug one instance of that hole. Once defect 8 resolves literal defaults through parsePostgresDefault, dbgenerated("{}::text[]") is a literal and never reaches the list check. The check stays as it was and the case works anyway. Refs: TML-3037 Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
Contributor
📝 WalkthroughWalkthroughThe PR improves Postgres infer-to-emit fidelity across relations, defaults, identity columns, dates, numeric types, indexes, and dangling foreign keys. It adds singular backrelation handling, library-based pluralization, a dedicated ChangesPostgres contract fidelity
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
@prisma-next/extension-author-tools
@prisma-next/mongo-runtime
@prisma-next/family-mongo
@prisma-next/sql-runtime
@prisma-next/family-sql
@prisma-next/extension-arktype-json
@prisma-next/middleware-cache
@prisma-next/mongo
@prisma-next/extension-paradedb
@prisma-next/extension-pgvector
@prisma-next/extension-postgis
@prisma-next/postgres
@prisma-next/sql-orm-client
@prisma-next/sqlite
@prisma-next/extension-supabase
@prisma-next/target-mongo
@prisma-next/adapter-mongo
@prisma-next/driver-mongo
@prisma-next/contract
@prisma-next/utils
@prisma-next/config
@prisma-next/errors
@prisma-next/framework-components
@prisma-next/operations
@prisma-next/ts-render
@prisma-next/contract-authoring
@prisma-next/ids
@prisma-next/psl-parser
@prisma-next/psl-printer
@prisma-next/cli
@prisma-next/cli-telemetry
@prisma-next/config-loader
@prisma-next/emitter
@prisma-next/language-server
@prisma-next/migration-tools
prisma-next
@prisma-next/vite-plugin-contract-emit
@prisma-next/mongo-codec
@prisma-next/mongo-contract
@prisma-next/mongo-value
@prisma-next/mongo-contract-psl
@prisma-next/mongo-contract-ts
@prisma-next/mongo-emitter
@prisma-next/mongo-schema-ir
@prisma-next/mongo-query-ast
@prisma-next/mongo-orm
@prisma-next/mongo-query-builder
@prisma-next/mongo-lowering
@prisma-next/mongo-wire
@prisma-next/sql-contract
@prisma-next/sql-errors
@prisma-next/sql-operations
@prisma-next/sql-schema-ir
@prisma-next/sql-contract-psl
@prisma-next/sql-contract-ts
@prisma-next/sql-contract-emitter
@prisma-next/sql-lane-query-builder
@prisma-next/sql-relational-core
@prisma-next/sql-builder
@prisma-next/target-postgres
@prisma-next/target-sqlite
@prisma-next/adapter-postgres
@prisma-next/adapter-sqlite
@prisma-next/driver-postgres
@prisma-next/driver-sqlite
commit: |
size-limit report 📦
|
![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: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/3-targets/3-targets/postgres/src/core/codec-helpers.ts`:
- Around line 146-155: Update the date-decoding logic around ISO_8601_DATE and
the Date.UTC construction to validate the resulting UTC year, month, and day
against the parsed yearText, monthText, and dayText before returning. Reject
normalized dates such as 2024-02-31 and invalid months, while preserving the
existing invalid-date error behavior; ensure years below 100 are compared
against the parsed year rather than Date.UTC’s 1900-based remapping, and add
regression coverage for the specified cases.
In `@packages/3-targets/3-targets/postgres/src/core/codecs.ts`:
- Around line 100-109: Update NumericParams and numericParamsSchema so the
numeric options are either empty or include required precision with optional
scale; preserve the existing precision and scale bounds while rejecting
configurations that specify scale without precision. Ensure
pgNumericRenderOutputType receives the validated precision/scale shape and no
longer silently ignores scale.
In
`@packages/3-targets/6-adapters/postgres/test/control-mutation-defaults.test.ts`:
- Around line 156-159: Update both ifDefined spreads in
packages/3-targets/6-adapters/postgres/test/control-mutation-defaults.test.ts at
lines 156-159 and 251-255 to pass an object containing nativeType only when
nativeType is defined, otherwise pass undefined; this must omit fieldContext
entirely when nativeType is missing and avoid the nested ifDefined/blindCast
behavior.
In `@test/integration/test/cli-journeys/infer-roundtrip-fidelity.e2e.test.ts`:
- Around line 140-149: Strengthen the assertions in
test/integration/test/cli-journeys/infer-roundtrip-fidelity.e2e.test.ts:140-149
by verifying the bare identities Identities? back-relation immediately after
runContractInfer and before fixListDefault/fixIndexTypes; at 240-248 assert the
inferred tags list default before emit; at 267-275 assert both inferred gin and
hash index declarations; at 297-305 match diagnostics against the dropped-FK
identifiers rather than preceding comments; and at 346-359 assert the inferred
JSONB literal default before emit and verify it afterward.
🪄 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: fc99cc0d-e24f-4809-a861-5dbc00f10d46
⛔ Files ignored due to path filters (4)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlprojects/infer-emit-roundtrip/plan.mdis excluded by!projects/**projects/infer-emit-roundtrip/reproduction.mdis excluded by!projects/**projects/infer-emit-roundtrip/spec.mdis excluded by!projects/**
📒 Files selected for processing (44)
packages/1-framework/1-core/framework-components/src/shared/mutation-default-types.tspackages/2-sql/1-core/schema-ir/src/ir/sql-column-ir.tspackages/2-sql/1-core/schema-ir/test/sql-column-ir.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/src/psl-field-resolution.tspackages/2-sql/2-authoring/contract-psl/src/psl-relation-resolution.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.relations.test.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.types.test.tspackages/2-sql/9-family/package.jsonpackages/2-sql/9-family/src/core/psl-contract-infer/name-transforms.tspackages/2-sql/9-family/test/psl-contract-infer/name-transforms.test.tspackages/3-extensions/supabase/scripts/generate-contract.tspackages/3-extensions/supabase/src/contract/CONTRACT-FIDELITY.mdpackages/3-extensions/supabase/src/contract/contract.d.tspackages/3-extensions/supabase/src/contract/contract.jsonpackages/3-extensions/supabase/src/contract/contract.prismapackages/3-targets/3-targets/postgres/src/core/codec-helpers.tspackages/3-targets/3-targets/postgres/src/core/codec-ids.tspackages/3-targets/3-targets/postgres/src/core/codecs.tspackages/3-targets/3-targets/postgres/src/core/default-normalizer.tspackages/3-targets/3-targets/postgres/src/core/descriptor-meta.tspackages/3-targets/3-targets/postgres/src/core/index-types.tspackages/3-targets/3-targets/postgres/src/core/psl-infer/infer-psl-contract.tspackages/3-targets/3-targets/postgres/src/exports/codecs.tspackages/3-targets/3-targets/postgres/src/exports/default-normalizer.tspackages/3-targets/3-targets/postgres/test/codecs-class.test.tspackages/3-targets/3-targets/postgres/test/codecs-class.types.test-d.tspackages/3-targets/3-targets/postgres/test/codecs-runtime-and-helpers.test.tspackages/3-targets/3-targets/postgres/test/codecs.test.tspackages/3-targets/3-targets/postgres/test/default-normalizer.test.tspackages/3-targets/3-targets/postgres/test/index-types.test.tspackages/3-targets/3-targets/postgres/test/psl-infer/infer-psl-contract-described-contracts.test.tspackages/3-targets/3-targets/postgres/test/psl-infer/infer-psl-contract.test.tspackages/3-targets/3-targets/postgres/test/psl-infer/print-psl/print-psl.enums.test.tspackages/3-targets/3-targets/postgres/test/psl-infer/print-psl/print-psl.relations.test.tspackages/3-targets/6-adapters/postgres/src/core/control-adapter.tspackages/3-targets/6-adapters/postgres/src/core/control-mutation-defaults.tspackages/3-targets/6-adapters/postgres/test/control-mutation-defaults.test.tspackages/3-targets/6-adapters/postgres/test/migrations/identity-column-introspection.integration.test.tsskills/extension-author/prisma-next-extension-upgrade/upgrades/0.15-to-0.16/instructions.mdskills/upgrade/prisma-next-upgrade/upgrades/0.15-to-0.16/instructions.mdtest/integration/test/cli-journeys/infer-roundtrip-fidelity.e2e.test.tstest/integration/test/infer-roundtrip-runtime.integration.test.ts
Comment on lines
+146
to
+155
| const match = ISO_8601_DATE.exec(json); | ||
| if (!match) { | ||
| throw new Error(`Invalid date string for pg/date@1: ${json}`); | ||
| } | ||
| const [, yearText, monthText, dayText] = match; | ||
| const date = new Date(Date.UTC(Number(yearText), Number(monthText) - 1, Number(dayText))); | ||
| if (Number.isNaN(date.getTime())) { | ||
| throw new Error(`Invalid date string for pg/date@1: ${json}`); | ||
| } | ||
| return date; |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Reject normalized calendar dates instead of silently changing them.
Date.UTC accepts values such as 2024-02-31 by normalizing them into March, and maps years 00–99 into 1900–1999. Compare the resulting UTC components with the parsed values before returning.
Proposed fix
const [, yearText, monthText, dayText] = match;
- const date = new Date(Date.UTC(Number(yearText), Number(monthText) - 1, Number(dayText)));
- if (Number.isNaN(date.getTime())) {
+ const year = Number(yearText);
+ const month = Number(monthText);
+ const day = Number(dayText);
+ const date = new Date(0);
+ date.setUTCHours(0, 0, 0, 0);
+ date.setUTCFullYear(year, month - 1, day);
+ if (
+ Number.isNaN(date.getTime()) ||
+ date.getUTCFullYear() !== year ||
+ date.getUTCMonth() !== month - 1 ||
+ date.getUTCDate() !== day
+ ) {
throw new Error(`Invalid date string for pg/date@1: ${json}`);
}Add regression coverage for 2024-02-31, 2024-13-01, and a year below 100.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const match = ISO_8601_DATE.exec(json); | |
| if (!match) { | |
| throw new Error(`Invalid date string for pg/date@1: ${json}`); | |
| } | |
| const [, yearText, monthText, dayText] = match; | |
| const date = new Date(Date.UTC(Number(yearText), Number(monthText) - 1, Number(dayText))); | |
| if (Number.isNaN(date.getTime())) { | |
| throw new Error(`Invalid date string for pg/date@1: ${json}`); | |
| } | |
| return date; | |
| const [, yearText, monthText, dayText] = match; | |
| const year = Number(yearText); | |
| const month = Number(monthText); | |
| const day = Number(dayText); | |
| const date = new Date(0); | |
| date.setUTCHours(0, 0, 0, 0); | |
| date.setUTCFullYear(year, month - 1, day); | |
| if ( | |
| Number.isNaN(date.getTime()) || | |
| date.getUTCFullYear() !== year || | |
| date.getUTCMonth() !== month - 1 || | |
| date.getUTCDate() !== day | |
| ) { | |
| throw new Error(`Invalid date string for pg/date@1: ${json}`); | |
| } | |
| return date; |
🧰 Tools
🪛 OpenGrep (1.25.0)
[ERROR] 146-146: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/3-targets/3-targets/postgres/src/core/codec-helpers.ts` around lines
146 - 155, Update the date-decoding logic around ISO_8601_DATE and the Date.UTC
construction to validate the resulting UTC year, month, and day against the
parsed yearText, monthText, and dayText before returning. Reject normalized
dates such as 2024-02-31 and invalid months, while preserving the existing
invalid-date error behavior; ensure years below 100 are compared against the
parsed year rather than Date.UTC’s 1900-based remapping, and add regression
coverage for the specified cases.
Comment on lines
+100
to
109
| type NumericParams = { readonly precision?: number; readonly scale?: number }; | ||
|
|
||
| const lengthParamsSchema = arktype({ | ||
| 'length?': 'number.integer > 0', | ||
| }) satisfies StandardSchemaV1<LengthParams>; | ||
|
|
||
| const numericParamsSchema = arktype({ | ||
| precision: 'number.integer > 0 & number.integer <= 1000', | ||
| 'precision?': 'number.integer > 0 & number.integer <= 1000', | ||
| 'scale?': 'number.integer >= 0', | ||
| }) satisfies StandardSchemaV1<NumericParams>; |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="packages/3-targets/3-targets/postgres/src/core/codecs.ts"
# Map the relevant symbols first.
ast-grep outline "$FILE" --view expanded || true
echo
echo "---- relevant lines ----"
sed -n '1,220p' "$FILE" | cat -n
echo
echo "---- search for renderer and type definitions ----"
rg -n "pgNumericRenderOutputType|NumericParams|lengthParamsSchema|numericParamsSchema|scale|precision" "$FILE" packages/3-targets/3-targets/postgres/src -g '!**/dist/**' -g '!**/build/**'Repository: prisma/prisma-next
Length of output: 48591
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the renderer and any surrounding helpers to understand behavior.
FILE="packages/3-targets/3-targets/postgres/src/core/codecs.ts"
sed -n '220,420p' "$FILE" | cat -n
echo
echo "---- nearby references in package ----"
rg -n "pgNumericRenderOutputType|NumericParams|precision.*scale|scale.*precision" packages/3-targets/3-targets/postgres/src -g '!**/dist/**' -g '!**/build/**'Repository: prisma/prisma-next
Length of output: 11705
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="packages/3-targets/3-targets/postgres/src/core/codecs.ts"
ast-grep outline "$FILE" --view expanded || true
echo
echo "---- relevant lines ----"
sed -n '1,220p' "$FILE" | cat -n
echo
echo "---- search for renderer and type definitions ----"
rg -n "pgNumericRenderOutputType|NumericParams|lengthParamsSchema|numericParamsSchema|scale|precision" "$FILE" packages/3-targets/3-targets/postgres/src -g '!**/dist/**' -g '!**/build/**'Repository: prisma/prisma-next
Length of output: 48591
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="packages/3-targets/3-targets/postgres/src/core/codecs.ts"
sed -n '220,420p' "$FILE" | cat -n
echo
echo "---- nearby references in package ----"
rg -n "pgNumericRenderOutputType|NumericParams|precision.*scale|scale.*precision" packages/3-targets/3-targets/postgres/src -g '!**/dist/**' -g '!**/build/**'Repository: prisma/prisma-next
Length of output: 11705
Require precision when scale is set.
numericParamsSchema accepts { scale: 2 }, but pgNumericRenderOutputType returns undefined whenever precision is missing, so scale is silently ignored. Model this as either empty or { precision, scale? } and enforce the same constraint in the schema.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/3-targets/3-targets/postgres/src/core/codecs.ts` around lines 100 -
109, Update NumericParams and numericParamsSchema so the numeric options are
either empty or include required precision with optional scale; preserve the
existing precision and scale bounds while rejecting configurations that specify
scale without precision. Ensure pgNumericRenderOutputType receives the validated
precision/scale shape and no longer silently ignores scale.
wmadden
requested changes
Jul 17, 2026
wmadden
left a comment
wmadden
left a comment
Contributor
There was a problem hiding this comment.
Rife with architectural violations
| * postgres control adapter stamps from this flag rather than from a raw | ||
| * expression when it's set. | ||
| */ | ||
| readonly identity?: boolean; |
Contributor
There was a problem hiding this comment.
Doesn't belong in sql core if it's a postgres thing. Why didn't this trip the forbidden words lint?
Contributor
Author
There was a problem hiding this comment.
Deleted in a2b9fb7. The field was pure duplication: the adapter already stamps resolvedDefault: autoincrement() at IR construction, and both consumers (the normalizer option, infer's flag check) existed only to re-derive that. All three are gone; postgres identity knowledge now lives at exactly one adapter-owned site.
Why the lint never tripped: the vocabulary lint's only scope was packages/1-framework — nothing linted 2-sql at all. Fixed alongside your other comment: a packages/2-sql scope now exists.
| nativeType: 'timestamptz', | ||
| }, | ||
| 'db.Date': { args: 'noArgs', baseType: 'DateTime', codecId: null, nativeType: 'date' }, | ||
| 'db.Date': { args: 'noArgs', baseType: 'DateTime', codecId: 'pg/date@1', nativeType: 'date' }, |
Contributor
There was a problem hiding this comment.
No postgres codecs in sql core. Add pg/* to the forbidden dict for the 2-sql dir
Contributor
Author
There was a problem hiding this comment.
Removed in 82ffdd2 — db.Date reverted to codecId: null; the id now arrives via the target-contributed scalarTypeDescriptors map (existing seam, keyed by the attribute name), so pg/date@1 is named only in target-postgres.
Lint scope added as requested: packages/2-sql forbidding pg/, threshold committed at 61 — the exact origin/main count, so this branch adds zero and the count can only shrink. The ~61 grandfathered sites (the rest of the db.* table + lanes) stay with the remove-db-attributes project (TML-2986/2987).
Comment on lines
+993
to
+1006
| // `GENERATED ALWAYS AS IDENTITY` ('a') and `GENERATED BY DEFAULT AS | ||
| // IDENTITY` ('d') both report a NULL column_default — Postgres tracks | ||
| // generation via attidentity, not a default expression — so neither | ||
| // variant is visible in `rawDefault` at all. The contract has no | ||
| // syntax to distinguish the two, so both resolve to the same | ||
| // `autoincrement()` a `serial` column's `nextval(...)` default | ||
| // already maps to. | ||
| const identity = | ||
| colRow.attidentity === 'a' || colRow.attidentity === 'd' ? true : undefined; | ||
| const resolvedDefault = identity | ||
| ? parsePostgresDefault(rawDefault ?? '', resolvedNativeType, { identity: true }) | ||
| : rawDefault !== undefined | ||
| ? parsePostgresDefault(rawDefault, resolvedNativeType) | ||
| : undefined; |
Contributor
There was a problem hiding this comment.
I don't love that this nonsense is leaking all over the place. Should we be compensating for this behavior once, in the schemaIR instead of in every consuming site?
Contributor
Author
There was a problem hiding this comment.
Yes — and your suggestion turned out to be the fix for the sibling comment too. Done in a2b9fb7: the adapter stamps resolvedDefault once at SchemaIR construction; the normalizer's {identity} option and infer's flag check are deleted, and infer reads the stamped answer instead.
The same compensate-once principle then drove the fieldContext fix (2803794) and the list-defaults fix (3ae4bb6): normalize at IR construction, consumers read the normalized value.
Comment on lines
+19
to
+30
| /** | ||
| * The shape the SQL authoring layer populates | ||
| * `DefaultFunctionLoweringContext.fieldContext` with (an opaque `unknown` | ||
| * at the framework boundary — see that field's doc comment). Declared here, | ||
| * not imported, because core cannot name it and the authoring layer has no | ||
| * reason to export a postgres-specific type; the two sides agree on the | ||
| * shape by convention, the same way `FuncCallSig` values below are agreed | ||
| * independently of the framework's `unknown`-typed `signature` field. | ||
| */ | ||
| interface PostgresFieldContext { | ||
| readonly nativeType?: string; | ||
| } |
Contributor
There was a problem hiding this comment.
This looks like we're shoving codec data into a field. Not ok
Contributor
Author
There was a problem hiding this comment.
Agreed, deleted in 2803794. lowerDbgenerated is byte-for-byte back to pre-branch; the drift fix moved to expected-side SchemaIR construction — contractToSchemaIR gained a resolveDefault hook (same IoC pattern as expandNativeType/renderDefault), postgres wires parsePostgresDefault into it, so both sides normalize identically at compare time. No opaque channel, no framework surface; contract.json stores the authored expression verbatim again, and the now-moot storageHash upgrade entry is deleted.
One knock-on worth your eye: reverting the lowering reopened the list-defaults defect (the interpreter rightly rejects dbgenerated on lists). Real fix landed in 3ae4bb6 — infer now prints the literal syntax PSL already has (@default([])) from the stamped resolvedDefault, instead of raw text emit rejects.
… at introspection Review comments 1 and 3 on PR #998: `identity` on SqlColumnIR was postgres vocabulary in sql core, and every consumer (default-normalizer option, infer's identity check) existed only to re-derive what the postgres control adapter already knows from `attidentity` and already stamps into `resolvedDefault`. The adapter now stamps `resolvedDefault: { kind: function, expression: autoincrement() }` directly for an identity column, with no intermediate option or field. `parsePostgresDefault` loses its `identity` option. Infer recognizes the same shape by its only distinguishing trait: a resolvedDefault of autoincrement() with no raw default. `SqlColumnIR.identity` is deleted entirely; the postgres adapter is the only place identity is recognized. Refs: TML-3037 Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
…ce at SchemaIR construction Review comment 4 on PR #998: DefaultFunctionLoweringContext.fieldContext was an opaque unknown channel smuggling postgres nativeType from SQL authoring into the adapter, held together by a blindCast and a comment. That field is deleted, and lowerDbgenerated reverts to keeping a dbgenerated(...) default verbatim as { kind: function, expression }, with no parsePostgresDefault call. The jsonb/text[] literal-default drift this was compensating for (resolvedDefaultsEqual comparing kind before content, so a dbgenerated literal never matched its introspected counterpart) is fixed once instead, at SchemaIR construction of the EXPECTED (contract-derived) side: contractToSchemaIR gains a target-supplied resolveDefault hook, mirroring the existing expandNativeType/renderDefault IoC pattern, and the postgres target wires postgresResolveDefault — reusing the same parsePostgresDefault introspection already calls — into both db verify and migration planning's expected-tree derivation. Consequence carried through: a dbgenerated(...) default on a *list* field still fails PSL_LIST_EXECUTION_DEFAULT_UNSUPPORTED at emit time, because the interpreter's check has no authoring-time signal left to distinguish a literal-shaped dbgenerated call from a genuine function — that signal only lived in the now-deleted fieldContext-driven resolution. auth.custom_oauth_providers.{acceptable_client_ids,scopes} (text[] literal defaults) go back into DEFAULT_OMISSIONS with the reason recorded; the D1 journey's matching list-default assertions are left unmodified, and reproduce this as a known regression relative to the branch this round reviewed, not a design this round endorses. The pack's jsonb dbgenerated defaults (attribute_mapping, authorization_params, transports, metadata) round-trip clean under the new compare-time fix, proven by extension-supabase/test/reference-fixture-verify.integration.test.ts. Upgrade instructions for the now-reverted authoring-side resolution are removed from both 0.15-to-0.16 instructions.md files — the fix is invisible to contract.json/storageHash, so no downstream action is needed. Refs: TML-3037 Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
…ql literal Review comment 2 on PR #998: this branch added codecId: pg/date@1 directly to db.Date's NATIVE_TYPE_SPECS row in psl-column-resolution.ts (2-sql), a postgres codec id literal in family code. db.Date goes back to codecId: null (matching origin/main). The noArgs case in resolveDbNativeTypeAttribute now falls back through the already-threaded scalarTypeDescriptors map (target-contributed, keyed everywhere else by PSL base type name) by the full attribute name ("db.Date") before falling back to the base type's own descriptor -- reusing the existing IoC seam instead of inventing one, since baseDescriptor alone would resolve to DateTime's pg/timestamptz@1, the original defect. The postgres adapter contributes the db.Date -> pg/date@1 mapping under that same key in postgresScalarTypeDescriptors (control-mutation-defaults.ts), so the concrete codec id lives in target-owned code. Extends lint:framework-vocabulary with a second scope: packages/2-sql, forbidden ["pg/"], threshold 61 -- the measured count after removing this branch's one new literal, unchanged from origin/main (every other pg/ occurrence in the scope is pre-existing debt, grandfathered per TML-2986/2987, not touched here). The ~30-literal db.* attribute table itself is not relocated; that is the existing remove-db-attributes project's scope. Refs: TML-3037 Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
…dDefault Two designs were tried and abandoned for defect 7 (array columns cant keep their default): relaxing the interpreters list-default check admitted DateTime[] @default(now()), which Postgres DDL refuses; riding defect 8s literal resolution at lowering time required smuggling postgres nativeType through a framework type and was rejected in review. Defect 8 landed at compare time instead, so it never rescues emit for a list column. The actual defect was narrower: PSL already has literal list syntax (@default([...])) and the interpreter already lowers it to kind: literal without touching the list check. Infer just never printed it — any raw default it could not otherwise map fell back to dbgenerated(<raw text>), which the interpreter always rejects on a list column. The postgres control adapter already resolves a list columns raw default to a structured literal at introspection time (resolvedDefault, the same mechanism defect 3 uses for identity columns), so infer now prints PSL literal-list syntax from that resolved value instead of the raw-text fallback. Deletes the acceptable_client_ids/scopes entries from DEFAULT_OMISSIONS and regenerates the Supabase pack; both columns now carry @default([]). Refs: TML-3037 Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
test/integration/test/cli-journeys/infer-roundtrip-fidelity.e2e.test.ts (1)
138-141: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAssert each inferred feature before exercising downstream behavior.
These tests can pass when inference omits the construct they are intended to protect. Please verify the expected feature in the PSL string before removing unrelated blockers.
test/integration/test/cli-journeys/infer-roundtrip-fidelity.e2e.test.ts#L138-L141: assert the bareidentities Identities?back-relation before callingfixIndexTypes.test/integration/test/cli-journeys/infer-roundtrip-fidelity.e2e.test.ts#L279-L281: assert both inferredginandhashindex declarations before callingfixOneToOneBackRelation.test/integration/test/cli-journeys/infer-roundtrip-fidelity.e2e.test.ts#L358-L366: assert the inferred JSONB literal default before callingfixOneToOneBackRelation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/integration/test/cli-journeys/infer-roundtrip-fidelity.e2e.test.ts` around lines 138 - 141, In test/integration/test/cli-journeys/infer-roundtrip-fidelity.e2e.test.ts, assert each inferred feature before applying the blocker-fixing helpers: at lines 138-141 verify the bare “identities Identities?” back-relation before fixIndexTypes; at lines 279-281 verify both inferred gin and hash index declarations before fixOneToOneBackRelation; and at lines 358-366 verify the inferred JSONB literal default before fixOneToOneBackRelation. Keep the existing downstream behavior checks intact.
🤖 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.
Duplicate comments:
In `@test/integration/test/cli-journeys/infer-roundtrip-fidelity.e2e.test.ts`:
- Around line 138-141: In
test/integration/test/cli-journeys/infer-roundtrip-fidelity.e2e.test.ts, assert
each inferred feature before applying the blocker-fixing helpers: at lines
138-141 verify the bare “identities Identities?” back-relation before
fixIndexTypes; at lines 279-281 verify both inferred gin and hash index
declarations before fixOneToOneBackRelation; and at lines 358-366 verify the
inferred JSONB literal default before fixOneToOneBackRelation. Keep the existing
downstream behavior checks intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d1f26335-99c6-483a-bdc9-156dd67136ea
📒 Files selected for processing (29)
packages/2-sql/1-core/schema-ir/test/sql-column-ir.test.tspackages/2-sql/2-authoring/contract-psl/src/psl-column-resolution.tspackages/2-sql/2-authoring/contract-psl/src/psl-named-type-resolution.tspackages/2-sql/2-authoring/contract-psl/test/fixtures.tspackages/2-sql/9-family/src/core/migrations/contract-to-schema-ir.tspackages/2-sql/9-family/src/exports/control.tspackages/2-sql/9-family/test/contract-to-schema-ir.test.tspackages/3-extensions/supabase/scripts/generate-contract.tspackages/3-extensions/supabase/src/contract/CONTRACT-FIDELITY.mdpackages/3-extensions/supabase/src/contract/contract.d.tspackages/3-extensions/supabase/src/contract/contract.jsonpackages/3-extensions/supabase/src/contract/contract.prismapackages/3-targets/3-targets/postgres/src/core/default-normalizer.tspackages/3-targets/3-targets/postgres/src/core/migrations/diff-database-schema.tspackages/3-targets/3-targets/postgres/src/core/psl-infer/infer-psl-contract.tspackages/3-targets/3-targets/postgres/src/exports/control.tspackages/3-targets/3-targets/postgres/src/exports/default-normalizer.tspackages/3-targets/3-targets/postgres/test/default-normalizer.test.tspackages/3-targets/3-targets/postgres/test/psl-infer/infer-psl-contract.test.tspackages/3-targets/6-adapters/postgres/src/core/control-adapter.tspackages/3-targets/6-adapters/postgres/src/core/control-mutation-defaults.tspackages/3-targets/6-adapters/postgres/test/control-mutation-defaults.test.tspackages/3-targets/6-adapters/postgres/test/migrations/identity-column-introspection.integration.test.tspackages/3-targets/6-adapters/postgres/test/migrations/planner-ddl-builders.test.tsprojects/infer-emit-roundtrip/spec.mdscripts/lint-framework-vocabulary.config.jsonskills/extension-author/prisma-next-extension-upgrade/upgrades/0.15-to-0.16/instructions.mdskills/upgrade/prisma-next-upgrade/upgrades/0.15-to-0.16/instructions.mdtest/integration/test/cli-journeys/infer-roundtrip-fidelity.e2e.test.ts
💤 Files with no reviewable changes (1)
- skills/upgrade/prisma-next-upgrade/upgrades/0.15-to-0.16/instructions.md
…er-output-round-trips-through-contract-emit Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io> # Conflicts: # skills/extension-author/prisma-next-extension-upgrade/upgrades/0.15-to-0.16/instructions.md # skills/upgrade/prisma-next-upgrade/upgrades/0.15-to-0.16/instructions.md
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
skills/extension-author/prisma-next-extension-upgrade/upgrades/0.15-to-0.16/instructions.md (1)
75-81: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winCorrect the stale
DEFAULT_OMISSIONSdocumentation.This block says the table shrinks from seven entries to three and that
acceptable_client_ids/scopesremain omitted. The currentpackages/3-extensions/supabase/scripts/generate-contract.tschange reduces it to a singleauth.users.phoneomission, so this upgrade note no longer describes the generated behavior.🤖 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 `@skills/extension-author/prisma-next-extension-upgrade/upgrades/0.15-to-0.16/instructions.md` around lines 75 - 81, Update the DEFAULT_OMISSIONS documentation in the upgrade instructions to state that the table is reduced to only the auth.users.phone omission. Remove the outdated references to the default-null round-trip issue and auth.custom_oauth_providers acceptable_client_ids/scopes, while preserving the surrounding generate-contract.ts workaround context.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In
`@skills/extension-author/prisma-next-extension-upgrade/upgrades/0.15-to-0.16/instructions.md`:
- Around line 75-81: Update the DEFAULT_OMISSIONS documentation in the upgrade
instructions to state that the table is reduced to only the auth.users.phone
omission. Remove the outdated references to the default-null round-trip issue
and auth.custom_oauth_providers acceptable_client_ids/scopes, while preserving
the surrounding generate-contract.ts workaround context.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: bfb4fc20-27ea-4951-a736-684b3223bfca
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
packages/3-extensions/supabase/scripts/generate-contract.tspackages/3-targets/6-adapters/postgres/src/core/control-adapter.tsskills/extension-author/prisma-next-extension-upgrade/upgrades/0.15-to-0.16/instructions.mdskills/upgrade/prisma-next-upgrade/upgrades/0.15-to-0.16/instructions.md
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/3-targets/6-adapters/postgres/src/core/control-adapter.ts
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.
Here is a fragment of
packages/3-extensions/supabase/scripts/generate-contract.ts, onmain, before this PR:We ship a script that repairs
contract infer's output before our owncontract emitwill accept it. Alongside that list are tables of column omissions, default omissions, and index omissions — each with a careful comment explaining which part of our own pipeline rejects our own output.This PR deletes 170 lines of that script and fixes what it was working around.
Why now
A user sent in a 260-line
fix-inferred-contract.tsthey run after everycontract infer. Auditing it claim-by-claim against the source turned up nine defects. They had written our script independently, against a different database, without ever seeing ours.Two people arriving at the same workaround is the finding.
contract inferwrites PSL thatcontract emitrejects, or thatdb verifythen reports as drift — and no test anywhere did introspect → infer → emit, which is why all nine shipped. The e2e suite had FK-bearing tables but never ran them through infer; identity columns had no coverage at all.So the first commit here is the instrument, not a fix: a round-trip journey against a live database carrying already-plural table names, 1:1 and 1:N and self-referencing FKs, both identity variants,
serial, bounded and unboundednumeric,date,text[],jsonb, GIN and hash indexes, and an FK pointing out of scope. Every defect below was reproduced by it before being fixed, and each fix is the commit where its assertion flips green.projects/infer-emit-roundtrip/reproduction.mdrecords the pre-fix state verbatim and is deliberately not updated.What was broken
Back-relation names double-pluralized.
pluralize()appendedesto anything ending ins, so already-plural table names — which is most real schemas — came outsessionses. Now uses the maintainedpluralizelibrary. Closes TML-3024, whose bar was "the Supabase generator override is removed and the regenerated contract is unchanged by its removal". It is, and it is.Infer printed a 1:1 back-relation emit couldn't parse. A bare
profile Profile?. The uniqueness detection producing it was correct; the interpreter collected back-relation candidates onlyif (field.list). We had a snapshot test asserting we print PSL our own emitter rejects.Decimalnever worked on Postgres. This one isn't an infer bug at all.Decimalmaps topg/numeric@1on the base-scalar path with notypeParams, so any schema with a plainamount Decimalfield throwsRUNTIME.CODEC_PARAMETERIZATION_MISMATCHat connect.NumericParams.precisionwas required while every sibling temporal codec's param is optional, and three other layers already handled a missing precision. Infer is simply the first thing that generates aDecimal— there isn't one in any.prismafile in this repo, which is why nobody hit it.No
pg/datecodec existed.@db.DatecarriedcodecId: null("inherit from base"), andDateTimeon Postgres ispg/timestamptz@1. Top-level rows survived becausedecode()is a passthrough over an already-parsedDate;.include()goes throughjson_agg→decodeJson(), which regex-checks for a full ISO timestamp and rejects"2024-01-15".Non-btree indexes could never emit. Infer prints
@@index(…, type: "gin"); the Postgres target registered zero index types, so emit threwunregistered index type "gin". Any real database has one.Identity columns lost their default. The columns query joined
pg_attributebut never selectedattidentity.serialworked only because it sets a realnextval(...)default.dbgeneratedliteral defaults drifted forever. Emit kept'{}'::jsonbaskind: 'function'; introspection parsed it tokind: 'literal';resolvedDefaultsEqualcompareskindfirst.db verifyreported such columnsnot-equalpermanently, even when the database matched exactly.Dangling FKs dropped silently. Infer correctly drops an FK whose target is outside the introspected scope, but said nothing — so a user loses every
auth.usersrelationship with no indication. It now says so, and points at the likely cause.Two decisions worth your attention
Identity maps onto
autoincrement(), symmetrically. The obvious fix — emit@default(autoincrement())for an identity column — trades the emit break for a verify break: the contract would carry a function default while the live column reports none, and it would drift forever. So both sides move: infer emits it, and the normalizer resolves a live identity column to it.At the contract's altitude
autoincrement()means "the database generates this value", and identity andserialare both that. PSL has no syntax to distinguish them, so this PR does not modelGENERATED ALWAYSvsBY DEFAULT. The accepted cost: a freshdb initfrom such a contract createsserial, not identity. That gap pre-exists — identity has never been authorable — and stays out of scope.The Postgres target registers its own built-in index types (
btree,hash,gin,gist,spgist,brin), via the sameIndexTypeRegistrymechanism ParadeDB uses forbm25. That's the extension point used as designed: the target declares its own built-ins, extensions contribute types they own.build-contract.tscomposes[target, ...extensionPacks], so they coexist. An unregistered type is still rejected — there's a test.Breaking changes
Upgrade instructions are recorded in
skills/upgrade/prisma-next-upgrade/upgrades/0.15-to-0.16/and the extension-author mirror.@db.Datecolumns changecodecIdfrompg/timestamptz@1topg/date@1on the next emit, with no source change — sostorageHashchanges.dbgenerated(...)literal defaults resolve differently on the next emit —storageHashchanges.contract inferre-run (sessionses→sessions). These are public field names consumers type, which is exactly why this was worth fixing before more people ran infer.db verify --strictonly; non-strict verify filters an undeclared live default out entirely.Index-type registration and the
Decimalfix are not breaking: both previously threw unconditionally, so nothing could have depended on them.Deliberately not fixed
@default(null)on a nullable column, and nullable list columns — two further round-trip gaps, documented ingenerate-contract.ts's own comments. Both real, both needing their own design.auth.users.phoneis the one entry left inDEFAULT_OMISSIONS, and its comment now says it's the only reason the table still exists.Also out: PSL
constraint: false(FK-less relations are TS-DSL-only), identity DDL authoring, and per-index-method option validation.One claim from the user's script resolved itself: the NUL-bytes-where-
@-should-be workaround (replaceAll('\0', '@'), the first thing their script does). That was a real bug, and we fixed it separately before 0.15.0 was cut. It reads as unreproducible here because we tested against post-fix code — their script is pinned to a version that predates the fix, which also explains why it's the one line in the file with no explanatory comment and the one omission from their own re-verification list. Nothing to do here; the report back tells them to delete the line and to shout if it recurs on 0.15.0.Alternatives considered
Eight separate PRs. Every defect is an instance of one class, they share one instrument and one acceptance bar, and the evidence they're worth fixing at all is collective. Split up, each reviewer sees a one-line change with no way to judge whether it's the right one-line change — and the pack script shrinking, which is the actual proof, wouldn't appear in any of them.
Make infer emit lists for 1:1 back-relations. Would have made emit pass by discarding real information. The contract already supports
'1:1'; the gap was PSL-side only.Relax the list-default check to permit storage function defaults. This is what the spec originally prescribed, and it's wrong — it admits
tags DateTime[] @default(now()), whose DDL Postgres refuses, moving the error from authoring time to apply time. Nothing in the authoring layer can tell whether a function returns an array. Review caught it; the tell was that the implementation needed a special case forautoincrement()to plug one instance of the hole it had opened. The array default works anyway, because the literal-default fix means it never reaches that check.Review trail
An Opus review pass before opening found five issues, three of which were this same failure repeating inside the PR's own diff — a symptom fixed instead of a class, and a test deleted rather than satisfied. All are addressed in the last four commits; the reproduction record and the spec's three correction blocks show where the plan was wrong and what the evidence said instead.
Refs: TML-3037
Closes: TML-3024
Summary by CodeRabbit
New Features
datesupport (YYYY-MM-DD) with correct encoding/decoding.@default(autoincrement()).Bug Fixes