Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis change rearchitects all codec authoring, registration, lookup, parameterization, column helpers, and downstream registry usage across every part of the framework, targets, adapters, and extensions to use a unified class-based descriptor and registry pattern. Old factory, registry, and parameterized codec split logic is completely supplanted. All tests, typegen, docs, contract logic, and downstream wiring are rewritten to match the new unified infrastructure. ChangesUnified Codec Descriptor Registry Migration
Sequence Diagram(s)sequenceDiagram
participant Author
participant CodecDescriptor
participant Registry
participant Runtime
participant Consumer
Author->>CodecDescriptor: implements CodecImpl, DescriptorImpl
CodecDescriptor->>Registry: descriptor registration (array)
Runtime->>Registry: lookup(by id, type, meta, render)
Registry->>CodecDescriptor: .factory(params)
Runtime->>Codec: encode/decode/render via registry
Consumer->>Registry: typegen or metadata lookup
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120+ minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
@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: |
wmadden
force-pushed
the
tml-2357-codec-registration-model-complete-the-unified
branch
from
5e0dee5 to
692178c
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/2-sql/5-runtime/src/sql-context.ts (1)
493-516:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftDon't collapse non-parameterized column codecs to one instance per
codecId.
CodecInstanceContextis per materialization site, but this cache makescontractCodecs.forColumn()hand back the same codec instance for every column sharing a non-parameterized id. The first column wins the capturedctx.name/usedAt, so any non-parameterized codec that derives state from column context will behave incorrectly.🛠️ Safer materialization shape
} else if (!isParameterized) { - let cached = byCodecId.get(column.codecId); - if (!cached) { - const ctx: SqlCodecInstanceContext = { - name: `<shared:${column.codecId}>`, - usedAt: [{ table: tableName, column: columnName }], - }; - const voidFactory = descriptor.factory as unknown as ( - params: undefined, - ) => (ctx: SqlCodecInstanceContext) => Codec; - cached = voidFactory(undefined)(ctx); - byCodecId.set(column.codecId, cached); - } - resolvedCodec = cached; + const ctx: SqlCodecInstanceContext = { + name: `<anon:${tableName}.${columnName}>`, + usedAt: [{ table: tableName, column: columnName }], + }; + const voidFactory = descriptor.factory as unknown as ( + params: undefined, + ) => (ctx: SqlCodecInstanceContext) => Codec; + resolvedCodec = voidFactory(undefined)(ctx); + byCodecId.set(column.codecId, byCodecId.get(column.codecId) ?? resolvedCodec); }If you still want a codec-id fallback, keep that representative instance separate from the per-column instances rather than reusing the column-scoped object for every site.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/2-sql/5-runtime/src/sql-context.ts` around lines 493 - 516, The code currently caches the concrete Codec instance in byCodecId (via voidFactory(undefined)(ctx)), causing every non-parameterized column to share the same instance and captured ctx; instead cache the parameterless factory function and invoke it per-column so each materialization site gets its own Codec with its own SqlCodecInstanceContext. Concretely: in the branch that handles non-parameterized codecs (where descriptor.factory is cast to voidFactory), store the function returned by voidFactory(undefined) in byCodecId (not the result of calling it with ctx), then for each column call that cached factory with a fresh SqlCodecInstanceContext to produce resolvedCodec; update uses of cached/voidFactory/byCodecId/resolvedCodec accordingly so contractCodecs.forColumn() returns per-column instances.
🧹 Nitpick comments (7)
examples/prisma-next-demo/test/no-emit-typed-flow.test-d.ts (1)
20-22: ⚡ Quick winUse package-export imports in this example test
Line 20 and Line 21 import internals via relative paths. For
examples/**/test/**, this should validate the public API surface by importing from package exports (@prisma-next/...) instead.As per coding guidelines, “Cross-package and integration tests: Import from package exports (
@prisma-next/...) to verify the public API.”🤖 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 `@examples/prisma-next-demo/test/no-emit-typed-flow.test-d.ts` around lines 20 - 22, The test imports package internals via relative paths; update the imports to use the package-export public API instead: replace the relative imports that bring in the types/symbols "contract" and "sql" with their corresponding exported package paths so the test validates the public surface (import the "contract" type and the "sql" helper from the package exports for prisma/prisma-no-emit rather than from ../prisma/contract and ../src/prisma-no-emit/context).packages/2-sql/4-lanes/relational-core/test/utils.ts (1)
4-11: ⚡ Quick winAvoid
/exports/barrel imports in this test utility.Line 10 and Line 11 import through
../src/exports/*; switch these to direct module imports to keep dependency boundaries explicit.As per coding guidelines: “Avoid barrel files for exports; use direct file imports instead” and “Import from shared plane packages using package name and export identifier … not
/exports/paths.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/2-sql/4-lanes/relational-core/test/utils.ts` around lines 4 - 11, The test utility currently imports Adapter, Codec, CodecRegistry, LoweredStatement, SelectAst and ExecutionContext from barrel paths ('../src/exports/*'); change these to import the same symbols directly from their concrete modules (the specific source files that define Adapter, Codec, CodecRegistry, LoweredStatement, SelectAst and ExecutionContext) so you avoid `/exports/` barrel imports and keep dependency boundaries explicit; update the import statements to reference the original modules that declare those types and remove the barrel paths.packages/3-targets/3-targets/postgres/src/core/codecs.ts (1)
55-72: 💤 Low valueRedundant undefined check for
precisionparameter.The function parameter declares
precisionasreadonly precision: number(required), but line 60 checksif (precision === undefined). This check is unreachable given the type signature.If
precisioncan actually be undefined at runtime (e.g., from untyped callers), consider changing the type toreadonly precision?: number. Otherwise, remove the redundant check.♻️ Suggested fix if precision is guaranteed
export const pgNumericRenderOutputType = (typeParams: { readonly precision: number; readonly scale?: number; }): string | undefined => { const precision = typeParams.precision; - if (precision === undefined) return undefined; if ( typeof precision !== 'number' || !Number.isFinite(precision) || !Number.isInteger(precision) ) {🤖 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 55 - 72, The check for precision being undefined in pgNumericRenderOutputType is unreachable because typeParams declares readonly precision: number; remove the if (precision === undefined) return undefined; branch and any related early-return logic; alternatively, if precision can be missing at runtime, change the signature to readonly precision?: number and keep the check — reference the function name pgNumericRenderOutputType and the property typeParams.precision when making the change.packages/2-sql/2-authoring/contract-psl/test/fixtures.ts (1)
160-164: ⚡ Quick winHarden codec-id presence checks with
Object.hasOwn.Line 160 and Line 163 currently rely on plain index lookups, which can accidentally treat prototype keys (for example,
"constructor") as valid codec ids.Suggested patch
export const postgresCodecLookup: CodecLookup = { get: (id: string) => { - if (!targetTypesByCodecId[id]) return undefined; + if (!Object.hasOwn(targetTypesByCodecId, id)) return undefined; return { id } as ReturnType<CodecLookup['get']>; }, - targetTypesFor: (id: string) => targetTypesByCodecId[id], + targetTypesFor: (id: string) => + Object.hasOwn(targetTypesByCodecId, id) ? targetTypesByCodecId[id] : undefined, metaFor: () => undefined, renderOutputTypeFor: () => undefined, };As per coding guidelines "Always use
Object.hasOwn()instead ofhasOwnProperty()to check for own properties."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/2-sql/2-authoring/contract-psl/test/fixtures.ts` around lines 160 - 164, Replace plain property index checks on targetTypesByCodecId with Object.hasOwn to avoid treating prototype keys as valid codec ids: in the implementation returning ReturnType<CodecLookup['get']> change the existence check from if (!targetTypesByCodecId[id]) to if (!Object.hasOwn(targetTypesByCodecId, id)) and in the targetTypesFor method ensure it only returns targetTypesByCodecId[id] when Object.hasOwn(targetTypesByCodecId, id) (otherwise return undefined); leave metaFor as-is.packages/1-framework/1-core/framework-components/test/codec.types.test-d.ts (1)
249-249: 💤 Low valueUse
toExtendinstead of deprecatedtoMatchTypeOf.Per coding guidelines,
toMatchTypeOf()is deprecated in Vitest'sexpectTypeOfAPI. UsetoExtend<T>()instead.♻️ Proposed fix
- expectTypeOf<typeof reg>().toMatchTypeOf<Map<string, AnyCodecDescriptor>>(); + expectTypeOf<typeof reg>().toExtend<Map<string, AnyCodecDescriptor>>();🤖 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/1-core/framework-components/test/codec.types.test-d.ts` at line 249, Replace the deprecated expectTypeOf(...).toMatchTypeOf usage with the supported generic toExtend form: locate the assertion using reg (expectTypeOf<typeof reg>().toMatchTypeOf<Map<string, AnyCodecDescriptor>>()) and change it to use toExtend with the same generic type (expectTypeOf<typeof reg>().toExtend<Map<string, AnyCodecDescriptor>>()); ensure the generic parameter remains Map<string, AnyCodecDescriptor> and the assertion target remains typeof reg.packages/1-framework/1-core/framework-components/src/shared/column-spec.ts (1)
34-55: ⚡ Quick winThread the
TCodecIdgeneric throughColumnSpecto preserve codec ID literals in helper return types.
ColumnTypeDescriptoris generic inTCodecId, butColumnSpecand thecolumn()function drop this parameter. Codec ID literals widen tostringat the helper boundary, losing type precision that downstream code (e.g.,contract-dsl.ts) already extracts viaDescriptor['codecId']. Threading the generic would maintain literal fidelity through the helper surface and align with the field-state pattern used in contract authoring.♻️ Suggested type-surface fix
-export interface ColumnSpec<R, P extends Record<string, unknown> | undefined> - extends ColumnTypeDescriptor { +export interface ColumnSpec< + TCodecId extends string, + R, + P extends Record<string, unknown> | undefined, +> extends ColumnTypeDescriptor<TCodecId> { readonly codecFactory: (ctx: CodecInstanceContext) => R; readonly typeParams: P; } -export function column<R, P extends Record<string, unknown> | undefined>( +export function column< + TCodecId extends string, + R, + P extends Record<string, unknown> | undefined, +>( codecFactory: (ctx: CodecInstanceContext) => R, - codecId: string, + codecId: TCodecId, typeParams: P, nativeType: string, -): ColumnSpec<R, P> { +): ColumnSpec<TCodecId, R, P> { return { codecFactory, codecId, typeParams, nativeType, }; } export type ColumnHelperFor<D extends CodecDescriptor<any>> = ( ...args: any[] -) => ColumnSpec<unknown, ColumnHelperParams<D>>; +) => ColumnSpec<D['codecId'], unknown, ColumnHelperParams<D>>; export type ColumnHelperForStrict<D extends CodecDescriptor<any>> = ( ...args: any[] -) => ColumnSpec<ReturnType<ReturnType<D['factory']>>, ColumnHelperParams<D>>; +) => ColumnSpec< + D['codecId'], + ReturnType<ReturnType<D['factory']>>, + ColumnHelperParams<D> +>;🤖 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/1-core/framework-components/src/shared/column-spec.ts` around lines 34 - 55, ColumnSpec currently drops the TCodecId generic from ColumnTypeDescriptor which widens codecId literals to string; update ColumnSpec to be generic in TCodecId and extend ColumnTypeDescriptor<TCodecId> (e.g. change the declaration to something like interface ColumnSpec<TCodecId extends string, R, P extends Record<string, unknown> | undefined> extends ColumnTypeDescriptor<TCodecId>), keep the existing codecFactory and typeParams members but use the P and R generics, and then propagate the TCodecId generic through the column() helper signature so helper return types preserve codec ID literals.packages/3-extensions/sql-orm-client/src/types.ts (1)
302-305: ⚡ Quick winUse
ifDefined()instead of inline conditional object spreads inparam().Line 302-Line 305 currently uses inline spread conditionals; this codepath should use the shared helper for consistency with repo conventions.
As per coding guidelines, "Use
ifDefined()from@prisma-next/utils/definedfor conditional object spreads instead of inline conditional spread patterns".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/3-extensions/sql-orm-client/src/types.ts` around lines 302 - 305, The inline conditional spreads in param() should be replaced with the shared helper ifDefined from `@prisma-next/utils/defined`: import ifDefined and pass ParamRef.of(value, ifDefined({ codecId }, { refs })) (or a single ifDefined call that combines the optional codecId and refs objects) instead of the current ...(codecId !== undefined ? { codecId } : {}), ...(refs !== undefined ? { refs } : {}); update the param() return to use ifDefined and remove the inline conditional spreads while keeping ParamRef.of and the same option shape.
🤖 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 `@docs/architecture` docs/adrs/ADR 184 - Codec-owned value serialization.md:
- Line 59: The paragraph claims "the defineCodec() factory provides these
defaults" but defineCodec() is retired; update the text that describes
encodeJson/decodeJson to either mark the defineCodec() statement as historical
("historically, defineCodec() provided these defaults") or reword it to the
current authoring surface (e.g., explain that the codec class/descriptor default
implementations make encodeJson/decodeJson identity for JSON-safe types and only
Date/binary codecs override them); do the same for the duplicate instance of
this sentence elsewhere so the ADR is internally consistent.
In `@docs/reference/codec-authoring-guide.md`:
- Around line 262-269: Update the docs' references that point to old filenames
so readers are directed to the new class-style implementations: change mentions
of the pre-migration files (e.g. "sql-codecs.ts" and "codecs.ts") to the
migrated filenames "sql-codecs-class.ts" and "codecs-class.ts" in the paragraphs
that describe the aliasCodec helper, CodecDescriptorImpl<P>, and the alias
factory pattern (around the aliasCodec/CodecDescriptorImpl example); ensure any
other occurrences (including the similar text at lines ~320-331) are updated
consistently so the guide links to the current implementations.
In `@examples/prisma-next-demo/test/no-emit-typed-flow.test-d.ts`:
- Around line 27-32: The test title claims the User.id is a string but current
assertions only check presence; update the test "field.id.uuidv4() produces a
string-typed id field on User" to actually assert the scalar type (or rename the
test to reflect presence-only checks). Specifically, in the test referencing
UserStorageFields and IdField (from (typeof
contract.models)['User']['storage']['fields']), add a type-level assertion that
IdField's column (or the appropriate scalar property on IdField) is string-typed
(e.g., assert IdField['column'] equals string) using expectTypeOf, or change the
test name to remove the "string-typed" claim.
In `@packages/2-sql/4-lanes/relational-core/src/ast/sql-codecs-class.ts`:
- Around line 67-76: lengthParamsSchema currently requires length but
LengthParams marks it optional and the sqlCharColumn/sqlVarcharColumn helpers
default params to {} which produces descriptors that validateTypeParams later
rejects; either relax lengthParamsSchema to make length optional (change to
'length?') so bare char/varchar is supported, or make LengthParams require
length and stop defaulting params to {} in the helper functions (sqlCharColumn,
sqlVarcharColumn and the other affected helpers mentioned) so callers must
provide length; update validateTypeParams usage accordingly to match the chosen
contract.
In `@packages/2-sql/4-lanes/relational-core/test/utils.ts`:
- Around line 39-41: profile.codecs() currently returns a new empty registry
every call (via emptyCodecRegistry()), causing registered codecs to be lost
between calls; fix by creating a single stable registry instance (e.g., const
codecRegistry = emptyCodecRegistry()) and have profile.codecs() return that same
codecRegistry so registrations persist; update the utils.ts factory that defines
profile.codecs() to initialize the registry once (module-scope or closure) and
return it on each call, leaving emptyCodecRegistry() usage but only invoked
once.
In `@packages/2-sql/5-runtime/src/codecs/encoding.ts`:
- Around line 56-62: When metadata.refs is present the resolution must use the
column path only and must not fall back to id-based resolution; in the block
that checks contractCodecs.forColumn(metadata.refs.table, metadata.refs.column)
(symbols: metadata.refs, contractCodecs, forColumn), change the logic so that if
byColumn is falsy you stop resolution immediately (e.g. return undefined or
throw a clear error) instead of continuing to
contractCodecs.forCodecId(metadata.codecId) or registry.get(metadata.codecId);
this ensures a missing column lookup fails fast rather than downgrading to
codec-id dispatch.
In `@packages/3-extensions/arktype-json/src/core/arktype-json-codec-class.ts`:
- Around line 144-154: Replace the blind cast in rehydrateSchema by adding a
type predicate (e.g. isArktypeSchemaLike) and use it to assert the return of
ark.schema(jsonIr) is an ArktypeSchemaLike before returning; if the predicate
fails throw the same runtimeError (preserving ARKTYPE_JSON_CODEC_ID and jsonIr)
so invalid shapes are rejected instead of being force-cast; keep the existing
try/catch and error message behavior around ark.schema and runtimeError.
In `@packages/3-extensions/arktype-json/test/arktype-json-codec-class.test.ts`:
- Around line 75-79: The test uses an explicit any cast and a biome-ignore;
replace that with a TypeScript compile-time suppression: remove the "as any" and
the biome lint suppression, and instead place a single-line "@ts-expect-error —
intentionally passing non-callable to test runtime guard" immediately above the
arktypeJsonColumn(notASchema) call (leaving notASchema typed as an object), so
the runtime negative-path assertion remains but the test follows the repo rule
against any/biome ignores; target the arktypeJsonColumn call in
arktype-json-codec-class.test.ts when making this change.
In
`@packages/3-extensions/arktype-json/test/arktype-json-codec-class.types.test-d.ts`:
- Around line 140-142: The type-test assertion for variable col uses the
deprecated matcher toMatchTypeOf; update the expectTypeOf(col) call to use
toExtend with the same generic target type ColumnSpec<ArktypeJsonCodecClass<{
name: string; price: number }>, ArktypeJsonTypeParams> so the assertion reads
expectTypeOf(col).toExtend<...>(), keeping the same generics (ColumnSpec,
ArktypeJsonCodecClass, ArktypeJsonTypeParams) and not changing the tested
variable name col.
In `@packages/3-extensions/sql-orm-client/src/types.ts`:
- Around line 292-294: refsFromLeft currently only handles kind === 'column-ref'
which drops refs for other AnyExpression shapes (so result.buildAst() loses refs
and breaks codec dispatch); update refsFromLeft (and the comparison factory
usage that passes result.buildAst()) to preserve/collect refs for any expression
by returning refs found on the expression (e.g., expression.refs if present) or
by recursively extracting column-ref nodes from
BinaryExpr/Unary/Identifier/Literal wrappers so column refs are not lost (keep
the return type { table: string; column: string } | undefined or adjust to
return refs array if needed and update callers accordingly); also replace the
inline conditional spreads in param() that do ...(codecId !== undefined ? {
codecId } : {}) and ...(refs !== undefined ? { refs } : {}) with the ifDefined()
helper from '@prisma-next/utils/defined' (add the import) to follow the
repository guideline.
In `@packages/3-targets/3-targets/postgres/src/core/codecs-class.ts`:
- Around line 110-126: The TypeScript type LengthParams marks length as optional
but lengthParamsSchema requires it, causing runtime validation failures when
helpers (e.g., pgBitColumn) pass {} as params; update lengthParamsSchema to make
length optional (change the schema key to 'length?' with the same constraint) so
the runtime arktype matches the LengthParams type and existing callers that
default params = {} continue to validate correctly.
In `@packages/3-targets/3-targets/sqlite/src/core/codecs-class.ts`:
- Around line 250-264: The decode paths (async decode and decodeJson) and the
encoder path (encode and encodeJson) should validate Date objects: after
constructing a Date in decode/decodeJson, check that date.getTime() is not NaN
and throw a TypeError (same contract message) for malformed/invalid ISO strings;
in encode/encodeJson ensure the provided Date is valid (check
isFinite(date.getTime()) or !isNaN) and throw TypeError if it is an Invalid Date
before calling toISOString; update the functions named encode, decode,
encodeJson, and decodeJson in codecs-class.ts accordingly.
---
Outside diff comments:
In `@packages/2-sql/5-runtime/src/sql-context.ts`:
- Around line 493-516: The code currently caches the concrete Codec instance in
byCodecId (via voidFactory(undefined)(ctx)), causing every non-parameterized
column to share the same instance and captured ctx; instead cache the
parameterless factory function and invoke it per-column so each materialization
site gets its own Codec with its own SqlCodecInstanceContext. Concretely: in the
branch that handles non-parameterized codecs (where descriptor.factory is cast
to voidFactory), store the function returned by voidFactory(undefined) in
byCodecId (not the result of calling it with ctx), then for each column call
that cached factory with a fresh SqlCodecInstanceContext to produce
resolvedCodec; update uses of cached/voidFactory/byCodecId/resolvedCodec
accordingly so contractCodecs.forColumn() returns per-column instances.
---
Nitpick comments:
In `@examples/prisma-next-demo/test/no-emit-typed-flow.test-d.ts`:
- Around line 20-22: The test imports package internals via relative paths;
update the imports to use the package-export public API instead: replace the
relative imports that bring in the types/symbols "contract" and "sql" with their
corresponding exported package paths so the test validates the public surface
(import the "contract" type and the "sql" helper from the package exports for
prisma/prisma-no-emit rather than from ../prisma/contract and
../src/prisma-no-emit/context).
In `@packages/1-framework/1-core/framework-components/src/shared/column-spec.ts`:
- Around line 34-55: ColumnSpec currently drops the TCodecId generic from
ColumnTypeDescriptor which widens codecId literals to string; update ColumnSpec
to be generic in TCodecId and extend ColumnTypeDescriptor<TCodecId> (e.g. change
the declaration to something like interface ColumnSpec<TCodecId extends string,
R, P extends Record<string, unknown> | undefined> extends
ColumnTypeDescriptor<TCodecId>), keep the existing codecFactory and typeParams
members but use the P and R generics, and then propagate the TCodecId generic
through the column() helper signature so helper return types preserve codec ID
literals.
In `@packages/1-framework/1-core/framework-components/test/codec.types.test-d.ts`:
- Line 249: Replace the deprecated expectTypeOf(...).toMatchTypeOf usage with
the supported generic toExtend form: locate the assertion using reg
(expectTypeOf<typeof reg>().toMatchTypeOf<Map<string, AnyCodecDescriptor>>())
and change it to use toExtend with the same generic type (expectTypeOf<typeof
reg>().toExtend<Map<string, AnyCodecDescriptor>>()); ensure the generic
parameter remains Map<string, AnyCodecDescriptor> and the assertion target
remains typeof reg.
In `@packages/2-sql/2-authoring/contract-psl/test/fixtures.ts`:
- Around line 160-164: Replace plain property index checks on
targetTypesByCodecId with Object.hasOwn to avoid treating prototype keys as
valid codec ids: in the implementation returning ReturnType<CodecLookup['get']>
change the existence check from if (!targetTypesByCodecId[id]) to if
(!Object.hasOwn(targetTypesByCodecId, id)) and in the targetTypesFor method
ensure it only returns targetTypesByCodecId[id] when
Object.hasOwn(targetTypesByCodecId, id) (otherwise return undefined); leave
metaFor as-is.
In `@packages/2-sql/4-lanes/relational-core/test/utils.ts`:
- Around line 4-11: The test utility currently imports Adapter, Codec,
CodecRegistry, LoweredStatement, SelectAst and ExecutionContext from barrel
paths ('../src/exports/*'); change these to import the same symbols directly
from their concrete modules (the specific source files that define Adapter,
Codec, CodecRegistry, LoweredStatement, SelectAst and ExecutionContext) so you
avoid `/exports/` barrel imports and keep dependency boundaries explicit; update
the import statements to reference the original modules that declare those types
and remove the barrel paths.
In `@packages/3-extensions/sql-orm-client/src/types.ts`:
- Around line 302-305: The inline conditional spreads in param() should be
replaced with the shared helper ifDefined from `@prisma-next/utils/defined`:
import ifDefined and pass ParamRef.of(value, ifDefined({ codecId }, { refs }))
(or a single ifDefined call that combines the optional codecId and refs objects)
instead of the current ...(codecId !== undefined ? { codecId } : {}), ...(refs
!== undefined ? { refs } : {}); update the param() return to use ifDefined and
remove the inline conditional spreads while keeping ParamRef.of and the same
option shape.
In `@packages/3-targets/3-targets/postgres/src/core/codecs.ts`:
- Around line 55-72: The check for precision being undefined in
pgNumericRenderOutputType is unreachable because typeParams declares readonly
precision: number; remove the if (precision === undefined) return undefined;
branch and any related early-return logic; alternatively, if precision can be
missing at runtime, change the signature to readonly precision?: number and keep
the check — reference the function name pgNumericRenderOutputType and the
property typeParams.precision when making the change.
🪄 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: 69fcb105-9c82-4733-9268-a3018eb3c54d
⛔ Files ignored due to path filters (6)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlprojects/codec-registration-completion/plan.mdis excluded by!projects/**projects/codec-registration-completion/spec.mdis excluded by!projects/**projects/codec-registration-completion/specs/class-based-codec-design.spec.mdis excluded by!projects/**projects/codec-registration-completion/specs/factory-defined-codec-types.spec.mdis excluded by!projects/**projects/codec-registration-completion/specs/typed-codec-flow.spec.mdis excluded by!projects/**
📒 Files selected for processing (218)
docs/README.mddocs/Testing Guide.mddocs/architecture docs/adrs/ADR 152 - Execution Plane Descriptors and Instances.mddocs/architecture docs/adrs/ADR 184 - Codec-owned value serialization.mddocs/architecture docs/adrs/ADR 186 - Codec-dispatched type rendering.mddocs/architecture docs/adrs/ADR 202 - Codec trait system.mddocs/architecture docs/adrs/ADR 204 - Single-Path Async Codec Runtime.mddocs/architecture docs/adrs/ADR 205 - SQL cast emission is adapter policy.mddocs/architecture docs/adrs/ADR 207 - Codec call context per-query AbortSignal and column metadata.mddocs/architecture docs/adrs/ADR 208 - Higher-order codecs for parameterized types.mddocs/architecture docs/adrs/ADR 209 - Mongo result-shape as a structural plan field.mddocs/architecture docs/subsystems/2. Contract Emitter & Types.mddocs/reference/codec-authoring-guide.mddocs/reference/community-generator-migration-analysis.mddocs/reference/framework-gaps.mddocs/reference/typescript-patterns.mdexamples/prisma-next-demo/test/no-emit-typed-flow.test-d.tspackages/1-framework/0-foundation/contract/package.jsonpackages/1-framework/0-foundation/contract/src/exports/types.tspackages/1-framework/0-foundation/contract/src/standard-schema-utils.tspackages/1-framework/1-core/framework-components/README.mdpackages/1-framework/1-core/framework-components/src/control/control-stack.tspackages/1-framework/1-core/framework-components/src/exports/codec.tspackages/1-framework/1-core/framework-components/src/shared/codec-descriptor.tspackages/1-framework/1-core/framework-components/src/shared/codec-types.tspackages/1-framework/1-core/framework-components/src/shared/codec.tspackages/1-framework/1-core/framework-components/src/shared/column-spec.tspackages/1-framework/1-core/framework-components/src/shared/framework-components.tspackages/1-framework/1-core/framework-components/test/codec-call-context.types.test-d.tspackages/1-framework/1-core/framework-components/test/codec-types.types.test-d.tspackages/1-framework/1-core/framework-components/test/codec.test.tspackages/1-framework/1-core/framework-components/test/codec.types.test-d.tspackages/1-framework/1-core/framework-components/test/control-stack.test.tspackages/1-framework/2-authoring/contract/README.mdpackages/1-framework/2-authoring/contract/src/descriptors.tspackages/1-framework/2-authoring/contract/src/index.tspackages/1-framework/2-authoring/contract/test/descriptors.test.tspackages/1-framework/2-authoring/ids/package.jsonpackages/1-framework/2-authoring/ids/src/index.tspackages/1-framework/3-tooling/cli/src/control-api/contract-enrichment.tspackages/1-framework/3-tooling/cli/test/config-types.test.tspackages/1-framework/3-tooling/cli/test/control-api/contract-enrichment.test.tspackages/1-framework/3-tooling/emitter/src/domain-type-generation.tspackages/1-framework/3-tooling/emitter/test/domain-type-generation.test.tspackages/2-mongo-family/1-foundation/mongo-codec/README.mdpackages/2-mongo-family/1-foundation/mongo-codec/src/codec-registry.tspackages/2-mongo-family/1-foundation/mongo-codec/src/codecs.tspackages/2-mongo-family/1-foundation/mongo-codec/src/exports/index.tspackages/2-mongo-family/1-foundation/mongo-codec/test/codecs-ctx.test-d.tspackages/2-mongo-family/1-foundation/mongo-codec/test/codecs-ctx.test.tspackages/2-mongo-family/1-foundation/mongo-codec/test/codecs.test-d.tspackages/2-mongo-family/1-foundation/mongo-codec/test/codecs.test.tspackages/2-mongo-family/2-authoring/contract-psl/src/derive-json-schema.tspackages/2-mongo-family/2-authoring/contract-psl/test/derive-json-schema.test.tspackages/2-mongo-family/2-authoring/contract-psl/test/interpreter.polymorphism.test.tspackages/2-mongo-family/2-authoring/contract-psl/test/interpreter.test.tspackages/2-mongo-family/7-runtime/README.mdpackages/2-mongo-family/7-runtime/src/mongo-execution-stack.tspackages/2-mongo-family/7-runtime/test/codecs/decoding.test.tspackages/2-mongo-family/7-runtime/test/decode.integration.test.tspackages/2-mongo-family/7-runtime/test/mongo-execution-stack.test.tspackages/2-mongo-family/7-runtime/test/mongo-middleware.test.tspackages/2-mongo-family/7-runtime/test/mongo-runtime-abort.test.tspackages/2-sql/2-authoring/contract-psl/src/provider.tspackages/2-sql/2-authoring/contract-psl/test/fixtures.tspackages/2-sql/2-authoring/contract-psl/test/provider.test.tspackages/2-sql/2-authoring/contract-psl/test/ts-psl-parity.test.tspackages/2-sql/2-authoring/contract-ts/src/contract-definition.tspackages/2-sql/2-authoring/contract-ts/src/contract-dsl.tspackages/2-sql/2-authoring/contract-ts/src/contract-lowering.tspackages/2-sql/2-authoring/contract-ts/test/config-types.test.tspackages/2-sql/2-authoring/contract-ts/test/contract-builder.contract-definition.test.tspackages/2-sql/2-authoring/contract-ts/test/contract-builder.value-objects.test.tspackages/2-sql/2-authoring/contract-ts/test/helpers/column-descriptor.tspackages/2-sql/3-tooling/emitter/test/emitter-hook.typeref-resolver.test.tspackages/2-sql/4-lanes/relational-core/README.mdpackages/2-sql/4-lanes/relational-core/package.jsonpackages/2-sql/4-lanes/relational-core/src/ast/codec-types.tspackages/2-sql/4-lanes/relational-core/src/ast/sql-codecs-class.tspackages/2-sql/4-lanes/relational-core/src/ast/sql-codecs.tspackages/2-sql/4-lanes/relational-core/src/ast/types.tspackages/2-sql/4-lanes/relational-core/src/ast/validate-param-refs.tspackages/2-sql/4-lanes/relational-core/src/exports/ast.tspackages/2-sql/4-lanes/relational-core/src/expression.tspackages/2-sql/4-lanes/relational-core/src/query-lane-context.tspackages/2-sql/4-lanes/relational-core/test/ast/codec-factory-ctx.test.tspackages/2-sql/4-lanes/relational-core/test/ast/codec-factory-ctx.types.test-d.tspackages/2-sql/4-lanes/relational-core/test/ast/codec-factory.test.tspackages/2-sql/4-lanes/relational-core/test/ast/codec-factory.types.test-d.tspackages/2-sql/4-lanes/relational-core/test/ast/codec-types.test.tspackages/2-sql/4-lanes/relational-core/test/ast/refs-propagation.test.tspackages/2-sql/4-lanes/relational-core/test/ast/sql-codecs-class.test.tspackages/2-sql/4-lanes/relational-core/test/ast/sql-codecs-class.types.test-d.tspackages/2-sql/4-lanes/relational-core/test/ast/sql-codecs.test.tspackages/2-sql/4-lanes/relational-core/test/ast/test-codec.tspackages/2-sql/4-lanes/relational-core/test/ast/test-helpers.tspackages/2-sql/4-lanes/relational-core/test/ast/validate-param-refs.test.tspackages/2-sql/4-lanes/relational-core/test/expression.test.tspackages/2-sql/4-lanes/relational-core/test/typed-codec-flow.test-d.tspackages/2-sql/4-lanes/relational-core/test/utils.tspackages/2-sql/4-lanes/sql-builder/src/runtime/builder-base.tspackages/2-sql/4-lanes/sql-builder/src/runtime/expression-impl.tspackages/2-sql/4-lanes/sql-builder/src/runtime/field-proxy.tspackages/2-sql/4-lanes/sql-builder/src/runtime/functions.tspackages/2-sql/4-lanes/sql-builder/src/runtime/mutation-impl.tspackages/2-sql/4-lanes/sql-builder/test/runtime/field-proxy.test.tspackages/2-sql/4-lanes/sql-builder/test/runtime/functions.test.tspackages/2-sql/5-runtime/src/codecs/decoding.tspackages/2-sql/5-runtime/src/codecs/encoding.tspackages/2-sql/5-runtime/src/codecs/json-schema-validation.tspackages/2-sql/5-runtime/src/sql-context.tspackages/2-sql/5-runtime/src/sql-runtime.tspackages/2-sql/5-runtime/test/before-compile-chain.test.tspackages/2-sql/5-runtime/test/codec-async.test.tspackages/2-sql/5-runtime/test/codec-decode-ctx.test.tspackages/2-sql/5-runtime/test/codec-encode-ctx.test.tspackages/2-sql/5-runtime/test/contract-codec-registry.test.tspackages/2-sql/5-runtime/test/encoding-dispatch.test.tspackages/2-sql/5-runtime/test/execution-stack.test.tspackages/2-sql/5-runtime/test/json-schema-validation.test.tspackages/2-sql/5-runtime/test/mutation-default-generators.test.tspackages/2-sql/5-runtime/test/parameterized-types.test.tspackages/2-sql/5-runtime/test/seeded-secret-codec.tspackages/2-sql/5-runtime/test/sql-context.test.tspackages/2-sql/5-runtime/test/sql-runtime-abort.test.tspackages/2-sql/5-runtime/test/sql-runtime.test.tspackages/2-sql/5-runtime/test/test-codec.tspackages/2-sql/5-runtime/test/utils.tspackages/3-extensions/arktype-json/README.mdpackages/3-extensions/arktype-json/src/core/arktype-json-codec-class.tspackages/3-extensions/arktype-json/src/core/arktype-json-codec.tspackages/3-extensions/arktype-json/src/core/pack-meta.tspackages/3-extensions/arktype-json/src/exports/codecs.tspackages/3-extensions/arktype-json/src/exports/column-types.tspackages/3-extensions/arktype-json/src/exports/control.tspackages/3-extensions/arktype-json/src/exports/runtime.tspackages/3-extensions/arktype-json/test/arktype-json-codec-class.test.tspackages/3-extensions/arktype-json/test/arktype-json-codec-class.types.test-d.tspackages/3-extensions/arktype-json/test/arktype-json-codec.test-d.tspackages/3-extensions/arktype-json/test/arktype-json-codec.test.tspackages/3-extensions/arktype-json/test/extension-descriptors.test.tspackages/3-extensions/pgvector/src/core/codecs-class.tspackages/3-extensions/pgvector/src/core/codecs.tspackages/3-extensions/pgvector/src/core/descriptor-meta.tspackages/3-extensions/pgvector/src/exports/column-types.tspackages/3-extensions/pgvector/src/exports/runtime.tspackages/3-extensions/pgvector/src/types/codec-types.tspackages/3-extensions/pgvector/test/codec-render-output-type.test.tspackages/3-extensions/pgvector/test/codecs-class.types.test-d.tspackages/3-extensions/pgvector/test/codecs.test.tspackages/3-extensions/pgvector/test/operations.test.tspackages/3-extensions/pgvector/test/typed-descriptor-flow.test-d.tspackages/3-extensions/sql-orm-client/src/query-plan-mutations.tspackages/3-extensions/sql-orm-client/src/types.tspackages/3-extensions/sql-orm-client/src/where-binding.tspackages/3-extensions/sql-orm-client/test/codec-async.types.test-d.tspackages/3-extensions/sql-orm-client/test/filters.test.tspackages/3-extensions/sql-orm-client/test/integration/codec-async.test.tspackages/3-extensions/sql-orm-client/test/integration/create.test.tspackages/3-extensions/sql-orm-client/test/model-accessor.test.tspackages/3-extensions/sql-orm-client/test/query-plan-mutations.test.tspackages/3-extensions/sql-orm-client/test/repository.test.tspackages/3-extensions/sql-orm-client/test/rich-filters-and-where.test.tspackages/3-extensions/sql-orm-client/test/test-codec.tspackages/3-mongo-target/1-mongo-target/src/exports/runtime.tspackages/3-mongo-target/2-mongo-adapter/src/core/codecs.tspackages/3-mongo-target/2-mongo-adapter/src/exports/control.tspackages/3-mongo-target/2-mongo-adapter/src/exports/runtime.tspackages/3-mongo-target/2-mongo-adapter/test/codecs.test.tspackages/3-mongo-target/2-mongo-adapter/test/lowering.test.tspackages/3-mongo-target/2-mongo-adapter/test/mongo-adapter-ctx.test.tspackages/3-mongo-target/2-mongo-adapter/test/mongo-adapter.test.tspackages/3-mongo-target/2-mongo-adapter/test/resolve-value-ctx.test.tspackages/3-mongo-target/2-mongo-adapter/test/resolve-value.test.tspackages/3-targets/3-targets/postgres/package.jsonpackages/3-targets/3-targets/postgres/src/core/codecs-class.tspackages/3-targets/3-targets/postgres/src/core/codecs.tspackages/3-targets/3-targets/postgres/src/core/descriptor-meta.tspackages/3-targets/3-targets/postgres/src/exports/codec-types.tspackages/3-targets/3-targets/postgres/src/exports/codecs.tspackages/3-targets/3-targets/postgres/src/exports/runtime.tspackages/3-targets/3-targets/postgres/test/codec-render-output-type.test.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.test.tspackages/3-targets/3-targets/postgres/test/typed-descriptor-flow.test-d.tspackages/3-targets/3-targets/sqlite/src/core/codecs-class.tspackages/3-targets/3-targets/sqlite/src/core/codecs.tspackages/3-targets/3-targets/sqlite/src/core/descriptor-meta.tspackages/3-targets/3-targets/sqlite/src/core/runtime-target.tspackages/3-targets/3-targets/sqlite/src/exports/codec-types.tspackages/3-targets/3-targets/sqlite/src/exports/codecs.tspackages/3-targets/3-targets/sqlite/test/codecs-class.types.test-d.tspackages/3-targets/3-targets/sqlite/test/typed-descriptor-flow.test-d.tspackages/3-targets/6-adapters/postgres/README.mdpackages/3-targets/6-adapters/postgres/src/core/adapter.tspackages/3-targets/6-adapters/postgres/src/core/codec-lookup.tspackages/3-targets/6-adapters/postgres/src/core/descriptor-meta.tspackages/3-targets/6-adapters/postgres/src/core/sql-renderer.tspackages/3-targets/6-adapters/postgres/src/exports/column-types.tspackages/3-targets/6-adapters/postgres/src/exports/runtime.tspackages/3-targets/6-adapters/postgres/test/adapter.test.tspackages/3-targets/6-adapters/postgres/test/descriptor-parity.test.tspackages/3-targets/6-adapters/postgres/test/sql-renderer.cast-policy.test.tspackages/3-targets/6-adapters/postgres/test/test-codec.tspackages/3-targets/6-adapters/sqlite/README.mdpackages/3-targets/6-adapters/sqlite/src/core/adapter.tspackages/3-targets/6-adapters/sqlite/src/core/runtime-adapter.tspackages/3-targets/6-adapters/sqlite/src/exports/codec-types.tspackages/3-targets/6-adapters/sqlite/test/codecs.test.tstest/integration/test/authoring/parity/ts-psl-parity.real-packs.test.tstest/integration/test/contract-builder.test.tstest/integration/test/cross-package/cross-family-codec.test.tstest/integration/test/cross-package/test-codec.tstest/integration/test/mongo/migration-psl-authoring.test.tstest/integration/test/pgvector.test.tstest/utils/README.mdtest/utils/src/column-descriptors.ts
💤 Files with no reviewable changes (11)
- packages/2-mongo-family/7-runtime/test/decode.integration.test.ts
- packages/2-mongo-family/1-foundation/mongo-codec/test/codecs-ctx.test.ts
- packages/2-sql/4-lanes/relational-core/test/ast/codec-types.test.ts
- packages/3-extensions/pgvector/src/core/codecs.ts
- packages/2-sql/5-runtime/src/codecs/json-schema-validation.ts
- packages/3-extensions/arktype-json/src/core/arktype-json-codec.ts
- packages/3-extensions/arktype-json/test/arktype-json-codec.test-d.ts
- packages/1-framework/2-authoring/contract/src/descriptors.ts
- packages/2-mongo-family/1-foundation/mongo-codec/test/codecs-ctx.test-d.ts
- packages/2-sql/5-runtime/test/json-schema-validation.test.ts
- packages/3-extensions/arktype-json/test/arktype-json-codec.test.ts
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (5)
packages/3-extensions/arktype-json/test/arktype-json-codec-class.types.test-d.ts (1)
140-142:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse the approved matcher for
.test-d.tstype compatibility assertions.Line 140 uses deprecated
toMatchTypeOf; switch totoExtendfor this subtype-style check.♻️ Proposed fix
- expectTypeOf(col).toMatchTypeOf< + expectTypeOf(col).toExtend< ColumnSpec<ArktypeJsonCodecClass<{ name: string; price: number }>, ArktypeJsonTypeParams> >();#!/bin/bash # Verify deprecated matcher usage in .test-d.ts files. rg -n --glob '**/*.test-d.ts' '\.toMatchTypeOf\s*<'As per coding guidelines, "
**/*.test-d.ts: Do not use the deprecatedtoMatchTypeOf()matcher in Vitest'sexpectTypeOfAPI. UsetoExtend()instead."🤖 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-extensions/arktype-json/test/arktype-json-codec-class.types.test-d.ts` around lines 140 - 142, Replace the deprecated matcher usage: locate the expectTypeOf assertion that references ColumnSpec<ArktypeJsonCodecClass<{ name: string; price: number }>, ArktypeJsonTypeParams> (the expectTypeOf(col) line) and change the matcher from toMatchTypeOf<...>() to toExtend<...>() so the subtype-style compatibility check uses the approved Vitest matcher for .test-d.ts files.packages/3-extensions/arktype-json/src/core/arktype-json-codec-class.ts (1)
144-147:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReplace the double-cast in schema rehydration with a predicate-based narrow.
Line 146 force-casts
ark.schema(jsonIr)withas unknown as ArktypeSchemaLike, which bypasses static safety and can mask shape drift.♻️ Proposed fix
function rehydrateSchema(jsonIr: object): ArktypeSchemaLike { try { - return ark.schema(jsonIr) as unknown as ArktypeSchemaLike; + const schema = ark.schema(jsonIr); + if (!isArktypeSchemaLike(schema)) { + throw new Error('Rehydrated schema is not callable with expression metadata'); + } + return schema; } catch (error) { throw runtimeError( 'RUNTIME.JSON_SCHEMA_VALIDATION_FAILED', `Failed to rehydrate arktype schema from contract IR: ${error instanceof Error ? error.message : String(error)}`, { codecId: ARKTYPE_JSON_CODEC_ID, jsonIr }, ); } }In current Arktype docs/types, what does `ark.schema(jsonIr)` return, and is the return value guaranteed to be callable with an `expression: string` property?As per coding guidelines, "
packages/**/*.{ts,tsx}: No blind casts in production code:as unknown as Xis forbidden outside tests. Prefer type predicates or fix the type surface."🤖 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-extensions/arktype-json/src/core/arktype-json-codec-class.ts` around lines 144 - 147, The rehydrateSchema function silently force-casts ark.schema(jsonIr) to ArktypeSchemaLike with "as unknown as", which violates the no-blind-casts rule; replace the double-cast with a runtime type-narrow: call ark.schema(jsonIr) into a local const (no cast), then implement and use a type predicate/is-check (for example isArktypeSchemaLike) that verifies the returned value is callable and has the expected shape (e.g., a function and the "expression" property of type string or other required members), throw a clear error if the check fails, and only then return the value typed as ArktypeSchemaLike; reference rehydrateSchema and ark.schema when locating code to change.packages/3-extensions/arktype-json/test/arktype-json-codec-class.test.ts (1)
75-79:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReplace
as any+ lint suppression with@ts-expect-errorin the guard test.Lines 77-78 currently bypass both typing and lint policy; this negative-path test can express intent without
anyorbiome-ignore.♻️ Proposed fix
it('rejects non-callable schema lookalikes at the call site', () => { const notASchema = { foo: 'bar' }; - // biome-ignore lint/suspicious/noExplicitAny: deliberately malformed input for the call-site guard - expect(() => arktypeJsonColumn(notASchema as any)).toThrow(/callable arktype Type/); + // `@ts-expect-error` -- intentionally passing non-callable value to test runtime guard + expect(() => arktypeJsonColumn(notASchema)).toThrow(/callable arktype Type/); });#!/bin/bash # Verify explicit any and biome suppressions in this test file. rg -n 'biome-ignore|as any|@ts-expect-error' packages/3-extensions/arktype-json/test/arktype-json-codec-class.test.ts -C2As per coding guidelines, "Never use
anytype", "Never suppress biome lints", and "Use@ts-expect-errorwith a short reason 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/3-extensions/arktype-json/test/arktype-json-codec-class.test.ts` around lines 75 - 79, The test currently silences type + lint rules by using "as any" and a "biome-ignore" comment; replace that with a TypeScript compile-time suppression using `@ts-expect-error` plus a short reason above the offending call to arktypeJsonColumn so the test still asserts the runtime guard without using any/biome ignores. Locate the failing test in arktype-json-codec-class.test.ts where arktypeJsonColumn(notASchema as any) is called and remove the "as any" cast and the biome-ignore, adding a single-line "@ts-expect-error TS-EXPLAIN: deliberate malformed input to exercise call-site guard" immediately above the call.packages/3-targets/3-targets/sqlite/src/core/codecs-class.ts (1)
245-258:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReject invalid
Datevalues at the codec boundary.
new Date(wire)/new Date(json)can produce anInvalid Date, andtoISOString()will also fail if callers hand this codec an invalidDate. Validating here keeps malformed storage values and contract JSON from escaping as poisonedDateinstances.🐛 Minimal validation
export class SqliteDatetimeCodec extends CodecImpl< @@ > { async encode(value: Date, _ctx: CodecCallContext): Promise<string> { + if (Number.isNaN(value.getTime())) { + throw new TypeError('sqlite/datetime@1 input value must be a valid Date'); + } return value.toISOString(); } async decode(wire: string, _ctx: CodecCallContext): Promise<Date> { - return new Date(wire); + const date = new Date(wire); + if (Number.isNaN(date.getTime())) { + throw new TypeError('sqlite/datetime@1 wire value must be a valid ISO-8601 string'); + } + return date; } encodeJson(value: Date): JsonValue { + if (Number.isNaN(value.getTime())) { + throw new TypeError('sqlite/datetime@1 contract value must be a valid Date'); + } return value.toISOString(); } decodeJson(json: JsonValue): Date { if (typeof json !== 'string') { throw new TypeError('sqlite/datetime@1 contract value must be an ISO-8601 string'); } - return new Date(json); + const date = new Date(json); + if (Number.isNaN(date.getTime())) { + throw new TypeError('sqlite/datetime@1 contract value must be a valid ISO-8601 string'); + } + return date; } }🤖 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/sqlite/src/core/codecs-class.ts` around lines 245 - 258, The codec must reject invalid Date instances and non-strings: in encode and encodeJson (functions encode(value: Date, _ctx: CodecCallContext) and encodeJson(value: Date)) check that value is a Date (instanceof Date) and that Number.isFinite(value.getTime())/!Number.isNaN(value.getTime()) before calling toISOString(), and throw a TypeError if not; in decode and decodeJson (decode(wire: string, _ctx: CodecCallContext) and decodeJson(json: JsonValue)) ensure the input is a string, construct the Date, then validate the produced Date with Number.isFinite(date.getTime())/!Number.isNaN(...) and throw a TypeError for invalid dates instead of returning them.packages/2-sql/4-lanes/relational-core/src/ast/sql-codecs-class.ts (1)
67-76:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
sqlCharColumn()/sqlVarcharColumn()still expose an impossible params shape.
lengthParamsSchemarequireslength, butLengthParamsmakes it optional and both helpers default to{}. That lets callers build column specs that this same descriptor later rejects during params validation. Either requirelengthend-to-end, or relax the schema if barechar/varcharis intentionally supported.🐛 Minimal fix
-type LengthParams = { readonly length?: number }; +type LengthParams = { readonly length: number }; -export const sqlCharColumn = (params: LengthParams = {}) => +export const sqlCharColumn = (params: LengthParams) => column(sqlCharDescriptor.factory(params), sqlCharDescriptor.codecId, params, 'char'); -export const sqlVarcharColumn = (params: LengthParams = {}) => +export const sqlVarcharColumn = (params: LengthParams) => column(sqlVarcharDescriptor.factory(params), sqlVarcharDescriptor.codecId, params, 'varchar');Also applies to: 245-246, 290-291
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/2-sql/4-lanes/relational-core/src/ast/sql-codecs-class.ts` around lines 67 - 76, The params shape is inconsistent: lengthParamsSchema currently requires length but the TypeScript type LengthParams and the sqlCharColumn/sqlVarcharColumn helpers treat length as optional and default to {}. Fix by making the contract consistent — either (A) relax lengthParamsSchema to allow an optional length (e.g., make the length rule accept undefined or mark it optional) so it matches LengthParams and the default {} behavior, or (B) make LengthParams require length and update sqlCharColumn/sqlVarcharColumn to require callers to supply length (and remove the default {}). Locate and update lengthParamsSchema, the LengthParams type, and the sqlCharColumn/sqlVarcharColumn call sites to implement the chosen option so params validation no longer rejects valid inputs.
🧹 Nitpick comments (4)
packages/3-targets/6-adapters/postgres/src/core/adapter.ts (1)
44-60: ⚡ Quick winDon’t leave the
factory(undefined)invariant implicit.This block now depends on every descriptor in
postgresCodecRegistrybeing safely materializable without params. That may be true for current builtins, but it is only enforced by comments right now. Please move this through a small helper owned next topostgresCodecRegistryor add a fail-fast assertion here so a future param-sensitive descriptor does not silently instantiate the wrong runtime codec.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/3-targets/6-adapters/postgres/src/core/adapter.ts` around lines 44 - 60, The code silently calls factory(undefined) for each descriptor from postgresCodecRegistry when constructing codecRegistry, which assumes every descriptor is paramless; add a fail-fast check or helper next to postgresCodecRegistry to enforce/verify that a descriptor is materializable without params before calling factory(undefined). Concretely, introduce a small predicate/helper (e.g., isParamlessDescriptor) or add an assertion in the codecRegistry initializer that inspects the descriptor shape and throws a clear error if params are required, and then only call (descriptor.factory)(undefined) (or use the helper) to produce the codec instance; reference codecRegistry, postgresCodecRegistry, CodecInstanceContext and the current factory(undefined) usage when making the change.packages/3-targets/6-adapters/sqlite/src/core/adapter.ts (1)
55-70: 🏗️ Heavy liftDon't materialize every descriptor with
factory(undefined).
sqliteCodecRegistrynow includes parameterized SQL descriptors, but this loop hard-casts everyfactoryto acceptunknownand instantiates it withundefined. The adapter registry therefore only works for descriptors whose runtime instances happen to ignore params; the first param-sensitive codec added here will be mis-materialized. Please keep this surface descriptor-based, or restrict this fallback registry to descriptors whose params are trulyvoid.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/3-targets/6-adapters/sqlite/src/core/adapter.ts` around lines 55 - 70, The current codecRegistry eagerly materializes every descriptor by calling descriptor.factory(undefined), which mis-instantiates parameterized descriptors from sqliteCodecRegistry; change this to preserve descriptor-based entries and only instantiate codecs when they are truly parameterless: in the codecRegistry initialization (and related logic around codecRegistry, sqliteCodecRegistry, CodecInstanceContext and factory), avoid calling factory(undefined) for all descriptors—instead detect parameterized descriptors (e.g., via a param-type flag or factory arity) and either keep the original descriptor in the registry or only call factory() for descriptors known to accept no params; ensure the fallback registry only materializes codecs whose params are void so parameterized codecs remain descriptor-based.packages/3-targets/3-targets/postgres/test/codecs.test.ts (1)
75-82: ⚡ Quick winDon't bypass parameterized descriptor factories here.
descriptor.factory(undefined as never)sidesteps the public params contract forcharacter,numeric,timestamp,enum, and the other parameterized descriptors in this table. The suite still checks encode/decode behavior, but it no longer proves those codecs can be constructed through their supported API shape. A smallparamsByScalarmap, or separate helpers for parameterized vs non-parameterized codecs, would keep these tests aligned with the descriptor-first surface.🤖 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/test/codecs.test.ts` around lines 75 - 82, The helper codecForScalar currently calls descriptor.factory(undefined as never) which bypasses the public params contract for parameterized descriptors; change codecForScalar to supply real params for parameterized descriptors (e.g. via a small paramsByScalar map or conditional helpers) and call descriptor.factory(params) for those scalars, falling back to descriptor.factory() or descriptor.factory(undefined) only for true non-parameterized descriptors so that descriptorByScalar/descriptor.factory and SYNTH_CTX are exercised via the intended public API shape.packages/3-targets/3-targets/sqlite/src/exports/codec-types.ts (1)
11-45: ⚡ Quick winKeep
src/exports/codec-types.tsas a thin re-export surface.
codecDescriptorMapandResolvemake this export file a second source of truth for SQLite codec membership. That raises the odds ofCodecTypesdrifting from the core registry/list when the next codec is added or removed. Please move the map/type derivation into../core/...and re-export the final alias from here so the public entrypoint stays aligned with the package’s single descriptor source.♻️ Suggested shape
-import type { ExtractCodecTypes } from '@prisma-next/sql-relational-core/ast'; -import { - sqlCharDescriptor, - sqlFloatDescriptor, - sqlIntDescriptor, - sqlVarcharDescriptor, -} from '@prisma-next/sql-relational-core/ast'; import type { JsonValue } from '../core/codecs'; -import { - sqliteBigintDescriptor, - sqliteBlobDescriptor, - sqliteDatetimeDescriptor, - sqliteIntegerDescriptor, - sqliteJsonDescriptor, - sqliteRealDescriptor, - sqliteTextDescriptor, -} from '../core/codecs-class'; - -const codecDescriptorMap = { - char: sqlCharDescriptor, - varchar: sqlVarcharDescriptor, - int: sqlIntDescriptor, - float: sqlFloatDescriptor, - text: sqliteTextDescriptor, - integer: sqliteIntegerDescriptor, - real: sqliteRealDescriptor, - blob: sqliteBlobDescriptor, - datetime: sqliteDatetimeDescriptor, - json: sqliteJsonDescriptor, - bigint: sqliteBigintDescriptor, -} as const; - -type Resolve<T> = { readonly [K in keyof T]: { readonly [P in keyof T[K]]: T[K][P] } }; - -export type CodecTypes = Resolve<ExtractCodecTypes<typeof codecDescriptorMap>>; +export type { CodecTypes } from '../core/codec-types'; export type { JsonValue };As per coding guidelines,
packages/**/src/exports/{adapter,codec-types,types}.tsre-export files must re-export from core without adding additional logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/2-sql/4-lanes/relational-core/src/codec-descriptor-registry.ts`:
- Around line 31-39: The registry currently overwrites duplicate codecId entries
causing inconsistent state; before calling byId.set(descriptor.codecId, widened)
in the loop over allDescriptors (where widened is cast to AnyDescriptor), check
if byId.has(descriptor.codecId) and throw a clear error (including the
conflicting codecId and descriptor identity) to fail-fast on duplicates; only
proceed to add to byTargetType when the codecId is not already present (so byId
and byTargetType remain consistent and descriptorFor(id) will match target-type
lists).
In `@packages/3-extensions/pgvector/src/core/codecs-class.ts`:
- Around line 82-118: The PgVectorCodec produced by factory() must retain
params.length (the declared dimension) and validate input across all ingress
paths: modify factory() to store the expected length on the PgVectorCodec
instance, add a single reused validator function (used by encode, decode, and
decodeJson) that checks the value is an array of numbers and that its length
equals the stored dimension, and update encodeJson/decodeJson/decode/encode to
call that validator and throw descriptive errors for wrong-dimension or
non-numeric entries; ensure contractCodecs.forColumn(table, column) returns a
codec that enforces the declared vector length rather than ignoring
params.length.
In `@packages/3-targets/3-targets/postgres/src/exports/codec-types.ts`:
- Around line 19-86: The file defines codecDescriptorMap, Resolve, and
CodecTypes inline which violates the export-only rule; move the
codecDescriptorMap, Resolve type helper, and the ExtractCodecTypes processing
into a new or existing module under src/core (e.g., export the computed type
there) and then change src/exports/codec-types.ts to only re-export the public
type (export type { CodecTypes } from '../core/...') so the exports file
contains no computation or logic and only exposes the type symbol CodecTypes.
In `@packages/3-targets/3-targets/sqlite/src/core/codecs-class.ts`:
- Around line 204-211: The encodeJson and decodeJson methods currently use
Node-only Buffer; replace that with a runtime-neutral base64 utility so src/core
stays platform-agnostic: implement or import cross-platform helpers (e.g.,
uint8ArrayToBase64 and base64ToUint8Array) that use Web APIs (atob/btoa or
TextEncoder/TextDecoder) and fall back to Buffer only when necessary, then
update encodeJson(value: Uint8Array) to return uint8ArrayToBase64(value) and
decodeJson(json: JsonValue) to validate string and return
base64ToUint8Array(json); keep the function names encodeJson and decodeJson
unchanged.
---
Duplicate comments:
In `@packages/2-sql/4-lanes/relational-core/src/ast/sql-codecs-class.ts`:
- Around line 67-76: The params shape is inconsistent: lengthParamsSchema
currently requires length but the TypeScript type LengthParams and the
sqlCharColumn/sqlVarcharColumn helpers treat length as optional and default to
{}. Fix by making the contract consistent — either (A) relax lengthParamsSchema
to allow an optional length (e.g., make the length rule accept undefined or mark
it optional) so it matches LengthParams and the default {} behavior, or (B) make
LengthParams require length and update sqlCharColumn/sqlVarcharColumn to require
callers to supply length (and remove the default {}). Locate and update
lengthParamsSchema, the LengthParams type, and the
sqlCharColumn/sqlVarcharColumn call sites to implement the chosen option so
params validation no longer rejects valid inputs.
In `@packages/3-extensions/arktype-json/src/core/arktype-json-codec-class.ts`:
- Around line 144-147: The rehydrateSchema function silently force-casts
ark.schema(jsonIr) to ArktypeSchemaLike with "as unknown as", which violates the
no-blind-casts rule; replace the double-cast with a runtime type-narrow: call
ark.schema(jsonIr) into a local const (no cast), then implement and use a type
predicate/is-check (for example isArktypeSchemaLike) that verifies the returned
value is callable and has the expected shape (e.g., a function and the
"expression" property of type string or other required members), throw a clear
error if the check fails, and only then return the value typed as
ArktypeSchemaLike; reference rehydrateSchema and ark.schema when locating code
to change.
In `@packages/3-extensions/arktype-json/test/arktype-json-codec-class.test.ts`:
- Around line 75-79: The test currently silences type + lint rules by using "as
any" and a "biome-ignore" comment; replace that with a TypeScript compile-time
suppression using `@ts-expect-error` plus a short reason above the offending call
to arktypeJsonColumn so the test still asserts the runtime guard without using
any/biome ignores. Locate the failing test in arktype-json-codec-class.test.ts
where arktypeJsonColumn(notASchema as any) is called and remove the "as any"
cast and the biome-ignore, adding a single-line "@ts-expect-error TS-EXPLAIN:
deliberate malformed input to exercise call-site guard" immediately above the
call.
In
`@packages/3-extensions/arktype-json/test/arktype-json-codec-class.types.test-d.ts`:
- Around line 140-142: Replace the deprecated matcher usage: locate the
expectTypeOf assertion that references ColumnSpec<ArktypeJsonCodecClass<{ name:
string; price: number }>, ArktypeJsonTypeParams> (the expectTypeOf(col) line)
and change the matcher from toMatchTypeOf<...>() to toExtend<...>() so the
subtype-style compatibility check uses the approved Vitest matcher for
.test-d.ts files.
In `@packages/3-targets/3-targets/sqlite/src/core/codecs-class.ts`:
- Around line 245-258: The codec must reject invalid Date instances and
non-strings: in encode and encodeJson (functions encode(value: Date, _ctx:
CodecCallContext) and encodeJson(value: Date)) check that value is a Date
(instanceof Date) and that
Number.isFinite(value.getTime())/!Number.isNaN(value.getTime()) before calling
toISOString(), and throw a TypeError if not; in decode and decodeJson
(decode(wire: string, _ctx: CodecCallContext) and decodeJson(json: JsonValue))
ensure the input is a string, construct the Date, then validate the produced
Date with Number.isFinite(date.getTime())/!Number.isNaN(...) and throw a
TypeError for invalid dates instead of returning them.
---
Nitpick comments:
In `@packages/3-targets/3-targets/postgres/test/codecs.test.ts`:
- Around line 75-82: The helper codecForScalar currently calls
descriptor.factory(undefined as never) which bypasses the public params contract
for parameterized descriptors; change codecForScalar to supply real params for
parameterized descriptors (e.g. via a small paramsByScalar map or conditional
helpers) and call descriptor.factory(params) for those scalars, falling back to
descriptor.factory() or descriptor.factory(undefined) only for true
non-parameterized descriptors so that descriptorByScalar/descriptor.factory and
SYNTH_CTX are exercised via the intended public API shape.
In `@packages/3-targets/6-adapters/postgres/src/core/adapter.ts`:
- Around line 44-60: The code silently calls factory(undefined) for each
descriptor from postgresCodecRegistry when constructing codecRegistry, which
assumes every descriptor is paramless; add a fail-fast check or helper next to
postgresCodecRegistry to enforce/verify that a descriptor is materializable
without params before calling factory(undefined). Concretely, introduce a small
predicate/helper (e.g., isParamlessDescriptor) or add an assertion in the
codecRegistry initializer that inspects the descriptor shape and throws a clear
error if params are required, and then only call (descriptor.factory)(undefined)
(or use the helper) to produce the codec instance; reference codecRegistry,
postgresCodecRegistry, CodecInstanceContext and the current factory(undefined)
usage when making the change.
In `@packages/3-targets/6-adapters/sqlite/src/core/adapter.ts`:
- Around line 55-70: The current codecRegistry eagerly materializes every
descriptor by calling descriptor.factory(undefined), which mis-instantiates
parameterized descriptors from sqliteCodecRegistry; change this to preserve
descriptor-based entries and only instantiate codecs when they are truly
parameterless: in the codecRegistry initialization (and related logic around
codecRegistry, sqliteCodecRegistry, CodecInstanceContext and factory), avoid
calling factory(undefined) for all descriptors—instead detect parameterized
descriptors (e.g., via a param-type flag or factory arity) and either keep the
original descriptor in the registry or only call factory() for descriptors known
to accept no params; ensure the fallback registry only materializes codecs whose
params are void so parameterized codecs remain descriptor-based.
🪄 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: 395e4d68-7371-4142-b775-7022bfe445bb
📒 Files selected for processing (46)
packages/2-sql/4-lanes/relational-core/package.jsonpackages/2-sql/4-lanes/relational-core/src/ast/sql-codecs-class.tspackages/2-sql/4-lanes/relational-core/src/codec-descriptor-registry.tspackages/2-sql/4-lanes/relational-core/src/exports/codec-descriptor-registry.tspackages/2-sql/4-lanes/relational-core/test/ast/sql-codecs-class.test.tspackages/2-sql/4-lanes/relational-core/test/ast/sql-codecs-class.types.test-d.tspackages/2-sql/4-lanes/relational-core/test/ast/sql-codecs.test.tspackages/2-sql/4-lanes/relational-core/test/typed-codec-flow.test-d.tspackages/2-sql/4-lanes/relational-core/tsdown.config.tspackages/2-sql/5-runtime/src/sql-context.tspackages/3-extensions/arktype-json/src/core/arktype-json-codec-class.tspackages/3-extensions/arktype-json/src/core/pack-meta.tspackages/3-extensions/arktype-json/src/exports/codecs.tspackages/3-extensions/arktype-json/test/arktype-json-codec-class.test.tspackages/3-extensions/arktype-json/test/arktype-json-codec-class.types.test-d.tspackages/3-extensions/arktype-json/test/extension-descriptors.test.tspackages/3-extensions/pgvector/src/core/codecs-class.tspackages/3-extensions/pgvector/src/core/descriptor-meta.tspackages/3-extensions/pgvector/src/core/registry.tspackages/3-extensions/pgvector/src/exports/runtime.tspackages/3-extensions/pgvector/test/codec-render-output-type.test.tspackages/3-extensions/pgvector/test/codecs-class.types.test-d.tspackages/3-extensions/pgvector/test/codecs.test.tspackages/3-extensions/pgvector/test/typed-descriptor-flow.test-d.tspackages/3-targets/3-targets/postgres/src/core/codecs-class.tspackages/3-targets/3-targets/postgres/src/core/registry.tspackages/3-targets/3-targets/postgres/src/exports/codec-types.tspackages/3-targets/3-targets/postgres/src/exports/codecs.tspackages/3-targets/3-targets/postgres/test/codec-render-output-type.test.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.test.tspackages/3-targets/3-targets/postgres/test/typed-descriptor-flow.test-d.tspackages/3-targets/3-targets/sqlite/src/core/codecs-class.tspackages/3-targets/3-targets/sqlite/src/core/registry.tspackages/3-targets/3-targets/sqlite/src/exports/codec-types.tspackages/3-targets/3-targets/sqlite/src/exports/codecs.tspackages/3-targets/3-targets/sqlite/test/codecs-class.types.test-d.tspackages/3-targets/3-targets/sqlite/test/typed-descriptor-flow.test-d.tspackages/3-targets/6-adapters/postgres/src/core/adapter.tspackages/3-targets/6-adapters/postgres/src/core/codec-lookup.tspackages/3-targets/6-adapters/postgres/src/core/descriptor-meta.tspackages/3-targets/6-adapters/postgres/src/exports/runtime.tspackages/3-targets/6-adapters/sqlite/src/core/adapter.tspackages/3-targets/6-adapters/sqlite/src/core/runtime-adapter.tspackages/3-targets/6-adapters/sqlite/test/codecs.test.ts
✅ Files skipped from review due to trivial changes (9)
- packages/2-sql/4-lanes/relational-core/package.json
- packages/2-sql/4-lanes/relational-core/src/exports/codec-descriptor-registry.ts
- packages/3-targets/3-targets/postgres/src/core/registry.ts
- packages/3-extensions/pgvector/src/core/registry.ts
- packages/3-targets/3-targets/sqlite/src/core/registry.ts
- packages/2-sql/4-lanes/relational-core/tsdown.config.ts
- packages/3-extensions/arktype-json/src/exports/codecs.ts
- packages/3-extensions/arktype-json/src/core/pack-meta.ts
- packages/3-targets/3-targets/sqlite/src/exports/codecs.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/3-targets/3-targets/postgres/src/exports/codecs.ts
- packages/3-targets/3-targets/postgres/test/codecs-class.types.test-d.ts
- packages/2-sql/5-runtime/src/sql-context.ts
- packages/3-targets/3-targets/postgres/src/core/codecs-class.ts
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/3-targets/3-targets/postgres/src/core/codecs.ts (1)
110-124:⚠️ Potential issue | 🟠 Major | ⚡ Quick winThe length-parameter helpers can emit params their descriptors reject.
lengthParamsSchemarequireslength, butLengthParamsis optional and these helpers default to{}. That meanspgBitColumn(),pgVarbitColumn(),pgCharColumn(), andpgVarcharColumn()can author column specs that fail descriptor validation later.Either make
lengthrequired at the helper boundary, or relax the relevant schemas if bare forms are intentionally supported. Right now the API surface and validator contract disagree.Also applies to: 733-735, 779-780, 1008-1035
🤖 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 110 - 124, The helper functions (e.g., pgBitColumn, pgVarbitColumn, pgCharColumn, pgVarcharColumn) are producing param objects that the validator lengthParamsSchema rejects because lengthParamsSchema requires length while the Type LengthParams and helpers allow an empty {}; fix by reconciling the contract: either relax lengthParamsSchema to accept an optional length (change length to 'number.integer > 0' -> 'number.integer > 0?') so bare forms validate, or make the helpers enforce/require length at their boundary (update the helper implementations to validate and throw or return an error when length is missing) and change LengthParams to require length; apply the same reconciliation for the related numeric/precision schemas and their helper functions referenced in the comment (numericParamsSchema, precisionParamsSchema and their callers).
♻️ Duplicate comments (3)
packages/3-extensions/pgvector/src/core/codecs.ts (1)
76-118:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCarry
lengthintoPgVectorCodecand validate every ingress path.
factory()still materializes the same codec forvector(3)andvector(1536), anddecodeJson()still force-casts JSON tonumber[]. That means wrong-dimension or non-numeric vectors can get past the codec boundary until much later.Suggested direction
export class PgVectorCodec extends CodecImpl< typeof VECTOR_CODEC_ID, readonly ['equality'], string, number[] > { + constructor( + descriptor: PgVectorDescriptor, + private readonly length: number, + ) { + super(descriptor); + } + + private assertVector(value: JsonValue | number[]): asserts value is number[] { + if (!Array.isArray(value)) { + throw new Error('Vector value must be an array of numbers'); + } + if (!value.every((v) => typeof v === 'number')) { + throw new Error('Vector value must contain only numbers'); + } + if (value.length !== this.length) { + throw new Error(`Vector length mismatch: expected ${this.length}, got ${value.length}`); + } + } + async encode(value: number[], _ctx: CodecCallContext): Promise<string> { - if (!Array.isArray(value)) { - throw new Error('Vector value must be an array of numbers'); - } - if (!value.every((v) => typeof v === 'number')) { - throw new Error('Vector value must contain only numbers'); - } + this.assertVector(value); return `[${value.join(',')}]`; } async decode(wire: string, _ctx: CodecCallContext): Promise<number[]> { @@ - return content.split(',').map((v) => { + const value = content.split(',').map((v) => { const num = Number.parseFloat(v.trim()); if (Number.isNaN(num)) { throw new Error(`Invalid vector value: "${v}" is not a number`); } return num; }); + this.assertVector(value); + return value; } encodeJson(value: number[]): JsonValue { + this.assertVector(value); return value; } decodeJson(json: JsonValue): number[] { - return json as number[]; + this.assertVector(json); + return json; } } @@ - override factory(_params: VectorParams): (ctx: CodecInstanceContext) => PgVectorCodec { - return () => new PgVectorCodec(this); + override factory(params: VectorParams): (ctx: CodecInstanceContext) => PgVectorCodec { + return () => new PgVectorCodec(this, params.length); } }Also applies to: 130-132
🤖 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-extensions/pgvector/src/core/codecs.ts` around lines 76 - 118, PgVectorCodec currently ignores the declared vector dimension so invalid-length or non-numeric vectors slip through; modify the factory() that constructs PgVectorCodec to accept and store the vector length into the codec instance, then validate that length in all ingress/egress paths: in encode(value), ensure value is an array of numbers of the expected length; in decode(wire) after parsing, ensure the resulting number[] has the expected length; in encodeJson(value) validate the array contents and length before returning; in decodeJson(json) validate that json is an array of numbers and the correct length (don’t force-cast). Update the PgVectorCodec class signature to carry a length field and use it in these methods to throw descriptive errors on mismatch or non-numeric entries.packages/2-sql/4-lanes/relational-core/src/ast/sql-codecs-class.ts (1)
67-76:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMake the
char/varcharhelper contract match the descriptor schema.Lines 75-76, 245-246, and 290-291 make
lengthoptional and default helper params to{}, butlengthParamsSchemaon Lines 67-69 still requires it. That letssqlCharColumn()/sqlVarcharColumn()produce type params that their own descriptors later reject.One fix if bare forms are not intended
-type LengthParams = { readonly length?: number }; +type LengthParams = { readonly length: number }; @@ -export const sqlCharColumn = (params: LengthParams = {}) => +export const sqlCharColumn = (params: LengthParams) => column(sqlCharDescriptor.factory(params), sqlCharDescriptor.codecId, params, 'char'); @@ -export const sqlVarcharColumn = (params: LengthParams = {}) => +export const sqlVarcharColumn = (params: LengthParams) => column(sqlVarcharDescriptor.factory(params), sqlVarcharDescriptor.codecId, params, 'varchar');If bare
char/varcharis intentional, relaxlengthParamsSchemainstead so validation matches the helper API.Also applies to: 245-246, 290-291
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/2-sql/4-lanes/relational-core/src/ast/sql-codecs-class.ts` around lines 67 - 76, The length validation in lengthParamsSchema conflicts with the helper defaults used by sqlCharColumn and sqlVarcharColumn—lengthParamsSchema currently requires length but the helpers make length optional and default to {}—so relax the schema to allow an optional length (e.g. change the key to 'length?' with the same numeric constraint) so the descriptor validation matches the helper API; alternatively, if you intend bare forms to be disallowed, make sqlCharColumn/sqlVarcharColumn require and pass a length instead, but the simpler fix is to update lengthParamsSchema to accept an optional length to align with LengthParams and the helpers.packages/3-extensions/arktype-json/test/arktype-json-codec.test.ts (1)
74-77:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDrop the
anycast and Biome suppression in this negative-path test.This still violates the repo’s test rules. A single
@ts-expect-erroron Line 77 keeps the runtime-guard coverage without suppressing lint or widening toany.Suggested cleanup
it('rejects non-callable schema lookalikes at the call site', () => { const notASchema = { foo: 'bar' }; - // biome-ignore lint/suspicious/noExplicitAny: deliberately malformed input for the call-site guard - expect(() => arktypeJsonColumn(notASchema as any)).toThrow(/callable arktype Type/); + // `@ts-expect-error` -- intentionally passing a non-callable to exercise the runtime guard + expect(() => arktypeJsonColumn(notASchema)).toThrow(/callable arktype Type/); });As per coding guidelines, "Never use
anytype", "Never suppress biome lints", and "Use@ts-expect-errorwith a short reason in tests instead of blanket@ts-ignore."🤖 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-extensions/arktype-json/test/arktype-json-codec.test.ts` around lines 74 - 77, The test currently uses an any cast and a biome-ignore to pass a malformed value into arktypeJsonColumn (variable notASchema) which violates project rules; remove the "as any" cast and the biome-ignore comment and instead put a single line with `@ts-expect-error` and a short reason immediately above the expect(() => arktypeJsonColumn(notASchema)).toThrow(...) call so the TypeScript compile-time error is expected while preserving the runtime negative-path assertion for arktypeJsonColumn.
🧹 Nitpick comments (2)
packages/3-targets/3-targets/sqlite/src/exports/codec-types.ts (1)
11-45: ⚡ Quick winKeep
src/exports/codec-types.tsas a pure re-export surface.This entrypoint now computes
codecDescriptorMap,Resolve, andCodecTypesinline. That breaks the repo’ssrc/exports/*rule; move the implementation into a core module and re-export the finished type from here.♻️ Suggested shape
-import type { ExtractCodecTypes } from '@prisma-next/sql-relational-core/ast'; -import { - sqlCharDescriptor, - sqlFloatDescriptor, - sqlIntDescriptor, - sqlVarcharDescriptor, -} from '@prisma-next/sql-relational-core/ast'; import type { JsonValue } from '../core/codec-helpers'; -import { - sqliteBigintDescriptor, - sqliteBlobDescriptor, - sqliteDatetimeDescriptor, - sqliteIntegerDescriptor, - sqliteJsonDescriptor, - sqliteRealDescriptor, - sqliteTextDescriptor, -} from '../core/codecs'; - -const codecDescriptorMap = { - char: sqlCharDescriptor, - varchar: sqlVarcharDescriptor, - int: sqlIntDescriptor, - float: sqlFloatDescriptor, - text: sqliteTextDescriptor, - integer: sqliteIntegerDescriptor, - real: sqliteRealDescriptor, - blob: sqliteBlobDescriptor, - datetime: sqliteDatetimeDescriptor, - json: sqliteJsonDescriptor, - bigint: sqliteBigintDescriptor, -} as const; - -type Resolve<T> = { readonly [K in keyof T]: { readonly [P in keyof T[K]]: T[K][P] } }; - -export type CodecTypes = Resolve<ExtractCodecTypes<typeof codecDescriptorMap>>; +export type { CodecTypes } from '../core/codec-types';As per coding guidelines, "Re-export files in
src/exports/(adapter.ts, codec-types.ts, types.ts) must re-export from core (../core/...) without adding additional logic."🤖 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/sqlite/src/exports/codec-types.ts` around lines 11 - 45, The file currently computes codecDescriptorMap, Resolve and CodecTypes inline; move that logic into a new core module (e.g. ../core/codec-types.ts) where you export the const codecDescriptorMap, the helper Resolve type and the resulting export type CodecTypes (using ExtractCodecTypes and the existing codec descriptors like sqliteTextDescriptor, sqliteIntegerDescriptor, etc.), then change this src/exports/codec-types.ts to be a pure re-export that only re-exports the finished type (e.g. export type { CodecTypes } from '../core/codec-types';) and any needed descriptor re-exports—remove all inline computation from the exports file.packages/2-sql/4-lanes/relational-core/src/ast/codec-types.ts (1)
238-248: 💤 Low valueBoth
inputandoutputcorrectly useDescriptorCodecInput— no issue found.The current implementation is semantically correct. Both
inputandoutputrepresent the application-facing type (TInput) per codec contract semantics: codecs encode application values to wire format and decode wire format back to application values. Tests confirm this:ContractCodecTypes['pg/int4@1']['output']correctly infersnumber.The naming could benefit from a brief inline comment clarifying that
inputandoutputboth refer to the application-side type (as opposed to wire format), but this is a minor optional clarity improvement, not a functional issue.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/2-sql/4-lanes/relational-core/src/ast/codec-types.ts` around lines 238 - 248, Add a brief inline comment in the ExtractCodecTypes type to clarify that both properties named input and output use DescriptorCodecInput and therefore represent the application-facing type (TInput) rather than the wire format; update the area around the ExtractCodecTypes declaration (reference the type name ExtractCodecTypes and the properties input/output that use DescriptorCodecInput and DescriptorCodecTraits) to include a one-line comment explaining this semantic choice for future readers.
🤖 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 55-72: The pgNumericRenderOutputType function currently allows any
JS number for typeParams.scale (including NaN/Infinity/floats) which can produce
invalid output like "Numeric<10, NaN>"; update pgNumericRenderOutputType to
validate typeParams.scale the same way precision is validated: check typeof
scale === 'number', Number.isFinite(scale), and Number.isInteger(scale) (and
throw a similar Error mentioning "scale" when invalid), and only render
`Numeric<precision, scale>` when the validated scale is present; otherwise fall
back to `Numeric<precision>`.
- Around line 103-114: The function pgEnumRenderOutputType currently coerces all
enum values with String(value); instead, validate that typeParams.values is an
array of strings and throw a descriptive Error if any element is not a string
before rendering. In pgEnumRenderOutputType inspect values (e.g. values.forEach
or values.some) and if an element's typeof !== 'string' throw an Error naming
the offending index/type (or a clear message) so only string literals are
processed, then proceed to escape and join the strings as before.
- Around line 75-96: Both pgTimestampDecodeJson and pgTimestamptzDecodeJson
currently accept any format the Date constructor parses; change them to first
validate the input string against a strict ISO 8601 regex (e.g., RFC3339-like
pattern) before constructing a Date, and remove the round-trip toISOString()
equality check (use the regex + Number.isNaN(date.getTime()) checks only) so
inputs like "2024-01-15T10:30:00Z" are accepted while non-ISO inputs are
rejected; update the error messages in pgTimestampDecodeJson and
pgTimestamptzDecodeJson to reflect the ISO-only requirement.
In `@packages/3-targets/3-targets/sqlite/src/core/codecs.ts`:
- Around line 245-259: The decode methods currently create Date objects from
strings but don't validate them, allowing invalid Dates to propagate; update
both decode(wire: string) and decodeJson(json: JsonValue) to parse the string
into a Date (using new Date(...)) and then check validity with
isNaN(date.getTime()) (or Number.isNaN(date.getTime())); if invalid, throw a
TypeError with the same contract message used now ('sqlite/datetime@1 contract
value must be an ISO-8601 string'); otherwise return the valid Date. Apply this
change to the decode and decodeJson functions to reject malformed datetime
strings early.
---
Outside diff comments:
In `@packages/3-targets/3-targets/postgres/src/core/codecs.ts`:
- Around line 110-124: The helper functions (e.g., pgBitColumn, pgVarbitColumn,
pgCharColumn, pgVarcharColumn) are producing param objects that the validator
lengthParamsSchema rejects because lengthParamsSchema requires length while the
Type LengthParams and helpers allow an empty {}; fix by reconciling the
contract: either relax lengthParamsSchema to accept an optional length (change
length to 'number.integer > 0' -> 'number.integer > 0?') so bare forms validate,
or make the helpers enforce/require length at their boundary (update the helper
implementations to validate and throw or return an error when length is missing)
and change LengthParams to require length; apply the same reconciliation for the
related numeric/precision schemas and their helper functions referenced in the
comment (numericParamsSchema, precisionParamsSchema and their callers).
---
Duplicate comments:
In `@packages/2-sql/4-lanes/relational-core/src/ast/sql-codecs-class.ts`:
- Around line 67-76: The length validation in lengthParamsSchema conflicts with
the helper defaults used by sqlCharColumn and
sqlVarcharColumn—lengthParamsSchema currently requires length but the helpers
make length optional and default to {}—so relax the schema to allow an optional
length (e.g. change the key to 'length?' with the same numeric constraint) so
the descriptor validation matches the helper API; alternatively, if you intend
bare forms to be disallowed, make sqlCharColumn/sqlVarcharColumn require and
pass a length instead, but the simpler fix is to update lengthParamsSchema to
accept an optional length to align with LengthParams and the helpers.
In `@packages/3-extensions/arktype-json/test/arktype-json-codec.test.ts`:
- Around line 74-77: The test currently uses an any cast and a biome-ignore to
pass a malformed value into arktypeJsonColumn (variable notASchema) which
violates project rules; remove the "as any" cast and the biome-ignore comment
and instead put a single line with `@ts-expect-error` and a short reason
immediately above the expect(() => arktypeJsonColumn(notASchema)).toThrow(...)
call so the TypeScript compile-time error is expected while preserving the
runtime negative-path assertion for arktypeJsonColumn.
In `@packages/3-extensions/pgvector/src/core/codecs.ts`:
- Around line 76-118: PgVectorCodec currently ignores the declared vector
dimension so invalid-length or non-numeric vectors slip through; modify the
factory() that constructs PgVectorCodec to accept and store the vector length
into the codec instance, then validate that length in all ingress/egress paths:
in encode(value), ensure value is an array of numbers of the expected length; in
decode(wire) after parsing, ensure the resulting number[] has the expected
length; in encodeJson(value) validate the array contents and length before
returning; in decodeJson(json) validate that json is an array of numbers and the
correct length (don’t force-cast). Update the PgVectorCodec class signature to
carry a length field and use it in these methods to throw descriptive errors on
mismatch or non-numeric entries.
---
Nitpick comments:
In `@packages/2-sql/4-lanes/relational-core/src/ast/codec-types.ts`:
- Around line 238-248: Add a brief inline comment in the ExtractCodecTypes type
to clarify that both properties named input and output use DescriptorCodecInput
and therefore represent the application-facing type (TInput) rather than the
wire format; update the area around the ExtractCodecTypes declaration (reference
the type name ExtractCodecTypes and the properties input/output that use
DescriptorCodecInput and DescriptorCodecTraits) to include a one-line comment
explaining this semantic choice for future readers.
In `@packages/3-targets/3-targets/sqlite/src/exports/codec-types.ts`:
- Around line 11-45: The file currently computes codecDescriptorMap, Resolve and
CodecTypes inline; move that logic into a new core module (e.g.
../core/codec-types.ts) where you export the const codecDescriptorMap, the
helper Resolve type and the resulting export type CodecTypes (using
ExtractCodecTypes and the existing codec descriptors like sqliteTextDescriptor,
sqliteIntegerDescriptor, etc.), then change this src/exports/codec-types.ts to
be a pure re-export that only re-exports the finished type (e.g. export type {
CodecTypes } from '../core/codec-types';) and any needed descriptor
re-exports—remove all inline computation from the exports file.
🪄 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: 9bcd7dd3-79d5-45a7-9aef-23e5489f668f
📒 Files selected for processing (42)
docs/reference/codec-authoring-guide.mdpackages/2-sql/4-lanes/relational-core/README.mdpackages/2-sql/4-lanes/relational-core/src/ast/codec-types.tspackages/2-sql/4-lanes/relational-core/src/ast/sql-codecs-class.tspackages/2-sql/4-lanes/relational-core/src/ast/sql-codecs.tspackages/2-sql/4-lanes/relational-core/test/ast/sql-codecs-class.types.test-d.tspackages/2-sql/4-lanes/relational-core/test/typed-codec-flow.test-d.tspackages/2-sql/5-runtime/src/sql-context.tspackages/3-extensions/arktype-json/src/core/arktype-json-codec.tspackages/3-extensions/arktype-json/src/core/pack-meta.tspackages/3-extensions/arktype-json/src/core/registry.tspackages/3-extensions/arktype-json/src/exports/codecs.tspackages/3-extensions/arktype-json/src/exports/column-types.tspackages/3-extensions/arktype-json/src/exports/runtime.tspackages/3-extensions/arktype-json/test/arktype-json-codec.test.tspackages/3-extensions/arktype-json/test/arktype-json-codec.types.test-d.tspackages/3-extensions/arktype-json/test/extension-descriptors.test.tspackages/3-extensions/pgvector/src/core/codecs.tspackages/3-extensions/pgvector/src/core/registry.tspackages/3-extensions/pgvector/test/codec-render-output-type.test.tspackages/3-extensions/pgvector/test/codecs-class.types.test-d.tspackages/3-extensions/pgvector/test/codecs.test.tspackages/3-extensions/pgvector/test/typed-descriptor-flow.test-d.tspackages/3-extensions/sql-orm-client/test/codec-async.types.test-d.tspackages/3-extensions/sql-orm-client/test/integration/codec-async.test.tspackages/3-targets/3-targets/postgres/src/core/codec-helpers.tspackages/3-targets/3-targets/postgres/src/core/codecs.tspackages/3-targets/3-targets/postgres/src/core/registry.tspackages/3-targets/3-targets/postgres/src/exports/codec-types.tspackages/3-targets/3-targets/postgres/src/exports/codecs.tspackages/3-targets/3-targets/postgres/test/codec-render-output-type.test.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.test.tspackages/3-targets/3-targets/postgres/test/typed-descriptor-flow.test-d.tspackages/3-targets/3-targets/sqlite/src/core/codec-helpers.tspackages/3-targets/3-targets/sqlite/src/core/codecs.tspackages/3-targets/3-targets/sqlite/src/core/registry.tspackages/3-targets/3-targets/sqlite/src/exports/codec-types.tspackages/3-targets/3-targets/sqlite/src/exports/codecs.tspackages/3-targets/3-targets/sqlite/test/codecs-class.types.test-d.tspackages/3-targets/3-targets/sqlite/test/typed-descriptor-flow.test-d.ts
✅ Files skipped from review due to trivial changes (16)
- packages/3-targets/3-targets/postgres/src/core/registry.ts
- packages/3-extensions/pgvector/src/core/registry.ts
- packages/3-extensions/sql-orm-client/test/integration/codec-async.test.ts
- packages/3-extensions/sql-orm-client/test/codec-async.types.test-d.ts
- packages/3-targets/3-targets/sqlite/src/core/registry.ts
- packages/3-targets/3-targets/postgres/src/exports/codecs.ts
- packages/3-extensions/pgvector/test/codec-render-output-type.test.ts
- packages/3-extensions/arktype-json/src/core/registry.ts
- packages/3-extensions/pgvector/test/typed-descriptor-flow.test-d.ts
- packages/3-extensions/arktype-json/src/exports/codecs.ts
- packages/2-sql/4-lanes/relational-core/README.md
- packages/3-targets/3-targets/postgres/test/codecs-class.types.test-d.ts
- packages/2-sql/4-lanes/relational-core/test/typed-codec-flow.test-d.ts
- docs/reference/codec-authoring-guide.md
- packages/2-sql/5-runtime/src/sql-context.ts
- packages/3-targets/3-targets/postgres/src/exports/codec-types.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/3-extensions/arktype-json/src/exports/column-types.ts
saevarb
reviewed
May 8, 2026
wmadden
commented
May 8, 2026
Contributor
Author
|
All actionable items in this review body are addressed in the inline thread replies; see those for closing commits. On the outside-diff finding for The nitpicks (column-spec generic threading, |
Contributor
Author
|
All actionable findings in this review body are duplicates of inline threads or other review bodies and are closed:
Nitpicks (nudges on |
Contributor
Author
|
All actionable findings in this review body are closed via the inline thread replies (F29, F34, F35, F36, F37, F39 — see those threads for closing commits). On the outside-diff finding for Nitpicks (descriptor-first shape for |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/2-sql/5-runtime/src/codecs/encoding.ts (1)
141-144:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve codec-authored runtime errors during encode.
encodeParamValue()wraps every thrown error asRUNTIME.ENCODE_FAILED. That double-wraps codec-generatedRUNTIME.ENCODE_FAILEDenvelopes and downgrades a codec-thrownRUNTIME.ABORTEDinto the wrong error code. Pass known runtime envelopes through before callingwrapEncodeFailure().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/2-sql/5-runtime/src/codecs/encoding.ts` around lines 141 - 144, In encodeParamValue's catch block around codec.encode(value, ctx), detect if the thrown error is already a runtime envelope (i.e., a codec-authored runtime error/exception) and rethrow it unchanged instead of wrapping; otherwise call wrapEncodeFailure(error, metadata, paramIndex, codec.id). Update the catch to explicitly pass through known runtime envelopes (identify by the runtime envelope shape/marker used in this project) before invoking wrapEncodeFailure so codec-thrown RUNTIME.* errors are preserved.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/2-sql/5-runtime/src/codecs/decoding.ts`:
- Around line 260-263: decodeField currently wraps every thrown error into
RUNTIME.DECODE_FAILED; change the catch in decodeField so it only calls
wrapDecodeFailure for non-runtime/foreign errors: if the caught error is already
a codec-produced runtime envelope (e.g., has the runtime failure shape or codes
like RUNTIME.DECODE_FAILED or RUNTIME.ABORTED) then rethrow it unchanged,
otherwise call wrapDecodeFailure(alias, ref, codec, wireValue) and throw that;
reference the decodeField function, the codec.decode call, and wrapDecodeFailure
when making this conditional check so codec-authored runtime envelopes are
preserved.
In `@packages/2-sql/5-runtime/src/sql-context.ts`:
- Around line 273-282: The partitioning logic currently checks
descriptor.paramsSchema !== voidParamsSchema which is brittle; change the branch
to use the descriptor.isParameterized boolean as the discriminator so
descriptors are routed correctly into the parameterized map
(parameterized.set(... as RuntimeParameterizedCodecDescriptor)) vs the
unparameterized path, ensuring validateTypeParams() and initializeTypeHelpers()
are invoked for parameterized descriptors; update any comments referencing
paramsSchema identity to reflect isParameterized as the source of truth.
- Around line 410-427: The current loop pre-creates and caches a single shared
codec instance in byCodecId and then reuses that same instance for column-bound
lookups, causing descriptor.factory to see the shared SqlCodecInstanceContext
instead of the real column context; change the logic so the pre-populated
instance remains only as the forCodecId() fallback, and when populating entries
for forColumn(table, column) always materialize a fresh codec instance via
descriptor.factory(...) with a column-specific SqlCodecInstanceContext (e.g.
name like `<col:${table}.${column}>` and usedAt set to the column site) rather
than reusing the shared instance; apply the same change in the second block
referenced (the code around the 492-512 region) so non-parameterized descriptors
are only cached as a fallback and per-column instances are created with real
column contexts.
In `@packages/3-mongo-target/2-mongo-adapter/src/core/codecs.ts`:
- Around line 121-124: The guard in renderOutputType that validates the variable
length currently only checks for number, finiteness, and integer-ness but allows
0 and negatives; update the validation in renderOutputType (in codecs.ts) to
also require length > 0 and throw the same (or a clearer) Error when length is
not a positive integer so invalid inputs cannot produce Vector<0> or Vector<-1>.
---
Outside diff comments:
In `@packages/2-sql/5-runtime/src/codecs/encoding.ts`:
- Around line 141-144: In encodeParamValue's catch block around
codec.encode(value, ctx), detect if the thrown error is already a runtime
envelope (i.e., a codec-authored runtime error/exception) and rethrow it
unchanged instead of wrapping; otherwise call wrapEncodeFailure(error, metadata,
paramIndex, codec.id). Update the catch to explicitly pass through known runtime
envelopes (identify by the runtime envelope shape/marker used in this project)
before invoking wrapEncodeFailure so codec-thrown RUNTIME.* errors are
preserved.
🪄 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: bd164814-d8cb-4c7c-9ca5-571e8aa245a4
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (61)
docs/reference/codec-authoring-guide.mdexamples/prisma-next-demo/test/no-emit-typed-flow.test-d.tspackages/1-framework/1-core/framework-components/src/control/control-stack.tspackages/1-framework/1-core/framework-components/src/shared/codec-descriptor.tspackages/1-framework/1-core/framework-components/src/shared/codec.tspackages/1-framework/1-core/framework-components/test/codec-types.types.test-d.tspackages/1-framework/1-core/framework-components/test/control-stack.test.tspackages/2-sql/4-lanes/relational-core/src/ast/adapter-types.tspackages/2-sql/4-lanes/relational-core/src/ast/codec-types.tspackages/2-sql/4-lanes/relational-core/src/ast/sql-codec-helpers.tspackages/2-sql/4-lanes/relational-core/src/ast/sql-codecs.tspackages/2-sql/4-lanes/relational-core/src/ast/validate-param-refs.tspackages/2-sql/4-lanes/relational-core/src/codec-descriptor-registry.tspackages/2-sql/4-lanes/relational-core/src/exports/ast.tspackages/2-sql/4-lanes/relational-core/src/query-lane-context.tspackages/2-sql/4-lanes/relational-core/test/ast/sql-codec-helpers.test.tspackages/2-sql/4-lanes/relational-core/test/ast/sql-codecs.test.tspackages/2-sql/4-lanes/relational-core/test/ast/sql-codecs.types.test-d.tspackages/2-sql/4-lanes/relational-core/test/ast/validate-param-refs.test.tspackages/2-sql/4-lanes/relational-core/test/typed-codec-flow.test-d.tspackages/2-sql/4-lanes/relational-core/test/utils.tspackages/2-sql/5-runtime/src/codecs/decoding.tspackages/2-sql/5-runtime/src/codecs/encoding.tspackages/2-sql/5-runtime/src/codecs/validation.tspackages/2-sql/5-runtime/src/sql-context.tspackages/2-sql/5-runtime/src/sql-runtime.tspackages/2-sql/5-runtime/test/before-compile-chain.test.tspackages/2-sql/5-runtime/test/codec-async.test.tspackages/2-sql/5-runtime/test/codec-decode-ctx.test.tspackages/2-sql/5-runtime/test/codec-encode-ctx.test.tspackages/2-sql/5-runtime/test/contract-codec-registry.test.tspackages/2-sql/5-runtime/test/encoding-dispatch.test.tspackages/2-sql/5-runtime/test/execution-stack.test.tspackages/2-sql/5-runtime/test/parameterized-types.test.tspackages/2-sql/5-runtime/test/sql-context.test.tspackages/2-sql/5-runtime/test/sql-family-adapter.test.tspackages/2-sql/5-runtime/test/sql-runtime-abort.test.tspackages/2-sql/5-runtime/test/sql-runtime.test.tspackages/2-sql/5-runtime/test/utils.tspackages/3-extensions/arktype-json/package.jsonpackages/3-extensions/arktype-json/src/core/arktype-json-codec.tspackages/3-extensions/arktype-json/test/arktype-json-codec.test.tspackages/3-extensions/arktype-json/test/arktype-json-codec.types.test-d.tspackages/3-extensions/pgvector/package.jsonpackages/3-extensions/pgvector/src/core/codecs.tspackages/3-extensions/pgvector/test/codecs.test.tspackages/3-extensions/sql-orm-client/src/types.tspackages/3-extensions/sql-orm-client/test/model-accessor.test.tspackages/3-mongo-target/2-mongo-adapter/src/core/codecs.tspackages/3-targets/3-targets/postgres/src/core/codec-helpers.tspackages/3-targets/3-targets/postgres/src/core/codec-type-map.tspackages/3-targets/3-targets/postgres/src/core/codecs.tspackages/3-targets/3-targets/postgres/src/exports/codec-types.tspackages/3-targets/3-targets/postgres/test/codecs-class.types.test-d.tspackages/3-targets/3-targets/sqlite/src/core/codecs.tspackages/3-targets/6-adapters/postgres/src/core/adapter.tspackages/3-targets/6-adapters/postgres/test/adapter.test.tspackages/3-targets/6-adapters/postgres/test/descriptor-parity.test.tspackages/3-targets/6-adapters/postgres/test/sql-renderer.cast-policy.test.tspackages/3-targets/6-adapters/sqlite/src/core/adapter.tstest/integration/test/cross-package/cross-family-codec.test.ts
💤 Files with no reviewable changes (2)
- packages/2-sql/4-lanes/relational-core/src/ast/adapter-types.ts
- packages/2-sql/5-runtime/test/sql-family-adapter.test.ts
✅ Files skipped from review due to trivial changes (2)
- packages/3-extensions/arktype-json/package.json
- packages/3-extensions/arktype-json/test/arktype-json-codec.types.test-d.ts
🚧 Files skipped from review as they are similar to previous changes (16)
- packages/2-sql/5-runtime/test/codec-async.test.ts
- packages/1-framework/1-core/framework-components/src/shared/codec-descriptor.ts
- packages/3-targets/3-targets/postgres/test/codecs-class.types.test-d.ts
- packages/1-framework/1-core/framework-components/test/control-stack.test.ts
- packages/1-framework/1-core/framework-components/test/codec-types.types.test-d.ts
- packages/2-sql/4-lanes/relational-core/src/codec-descriptor-registry.ts
- packages/1-framework/1-core/framework-components/src/control/control-stack.ts
- packages/2-sql/5-runtime/test/contract-codec-registry.test.ts
- examples/prisma-next-demo/test/no-emit-typed-flow.test-d.ts
- packages/2-sql/5-runtime/test/execution-stack.test.ts
- packages/2-sql/5-runtime/test/encoding-dispatch.test.ts
- packages/3-extensions/pgvector/src/core/codecs.ts
- packages/2-sql/4-lanes/relational-core/test/typed-codec-flow.test-d.ts
- packages/2-sql/4-lanes/relational-core/src/ast/sql-codecs.ts
- packages/3-targets/3-targets/postgres/src/core/codec-helpers.ts
- packages/3-targets/3-targets/postgres/src/core/codecs.ts
wmadden
force-pushed
the
tml-2357-codec-registration-model-complete-the-unified
branch
from
0e3aafc to
c99c1ba
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/architecture docs/adrs/ADR 204 - Single-Path Async Codec Runtime.md (1)
43-56:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winClarify this section as historical to avoid implying
defineCodec()is current.This section reads as present-day behavior (
defineCodec()lifting sync methods), but the retrospective note says that authoring surface is retired. Please mark these statements as historical-era mechanics so the ADR stays internally consistent.As per coding guidelines, "
**/*.{md,mdx}: Keep docs current (READMEs, rules, links)`."🤖 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 `@docs/architecture` docs/adrs/ADR 204 - Single-Path Async Codec Runtime.md around lines 43 - 56, This section currently describes defineCodec()/mongoCodec() and the Codec interface as if they are current; mark these sentences as historical by adding a short qualifier (e.g., "Historical:", "Legacy behavior:", or "Former design:") before the paragraph that explains that defineCodec() lifts sync methods to Promise-returning query-time methods and the listed Codec shape (encode/decode behavior), and update any present-tense verbs to past-tense for symbols like defineCodec(), mongoCodec(), Codec, encode, decode, encodeJson, decodeJson so it clearly reads as retired/legacy mechanics rather than current behavior.
🧹 Nitpick comments (2)
docs/architecture docs/adrs/ADR 204 - Single-Path Async Codec Runtime.md (1)
237-237: ⚡ Quick winMove implementation mechanics out of the ADR body.
This line includes concrete internal wiring and file-path implementation detail. Prefer keeping ADR text at decision/constraint level and linking to subsystem docs for mechanics.
Based on learnings, "ADRs in prisma/prisma-next should document architectural design decisions and the key constraints/assumptions only. Do not include implementation algorithms, emitter derivation logic, or step-by-step derivation/migration rules."
🤖 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 `@docs/architecture` docs/adrs/ADR 204 - Single-Path Async Codec Runtime.md at line 237, The ADR includes low-level wiring and file-path detail; remove the implementation mechanics and file paths and rephrase to the decision/constraint level: state that component descriptors declare codecs (ComponentMetadata.types.codecTypes.codecDescriptors), the framework aggregates those via the execution-stack into a single MongoCodecRegistry exposed on the execution context (context.codecs) by createMongoExecutionContext, and that MongoRuntimeOptions no longer accepts a codecs field; move the detailed folding algorithm and any package/file-specific implementation (e.g., the previous package paths) into a linked subsystem implementation doc instead of the ADR body.docs/architecture docs/adrs/ADR 208 - Higher-order codecs for parameterized types.md (1)
166-175: ⚡ Quick winMove the numbered runtime build sequence out of the ADR.
This section drops into implementation algorithm detail (
forColumnprebuild steps, caching order, and validation flow) rather than the architectural decision itself. Keeping the ADR at the invariant level here would age better; the step-by-step assembly belongs in subsystem/runtime docs.Based on learnings, "ADRs in prisma/prisma-next should document architectural design decisions and the key constraints/assumptions only. Do not include implementation algorithms, emitter derivation logic, or step-by-step derivation/migration rules."
🤖 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 `@docs/architecture` docs/adrs/ADR 208 - Higher-order codecs for parameterized types.md around lines 166 - 175, The ADR currently includes implementation-level algorithmic steps (the numbered "forColumn" build sequence, caching/validation order and materialization details) that should be removed from the ADR and moved to runtime/subsystem documentation; edit the ADR so it only documents the high-level decision and invariants (that a descriptor map keyed by codecId exists, that descriptorFor(codecId) exposes metadata, and forColumn(table, column) resolves per-instance vs singleton codecs) and remove the detailed prebuild steps referencing storage.tables[].columns[], descriptor.factory(...), descriptor.paramsSchema['~standard'].validate(...), and decode(...) validation behavior; instead add a short pointer/note in the ADR referencing the runtime docs where the concrete assembly, caching, and JSON-schema validation implementation is described.
🤖 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 `@docs/architecture` docs/adrs/ADR 208 - Higher-order codecs for parameterized
types.md:
- Around line 258-259: Replace the bullet that mentions "Mongo control-plane
parameterized-codecs slot" and any reference to "parameterizedCodecs:" with
phrasing that Mongo will adopt the unified "codecs:" registration surface (e.g.,
"Mongo control-plane to adopt unified codecs: registration surface") so the ADR
no longer reintroduces the retired `parameterized-codecs slot` vocabulary and
matches the deleted `parameterizedCodecs:` slot stated earlier.
In `@packages/1-framework/1-core/framework-components/README.md`:
- Line 34: The README incorrectly states that TypeScript bivariance permits
synchronous method bodies to satisfy Promise return types and shows an example
using property syntax and an optional ctx; fix the text and example: update the
explanation to state that bivariance only affects parameter type checking and
that methods declared to return Promise<T> must return a compatible Promise (use
async or explicitly return a Promise), change the sketch to extend CodecImpl and
implement encode(...) and decode(...) as class methods (not property shorthand),
and make the ctx parameter required and use ctx.signal (not ctx?.signal) in the
method signatures and bodies; reference the CodecImpl class and the
encode/decode methods when making these edits.
---
Outside diff comments:
In `@docs/architecture` docs/adrs/ADR 204 - Single-Path Async Codec Runtime.md:
- Around line 43-56: This section currently describes defineCodec()/mongoCodec()
and the Codec interface as if they are current; mark these sentences as
historical by adding a short qualifier (e.g., "Historical:", "Legacy behavior:",
or "Former design:") before the paragraph that explains that defineCodec() lifts
sync methods to Promise-returning query-time methods and the listed Codec shape
(encode/decode behavior), and update any present-tense verbs to past-tense for
symbols like defineCodec(), mongoCodec(), Codec, encode, decode, encodeJson,
decodeJson so it clearly reads as retired/legacy mechanics rather than current
behavior.
---
Nitpick comments:
In `@docs/architecture` docs/adrs/ADR 204 - Single-Path Async Codec Runtime.md:
- Line 237: The ADR includes low-level wiring and file-path detail; remove the
implementation mechanics and file paths and rephrase to the decision/constraint
level: state that component descriptors declare codecs
(ComponentMetadata.types.codecTypes.codecDescriptors), the framework aggregates
those via the execution-stack into a single MongoCodecRegistry exposed on the
execution context (context.codecs) by createMongoExecutionContext, and that
MongoRuntimeOptions no longer accepts a codecs field; move the detailed folding
algorithm and any package/file-specific implementation (e.g., the previous
package paths) into a linked subsystem implementation doc instead of the ADR
body.
In `@docs/architecture` docs/adrs/ADR 208 - Higher-order codecs for parameterized
types.md:
- Around line 166-175: The ADR currently includes implementation-level
algorithmic steps (the numbered "forColumn" build sequence, caching/validation
order and materialization details) that should be removed from the ADR and moved
to runtime/subsystem documentation; edit the ADR so it only documents the
high-level decision and invariants (that a descriptor map keyed by codecId
exists, that descriptorFor(codecId) exposes metadata, and forColumn(table,
column) resolves per-instance vs singleton codecs) and remove the detailed
prebuild steps referencing storage.tables[].columns[], descriptor.factory(...),
descriptor.paramsSchema['~standard'].validate(...), and decode(...) validation
behavior; instead add a short pointer/note in the ADR referencing the runtime
docs where the concrete assembly, caching, and JSON-schema validation
implementation is described.
🪄 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: 788f2e6c-0cb2-4466-aa83-d810249436a2
📒 Files selected for processing (114)
docs/README.mddocs/Testing Guide.mddocs/architecture docs/adrs/ADR 152 - Execution Plane Descriptors and Instances.mddocs/architecture docs/adrs/ADR 184 - Codec-owned value serialization.mddocs/architecture docs/adrs/ADR 186 - Codec-dispatched type rendering.mddocs/architecture docs/adrs/ADR 202 - Codec trait system.mddocs/architecture docs/adrs/ADR 204 - Single-Path Async Codec Runtime.mddocs/architecture docs/adrs/ADR 205 - SQL cast emission is adapter policy.mddocs/architecture docs/adrs/ADR 207 - Codec call context per-query AbortSignal and column metadata.mddocs/architecture docs/adrs/ADR 208 - Higher-order codecs for parameterized types.mddocs/architecture docs/adrs/ADR 209 - Mongo result-shape as a structural plan field.mddocs/architecture docs/subsystems/2. Contract Emitter & Types.mddocs/reference/codec-authoring-guide.mddocs/reference/community-generator-migration-analysis.mddocs/reference/framework-gaps.mddocs/reference/typescript-patterns.mdexamples/prisma-next-demo/test/no-emit-typed-flow.test-d.tspackages/1-framework/0-foundation/contract/package.jsonpackages/1-framework/1-core/framework-components/README.mdpackages/1-framework/1-core/framework-components/src/control/control-stack.tspackages/1-framework/1-core/framework-components/src/exports/codec.tspackages/1-framework/1-core/framework-components/src/shared/codec-descriptor.tspackages/1-framework/1-core/framework-components/src/shared/codec-types.tspackages/1-framework/1-core/framework-components/src/shared/codec.tspackages/1-framework/1-core/framework-components/src/shared/column-spec.tspackages/1-framework/1-core/framework-components/src/shared/framework-components.tspackages/1-framework/1-core/framework-components/test/codec-call-context.types.test-d.tspackages/1-framework/1-core/framework-components/test/codec-types.types.test-d.tspackages/1-framework/1-core/framework-components/test/codec.test.tspackages/1-framework/1-core/framework-components/test/codec.types.test-d.tspackages/1-framework/1-core/framework-components/test/control-stack.test.tspackages/1-framework/2-authoring/contract/README.mdpackages/1-framework/2-authoring/contract/src/descriptors.tspackages/1-framework/2-authoring/contract/src/index.tspackages/1-framework/2-authoring/contract/test/descriptors.test.tspackages/1-framework/2-authoring/ids/package.jsonpackages/1-framework/2-authoring/ids/src/index.tspackages/1-framework/3-tooling/cli/src/control-api/contract-enrichment.tspackages/1-framework/3-tooling/cli/test/config-types.test.tspackages/1-framework/3-tooling/cli/test/control-api/contract-enrichment.test.tspackages/1-framework/3-tooling/emitter/src/domain-type-generation.tspackages/1-framework/3-tooling/emitter/test/domain-type-generation.test.tspackages/2-mongo-family/1-foundation/mongo-codec/README.mdpackages/2-mongo-family/1-foundation/mongo-codec/src/codec-registry.tspackages/2-mongo-family/1-foundation/mongo-codec/src/codecs.tspackages/2-mongo-family/1-foundation/mongo-codec/src/exports/index.tspackages/2-mongo-family/1-foundation/mongo-codec/test/codecs-ctx.test-d.tspackages/2-mongo-family/1-foundation/mongo-codec/test/codecs-ctx.test.tspackages/2-mongo-family/1-foundation/mongo-codec/test/codecs.test-d.tspackages/2-mongo-family/1-foundation/mongo-codec/test/codecs.test.tspackages/2-mongo-family/2-authoring/contract-psl/src/derive-json-schema.tspackages/2-mongo-family/2-authoring/contract-psl/test/derive-json-schema.test.tspackages/2-mongo-family/2-authoring/contract-psl/test/interpreter.polymorphism.test.tspackages/2-mongo-family/2-authoring/contract-psl/test/interpreter.test.tspackages/2-mongo-family/7-runtime/README.mdpackages/2-mongo-family/7-runtime/src/mongo-execution-stack.tspackages/2-mongo-family/7-runtime/test/codecs/decoding.test.tspackages/2-mongo-family/7-runtime/test/decode.integration.test.tspackages/2-mongo-family/7-runtime/test/mongo-execution-stack.test.tspackages/2-mongo-family/7-runtime/test/mongo-middleware.test.tspackages/2-mongo-family/7-runtime/test/mongo-runtime-abort.test.tspackages/2-sql/2-authoring/contract-psl/src/provider.tspackages/2-sql/2-authoring/contract-psl/src/psl-column-resolution.tspackages/2-sql/2-authoring/contract-psl/test/fixtures.tspackages/2-sql/2-authoring/contract-psl/test/provider.test.tspackages/2-sql/2-authoring/contract-psl/test/ts-psl-parity.test.tspackages/2-sql/2-authoring/contract-ts/src/contract-definition.tspackages/2-sql/2-authoring/contract-ts/src/contract-dsl.tspackages/2-sql/2-authoring/contract-ts/src/contract-lowering.tspackages/2-sql/2-authoring/contract-ts/test/config-types.test.tspackages/2-sql/2-authoring/contract-ts/test/contract-builder.contract-definition.test.tspackages/2-sql/2-authoring/contract-ts/test/contract-builder.value-objects.test.tspackages/2-sql/2-authoring/contract-ts/test/helpers/column-descriptor.tspackages/2-sql/3-tooling/emitter/test/emitter-hook.typeref-resolver.test.tspackages/2-sql/4-lanes/relational-core/README.mdpackages/2-sql/4-lanes/relational-core/package.jsonpackages/2-sql/4-lanes/relational-core/src/ast/adapter-types.tspackages/2-sql/4-lanes/relational-core/src/ast/codec-types.tspackages/2-sql/4-lanes/relational-core/src/ast/sql-codec-helpers.tspackages/2-sql/4-lanes/relational-core/src/ast/sql-codecs.tspackages/2-sql/4-lanes/relational-core/src/ast/types.tspackages/2-sql/4-lanes/relational-core/src/ast/validate-param-refs.tspackages/2-sql/4-lanes/relational-core/src/codec-descriptor-registry.tspackages/2-sql/4-lanes/relational-core/src/exports/ast.tspackages/2-sql/4-lanes/relational-core/src/exports/codec-descriptor-registry.tspackages/2-sql/4-lanes/relational-core/src/expression.tspackages/2-sql/4-lanes/relational-core/src/query-lane-context.tspackages/2-sql/4-lanes/relational-core/test/ast/codec-factory-ctx.test.tspackages/2-sql/4-lanes/relational-core/test/ast/codec-factory-ctx.types.test-d.tspackages/2-sql/4-lanes/relational-core/test/ast/codec-factory.test.tspackages/2-sql/4-lanes/relational-core/test/ast/codec-factory.types.test-d.tspackages/2-sql/4-lanes/relational-core/test/ast/codec-types.test.tspackages/2-sql/4-lanes/relational-core/test/ast/refs-propagation.test.tspackages/2-sql/4-lanes/relational-core/test/ast/sql-codec-helpers.test.tspackages/2-sql/4-lanes/relational-core/test/ast/sql-codecs.test.tspackages/2-sql/4-lanes/relational-core/test/ast/sql-codecs.types.test-d.tspackages/2-sql/4-lanes/relational-core/test/ast/test-codec.tspackages/2-sql/4-lanes/relational-core/test/ast/test-helpers.tspackages/2-sql/4-lanes/relational-core/test/ast/validate-param-refs.test.tspackages/2-sql/4-lanes/relational-core/test/expression.test.tspackages/2-sql/4-lanes/relational-core/test/typed-codec-flow.test-d.tspackages/2-sql/4-lanes/relational-core/test/utils.tspackages/2-sql/4-lanes/relational-core/tsdown.config.tspackages/2-sql/4-lanes/sql-builder/src/runtime/builder-base.tspackages/2-sql/4-lanes/sql-builder/src/runtime/expression-impl.tspackages/2-sql/4-lanes/sql-builder/src/runtime/field-proxy.tspackages/2-sql/4-lanes/sql-builder/src/runtime/functions.tspackages/2-sql/4-lanes/sql-builder/src/runtime/mutation-impl.tspackages/2-sql/4-lanes/sql-builder/test/runtime/field-proxy.test.tspackages/2-sql/4-lanes/sql-builder/test/runtime/functions.test.tspackages/2-sql/5-runtime/src/codecs/decoding.tspackages/2-sql/5-runtime/src/codecs/encoding.tspackages/2-sql/5-runtime/src/codecs/json-schema-validation.tspackages/2-sql/5-runtime/src/codecs/validation.ts
💤 Files with no reviewable changes (7)
- packages/1-framework/2-authoring/contract/src/descriptors.ts
- packages/2-sql/5-runtime/src/codecs/json-schema-validation.ts
- packages/2-mongo-family/7-runtime/test/decode.integration.test.ts
- packages/2-sql/4-lanes/relational-core/test/ast/codec-types.test.ts
- packages/2-sql/4-lanes/relational-core/src/ast/adapter-types.ts
- packages/2-mongo-family/1-foundation/mongo-codec/test/codecs-ctx.test.ts
- packages/2-mongo-family/1-foundation/mongo-codec/test/codecs-ctx.test-d.ts
✅ Files skipped from review due to trivial changes (33)
- packages/2-sql/4-lanes/relational-core/src/exports/codec-descriptor-registry.ts
- packages/2-sql/4-lanes/relational-core/tsdown.config.ts
- packages/1-framework/0-foundation/contract/package.json
- docs/architecture docs/adrs/ADR 202 - Codec trait system.md
- packages/2-sql/2-authoring/contract-ts/test/helpers/column-descriptor.ts
- packages/1-framework/1-core/framework-components/test/codec-call-context.types.test-d.ts
- packages/2-sql/2-authoring/contract-ts/test/config-types.test.ts
- docs/architecture docs/subsystems/2. Contract Emitter & Types.md
- docs/architecture docs/adrs/ADR 205 - SQL cast emission is adapter policy.md
- packages/2-sql/2-authoring/contract-ts/src/contract-definition.ts
- packages/2-mongo-family/7-runtime/src/mongo-execution-stack.ts
- packages/1-framework/2-authoring/ids/package.json
- packages/1-framework/2-authoring/ids/src/index.ts
- packages/2-sql/3-tooling/emitter/test/emitter-hook.typeref-resolver.test.ts
- docs/architecture docs/adrs/ADR 186 - Codec-dispatched type rendering.md
- docs/README.md
- packages/2-sql/2-authoring/contract-ts/src/contract-dsl.ts
- docs/reference/community-generator-migration-analysis.md
- docs/architecture docs/adrs/ADR 152 - Execution Plane Descriptors and Instances.md
- packages/1-framework/2-authoring/contract/README.md
- packages/2-sql/4-lanes/relational-core/test/ast/codec-factory.test.ts
- packages/2-sql/2-authoring/contract-ts/test/contract-builder.value-objects.test.ts
- docs/architecture docs/adrs/ADR 209 - Mongo result-shape as a structural plan field.md
- packages/2-mongo-family/7-runtime/README.md
- docs/reference/framework-gaps.md
- packages/2-mongo-family/1-foundation/mongo-codec/test/codecs.test.ts
- packages/2-sql/4-lanes/relational-core/test/ast/validate-param-refs.test.ts
- packages/2-mongo-family/7-runtime/test/mongo-execution-stack.test.ts
- docs/architecture docs/adrs/ADR 207 - Codec call context per-query AbortSignal and column metadata.md
- docs/reference/codec-authoring-guide.md
- packages/2-sql/4-lanes/relational-core/test/ast/codec-factory-ctx.test.ts
- packages/2-sql/4-lanes/relational-core/README.md
- docs/Testing Guide.md
🚧 Files skipped from review as they are similar to previous changes (60)
- packages/2-sql/4-lanes/relational-core/package.json
- packages/1-framework/2-authoring/contract/src/index.ts
- packages/2-mongo-family/7-runtime/test/mongo-runtime-abort.test.ts
- packages/2-sql/2-authoring/contract-psl/test/ts-psl-parity.test.ts
- packages/2-sql/2-authoring/contract-psl/src/provider.ts
- packages/1-framework/3-tooling/cli/test/config-types.test.ts
- packages/2-sql/4-lanes/relational-core/src/exports/ast.ts
- packages/2-sql/2-authoring/contract-ts/src/contract-lowering.ts
- packages/2-sql/4-lanes/sql-builder/src/runtime/builder-base.ts
- packages/2-sql/5-runtime/src/codecs/validation.ts
- packages/1-framework/3-tooling/emitter/src/domain-type-generation.ts
- packages/2-mongo-family/2-authoring/contract-psl/test/derive-json-schema.test.ts
- packages/2-sql/4-lanes/relational-core/src/expression.ts
- packages/1-framework/2-authoring/contract/test/descriptors.test.ts
- packages/1-framework/1-core/framework-components/src/shared/framework-components.ts
- packages/2-sql/4-lanes/sql-builder/src/runtime/mutation-impl.ts
- packages/1-framework/1-core/framework-components/src/shared/codec.ts
- packages/2-sql/4-lanes/relational-core/test/ast/refs-propagation.test.ts
- packages/2-sql/4-lanes/relational-core/test/ast/codec-factory.types.test-d.ts
- packages/2-sql/4-lanes/relational-core/src/ast/validate-param-refs.ts
- packages/1-framework/1-core/framework-components/test/codec-types.types.test-d.ts
- packages/1-framework/3-tooling/cli/test/control-api/contract-enrichment.test.ts
- packages/2-sql/4-lanes/sql-builder/src/runtime/expression-impl.ts
- packages/2-sql/4-lanes/relational-core/test/ast/codec-factory-ctx.types.test-d.ts
- packages/2-sql/4-lanes/relational-core/src/codec-descriptor-registry.ts
- packages/2-sql/4-lanes/relational-core/test/typed-codec-flow.test-d.ts
- packages/2-sql/2-authoring/contract-ts/test/contract-builder.contract-definition.test.ts
- packages/2-sql/4-lanes/relational-core/src/ast/sql-codecs.ts
- packages/2-sql/4-lanes/relational-core/test/expression.test.ts
- packages/2-sql/4-lanes/relational-core/test/ast/sql-codecs.test.ts
- packages/1-framework/1-core/framework-components/src/shared/column-spec.ts
- packages/2-sql/2-authoring/contract-psl/test/fixtures.ts
- packages/2-sql/5-runtime/src/codecs/encoding.ts
- packages/1-framework/3-tooling/cli/src/control-api/contract-enrichment.ts
- packages/2-sql/4-lanes/sql-builder/test/runtime/functions.test.ts
- packages/2-sql/4-lanes/relational-core/test/ast/sql-codecs.types.test-d.ts
- packages/2-sql/4-lanes/relational-core/test/ast/sql-codec-helpers.test.ts
- packages/1-framework/1-core/framework-components/test/codec.types.test-d.ts
- packages/2-sql/4-lanes/relational-core/src/ast/types.ts
- packages/2-mongo-family/1-foundation/mongo-codec/README.md
- packages/2-sql/4-lanes/relational-core/src/ast/sql-codec-helpers.ts
- packages/2-mongo-family/2-authoring/contract-psl/test/interpreter.test.ts
- examples/prisma-next-demo/test/no-emit-typed-flow.test-d.ts
- packages/2-mongo-family/7-runtime/test/codecs/decoding.test.ts
- packages/1-framework/3-tooling/emitter/test/domain-type-generation.test.ts
- packages/2-mongo-family/1-foundation/mongo-codec/src/codec-registry.ts
- packages/1-framework/1-core/framework-components/src/control/control-stack.ts
- packages/2-mongo-family/1-foundation/mongo-codec/test/codecs.test-d.ts
- packages/2-sql/5-runtime/src/codecs/decoding.ts
- packages/2-sql/2-authoring/contract-psl/test/provider.test.ts
- packages/2-mongo-family/7-runtime/test/mongo-middleware.test.ts
- packages/2-mongo-family/2-authoring/contract-psl/test/interpreter.polymorphism.test.ts
- packages/1-framework/1-core/framework-components/test/control-stack.test.ts
- packages/2-sql/4-lanes/relational-core/test/utils.ts
- packages/2-mongo-family/1-foundation/mongo-codec/src/codecs.ts
- packages/2-sql/4-lanes/sql-builder/src/runtime/field-proxy.ts
- packages/1-framework/1-core/framework-components/src/exports/codec.ts
- packages/1-framework/1-core/framework-components/src/shared/codec-descriptor.ts
- packages/2-sql/4-lanes/relational-core/src/query-lane-context.ts
- packages/2-sql/4-lanes/relational-core/src/ast/codec-types.ts
wmadden
force-pushed
the
tml-2357-codec-registration-model-complete-the-unified
branch
2 times, most recently
from
aa0e4b3 to
70b386c
Compare
Completes the registration-side migration codec-registry-unification
(TML-2229) deferred. Branched off origin/main post-TML-2229 merge.
Six structural defects in the codec registration model retire across
four milestones:
- M1: narrow runtime Codec instance to `{ id, encode, decode, encodeJson,
decodeJson }`; route emit-path renderOutputType through
descriptorFor(codecId).
- M2: every codec ships native CodecDescriptor through unified codecs:
slot; synthesis bridge, parameterizedCodecs: slot, CodecParamsDescriptor,
aliasCodec, and arktypeJsonEmitCodec all delete; aliasDescriptor
replaces aliasCodec.
- M3: ParamRef gains refs?: { table, column }; validator-pass enforces
refs-required for parameterized codec ids; encode-side dispatch goes
through forColumn; forCodecId fallback retires for parameterized codec
ids.
- M4: JsonSchemaValidatorRegistry deletes; validation lives inline in the
resolved codec decode body (matching the arktype-json pattern parent
established); json-validator trait retires if no consumer remains.
Mongo registration migration is folded into TML-2324.
…me Codec instance
Mid-flight WIP from the M1 implementer subagent (implementer-m1-r1).
Captured at user request before subagent return.
Substantive shape:
- Narrow framework Codec interface: drop traits / targetTypes /
renderOutputType? from the instance; encode TTraits as a phantom-
symbol carrier so trait-gated downstream type machinery still works
without a runtime field.
- Extend CodecLookup with descriptor-routed accessors
(targetTypesFor, metaFor, renderOutputTypeFor) so consumer sites
reach codec-id-keyed metadata without reading off the codec
instance.
- Migrate consumer reads in framework-components/control-stack,
emitter/domain-type-generation, mongo-codec, mongo-contract-psl,
sql-contract-psl/provider, relational-core/ast/codec-types,
sql-runtime/sql-context, and postgres-adapter/sql-renderer to read
through the new accessors.
- Thread explicit { traits, targetTypes, meta } args into
synthesizeNonParameterizedDescriptor so contributor code keeps
compiling while the bridge survives until M2.
Known incomplete: workspace typecheck fails in
@prisma-next/mongo-codec on an unused-import lint (`ifDefined`); the
subagent has not yet completed its M1 R1 round. Pushing as a
checkpoint per user request; subagent return + cleanup will land in a
follow-up commit.
Picks up from WIP commit 692178c and lands the changes the prior implementer needed to reach a green validation gate. What this commit adds, on top of the WIP: - mongo-codec: drop the unused `ifDefined` import-then-unused regression, re-shape `MongoCodec` to extend the narrow framework `Codec` while keeping `traits` / `targetTypes` / `renderOutputType` as optional transitional fields (per the M1 plan, these survive on family-specific Codec extensions until M2). Re-thread the runtime factory so `mongoCodec()` still emits `targetTypes` / `traits` / `renderOutputType` on the resolved instance for downstream consumers. - relational-core/ast/codec-types: keep SQL `Codec` extending the narrow framework `Codec` and re-declare `traits` / `targetTypes` / `meta` / `paramsSchema` / `init` / `renderOutputType` as optional transitional fields for M1; `targetTypes` becomes optional throughout. Defensively default `codec.targetTypes ?? []` in `CodecRegistryImpl.register`. - sql-runtime/sql-context: pass `targetTypes ?? []` into the synthesis bridge so a codec without that field still lifts to a descriptor. - arktype-json: type the emit-only shim against the SQL `Codec` (which still carries the transitional metadata fields) rather than the narrowed framework `Codec`. - adapter-postgres/codec-lookup: implement the new `CodecLookup` accessors (`targetTypesFor`, `metaFor`, `renderOutputTypeFor`) on top of the built-in postgres codec map. Test mocks updated to fully implement the widened `CodecLookup` shape: emitter, sql-renderer cast-policy, mongo / sql contract-psl, contract-ts, cli config + contract-enrichment, and integration tests (`ts-psl-parity.real-packs`, `migration-psl-authoring`). Where tests read `codec.targetTypes` directly, they now read through `codecLookup.targetTypesFor(codecId)`. No production reads of the removed framework-`Codec` fields remain: the only direct `codec.traits` / `codec.targetTypes` / `codec.meta` reads under packages/ are the synthesis bridge call site (sql-context.ts) and the SQL `CodecRegistryImpl.register` loop, both operating on SQL `Codec` whose interface still carries them in M1. Validation: `pnpm typecheck`, `pnpm lint:deps`, `pnpm test:packages`, `pnpm test:e2e`, `pnpm build`, and demo emit byte-identical against `origin/main` all pass. Refs: TML-2357
…nts (D1, D2) - Spec AC-3 § 2: split family-extension narrow across M1 (framework) / M2 (family extensions, alongside synthesis-bridge deletion). The two-stage ordering is intentional and was implicit in the plan; the spec now states it explicitly so future readers and the AC scoreboard agree on the milestone boundary. - Plan T1.4: tighten the emit-path file list to the single production site on origin/main (domain-type-generation.ts). The original enumeration of emit.ts / emit-types.ts / generate-contract-dts.ts was aspirational; reviewer grep on origin/main confirmed only one real read site. Sourced from m1 R1 reviewer report § Items for the user's attention (D1, D2). Refs TML-2357.
Drops two unnecessary `as unknown as ...` casts in `adapter-postgres/codec-lookup.ts` and tightens the existing comment block on the genuinely-transitional cast in `framework-components/control-stack.ts` to name F1 explicitly. Site (1) — `definition.codec as unknown as SqlCodec` (line 22): the `defineCodecs` builder already types `definition.codec` as the SQL `Codec` extension, so the cast was a no-op fossil from R1 plumbing. Drop it; `byId.set(definition.codec.id, definition.codec)` typechecks directly. Site (2) — `byId.get(id) as Codec | undefined` (line 25): `SqlCodec` extends the framework `Codec`, so the lookup return type widens without an explicit cast. Drop it; `get: (id) => byId.get(id)` typechecks under the `CodecLookup` interface contract directly. Site (3) — `legacyMeta = codec as unknown as LegacyCodecInstanceMeta` in `extractCodecLookup` (line 377): genuinely transitional. The framework `Codec` interface narrowed in M1 but family-`Codec` extensions still carry `traits` / `targetTypes` / `meta` / `renderOutputType` as optional transitional fields, and the assembled `CodecLookup` has to surface them to the new descriptor-routed accessors until every contributor ships a native `CodecDescriptor`. Comment block now names this disposition explicitly so the next reader knows the cast is documented M2-bundled cleanup, not a forgotten loose end. Validation: `pnpm typecheck`, `pnpm lint:deps`, `pnpm test:packages`, `pnpm test:e2e`, `pnpm build`, demo emit byte-identical against `origin/main` all pass. Refs: TML-2357
Composes a derived CodecDescriptor by overlaying codecId, targetTypes, and optional meta on a base descriptor. The alias factory delegates to the base factory and rewrites `id` on the resolved codec. Replaces the legacy `aliasCodec` helper that composed at the codec- instance level. Lives in framework-components so SQL families can share it without a target-specific dep. Refs TML-2357 T2.1.
…() parallel APIs
Introduces the descriptor-shaped factory + builder in
sql-relational-core that the per-package M2 migrations will adopt,
without disturbing the legacy codec()/defineCodecs() that 28+ existing
consumers (production + tests) still depend on.
The plan T2.2 deferred the absorb-vs-parallel choice to implementation
start, biased to absorb when "structurally clean". Implementation-time
recon found absorb is not clean: 28+ direct codec() consumers would
break simultaneously and 15+ test files read transitional fields off
the resolved Codec instance. Per the plan's fallback ("only introduce
a parallel defineCodecDescriptors if defineCodecs doesn't admit a
clean shape"), this is the parallel-API path.
Setup-commit only: every existing consumer compiles and tests pass.
Per-package migrations land in subsequent commits; the M2 cleanup
commit deletes both the legacy and parallel APIs once the migration
chain finishes.
…nal-core base codecs Each of the six SQL base codecs (char, varchar, int, float, text, timestamp) gains a sibling `*Descriptor` export authored natively via `codecDescriptor()`. The descriptor builder is wired through `defineCodecDescriptors()` and exposed as `sqlCodecDescriptorDefinitions` and `sqlCodecDescriptorList` (mirroring the legacy `sqlCodecDefinitions`). Encode/decode/render functions are extracted to module-level constants so the legacy codec form and the descriptor form share a single source of runtime behaviour during the migration window. The legacy codec exports stay so postgres + sqlite + sql-codecs.test.ts keep compiling; they migrate to the descriptor exports in subsequent per-package commits and the legacy form deletes in the M2 cleanup. Adds an `AnyCodecDescriptor` variance-erasure alias to `codec-types.ts` so the descriptor builder can collect heterogeneous descriptors (the descriptor `factory` and `renderOutputType` slots are contravariant in `P`, so heterogeneous storage cannot use `CodecDescriptor<unknown>`).
…rget codecs Each of the 22 postgres codecs gains a sibling `*Descriptor` definition authored via `codecDescriptor()` (or composed from a base SQL descriptor via `aliasDescriptor`). The descriptor builder is wired through `defineCodecDescriptors()` and exposed as `codecDescriptorDefinitions` and `codecDescriptorList`, mirroring the existing `codecDefinitions` / `dataTypes` shape. The legacy `Codec` instances + `aliasCodec` helper stay so the postgres adapter (codec-lookup.ts, runtime.ts, descriptor-meta.ts, adapter.ts) and the postgres test suite keep reading codec instances out of `codecDefinitions[k].codec` until the unified `codecs:` slot reshape (later in M2). Both shapes retire to descriptor-only in the M2 cleanup commit.
…et codecs Each of the seven sqlite codecs (text, integer, real, blob, datetime, json, bigint) gains a sibling `*Descriptor` authored via `codecDescriptor()`. The descriptor builder is wired through `defineCodecDescriptors()` and exposed as `codecDescriptorDefinitions` and `codecDescriptorList`, mirroring the existing `codecDefinitions` / `dataTypes` shape (and reusing the shared SQL base descriptors — char, varchar, int, float — by reference). The legacy `Codec` exports stay so the sqlite adapter (codec-lookup, runtime-adapter, adapter) and the sqlite test suite keep reading codec instances out of `codecDefinitions[k].codec` until the unified `codecs:` slot reshape later in M2; both shapes retire to descriptor- only in the M2 cleanup commit.
The vector codec gains a native `pgVectorDescriptor` authored via `codecDescriptor()` in core/codecs.ts (with `paramsSchema` and `renderOutputType` consolidated into the descriptor). The runtime extension drops its hand-rolled descriptor literal and references `pgVectorDescriptor` directly; the legacy `parameterizedCodecs:` slot still echoes the descriptor through the `RuntimeParameterizedCodecDescriptor<P>` alias for the contributor protocol. Both the legacy `codecDefinitions[k].codec` surface and the new `codecDescriptorList` ship through this commit; the M2 cleanup commit collapses the parameterized + non-parameterized slots into the unified `codecs:` slot.
…egistry The previous register(codec) method on the SQL CodecRegistry interface was used in a strict build-then-read pattern: every consumer assembled a fresh registry once and then read from it. The mutation surface was exposed externally but never used to overwrite or alter a live registry post-read. Collapse the surface to a single-shot builder: - Add buildCodecRegistry(codecs: readonly Codec<string>[]): CodecRegistry that returns a read-only registry built once from the input array. - Drop register(codec) from the CodecRegistry interface; drop the newCodecRegistry() factory (no remaining consumers). - Migrate the production SQL execution-context construction in sql-context.ts and the postgres / sqlite adapter codec-registry closures to assemble a Codec[] up-front and call buildCodecRegistry once. - Migrate the ~25 test fixtures (sql-runtime, relational-core, sql-orm-client, integration) to the same up-front-array + builder pattern. The pattern was mechanical — repeated registry.register(codec) calls collapse into a single array literal passed to the builder. No behavioural change: duplicates still raise on construction; the returned registry exposes the same get/has/values/iterator surface.
…criptorRegistry After Phase 1 narrowed CodecRegistry to a build-then-read map of resolved Codec instances, audit every CodecRegistry.get(id) consumer to determine whether the resolved-instance contract is still required: - ExecutionContext.codecs (sql-runtime): purely a relay to encode/decode call sites. Migrate-able (a) — replace with direct ContractCodecRegistry lookup at the encode/decode call site. - encodeParam / decodeRow registry parameter (sql-runtime): hot-path dispatch. Migrate-able (a) — pre-populate ContractCodecRegistry.byCodecId with one canonical instance per non-parameterized descriptor at registry build time, so forCodecId(id) covers refs-less call sites without an external registry parameter. - AdapterProfile.codecs (lane core): test instrumentation only — no production consumers post Phase 1. Replaced by a test-only StubAdapter shape that exposes the codec array directly. No surviving (c) consumers. Retire the CodecRegistry interface and the buildCodecRegistry factory entirely: - Drop CodecRegistry / buildCodecRegistry from @prisma-next/sql-relational-core/ast. - Drop AdapterProfile.codecs() (test-only surface). - Drop ExecutionContext.codecs (lane-core query-lane-context); call sites read context.contractCodecs / context.codecDescriptors instead. - Drop the registry parameter from encodeParam / encodeParams / decodeRow / buildDecodeContext / resolveProjectionCodec / resolveParamCodec. - buildContractCodecRegistry now pre-populates byCodecId with non- parameterized canonical instances and a parameterizedRepresentatives fallback for parameterized descriptors whose factory tolerates factory(undefined). Refs-less forCodecId(id) lookups read both maps; parameterized descriptors that need real params (arktype-json, pgvector) fall through to column-aware dispatch via forColumn(table, column). - Postgres / SQLite adapters drop their internal codec-registry closures. - sql-orm-client model-accessor and sql-family-adapter tests drop the legacy codecs lambda from their test contexts. - Cross-family integration test exercises the codec directly rather than going through the SQL CodecRegistry. Migrate ~25 test fixtures across sql-runtime / sql-orm-client / relational-core / integration to: - Pass the codec array directly (replaces buildCodecRegistry([...]) calls). - Use buildTestContractCodecs(codecs) to construct a ContractCodecRegistry for tests that exercise encode/decode paths. - Use descriptorsFromCodecs(codecs) to build a CodecDescriptorRegistry. The descriptor-parity test in adapter-postgres becomes obsolete (it asserted parity between AdapterProfile.codecs() and the descriptor list, which no longer exists) and is deleted. Net effect: a single codec registry — CodecDescriptorRegistry — survives. The runtime materializes resolved Codec instances on demand through ContractCodecRegistry, which combines column-aware dispatch (forColumn) with descriptor-derived canonical instances (forCodecId). Cross-package gates pass: - grep "registry\.register" packages/ — no SQL CodecRegistry hits remain (only unrelated mongo / operations / pgvector op registries). - grep "interface CodecRegistry" packages/ — 0 hits in SQL family. - grep "buildCodecRegistry|newCodecRegistry" packages/ — 0 hits.
…rebase - psl-column-resolution.ts: ColumnDescriptor typeParams accepts undefined to match framework-components ColumnTypeDescriptor under exactOptionalPropertyTypes. - intercept-decoding / marker-vs-intercept-ordering tests: replace retired createCodecRegistry/codec/parameterizedCodecs surface with defineTestCodec + descriptorsFromCodecs (the post-M2 codec registration shape). - sql-context tests: drop parameterizedCodecs and createCodecRegistry residue from new mutation-default-generator stability tests. - target-postgres codecs.test.ts: route the new bytea cases through codecForScalar like its siblings (codecDefinitions never existed on this branch).
Codec-authored RUNTIME.DECODE_FAILED and RUNTIME.ABORTED were being double-wrapped (or coerced) by decodeFields catch. Rethrow runtime envelopes unchanged; only wrap foreign errors. Closes CodeRabbit thread on decoding.ts:263.
…hrough The pre-existing codec-async test asserted the old (regression) behaviour where decodeField double-wrapped a codec-authored RUNTIME.JSON_SCHEMA_VALIDATION_FAILED into RUNTIME.DECODE_FAILED. With F40 in place, codec-authored runtime envelopes pass through unchanged, so the assertion now pins the correct surfaced code + details directly.
descriptor.paramsSchema !== voidParamsSchema is brittle — any descriptor that doesn't reuse the exact singleton gets misrouted. Use the descriptor.isParameterized getter (F12) as the canonical discriminator. Closes CodeRabbit thread on sql-context.ts:282.
The byCodecId pre-populated representative was being reused for every
forColumn(table, column) entry, so non-parameterized descriptors saw
the shared context instead of the real column site even on
column-aware dispatch. Keep the representative as the forCodecId
fallback only; materialise fresh instances for forColumn entries with
the column-bound SqlCodecInstanceContext (name: <col:table.column>,
usedAt: [{ table, column }]).
Closes CodeRabbit thread on sql-context.ts:427 (also applies to 492-512).
Vector<0> and Vector<-1> previously rendered through the guard. Tighten to require length > 0. Fix is local to mongo adapter renderOutputType; parallel guards (e.g. pgvector) are out of scope for this thread. Closes CodeRabbit thread on mongo-adapter codecs.ts:124.
…ork-components README codec sketch F44: ADR 208 future-work bullet referenced the retired `parameterized-codecs slot` vocabulary, contradicting the same ADR a few lines up; rephrase to "unified `codecs:` registration surface". F45: framework-components README codec-call-context sketch had two errors: (a) line 34 incorrectly invoked TypeScript bivariance to justify sync method bodies satisfying `Promise<T>` return types — bivariance is parameter-position only; reframe as "mark the method `async`, or return `Promise.resolve(...)` explicitly"; (b) the `encode` sketch used property syntax + optional `ctx?.signal` inconsistent with the `CodecImpl` class story — rewrite as a real class method with required `ctx: CodecCallContext`. Closes CodeRabbit threads on ADR 208:259 and framework-components README:34.
…factory param CI lint flagged `_params: void` in a parameter position. `void` outside a return type is confusing per biome (lint/suspicious/noConfusingVoidType); switch the F42 test fixture to `undefined`. Behaviour unchanged — the factory body never reads the param, and the cast through unknown still pins it to `CodecDescriptor<void>["factory"]`.
Deleted the standalone phantom-property block between imports and the Codec interface (redundant with __codecTraits inline doc; merged the string-key-vs-symbol rationale into that line). Reflowed framework codec surface JSDoc (codec.ts, codec-descriptor.ts, column-spec.ts, codec-types.ts, exports/codec.ts) so paragraphs are not manually hard-wrapped mid-sentence. Added `.cursor/rules/jsdoc-no-artificial-line-wraps.mdc` (force-added; `.cursor/rules` is gitignored) and indexed it in `.cursor/rules/README.md`.
…s/rules Canonical rulecard lives under `.agents/rules/` for agent discovery; removed duplicate from `.cursor/rules/`. Updated `.cursor/rules/README.md` index entry.
…links - Rewrap codec-cluster retrospective blockquotes as single-line paragraphs; replace transient codenames with class-based CodecImpl/CodecDescriptorImpl wording; drop ticket IDs from note titles. - ADR 208: rename status heading; remove Pattern E / ticket bullets; Mongo follow-up without Linear URL. - ADR 204 async: framework-gaps G9/G4 instead of tracker URLs; prose-only Mongo decode sentence. - ADR 207: framework-gaps G4/G1 instead of tracker URLs. - ADR 180 implementation note: drop Linear URL (same paragraph reflow). - docs/README + codec-authoring guide: align wording with stable terminology. - Extend `.agents/rules/jsdoc-no-artificial-line-wraps.mdc` with ADR guidance.
…olumns
The runtime sql-context materializes a representative codec for parameterized descriptors via factory(undefined)(ctx), used as the encode/decode fallback for columns whose codec id is parameterized but whose contract entry has no typeParams (e.g. the undimensioned vectorColumn variant). PgVectorDescriptor.factory threw when params was undefined because it eagerly read params.length, leaving forCodecId('pg/vector@1') with no representative. As a result, vector arrays passed through unencoded and node-postgres formatted them as PG array literals ({"'0.1'","'0.2'"}), which Postgres rejected with "invalid input syntax for type vector".
Make PgVectorCodec accept length: number | undefined and skip the dimension check when length is undefined; tolerate factory(undefined) by reading the optional length defensively. Typed call sites (pgVectorColumn(N), vector(N)) keep their strict signature so length is still required at column construction.
Adds a regression test that asserts factory(undefined) yields a length-agnostic codec encoding/decoding the wire format correctly.
Coverage gaps reported by CI fell into two clusters:
- New / heavily rewritten files on this branch had no targeted tests yet:
- relational-core codec-descriptor-registry (0% — fully untested)
- relational-core sql-codecs encodeJson/decodeJson + column helpers
- relational-core validate-param-refs unknown-codec-id branch
- pgvector codecs paramsSchema narrow, NaN decode, encodeJson/decodeJson, pgVectorColumn helper
- arktype-json column-author and renderOutputType edge branches
- mongo-adapter descriptor.factory invocation, defensive introspect parsing,
and extractDb error path
- A handful of unmodified files dropped just below threshold; covered the
reachable branches with targeted tests rather than relaxing thresholds:
- sql-orm-client query-plan-meta missing-profileHash branch
- sql-orm-client query-plan-mutations: unknown table/column errors,
UPDATE/DELETE WHERE preservation, split helpers empty-rows guard
Defensive paths in arktype-json rehydration that arktype itself cannot reach
(non-Error throws and post-rehydrate non-callable returns) are marked with
`c8 ignore` rather than synthesised through mocks.
Verified with node scripts/coverage-report.mjs (61 packages, 0 failures).
…ynamic import The contributes-parameterized-codec-descriptors test uses await import() on src/exports/runtime, triggering vitests first-pass transform of the runtime module graph. On cold CI workers this can exceed the package default of 200ms (timeouts.default x multiplier). Apply timeouts.coldTransformImport to the test directly, matching the pattern used elsewhere in the repo for lazy-import suites.
…ode dispatch
Three related runtime issues surfaced together when the demo exercised
parameterized codecs through the ORM (cosineDistance/lt) and via SQL
DSL self-joins:
1. forColumn dispatch trusted refs blindly. The ORM lifts the column
ref out of an OperationExpr that changed the codec id (e.g.
`cosineDistance(p.embedding, x).lt(1)` carries refs={post,embedding}
but a `pg/float8@1` comparison codec). forColumn returned the
vector codec, which then rejected the float literal.
resolveParamCodec / resolveProjectionCodec now require byColumn.id
to agree with the declared codec id; a mismatch falls through to
forCodecId so the float literal dispatches as float8.
2. forColumn missed for aliased self-joins (`db.sql.post.as(p1)
.innerJoin(db.sql.post.as(p2), …)` projects p1.id, p2.id). byColumn
is keyed by source table; the alias never matched, fell through to
forCodecId(sql/char@1), and threw "ambiguous parameterized
instances".
Builds an alias→source table map from the query AST and resolves
refs through it before forColumn dispatch (decode and encode share
the helper in alias-resolver.ts).
3. F42 detached factory from its descriptor: `voidFactory(undefined)
(ctx)` lost the `this`-on-descriptor binding, so descriptors whose
factory body captures `this` (e.g. PgTextDescriptor: `() => new
PgTextCodec(this)`) produced codecs with `descriptor: undefined`.
Reading `codec.id` then threw. The bug was latent until #1 above
started inspecting `byColumn.id`.
Both factory call sites now use `factory.bind(descriptor)`.
Updates the demo offline-runtime test expectation: ProjectionItem refs
are now populated for top-level identifier projections (a side effect
of the existing select-shortcut path that the test was written before).
The alias resolver was casting `{ kind: string }` to per-branch shapes
(`{ name, alias? }` and `{ alias }`) inside the switch, which TS rejects
under `noImplicitAny`/strict structural checks once the surrounding code
matures. Type the helper directly as `AnyFromSource` so each branch
discriminates safely without unsafe casts.
Adds a unit test covering the resolver across SELECT, self-join,
derived table, and DELETE shapes.
…d runtime CI per-file coverage thresholds tripped on: - src/sql-runtime.ts (functions 83% < 92%) - src/middleware/budgets.ts (statements 88% < 90%) - src/middleware/lints.ts (statements 64% < 90%, branches 51% < 80%) Tightens budgets.ts so primaryTableFromAst/estimateRowsFromAst return non-nullable types, removes a dead `if (this.verified) return` defensive check inside verifyMarker, and folds the default no-op log into a single shared sink. Adds tests covering: - alias resolver -> via the new test file in the previous commit. - lints severity overrides for every rule code, raw guardrail fallback, and skip-fallback configuration. - budgets unbounded SELECT with estimate below maxRows, derived-table source rows lookup. - verifyMarker MARKER_MISSING, marker-passes, MARKER_MISMATCH on profile hash, and startup-mode caching. - middleware ctx surface (now() and contentHash()) on a runtime using the default no-op log.
Removes references to in-flight code review issue IDs (F##, M0 R#, AC-CB-#, Phase A/B markers) and strips malformed `// ---...---` decorative comment lines from comments and test descriptions across the codec stack. The surrounding technical content is preserved; only the transient pointers into project review artifacts are dropped.
wmadden
force-pushed
the
tml-2357-codec-registration-model-complete-the-unified
branch
from
ebcadb5 to
60d75b8
Compare
Replaces `.agents/rules/jsdoc-no-artificial-line-wraps.mdc` with `.agents/rules/jsdoc-line-width.mdc`. Doc-comment prose now wraps at the same `lineWidth` the biome formatter uses (currently 100, read from `biome.jsonc`) instead of being kept as one continuous line. Markdown is explicitly exempt and continues to defer to the markdown-no-artificial-line-wraps skill.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/architecture docs/adrs/ADR 208 - Higher-order codecs for parameterized types.md (1)
33-56:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd
isParameterizedto the descriptor snippet.The
CodecDescriptor/CodecDescriptorImplexample no longer matches the implementation:isParameterizedis part of the real surface, and later prose in this ADR relies on it. Omitting it here makes the decision block self-inconsistent for readers copying the shape.As per coding guidelines, "Keep docs, READMEs, and rules up-to-date during code changes."
🤖 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 `@docs/architecture` docs/adrs/ADR 208 - Higher-order codecs for parameterized types.md around lines 33 - 56, The snippet for the descriptor types is missing the isParameterized property; update both the consumer interface CodecDescriptor<P> and the author abstract class CodecDescriptorImpl<P> to include a readonly isParameterized: boolean property (same name and type in both places) so the example matches the real surface and the later prose that uses isParameterized.
🧹 Nitpick comments (1)
docs/architecture docs/adrs/ADR 208 - Higher-order codecs for parameterized types.md (1)
243-252: ⚡ Quick winMove the migration close-out inventory out of the ADR.
This section reads like rollout status and deleted-symbol bookkeeping rather than a durable architectural decision, so it will drift as follow-ups land. I'd keep the ADR on the steady-state shape and move the completion inventory to subsystem docs or project tracking.
Based on learnings, "ADRs in prisma/prisma-next should document architectural design decisions and the key constraints/assumptions only. Do not include implementation algorithms, emitter derivation logic, or step-by-step derivation/migration rules."
🤖 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 `@docs/architecture` docs/adrs/ADR 208 - Higher-order codecs for parameterized types.md around lines 243 - 252, Remove the migration close-out inventory paragraph headed "Status — Unified descriptor authoring" (and the detailed bullet list referencing defineCodec, mkCodec, defineCodecGroup, synthesizeNonParameterizedDescriptor, CodecParamsDescriptor, ParamRef.refs, forColumn, forCodecId, arktypeJsonEmitCodec, JsonSchemaValidatorRegistry, etc.) from the ADR and relocate that rollout/deleted-symbol bookkeeping into a separate subsystem migration or project-tracking document; edit the ADR so it instead contains a short, steady-state summary of the final shape (class-based authoring, single codecs: slot, narrow runtime Codec interface, ParamRef.refs behavior, and JSON-schema validation in decode) without the step-by-step migration status or per-symbol deletion notes.
🤖 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 @.agents/rules/jsdoc-line-width.mdc:
- Line 20: Update the "5. **Markdown is exempt** — Do not apply this rule to
`.md` files..." sentence to remove ambiguity and fix the malformed path token;
specifically, change the phrasing in the rule header text (the line starting
with "5. **Markdown is exempt**") to explicitly list the Markdown file types
(for example: ".md files such as README.md, ADRs, PR bodies, and rulecards") and
correct the path token to two distinct directories (e.g., "ADR callouts and
narrative Markdown under docs/architecture/ and docs/adrs/ follow the Markdown
rule, not this one") so there is no confusion about scope or tokenization.
In
`@packages/1-framework/1-core/framework-components/src/shared/codec-descriptor.ts`:
- Around line 73-76: The isParameterized getter currently uses object identity
(this.paramsSchema !== voidParamsSchema) which misclassifies equivalent schemas;
update the CodecDescriptor surface to use an explicit marker/property instead:
add a boolean field (e.g., isParameterizedFlag) to the descriptor and set it
when the descriptor is constructed, change get isParameterized() to return that
flag (reference symbols: isParameterized, paramsSchema, voidParamsSchema,
CodecDescriptor constructor), and update any creation sites so descriptors that
represent "no params" set the flag false; ensure codepaths that read it
(descriptorFor(codecId).isParameterized, forColumn, validateParamRefRefs) use
the new property.
---
Outside diff comments:
In `@docs/architecture` docs/adrs/ADR 208 - Higher-order codecs for parameterized
types.md:
- Around line 33-56: The snippet for the descriptor types is missing the
isParameterized property; update both the consumer interface CodecDescriptor<P>
and the author abstract class CodecDescriptorImpl<P> to include a readonly
isParameterized: boolean property (same name and type in both places) so the
example matches the real surface and the later prose that uses isParameterized.
---
Nitpick comments:
In `@docs/architecture` docs/adrs/ADR 208 - Higher-order codecs for parameterized
types.md:
- Around line 243-252: Remove the migration close-out inventory paragraph headed
"Status — Unified descriptor authoring" (and the detailed bullet list
referencing defineCodec, mkCodec, defineCodecGroup,
synthesizeNonParameterizedDescriptor, CodecParamsDescriptor, ParamRef.refs,
forColumn, forCodecId, arktypeJsonEmitCodec, JsonSchemaValidatorRegistry, etc.)
from the ADR and relocate that rollout/deleted-symbol bookkeeping into a
separate subsystem migration or project-tracking document; edit the ADR so it
instead contains a short, steady-state summary of the final shape (class-based
authoring, single codecs: slot, narrow runtime Codec interface, ParamRef.refs
behavior, and JSON-schema validation in decode) without the step-by-step
migration status or per-symbol deletion notes.
🪄 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: 959fe392-d270-44a6-a14c-064039a9fa1b
📒 Files selected for processing (67)
.agents/rules/jsdoc-line-width.mdc.cursor/rules/README.md.vscode/settings.jsondocs/README.mddocs/Testing Guide.mddocs/architecture docs/adrs/ADR 152 - Execution Plane Descriptors and Instances.mddocs/architecture docs/adrs/ADR 180 - Dot-path field accessor.mddocs/architecture docs/adrs/ADR 184 - Codec-owned value serialization.mddocs/architecture docs/adrs/ADR 186 - Codec-dispatched type rendering.mddocs/architecture docs/adrs/ADR 202 - Codec trait system.mddocs/architecture docs/adrs/ADR 204 - Single-Path Async Codec Runtime.mddocs/architecture docs/adrs/ADR 204 - Single-tier runtime.mddocs/architecture docs/adrs/ADR 205 - SQL cast emission is adapter policy.mddocs/architecture docs/adrs/ADR 207 - Codec call context per-query AbortSignal and column metadata.mddocs/architecture docs/adrs/ADR 208 - Higher-order codecs for parameterized types.mddocs/architecture docs/adrs/ADR 209 - Mongo result-shape as a structural plan field.mddocs/architecture docs/subsystems/2. Contract Emitter & Types.mddocs/reference/codec-authoring-guide.mddocs/reference/community-generator-migration-analysis.mddocs/reference/framework-gaps.mddocs/reference/typescript-patterns.mdexamples/prisma-next-demo/test/no-emit-typed-flow.test-d.tsexamples/prisma-next-demo/test/runtime.offline.integration.test.tspackages/1-framework/0-foundation/contract/package.jsonpackages/1-framework/1-core/framework-components/README.mdpackages/1-framework/1-core/framework-components/src/control/control-stack.tspackages/1-framework/1-core/framework-components/src/exports/codec.tspackages/1-framework/1-core/framework-components/src/shared/codec-descriptor.tspackages/1-framework/1-core/framework-components/src/shared/codec-types.tspackages/1-framework/1-core/framework-components/src/shared/codec.tspackages/1-framework/1-core/framework-components/src/shared/column-spec.tspackages/1-framework/1-core/framework-components/src/shared/framework-components.tspackages/1-framework/1-core/framework-components/test/codec-call-context.types.test-d.tspackages/1-framework/1-core/framework-components/test/codec-types.types.test-d.tspackages/1-framework/1-core/framework-components/test/codec.test.tspackages/1-framework/1-core/framework-components/test/codec.types.test-d.tspackages/1-framework/1-core/framework-components/test/control-stack.test.tspackages/1-framework/2-authoring/contract/README.mdpackages/1-framework/2-authoring/contract/src/descriptors.tspackages/1-framework/2-authoring/contract/src/index.tspackages/1-framework/2-authoring/contract/test/descriptors.test.tspackages/1-framework/2-authoring/ids/package.jsonpackages/1-framework/2-authoring/ids/src/index.tspackages/1-framework/3-tooling/cli/src/control-api/contract-enrichment.tspackages/1-framework/3-tooling/cli/test/config-types.test.tspackages/1-framework/3-tooling/cli/test/control-api/contract-enrichment.test.tspackages/1-framework/3-tooling/emitter/src/domain-type-generation.tspackages/1-framework/3-tooling/emitter/test/domain-type-generation.test.tspackages/2-mongo-family/1-foundation/mongo-codec/README.mdpackages/2-mongo-family/1-foundation/mongo-codec/src/codec-registry.tspackages/2-mongo-family/1-foundation/mongo-codec/src/codecs.tspackages/2-mongo-family/1-foundation/mongo-codec/src/exports/index.tspackages/2-mongo-family/1-foundation/mongo-codec/test/codecs-ctx.test-d.tspackages/2-mongo-family/1-foundation/mongo-codec/test/codecs-ctx.test.tspackages/2-mongo-family/1-foundation/mongo-codec/test/codecs.test-d.tspackages/2-mongo-family/1-foundation/mongo-codec/test/codecs.test.tspackages/2-mongo-family/2-authoring/contract-psl/src/derive-json-schema.tspackages/2-mongo-family/2-authoring/contract-psl/test/derive-json-schema.test.tspackages/2-mongo-family/2-authoring/contract-psl/test/interpreter.polymorphism.test.tspackages/2-mongo-family/2-authoring/contract-psl/test/interpreter.test.tspackages/2-mongo-family/7-runtime/README.mdpackages/2-mongo-family/7-runtime/src/mongo-execution-stack.tspackages/2-mongo-family/7-runtime/test/codecs/decoding.test.tspackages/2-mongo-family/7-runtime/test/decode.integration.test.tspackages/2-mongo-family/7-runtime/test/mongo-execution-stack.test.tspackages/2-mongo-family/7-runtime/test/mongo-middleware.test.tspackages/2-mongo-family/7-runtime/test/mongo-runtime-abort.test.ts
💤 Files with no reviewable changes (3)
- packages/1-framework/2-authoring/contract/src/descriptors.ts
- packages/2-mongo-family/1-foundation/mongo-codec/test/codecs-ctx.test-d.ts
- packages/2-mongo-family/1-foundation/mongo-codec/test/codecs-ctx.test.ts
✅ Files skipped from review due to trivial changes (22)
- .vscode/settings.json
- docs/architecture docs/adrs/ADR 204 - Single-tier runtime.md
- .cursor/rules/README.md
- packages/1-framework/2-authoring/contract/README.md
- docs/architecture docs/subsystems/2. Contract Emitter & Types.md
- docs/README.md
- docs/reference/framework-gaps.md
- packages/2-mongo-family/7-runtime/README.md
- packages/1-framework/1-core/framework-components/test/codec-call-context.types.test-d.ts
- packages/1-framework/2-authoring/ids/src/index.ts
- docs/architecture docs/adrs/ADR 180 - Dot-path field accessor.md
- docs/reference/community-generator-migration-analysis.md
- packages/1-framework/3-tooling/cli/test/control-api/contract-enrichment.test.ts
- docs/architecture docs/adrs/ADR 184 - Codec-owned value serialization.md
- docs/architecture docs/adrs/ADR 202 - Codec trait system.md
- docs/architecture docs/adrs/ADR 209 - Mongo result-shape as a structural plan field.md
- docs/architecture docs/adrs/ADR 186 - Codec-dispatched type rendering.md
- docs/architecture docs/adrs/ADR 205 - SQL cast emission is adapter policy.md
- docs/Testing Guide.md
- examples/prisma-next-demo/test/no-emit-typed-flow.test-d.ts
- packages/2-mongo-family/1-foundation/mongo-codec/README.md
- docs/reference/codec-authoring-guide.md
🚧 Files skipped from review as they are similar to previous changes (33)
- docs/architecture docs/adrs/ADR 152 - Execution Plane Descriptors and Instances.md
- packages/1-framework/0-foundation/contract/package.json
- packages/2-mongo-family/2-authoring/contract-psl/test/interpreter.test.ts
- packages/2-mongo-family/7-runtime/src/mongo-execution-stack.ts
- packages/2-mongo-family/7-runtime/test/mongo-middleware.test.ts
- packages/1-framework/1-core/framework-components/src/shared/codec.ts
- packages/2-mongo-family/1-foundation/mongo-codec/src/exports/index.ts
- packages/1-framework/3-tooling/emitter/src/domain-type-generation.ts
- packages/1-framework/2-authoring/contract/src/index.ts
- packages/2-mongo-family/2-authoring/contract-psl/src/derive-json-schema.ts
- packages/1-framework/2-authoring/contract/test/descriptors.test.ts
- packages/2-mongo-family/7-runtime/test/mongo-runtime-abort.test.ts
- packages/1-framework/3-tooling/cli/test/config-types.test.ts
- packages/2-mongo-family/2-authoring/contract-psl/test/interpreter.polymorphism.test.ts
- packages/1-framework/2-authoring/ids/package.json
- packages/2-mongo-family/1-foundation/mongo-codec/test/codecs.test-d.ts
- packages/1-framework/1-core/framework-components/test/codec.test.ts
- packages/2-mongo-family/7-runtime/test/mongo-execution-stack.test.ts
- packages/1-framework/1-core/framework-components/README.md
- packages/2-mongo-family/2-authoring/contract-psl/test/derive-json-schema.test.ts
- packages/2-mongo-family/1-foundation/mongo-codec/test/codecs.test.ts
- packages/1-framework/1-core/framework-components/test/codec.types.test-d.ts
- packages/1-framework/1-core/framework-components/src/exports/codec.ts
- packages/1-framework/1-core/framework-components/src/control/control-stack.ts
- packages/2-mongo-family/1-foundation/mongo-codec/src/codec-registry.ts
- packages/1-framework/1-core/framework-components/src/shared/framework-components.ts
- packages/1-framework/1-core/framework-components/src/shared/column-spec.ts
- docs/reference/typescript-patterns.md
- packages/1-framework/1-core/framework-components/test/control-stack.test.ts
- packages/1-framework/1-core/framework-components/test/codec-types.types.test-d.ts
- packages/1-framework/1-core/framework-components/src/shared/codec-types.ts
- packages/2-mongo-family/7-runtime/test/codecs/decoding.test.ts
- packages/2-mongo-family/1-foundation/mongo-codec/src/codecs.ts
Comment on lines
+73
to
+76
| /** Boolean derived from `paramsSchema`: `true` whenever the schema is not the singleton `voidParamsSchema`. The framework registry's `validateParamRefRefs` pass reads this through `descriptorFor(codecId).isParameterized` to gate column-ref enforcement. */ | ||
| get isParameterized(): boolean { | ||
| return this.paramsSchema !== voidParamsSchema; | ||
| } |
There was a problem hiding this comment.
Don't derive parameterization from schema object identity.
isParameterized currently depends on this.paramsSchema !== voidParamsSchema. That makes correct dispatch depend on every non-parameterized descriptor reusing one sentinel instance; an equivalent StandardSchemaV1<void> will be misclassified and pushed through the parameterized-only forColumn / validateParamRefRefs paths. Please make this explicit on the descriptor surface or key it off a dedicated marker instead of raw reference equality.
🤖 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/1-core/framework-components/src/shared/codec-descriptor.ts`
around lines 73 - 76, The isParameterized getter currently uses object identity
(this.paramsSchema !== voidParamsSchema) which misclassifies equivalent schemas;
update the CodecDescriptor surface to use an explicit marker/property instead:
add a boolean field (e.g., isParameterizedFlag) to the descriptor and set it
when the descriptor is constructed, change get isParameterized() to return that
flag (reference symbols: isParameterized, paramsSchema, voidParamsSchema,
CodecDescriptor constructor), and update any creation sites so descriptors that
represent "no params" set the flag false; ensure codepaths that read it
(descriptorFor(codecId).isParameterized, forColumn, validateParamRefRefs) use
the new property.
Contributor
Author
There was a problem hiding this comment.
Valid concern, out of scope — the identity-with-singleton check is intentional design ratified by ADR 208 and documented in the getter's docstring. The fix would touch the public CodecDescriptor surface and every codec-descriptor subclass in the codebase. Not minimal, not a regression introduced on this branch.
There was a problem hiding this comment.
@wmadden, understood — I'll accept this as intentional design and won't flag it again. Adding a learning to reflect this.
✏️ Learnings added
Learnt from: wmadden
Repo: prisma/prisma-next PR: 417
File: packages/1-framework/1-core/framework-components/src/shared/codec-descriptor.ts:73-76
Timestamp: 2026-05-09T16:53:21.419Z
Learning: In prisma/prisma-next, the `isParameterized` getter on `CodecDescriptorImpl` (in `packages/1-framework/1-core/framework-components/src/shared/codec-descriptor.ts`) intentionally uses object-identity comparison against the singleton `voidParamsSchema` (`this.paramsSchema !== voidParamsSchema`). This is by design, ratified by ADR 208, and documented in the getter's JSDoc. All non-parameterized descriptors are required to reuse the singleton `voidParamsSchema` instance. Do not flag this pattern as unsafe object-identity comparison or suggest replacing it with an explicit boolean marker property.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: wmadden
Repo: prisma/prisma-next PR: 38
File: packages/2-sql/3-tooling/family/src/core/migrations/plan-helpers.ts:40-47
Timestamp: 2025-12-26T23:02:40.679Z
Learning: In Prisma-next's TypeScript codebase, prefer using well-established utility libraries (e.g., Lodash) rather than building custom utilities for complex object operations such as deep freezing or deep cloning. Favor reliable, battle-tested functions and ensure dependencies are reviewed for tree-shaking and bundle size. If a lightweight utility suffices, consider modern native approaches or smaller libs.
Learnt from: wmadden
Repo: prisma/prisma-next PR: 191
File: packages/1-framework/3-tooling/cli/src/commands/contract-emit.ts:5-5
Timestamp: 2026-03-01T13:54:21.863Z
Learning: In the Prisma-next repository, prefer importing and using 'pathe' over the built-in 'node:path' module for path operations across the codebase (including CLI commands and tooling files). Apply this in TypeScript files, replacing imports from 'node:path' with 'pathe' and adjusting API usage accordingly. Ensure consistent usage across all modules and update any affected tests or tooling imports when refactoring.
Learnt from: wmadden
Repo: prisma/prisma-next PR: 234
File: packages/2-sql/4-lanes/sql-lane/test/rich-mutation.test.ts:2-2
Timestamp: 2026-03-23T10:01:15.075Z
Learning: In the prisma/prisma-next repo, prefer using `pathe` over Node’s built-in `node:path`. For any TypeScript file (including tests, test helpers, and integration tooling) replace imports like `import { dirname, join } from 'node:path'` with `import { dirname, join } from 'pathe'`—especially when code reads files/fixtures from disk.
Learnt from: wmadden
Repo: prisma/prisma-next PR: 294
File: packages/2-mongo-family/2-query/query-ast/src/read-plan.ts:4-10
Timestamp: 2026-04-04T10:08:34.220Z
Learning: In prisma/prisma-next, don’t flag declaration-emit/typecheck warnings/errors when a file uses an intentional nominal/branding pattern with a non-exported `declare const <brand>: unique symbol` as a computed property key on an exported interface (e.g., `readonly [__mongoReadPlanRow]: ...` / `readonly [aggregateResultBrand]: ...`). The `unique symbol` must remain unexported to prevent external forging, while the branded property key staying on the exported interface provides compile-time discrimination. If the build correctly emits declaration files for this pattern, treat it as intentional rather than a declaration-emit error.
Learnt from: wmadden
Repo: prisma/prisma-next PR: 293
File: packages/1-framework/1-core/shared/contract/src/canonicalization.ts:203-216
Timestamp: 2026-04-04T12:19:05.250Z
Learning: In the prisma/prisma-next repository, do not treat `Array.prototype.sort` comparators as “potentially non-deterministic” solely because the comparator does not include explicit tie-breaking keys. This is because the repo’s required runtime is Node >= 24, where `Array.prototype.sort` is stable (equal elements retain their original relative order via a stable sort such as TimSort). Therefore, equal-comparing elements should remain deterministic without additional tie-break logic.
Learnt from: wmadden
Repo: prisma/prisma-next PR: 291
File: packages/1-framework/1-core/migration/control-plane/src/emission/emit.ts:112-119
Timestamp: 2026-04-05T07:41:53.422Z
Learning: In the prisma/prisma-next codebase, the in-memory `Contract` model stores hashes only in nested locations: `contract.storage.storageHash` (and `contract.execution.executionHash`). These values are promoted to top-level fields only during canonicalization/emission via `canonicalizeContractToObject`. Therefore, runtime code that consumes in-memory `Contract` objects (e.g., sql-builder, mongo ORM, other runtime processors) must read `contract.storage.storageHash` from the nested path, not `contract.storageHash`. Do not flag runtime accesses to `contract.storage.storageHash` as requiring migration to a top-level property.
Learnt from: wmadden
Repo: prisma/prisma-next PR: 291
File: packages/1-framework/1-core/migration/control-plane/src/emission/emit.ts:112-119
Timestamp: 2026-04-05T07:49:50.627Z
Learning: In this prisma/prisma-next codebase, the in-memory runtime `Contract` model does not have top-level `storageHash` or `executionHash` fields. Hashes live only in nested properties: `contract.storage.storageHash` and `contract.execution.executionHash`, and they are only promoted to top-level fields during canonicalization/emission via `canonicalizeContractToObject` (and thus only appear in the emitted `contract.json` artifact). When reviewing runtime code that consumes in-memory `Contract` objects (e.g., sql-builder, mongo ORM, other runtime processors), require reads from the nested paths (`contract.storage.storageHash` / `contract.execution.executionHash`). Do not flag such nested accesses as needing a migration to top-level properties.
Learnt from: wmadden
Repo: prisma/prisma-next PR: 334
File: packages/2-mongo-family/9-family/src/core/control-instance.ts:204-228
Timestamp: 2026-04-13T15:54:52.337Z
Learning: When reviewing TypeScript code in this repo, do not assume a property is potentially `undefined` at a call site just because an arktype schema definition uses the DSL syntax `'key?': 'type'` (a quoted string key with a trailing `?`). This DSL notation is not the same as TypeScript optional properties (`key?: type`). To determine whether `key` is truly optional/undefined-capable, verify the actual exported TypeScript type declaration (e.g., `contract-types.ts`) and the runtime validation schema (e.g., `validate-contract.ts`) for that property; only then should the review flag call-site handling for potential `undefined`.
Learnt from: wmadden
Repo: prisma/prisma-next PR: 339
File: test/integration/test/cli.init-templates.e2e.test.ts:96-96
Timestamp: 2026-04-15T19:33:48.733Z
Learning: In prisma/prisma-next, when writing tests that run TypeScript compilation (e.g., via `tsc --noEmit`), avoid hardcoded timeout numbers (such as `30_000`). Instead, use `prisma-next/test-utils` timeout helpers—specifically `timeouts.typeScriptCompilation` (implemented in `test/utils/src/timeouts.ts`) for `tsc` compilation scenarios. Use `timeouts.spinUpPpgDev` for PostgreSQL server startup and `timeouts.databaseOperation` for database operation scenarios.
Learnt from: saevarb
Repo: prisma/prisma-next PR: 394
File: packages/1-framework/3-tooling/migration/src/graph-ops.ts:50-54
Timestamp: 2026-04-29T12:44:59.376Z
Learning: In TypeScript overload implementations, an internal “implementation-only” cast (e.g., `state as unknown as <ReturnType>`) can be safe and intentional when the public overload signatures and their structural typing prevent callers from reaching the implementation with an incompatible type. In these cases, reviewers should not flag the cast as a blind/unsafe cast, since compile-time overload enforcement substitutes for a runtime type guard. Don’t require an added `typeof`/runtime check solely to silence the cast when the overload design already prevents invalid inputs.
Learnt from: wmadden
Repo: prisma/prisma-next PR: 417
File: packages/3-extensions/sql-orm-client/src/types.ts:309-318
Timestamp: 2026-05-08T17:07:07.145Z
Learning: In prisma/prisma-next code, prefer `ifDefined` from `prisma-next/utils/defined` for conditional object spreads instead of inline ternary-based spreads. For example, when building option objects for `ParamRef.of` (or similar codec/param construction), wrap optional values with `ifDefined(...)` so that the spread only includes the property when the value is defined, improving readability and avoiding “{...cond ? {a: b} : {}}”-style patterns. Ensure you import `ifDefined` from `prisma-next/utils/defined` and use it consistently for these conditional spreads.
- jsdoc-line-width.mdc: drop the redundant ADR-callout sentence; the Markdown-exempt list already covers ADRs and PR bodies, and the embedded path with a space in `architecture docs` was visually confusing for readers. - ADR 208: add the missing `readonly isParameterized: boolean` field to both the `CodecDescriptor<P>` interface and the `CodecDescriptorImpl<P>` abstract-class snippet so the example matches the real surface in `framework-components/src/shared/codec-descriptor.ts`. - ADR 208: remove the "Status — Unified descriptor authoring" close-out inventory section. The migration rollout and per-symbol deletion notes belong in the project tracker (`projects/codec-registration-completion/spec.md`, which already carries them); the steady-state design is already described in the ADRs Decision / How it composes / Trade-offs sections.
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
3 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.
Completes the registration-side migration that the parent codec-registry-unification project (merged via ADR 208) deliberately deferred. (TML-2357)
Spec & plan
projects/codec-registration-completion/spec.md— eight ACs (AC-0..AC-7), three pinning cases.projects/codec-registration-completion/plan.md— five milestones with validation gates and risks.projects/codec-registration-completion/specs/class-based-codec-design.spec.md— six implementation-level ACs (AC-CB-1..6) covering the abstract-class hierarchy.Milestones
a210fa1c5Codecinstance + descriptor-keyed metadata reads1be7564c4; reverified through M4aliasCodec/arktypeJsonEmitCodecdeletionParamRef.refsplumbing + encode-sideforColumn+forCodecIdretirement3f0ec224aJsonSchemaValidatorRegistry; retire'json-validator'traite055c9455c4d81ad1cCodecRegistryconsolidated intoCodecDescriptorRegistry0e3aafc0bWhat landed
interface CodecDescriptor<P>+abstract class CodecDescriptorImpl<P>, paired withinterface Codec<Id, Traits, Wire, Input>+abstract class CodecImpl<...>. Per-codec column helpers (e.g.vectorColumn(N)) directly invokedescriptor.factory(params)to preserve method-level generics;satisfies ColumnHelperFor<D>ties helpers to their descriptor without polymorphism.AnyCodecDescriptor(alias forCodecDescriptor<any>) is the canonical heterogeneous-storage type.mkCodec,defineCodec,defineCodecGroup,defineCodecBundle,CodecDefBuilder*,synthesizeNonParameterizedDescriptor, instance-keyedExtractCodecTypes,byScalarmaps,dataTypesexports,sqlCodecDefinitions,codecDescriptorDefinitions,pgVectorRepresentativeCodecplaceholder,parameterizedCodecs:slot,CodecParamsDescriptor,arktypeJsonEmitCodec,aliasCodec,aliasDescriptorfunction form, the'json-validator'trait,JsonSchemaValidatorRegistryinfrastructure,arktypeParamsSchemahelper, the SQL-familyCodecRegistryinterface (consolidated intoCodecDescriptorRegistry). Closing-grep zero call-sites.ParamRef+ProjectionItemcarryrefs?: { table; column };validateParamRefRefsbuilder-pipeline pass enforces refs for parameterized codec ids; encode/decode dispatch consultsmetadata.refsfirst viacontractCodecs.forColumn(table, column).decodebody (arktype-json's inline pattern from TML-2229). Decode-error envelope equivalence verified viacodec-async.test.ts:408-456.ColumnTypeDescriptorrelocated from@prisma-next/contract-authoring(layer 2) to@prisma-next/framework-components(layer 1) so codec base types live with the framework primitives.fae3bd688):CodecTypesexposed at the publicexports/codec-types.tsentry point with aResolve<T>materializer to break tsdown's chunk-private path reference and restore consumer-side typecheck.3d57f9ecd..c4d81ad1c): each codec-shipping package now exposes only column helpers + a<package>CodecRegistryinstance +typere-exports of descriptor types. InternalcodecDescriptorMap/codecDescriptorClassListno longer surface through public exports. Transition vocabulary (Classsuffixes,class-form/class-basedin comments,codecs-class.tsfilenames) scrubbed.Post-feedback hygiene round (F11–F39)
19 commits between
c4d81ad1c..0e3aafc0bclose 27 findings surfaced from orchestrator-principal review and 30 GitHub PR review threads. Highlights:f725db1f8) — renameast/sql-codecs-class.ts→ast/sql-codecs.ts, split helpers intoast/sql-codec-helpers.ts.7d56a39b4) — move parameterization predicate ontoCodecDescriptorImpl.isParameterizedgetter; retire the standaloneIsParameterizedCodecIdcallback type.cfa078a23) — align arktype params schemas withTParamsoptionality ('length?': '...'); deletearktypeParamsSchemahelper; consumer sites direct-assign with: StandardSchemaV1<TParams>. Thearktyperuntime dep stays in@prisma-next/contractbecausevalidate-contract.tsconsumes it directly for structural validation — that's by design, not a follow-up.1051f24e8) — replaceunique symboltrait phantom with a string-key phantom property (__codecTraits) to avoid Node bundling failure modes.083fff350,a278bd034) — rewrite codec-authoring-guide alias section + fix self-referential JSDoc helper-source reference.7ee52fae0) — assert leaf scalar type inno-emit-typed-flow.test-d.tsto make AC-CB-6's literal claim explicit.81c55248f,bc4e8d716) — hardenextractCodecLookup: lift inline imports, tightenidto non-optionalstring, refine the silent catch so non-parameterized codecs throw immediately while parameterized factories that tolerate empty params still materialise representatives.f1be14d55) —createStubAdapterreturns a stable codec registry instance (no per-call rebuild).20f99bffd) — dispatch correctness:refsFromLeftwalks viacollectColumnRefsto preserve refs for wrapping expressions; encode-sideforColumnfall-through is structurally safe (F19 +ambiguousCodecIdsrejection +byCodecIdcolumn-correct materialisation); pgvectorlengththreaded intoPgVectorCodecconstructor withassertVectorvalidating the dimension at every ingress.48ed1d135) — type predicateisArktypeSchemaLikereplaces blind cast inrehydrateSchema;@ts-expect-errorreplacesas any+ biome-ignore in tests;toExtendreplaces deprecatedtoMatchTypeOfmatcher.21b4cbca7,0e3aafc0b) — retire SQL-familyCodecRegistry.register()mutation surface in favour ofbuildCodecRegistry(descriptors)builder; phase-2 deletes theCodecRegistryinterface entirely. Single registry surface in the SQL family is nowCodecDescriptorRegistry.9881a7efe) —buildCodecDescriptorRegistrythrows on duplicatecodecId.7a3faf20f) —codecDescriptorMaprelocated tocore/codec-type-map.ts;Resolve<T>materialisation kept at theexports/boundary per F8.4178072fa) — postgres render hygiene: scale validation inpgNumericRenderOutputType; ISO 8601 regex for timestamps; string validation for enum values.182d10c88) —SqliteDatetimeCodecrejectsInvalid Datein decode/decodeJson.CodecImplwould constrain its type signature and obscure where real conversion work happens. Convention stays: explicit identity overrides at codec-author site.80ba4fd60) —enumParamsSchemaandEnumParamstightened toreadonly string[].Closure-mechanism note for F22: the implementer chose a structural argument over fail-fast on
forColumnmiss — F19's refinement makesextractCodecLookupskip parameterized descriptors that don't tolerate empty params;buildContractCodecRegistry'sambiguousCodecIdsset throwsRUNTIME.TYPE_PARAMS_INVALIDon multi-instance ids; for the non-ambiguous parameterized casebyCodecIdstores the column-correct per-instance codec. Reviewer cross-checked all three legs and accepted.Notable side effect: 16 pre-existing e2e failures resolved by M3
Before M3, the e2e suite ran 75/91 with 16 failures of the form
Codec '...' resolves to multiple parameterized instances; column-aware dispatch is required.— top-level field shortcuts (select('vectorCol')) emittedIdentifierRefAST that didn't carry(table, column)context, so decode-sideresolveProjectionCodecfell back toforCodecIdand threw. M3'sProjectionItem.refsextension + decode-side parity indecoding.tsclosed the path. e2e is now 91/91.Validation gates at HEAD
0e3aafc0bpnpm typecheckpnpm lint:depspnpm fixtures:checkorigin/main)pnpm buildpnpm test:e2epnpm test:packagesReview artifacts
projects/codec-registration-completion/reviews/carries three review artifacts produced by the reviewer subagent during orchestration. After user feedback that the initial structure didn't match the canonical skill output, all three were rewritten to conform to/drive-pr-local-review(flat F-numbered findings + AC verification table) and/drive-pr-walkthrough(intent-first semantic narrative). Final reviewer verdict at HEAD0e3aafc0b: SATISFIED with 12 PASS / 2 WEAK on the AC scoreboard. The two WEAKs are AC-7 (acknowledged baseline test failures in sql-orm-client pgvector wire format + TML-2402 parallel flake) and AC-CB-5 (a single internaldescriptor as unknown as AnyDescriptorcast insidebuildCodecDescriptorRegistry— purely a registry-internal heterogeneous-storage erasure, not a public surface concern).Linear follow-ups filed during close-out
pnpm test:packagesparallel-execution flake (adapter-postgres/cli/sql-orm-client; passes cleanly in isolation). P3.pnpm build --force). P4.(table, column)lookup keys on the AST. The current shape's validator pass is a smell; instance-on-Plan would retireforColumn/forCodecIdfrom the runtime dispatch surface entirely. Architectural successor to this work. P3.Out of scope
forColumnplumbing).Codec— type name stays; only the field set narrows.CodecCallContext(ADR 207).pgEnumCodecplaceholder factory audit (already clean at HEAD; documented in ADR 208 § Future work).CodecLookup.get(id)andProjectionItem.refs/ParamRef.refslookup-key carriers from the AST — TML-2405.Note on project artifacts
Per the user's mid-flight directive at close-out,
projects/codec-registration-completion/is preserved in-tree (the standard transient-directory deletion was reverted at5b0113a5a). The directory's review artifacts underreviews/are gitignored and don't appear in the diff; they live in the working tree only as historical context.Summary by CodeRabbit
New Features
Documentation
Improvements
Tests