feat(operations): author sql operations as typescript functions#374
Merged
feat(operations): author sql operations as typescript functions#374
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (5)
📒 Files selected for processing (42)
📝 WalkthroughWalkthroughThis pull request refactors the operation specification model across the framework, moving from parameter/return metadata ( Changes
Sequence Diagram(s)sequenceDiagram
participant Author as Operation Author
participant Builder as SQL Builder
participant Expr as Expression System
participant Runtime as Runtime Engine
Author->>Builder: Define operation as TypeScript function<br/>(self: TraitExpression, arg: CodecExpression)
Builder->>Expr: Call buildOperation(spec)<br/>method, self, args, returns, lowering
Expr->>Expr: Convert args via toExpr<br/>wrap in ParamRef/ColumnRef
Expr->>Expr: Create OperationExpr AST<br/>with method, self, args, lowering
Expr->>Expr: Wrap AST in Expression<T><br/>attach returnType metadata
Expr->>Builder: Return Expression<T>
Builder->>Runtime: Use expression in query<br/>call buildAst() for AST
Runtime->>Runtime: Execute operation<br/>apply lowering, return result
Runtime->>Builder: Result with codec identity
Builder->>Author: Typed query result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 53 minutes and 54 seconds.Comment |
@prisma-next/mongo-runtime
@prisma-next/family-mongo
@prisma-next/sql-runtime
@prisma-next/family-sql
@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: |
SevInf
force-pushed
the
worktree/op-registry-ts
branch
from
95aac77 to
05ae897
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: 6
🧹 Nitpick comments (5)
packages/3-extensions/pgvector/test/operations.test.ts (1)
29-55: Prefer non-null assertion aftertoBeDefined().
cosineDistanceOpandcosineSimilarityOpare asserted defined one line above, so the optional chaining on?.impl(...)is defensive-check noise. Per the repo guideline "Prefer assertions over defensive checks when data is guaranteed to be valid", use!:Proposed fix
- const distExpr = cosineDistanceOp?.impl( + const distExpr = cosineDistanceOp!.impl( ParamRef.of([1, 2], { codecId: 'pg/vector@1' }) as never, [3, 4] as never, ) as unknown as { buildAst(): OperationExpr }; ... - const simExpr = cosineSimilarityOp?.impl( + const simExpr = cosineSimilarityOp!.impl( ParamRef.of([1, 2], { codecId: 'pg/vector@1' }) as never, [3, 4] as never, ) as unknown as { buildAst(): OperationExpr };As per coding guidelines: "Prefer assertions over defensive checks when data is guaranteed to be valid - use non-null assertions (e.g.,
columnMeta!) after validation checks instead of optional chaining".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/3-extensions/pgvector/test/operations.test.ts` around lines 29 - 55, The test currently uses optional chaining on cosineDistanceOp?.impl(...) and cosineSimilarityOp?.impl(...) immediately after expect(...).toBeDefined(); update both to use non-null assertions instead (cosineDistanceOp!.impl(...) and cosineSimilarityOp!.impl(...)) so the code reflects the prior assertion; keep the rest of the invocation and casts (ParamRef.of, as never, buildAst) unchanged and still assert the resulting distAst/simAst are OperationExpr instances and their lowering values.packages/3-extensions/sql-orm-client/test/extension-operations.test-d.ts (1)
40-62: These tests only verify that calls compile, not the parameter type.Replacing the previous
toEqualTypeOf<[number[] | null]>()assertion with plainfn(...)calls loses the type-level assertion: any widening of the argument type (e.g. accidentally becomingunknown) would still pass these tests. If the intent is to document accepted call sites, consider pinning the parameter type withexpectTypeOfalongside the exercise, e.g.:expectTypeOf<Parameters<Fn>[0]>().toExtend<number[] | null | { buildAst(): unknown }>();so regressions in the impl signature are actually caught. Otherwise these tests mostly just assert
toBeFunction().🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/3-extensions/sql-orm-client/test/extension-operations.test-d.ts` around lines 40 - 62, The tests for PostAccessor['embedding']['cosineDistance'] and ['cosineSimilarity'] currently only assert the calls compile; pin the parameter type to catch regressions by adding a type-level assertion for Parameters<Fn>[0] for each test (e.g., use expectTypeOf<Parameters<Fn>[0]>().toExtend<number[] | null | { buildAst(): unknown }>()), keeping the existing runtime call checks; reference the Fn type alias, PostAccessor, and the cosineDistance/cosineSimilarity functions when you update each test.packages/3-mongo-target/2-mongo-adapter/test/codecs.test.ts (1)
149-153: Tautological placeholder test.This test only verifies the placeholder behavior of the current stub (
() => undefined as never). If/when Mongo actually lowersnear, this test will start failing for the right reason — but it provides no behavioral value today and encodes the stub into the spec. Consider removing it and instead asserting only the descriptor shape (method/self), or replace with a TODO-skipped test.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/3-mongo-target/2-mongo-adapter/test/codecs.test.ts` around lines 149 - 153, The test asserting that mongoVectorNearOperation.impl() returns undefined is tautological and encodes the current stub; remove or replace it so the spec doesn't lock in the stub. Update the test for mongoVectorNearOperation to either (a) assert only the descriptor shape (e.g., that mongoVectorNearOperation has the expected properties such as .impl being a function, and descriptor fields like .method/.self where applicable) or (b) convert the test to a skipped/TODO test noting that behavior will change when Mongo lowers `near`; reference mongoVectorNearOperation.impl and the descriptor name in the replacement.packages/3-extensions/sql-orm-client/test/model-accessor.test.ts (1)
103-123: Cross-column test doesn't cross columns.The test name and comment emphasize "cross-column composition", but both
postandotherPostresolve toColumnRef.of('posts', 'embedding')(same model, same field). The assertion passes, but it would equally pass iftoExprcollapsed the second argument to the sameColumnRefasself— the test doesn't distinguish.Consider threading the argument through a distinct field/model to make the "column-handle, not raw value" branch observable:
💡 Suggested tightening
- const post = createModelAccessor(context, 'Post'); - const otherPost = createModelAccessor(context, 'Post'); - - const result = post['embedding']!.cosineDistance(otherPost['embedding']!) as unknown as Record< - string, - unknown - >; + const post = createModelAccessor(context, 'Post'); + // Use a different vector-typed field (or a different aliased model) so + // `self` and `arg0` resolve to distinct ColumnRefs. + const result = post['embedding']!.cosineDistance(post['otherEmbedding']!) as unknown as Record< + string, + unknown + >; ... - expect(opExpr.args[0]).toEqual(ColumnRef.of('posts', 'embedding')); + expect(opExpr.args[0]).toEqual(ColumnRef.of('posts', 'other_embedding'));Skip if the fixture doesn't have a second vector column handy — the current test still exercises the
isExpressionLikebranch.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/3-extensions/sql-orm-client/test/model-accessor.test.ts` around lines 103 - 123, The test claims to verify cross-column composition but uses two identical column handles, so change the second accessor to point to a different model/field so the produced ColumnRef is distinct and the test actually verifies that cosineDistance treats the second arg as an Expression (not a ParamRef); specifically modify the second accessor created by createModelAccessor (otherPost) to reference a different model or vector field (e.g., use createModelAccessor(context, 'Comment') or a different field name instead of 'embedding') and keep assertions on OperationExpr.method === 'cosineDistance' and that opExpr.args[0] is a ColumnRef unequal to opExpr.self.packages/1-framework/1-core/operations/src/index.ts (1)
12-14: Consider non-empty tuple fortraitsto mirror runtime check.
readonly string[]allows{ traits: [] }at compile time, but the runtime validator at line 42 rejects it. Tightening the type closes the gap and surfaces the error at authoring time:♻️ Suggested tightening
export type SelfSpec = | { readonly codecId: string; readonly traits?: never } - | { readonly traits: readonly string[]; readonly codecId?: never }; + | { readonly traits: readonly [string, ...string[]]; readonly codecId?: never };Not a functional bug — the runtime guard already catches it — but it keeps the
selfvocabulary consistent across compile/runtime.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/1-framework/1-core/operations/src/index.ts` around lines 12 - 14, The SelfSpec union allows traits to be an empty array at compile time but the runtime validator rejects empty traits; update the SelfSpec type so traits is a non-empty tuple (i.e., require a first string element then zero-or-more strings) instead of plain readonly string[] to reflect the runtime check and surface empty-traits errors at compile time; adjust the existing union branches (the variant with readonly traits and the codecId-exclusive variant) accordingly while preserving the readonly and mutual-exclusion semantics.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/2-sql/4-lanes/relational-core/src/expression.ts`:
- Around line 71-85: The current isExpressionLike type guard only checks for a
buildAst function and allows arbitrary objects to be treated as
Expression<ScopeField>, letting raw codec inputs bypass ParamRef; update
isExpressionLike (used by toExpr) to also verify the presence and shape of a
returnType property (e.g., ensure value.returnType is an object and has codecId
of type string and nullable of type boolean) so that only true Expression-like
objects with both buildAst() and a valid returnType pass the guard and other
values fall through to ParamRef.of.
- Around line 20-29: The conditional in the CodecIdsWithTrait type is comparing
RequiredTraits[number] to the inferred traits tuple T itself, causing mismatches
when traits are arrays/tuples; change the check to compare against the element
union of the tuple by using T[number] (keep the tuple-wrapping trick to avoid
distributive conditionals). Concretely, in CodecIdsWithTrait update the
conditional from checking [RequiredTraits[number]] extends [T] to
[RequiredTraits[number]] extends [T[number]] so the type extracts the trait
element types correctly (refer to CodecIdsWithTrait, CT, RequiredTraits, and the
inferred T).
In `@packages/3-extensions/pgvector/src/core/descriptor-meta.ts`:
- Around line 23-30: The impls for cosineDistance and cosineSimilarity declare
operands as CodecExpression<'pg/vector@1', boolean, CT> but always return
Expression with returns.nullable: false, causing nullable vector inputs to be
incorrectly typed as non-null; fix by either changing the operand nullability
parameter from boolean to false (i.e., CodecExpression<'pg/vector@1', false,
CT>) to forbid nullable operands, or propagate operand nullability into the
return metadata (compute returns.nullable = self.nullable || other.nullable) by
reading the boolean nullability of the incoming CodecExpression and using that
value when building the return Expression in the buildOperation call (update the
impls for both cosineDistance and cosineSimilarity and ensure toExpr usage
remains correct).
In `@packages/3-extensions/pgvector/src/types/operation-types.ts`:
- Around line 30-40: The exported operation types currently accept nullable
vector operands (CodecExpression<'pg/vector@1', boolean, CT>) but return a
non-null float result; mirror the pgvector nullability contract by making the
operand nullability explicit—change the operand types in the impl signature(s)
to non-null (CodecExpression<'pg/vector@1', false, CT>) or alternatively make
the returned Expression nullable when operands can be nullable; specifically
update the impl signatures for cosineSimilarity (and any sibling impl shown) to
use false for the operand nullability so the public types match the descriptor
contract.
In `@packages/3-mongo-target/2-mongo-adapter/src/core/operations.ts`:
- Around line 4-8: The placeholder impl on mongoVectorNearOperation currently
returns undefined and can silently propagate errors; change the impl (on
mongoVectorNearOperation) to immediately throw a clear runtime Error (e.g.,
"mongo vector 'near' operation not implemented/registered") instead of returning
undefined so any premature dispatch fails loudly, remove the unnecessary "as
never" cast, and update the placeholder test in codecs.test.ts to expect/verify
that calling the impl throws.
In `@packages/3-targets/6-adapters/postgres/src/core/descriptor-meta.ts`:
- Around line 147-155: The ilike operation's pattern parameter is incorrectly
typed as CodecExpression<'pg/text@1', false, CT> which prevents
cross-textual-codec use; change the pattern parameter type in both the ilike
signatures (the one in operation-types and the impl in descriptor-meta for
ilike) to TraitExpression<readonly ['textual'], false, CT> so it matches self;
keep the existing toExpr(pattern, PG_TEXT_CODEC_ID) normalization call and
returns/lowering unchanged (symbols to update: ilike, TraitExpression,
CodecExpression, toExpr, PG_TEXT_CODEC_ID).
---
Nitpick comments:
In `@packages/1-framework/1-core/operations/src/index.ts`:
- Around line 12-14: The SelfSpec union allows traits to be an empty array at
compile time but the runtime validator rejects empty traits; update the SelfSpec
type so traits is a non-empty tuple (i.e., require a first string element then
zero-or-more strings) instead of plain readonly string[] to reflect the runtime
check and surface empty-traits errors at compile time; adjust the existing union
branches (the variant with readonly traits and the codecId-exclusive variant)
accordingly while preserving the readonly and mutual-exclusion semantics.
In `@packages/3-extensions/pgvector/test/operations.test.ts`:
- Around line 29-55: The test currently uses optional chaining on
cosineDistanceOp?.impl(...) and cosineSimilarityOp?.impl(...) immediately after
expect(...).toBeDefined(); update both to use non-null assertions instead
(cosineDistanceOp!.impl(...) and cosineSimilarityOp!.impl(...)) so the code
reflects the prior assertion; keep the rest of the invocation and casts
(ParamRef.of, as never, buildAst) unchanged and still assert the resulting
distAst/simAst are OperationExpr instances and their lowering values.
In `@packages/3-extensions/sql-orm-client/test/extension-operations.test-d.ts`:
- Around line 40-62: The tests for PostAccessor['embedding']['cosineDistance']
and ['cosineSimilarity'] currently only assert the calls compile; pin the
parameter type to catch regressions by adding a type-level assertion for
Parameters<Fn>[0] for each test (e.g., use
expectTypeOf<Parameters<Fn>[0]>().toExtend<number[] | null | { buildAst():
unknown }>()), keeping the existing runtime call checks; reference the Fn type
alias, PostAccessor, and the cosineDistance/cosineSimilarity functions when you
update each test.
In `@packages/3-extensions/sql-orm-client/test/model-accessor.test.ts`:
- Around line 103-123: The test claims to verify cross-column composition but
uses two identical column handles, so change the second accessor to point to a
different model/field so the produced ColumnRef is distinct and the test
actually verifies that cosineDistance treats the second arg as an Expression
(not a ParamRef); specifically modify the second accessor created by
createModelAccessor (otherPost) to reference a different model or vector field
(e.g., use createModelAccessor(context, 'Comment') or a different field name
instead of 'embedding') and keep assertions on OperationExpr.method ===
'cosineDistance' and that opExpr.args[0] is a ColumnRef unequal to opExpr.self.
In `@packages/3-mongo-target/2-mongo-adapter/test/codecs.test.ts`:
- Around line 149-153: The test asserting that mongoVectorNearOperation.impl()
returns undefined is tautological and encodes the current stub; remove or
replace it so the spec doesn't lock in the stub. Update the test for
mongoVectorNearOperation to either (a) assert only the descriptor shape (e.g.,
that mongoVectorNearOperation has the expected properties such as .impl being a
function, and descriptor fields like .method/.self where applicable) or (b)
convert the test to a skipped/TODO test noting that behavior will change when
Mongo lowers `near`; reference mongoVectorNearOperation.impl and the descriptor
name in the replacement.
🪄 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: 3b77b6d9-0fb7-4cb2-ac27-1db8bfcea834
⛔ Files ignored due to path filters (5)
packages/2-sql/4-lanes/sql-builder/test/fixtures/generated/contract.d.tsis excluded by!**/generated/**packages/3-extensions/sql-orm-client/test/fixtures/generated/contract.d.tsis excluded by!**/generated/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yamltest/e2e/framework/test/fixtures/generated/contract.d.tsis excluded by!**/generated/**test/integration/test/sql-builder/fixtures/generated/contract.d.tsis excluded by!**/generated/**
📒 Files selected for processing (42)
docs/architecture docs/ADR-INDEX.mddocs/architecture docs/adrs/ADR 204 - Operations as TypeScript functions.mdexamples/prisma-next-demo/src/prisma/contract.d.tspackages/1-framework/1-core/operations/src/index.tspackages/1-framework/1-core/operations/test/operations-registry.test.tspackages/2-sql/1-core/contract/src/exports/types.tspackages/2-sql/1-core/contract/src/types.tspackages/2-sql/1-core/operations/package.jsonpackages/2-sql/1-core/operations/src/index.tspackages/2-sql/1-core/operations/test/operations-registry.test.tspackages/2-sql/3-tooling/emitter/src/index.tspackages/2-sql/4-lanes/relational-core/package.jsonpackages/2-sql/4-lanes/relational-core/src/exports/expression.tspackages/2-sql/4-lanes/relational-core/src/expression.tspackages/2-sql/4-lanes/relational-core/src/index.tspackages/2-sql/4-lanes/relational-core/test/expression.test.tspackages/2-sql/4-lanes/relational-core/tsdown.config.tspackages/2-sql/4-lanes/sql-builder/src/expression.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/functions.tspackages/2-sql/4-lanes/sql-builder/src/scope.tspackages/2-sql/4-lanes/sql-builder/test/runtime/expression-impl.test.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/test/execution-stack.test.tspackages/2-sql/5-runtime/test/sql-context.test.tspackages/3-extensions/pgvector/src/core/descriptor-meta.tspackages/3-extensions/pgvector/src/exports/control.tspackages/3-extensions/pgvector/src/exports/runtime.tspackages/3-extensions/pgvector/src/types/operation-types.tspackages/3-extensions/pgvector/test/operations.test.tspackages/3-extensions/sql-orm-client/src/model-accessor.tspackages/3-extensions/sql-orm-client/src/types.tspackages/3-extensions/sql-orm-client/test/extension-operations.test-d.tspackages/3-extensions/sql-orm-client/test/model-accessor.test.tspackages/3-mongo-target/2-mongo-adapter/src/core/operations.tspackages/3-mongo-target/2-mongo-adapter/test/codecs.test.tspackages/3-targets/6-adapters/postgres/src/core/descriptor-meta.tspackages/3-targets/6-adapters/postgres/src/exports/runtime.tspackages/3-targets/6-adapters/postgres/src/types/operation-types.tstest/integration/test/fixtures/contract.d.ts
Comment on lines
+20
to
+29
| type CodecIdsWithTrait< | ||
| CT extends Record<string, { readonly input: unknown }>, | ||
| RequiredTraits extends readonly string[], | ||
| > = { | ||
| [K in keyof CT & string]: CT[K] extends { readonly traits: infer T } | ||
| ? [RequiredTraits[number]] extends [T] | ||
| ? K | ||
| : never | ||
| : never; | ||
| }[keyof CT & string]; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect codec type maps to confirm whether `traits` is represented as an array/tuple.
# Expected: If matches show `traits: readonly [...]` or `traits: [...]`, the current conditional should use `traits[number]`.
rg -n -C3 "readonly traits|traits:" --glob '*.ts' packages test examplesRepository: prisma/prisma-next
Length of output: 50374
🏁 Script executed:
sed -n '20,29p' packages/2-sql/4-lanes/relational-core/src/expression.tsRepository: prisma/prisma-next
Length of output: 366
🏁 Script executed:
rg -A5 "CodecIdsWithTrait" packages/2-sql/4-lanes/relational-core/src/expression.tsRepository: prisma/prisma-next
Length of output: 765
🏁 Script executed:
rg "TraitExpression" packages/2-sql/4-lanes/relational-core/src/ --type ts -A2 | head -50Repository: prisma/prisma-next
Length of output: 325
🏁 Script executed:
rg "TraitExpression<" packages/2-sql/ packages/3-targets/ test/ --type ts -B2 -A3 | head -100Repository: prisma/prisma-next
Length of output: 1997
🏁 Script executed:
find packages/2-sql/4-lanes/relational-core -name "*.test-d.ts" -o -name "*.test.ts" | head -5Repository: prisma/prisma-next
Length of output: 389
🏁 Script executed:
find packages/2-sql/4-lanes/relational-core -name "*expression*" -type fRepository: prisma/prisma-next
Length of output: 246
🏁 Script executed:
cat packages/2-sql/4-lanes/relational-core/test/expression.test.tsRepository: prisma/prisma-next
Length of output: 4495
🏁 Script executed:
find packages/2-sql/4-lanes/relational-core -name "*.test-d.ts"Repository: prisma/prisma-next
Length of output: 138
🏁 Script executed:
cat packages/2-sql/4-lanes/relational-core/test/ast/driver-types.types.test-d.tsRepository: prisma/prisma-next
Length of output: 1776
🏁 Script executed:
rg -B5 -A10 "TraitExpression.*readonly.*\[" packages/3-targets/6-adapters/postgres/src/types/operation-types.ts | head -30Repository: prisma/prisma-next
Length of output: 382
🏁 Script executed:
rg -B5 "readonly traits.*readonly \[" packages/2-sql/4-lanes/relational-core/src/ast/codec-types.ts -A3Repository: prisma/prisma-next
Length of output: 44
🏁 Script executed:
sed -n '240,250p' packages/2-sql/4-lanes/relational-core/src/ast/codec-types.tsRepository: prisma/prisma-next
Length of output: 452
🏁 Script executed:
rg "type CodecTraits" packages/2-sql/4-lanes/relational-core/src/ast/codec-types.ts -A5Repository: prisma/prisma-next
Length of output: 241
🏁 Script executed:
rg "type CodecTraits" packages/2-sql/4-lanes/relational-core/src/ast/codec-types.ts -B2 -A8Repository: prisma/prisma-next
Length of output: 490
🏁 Script executed:
rg "CodecTypes\[" packages/2-sql/4-lanes/relational-core/test/ -B2 -A2 | head -40Repository: prisma/prisma-next
Length of output: 44
🏁 Script executed:
rg "CodecIdsWithTrait" packages/ test/ -B2 -A2Repository: prisma/prisma-next
Length of output: 1394
Fix trait matching in CodecIdsWithTrait to extract array element types.
Line 25 compares required trait literals to the inferred traits type. When codec traits are defined as arrays (e.g., traits: ['equality', 'order']), the inferred type T is the tuple itself, not the element union. The check [RequiredTraits[number]] extends [T] then fails because it compares a string literal against a tuple type instead of the tuple's elements.
The fix extracts the element type using [number] indexing and checks for subset membership:
Proposed fix
type CodecIdsWithTrait<
CT extends Record<string, { readonly input: unknown }>,
RequiredTraits extends readonly string[],
> = {
- [K in keyof CT & string]: CT[K] extends { readonly traits: infer T }
- ? [RequiredTraits[number]] extends [T]
+ [K in keyof CT & string]: CT[K] extends { readonly traits: readonly string[] }
+ ? Exclude<RequiredTraits[number], CT[K]['traits'][number]> extends never
? K
: never
: never;
}[keyof CT & string];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/2-sql/4-lanes/relational-core/src/expression.ts` around lines 20 -
29, The conditional in the CodecIdsWithTrait type is comparing
RequiredTraits[number] to the inferred traits tuple T itself, causing mismatches
when traits are arrays/tuples; change the check to compare against the element
union of the tuple by using T[number] (keep the tuple-wrapping trick to avoid
distributive conditionals). Concretely, in CodecIdsWithTrait update the
conditional from checking [RequiredTraits[number]] extends [T] to
[RequiredTraits[number]] extends [T[number]] so the type extracts the trait
element types correctly (refer to CodecIdsWithTrait, CT, RequiredTraits, and the
inferred T).
Comment on lines
+23
to
+30
| impl: ( | ||
| self: CodecExpression<'pg/vector@1', boolean, CT>, | ||
| other: CodecExpression<'pg/vector@1', boolean, CT>, | ||
| ): Expression<{ codecId: 'pg/float8@1'; nullable: false }> => | ||
| buildOperation({ | ||
| method: 'cosineDistance', | ||
| args: [toExpr(self, pgvectorTypeId), toExpr(other, pgvectorTypeId)], | ||
| returns: { codecId: 'pg/float8@1', nullable: false }, |
There was a problem hiding this comment.
Align operand nullability with the advertised return type.
These impls accept nullable vector expressions via boolean, but always return nullable: false. Either restrict operands to non-null expressions (false) or propagate operand nullability into the returned Expression metadata; otherwise nullable vector columns are typed as producing non-null distances.
Possible narrow fix if nullable operands are not supported
impl: (
- self: CodecExpression<'pg/vector@1', boolean, CT>,
- other: CodecExpression<'pg/vector@1', boolean, CT>,
+ self: CodecExpression<'pg/vector@1', false, CT>,
+ other: CodecExpression<'pg/vector@1', false, CT>,
): Expression<{ codecId: 'pg/float8@1'; nullable: false }> =>Apply the same change to cosineSimilarity.
Also applies to: 41-48
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/3-extensions/pgvector/src/core/descriptor-meta.ts` around lines 23 -
30, The impls for cosineDistance and cosineSimilarity declare operands as
CodecExpression<'pg/vector@1', boolean, CT> but always return Expression with
returns.nullable: false, causing nullable vector inputs to be incorrectly typed
as non-null; fix by either changing the operand nullability parameter from
boolean to false (i.e., CodecExpression<'pg/vector@1', false, CT>) to forbid
nullable operands, or propagate operand nullability into the return metadata
(compute returns.nullable = self.nullable || other.nullable) by reading the
boolean nullability of the incoming CodecExpression and using that value when
building the return Expression in the buildOperation call (update the impls for
both cosineDistance and cosineSimilarity and ensure toExpr usage remains
correct).
Contributor
Author
There was a problem hiding this comment.
Pre-existing issue, out of scope for this PR
There was a problem hiding this comment.
@SevInf, understood — I'll drop this for the current PR. Would you like me to open a follow-up issue to track the operand nullability alignment for cosineDistance and cosineSimilarity so it isn't lost?
🧠 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: 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.
Comment on lines
+30
to
+40
| readonly impl: ( | ||
| self: CodecExpression<'pg/vector@1', boolean, CT>, | ||
| other: CodecExpression<'pg/vector@1', boolean, CT>, | ||
| ) => Expression<{ codecId: 'pg/float8@1'; nullable: false }>; | ||
| }; | ||
| readonly cosineSimilarity: { | ||
| readonly self: { readonly codecId: 'pg/vector@1' }; | ||
| readonly impl: ( | ||
| self: CodecExpression<'pg/vector@1', boolean, CT>, | ||
| other: CodecExpression<'pg/vector@1', boolean, CT>, | ||
| ) => Expression<{ codecId: 'pg/float8@1'; nullable: false }>; |
There was a problem hiding this comment.
Mirror the pgvector nullability contract in the exported types.
The public signatures accept nullable vector expressions (boolean) but return a non-null float expression. Keep this in sync with the descriptor fix: either make operands non-null (false) or return an expression whose nullable metadata reflects nullable operands.
Possible narrow fix if nullable operands are not supported
readonly impl: (
- self: CodecExpression<'pg/vector@1', boolean, CT>,
- other: CodecExpression<'pg/vector@1', boolean, CT>,
+ self: CodecExpression<'pg/vector@1', false, CT>,
+ other: CodecExpression<'pg/vector@1', false, CT>,
) => Expression<{ codecId: 'pg/float8@1'; nullable: false }>;Apply the same change to cosineSimilarity.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/3-extensions/pgvector/src/types/operation-types.ts` around lines 30
- 40, The exported operation types currently accept nullable vector operands
(CodecExpression<'pg/vector@1', boolean, CT>) but return a non-null float
result; mirror the pgvector nullability contract by making the operand
nullability explicit—change the operand types in the impl signature(s) to
non-null (CodecExpression<'pg/vector@1', false, CT>) or alternatively make the
returned Expression nullable when operands can be nullable; specifically update
the impl signatures for cosineSimilarity (and any sibling impl shown) to use
false for the operand nullability so the public types match the descriptor
contract.
Contributor
Author
There was a problem hiding this comment.
Pre-existing issue, out of scope
There was a problem hiding this comment.
@SevInf, understood — I'll drop this concern for the current PR. Would you like me to open a follow-up issue to track the nullability alignment for the pgvector operation types?
🧠 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: 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.
Comment on lines
4
to
8
| export const mongoVectorNearOperation = Object.freeze({ | ||
| method: 'near', | ||
| args: [{ codecId: MONGO_VECTOR_CODEC_ID, nullable: false }], | ||
| returns: { codecId: MONGO_INT32_CODEC_ID, nullable: false }, | ||
| self: { codecId: MONGO_VECTOR_CODEC_ID }, | ||
| impl: () => undefined as never, | ||
| }) satisfies OperationDescriptor; |
There was a problem hiding this comment.
Placeholder impl will silently return undefined if ever dispatched.
impl: () => undefined as never satisfies the type but has no runtime guard. If anything in the query pipeline ever calls this impl (directly or via the registry) before Mongo wires up near, it will produce undefined and propagate through AST construction / buildOperation paths with a very confusing downstream failure.
Prefer a loud failure so the gap is obvious:
Proposed fix
- impl: () => undefined as never,
+ impl: () => {
+ throw new Error('mongoVectorNearOperation: not yet implemented for the Mongo adapter');
+ },This also lets you drop the as never cast (see repo guideline "Minimize type casts"). The placeholder test in codecs.test.ts would need a corresponding toThrow update.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/3-mongo-target/2-mongo-adapter/src/core/operations.ts` around lines
4 - 8, The placeholder impl on mongoVectorNearOperation currently returns
undefined and can silently propagate errors; change the impl (on
mongoVectorNearOperation) to immediately throw a clear runtime Error (e.g.,
"mongo vector 'near' operation not implemented/registered") instead of returning
undefined so any premature dispatch fails loudly, remove the unnecessary "as
never" cast, and update the placeholder test in codecs.test.ts to expect/verify
that calling the impl throws.
Contributor
Author
There was a problem hiding this comment.
It will never be dispatched before getting replaced with an actual operation
There was a problem hiding this comment.
@SevInf, understood — thanks for the clarification. If the registry/dispatch lifecycle guarantees the placeholder is always replaced with a concrete implementation before any call site can invoke it, the undefined as never stub is safe as a structural placeholder to satisfy OperationDescriptor.
✏️ Learnings added
Learnt from: SevInf
Repo: prisma/prisma-next PR: 374
File: packages/3-mongo-target/2-mongo-adapter/src/core/operations.ts:4-8
Timestamp: 2026-04-23T17:08:52.146Z
Learning: In prisma/prisma-next, the Mongo adapter's `mongoVectorNearOperation` uses `impl: () => undefined as never` as an intentional structural placeholder to satisfy the `OperationDescriptor` type shape at registration time. This placeholder is guaranteed to be replaced with a real implementation before any dispatch occurs, so it will never actually be called. Do not flag this as a missing runtime guard or suggest converting it to a throwing stub.
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: 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.
Comment on lines
+147
to
+155
| impl: ( | ||
| self: TraitExpression<readonly ['textual'], false, CT>, | ||
| pattern: CodecExpression<'pg/text@1', false, CT>, | ||
| ): Expression<{ codecId: 'pg/bool@1'; nullable: false }> => | ||
| buildOperation({ | ||
| method: 'ilike', | ||
| args: [toExpr(self), toExpr(pattern, PG_TEXT_CODEC_ID)], | ||
| returns: { codecId: PG_BOOL_CODEC_ID, nullable: false }, | ||
| lowering: { targetFamily: 'sql', strategy: 'infix', template: '{{self}} ILIKE {{arg0}}' }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the Postgres ilike operation type surface matches the descriptor signature.
# Expect: both descriptor-meta.ts and any operation-types.ts ilike signatures use TraitExpression
# for non-raw textual expression operands, not only CodecExpression<'pg/text@1'>.
rg -n -C4 --type=ts "ilike|CodecExpression<'pg/text@1'|TraitExpression<readonly \\['textual'\\]" packages/3-targets/6-adapters/postgresRepository: prisma/prisma-next
Length of output: 3177
Change ilike pattern parameter to accept any textual codec.
The pattern parameter is narrowed to CodecExpression<'pg/text@1', false, CT> in both operation-types.ts (line 17) and descriptor-meta.ts (line 149), while self is trait-typed as TraitExpression<readonly ['textual'], false, CT>. This inconsistency blocks cross-textual-codec composition (e.g., varchar ILIKE varchar).
Update both signatures to use TraitExpression<readonly ['textual'], false, CT> for the pattern parameter. The toExpr(pattern, PG_TEXT_CODEC_ID) call will continue to normalize the pattern at the semantic level.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/3-targets/6-adapters/postgres/src/core/descriptor-meta.ts` around
lines 147 - 155, The ilike operation's pattern parameter is incorrectly typed as
CodecExpression<'pg/text@1', false, CT> which prevents cross-textual-codec use;
change the pattern parameter type in both the ilike signatures (the one in
operation-types and the impl in descriptor-meta for ilike) to
TraitExpression<readonly ['textual'], false, CT> so it matches self; keep the
existing toExpr(pattern, PG_TEXT_CODEC_ID) normalization call and
returns/lowering unchanged (symbols to update: ilike, TraitExpression,
CodecExpression, toExpr, PG_TEXT_CODEC_ID).
wmadden
reviewed
Apr 28, 2026
Contributor
wmadden
left a comment
wmadden
left a comment
There was a problem hiding this comment.
Review (comment, not blocking automation)
One substantive concern.
The change is structurally complete for SQL but not for Mongo, and the framework permits this asymmetry. SQL ships the full new authoring model — SqlOperationEntry tightening, QueryOperations<CT>() factory shape, QueryOperationTypes<CT> type-only mirror, contract-emitter intersection, accessor dispatch in createExtensionMethodFactory. Mongo ships one entry (mongoVectorNearOperation) using the framework's OperationDescriptor type and nothing else: no Mongo-shaped entry tightening, no factory, no type-only mirror, no Mongo emitter support, no Mongo accessor dispatch site. The descriptor is exported and consumed only by test/codecs.test.ts.
SQL and Mongo can't dispatch into each other (separate contracts, builders, lowering paths), so this is not a runtime hazard. It is a framework-level structural problem: @prisma-next/operations defines the descriptor and a self-validating registry, but every higher-level concern (return tightening, factory shape, type mirror, emitter integration, accessor dispatch) was built ad-hoc inside the SQL packages. The framework neither provides those primitives nor structurally requires that targets supply them. SQL built it well; Mongo skipped it; the type system noticed nothing. The same drift will be available to the next target.
Preferred resolution. Lift the cross-cutting infrastructure into @prisma-next/operations as a target-parametric contract — OperationTarget<Return, ContractCT>, with OperationEntryFor<T>, OperationFactoryFor<T>, OperationTypeMirrorFor<T>, plus framework-provided primitives for registry validation, contract-emitter intersection, and accessor-dispatch resolution that are generic over OperationTarget. SQL becomes SqlOperationTarget extends OperationTarget<QueryOperationReturn, SqlCodecTypesBase>. Mongo becomes MongoOperationTarget extends OperationTarget<MongoOperationReturn, MongoCodecTypesBase>. Adding a target then means instantiating the contract; the framework refuses to wire up a half-built one. Most of D1–D6 stays where it is; the cross-cutting infrastructure moves up a layer. This pre-empts the next drift, not just this one.
Acceptable fallbacks for this PR.
- Apply the SQL-shaped infrastructure to Mongo by hand here.
- Cut the Mongo stub from this PR (delete
mongoVectorNearOperation,mongoVectorOperationDescriptors, the export, and the correspondingcodecs.test.tscases) and add a "Future work — Mongo" section to ADR 204 explicitly scoping the revamp to SQL with Mongo named as the next target.
As shipped, the Mongo entry asserts more than it delivers. SQL-side work is solid and ready otherwise.
| args: [{ codecId: MONGO_VECTOR_CODEC_ID, nullable: false }], | ||
| returns: { codecId: MONGO_INT32_CODEC_ID, nullable: false }, | ||
| self: { codecId: MONGO_VECTOR_CODEC_ID }, | ||
| impl: () => undefined as never, |
Contributor
There was a problem hiding this comment.
This file is the symptom of the framework-level structural issue called out in the top-level review.
The SQL side of the PR ships a full operation-authoring system (entry tightening, factory shape, type-only mirror, contract-emitter intersection, accessor dispatch). The Mongo side ships this — an OperationDescriptor re-using the framework's type, a placeholder impl: () => undefined as never, and a single-element descriptors array — exported from src/exports/index.ts and consumed only by test/codecs.test.ts. There is no Mongo QueryOperations<CT>() analogue, no type-only mirror, no Mongo contract-emitter integration, no Mongo accessor dispatch site. Nothing in the codebase actually uses this entry.
The framework permits this because @prisma-next/operations defines OperationDescriptor + a self-validating registry and stops there. Everything else SQL needed was built inside sql-operations / sql-emitter / sql-orm-client ad-hoc.
Preferred fix (in the top-level review): introduce a target-parametric OperationTarget<Return, ContractCT> contract in @prisma-next/operations with framework-provided primitives that SQL and Mongo both instantiate.
Fallback for this PR: either build the SQL-shaped infrastructure for Mongo by hand, or delete this file (and mongoVectorOperationDescriptors, the export, and the test cases) and explicitly scope ADR 204 to SQL with Mongo as a named follow-up. Shipping as-is asserts a Mongo migration that hasn't actually happened.
SevInf
force-pushed
the
worktree/op-registry-ts
branch
from
05ae897 to
b93ba3b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docs/architecture docs/adrs/ADR 206 - Operations as TypeScript functions.md (1)
13-35: Trim implementation mechanics from the ADR.This section goes beyond the architectural decision and into step-by-step runtime/type-matching behavior. Keep the ADR at the decision/constraint level, and move the
toExpr/buildOperation/ registry-walk details to subsystem docs or implementation notes.As per coding guidelines, ADRs should document architectural decisions and key constraints only, not implementation algorithms, emitter derivation logic, or step-by-step derivation/migration rules.
♻️ Suggested rewrite direction
-Operations are authored as TypeScript functions. The function's signature is the type-level surface. The function's body is the runtime — it receives user arguments ..., wraps them into parameter references ..., and returns an AST expression node ... +Operations are authored as TypeScript functions, with the signature defining the type-level surface and the descriptor carrying only minimal dispatch metadata.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/architecture` docs/adrs/ADR 206 - Operations as TypeScript functions.md around lines 13 - 35, The ADR currently mixes architectural decisions with low-level implementation mechanics; remove or relocate the procedural details about toExpr, buildOperation, SqlOperationDescriptor internals, registry-walk behavior, OperationExpr shape, and factory wiring (QueryOperationTypes/ModelAccessor indexing) from the ADR body into implementation/subsystem docs. Keep only the decision-level statements: that operations are authored as TypeScript functions, the descriptor carries minimal dispatch metadata (method/self/impl), the codec-types map is bound at factory-call time, and that predicate vs non-predicate is determined by the return codec's boolean trait; move the step-by-step wrapping, AST construction, descriptor indexing, and runtime/type-matching algorithms into a new implementation note or subsystem document.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/1-framework/1-core/operations/test/operations-registry.test.ts`:
- Around line 58-65: Remove the compile-time suppression and instead cast the
invalid test payload at the call boundary so the runtime test still passes
TypeScript checks; replace the `// `@ts-expect-error`` usage before the
registry.register(...) call with an explicit narrow cast like `...register({
method: 'bad', self: {} } as unknown as any)` (or `as unknown as
SelfSpec`/`Partial<SelfSpec>` as appropriate) so the code compiles while
exercising the runtime validation in register; apply the same pattern to the
other negative case that checks the `codecId + traits` combination (the second
failing register call around lines 83-90).
---
Nitpick comments:
In `@docs/architecture` docs/adrs/ADR 206 - Operations as TypeScript functions.md:
- Around line 13-35: The ADR currently mixes architectural decisions with
low-level implementation mechanics; remove or relocate the procedural details
about toExpr, buildOperation, SqlOperationDescriptor internals, registry-walk
behavior, OperationExpr shape, and factory wiring
(QueryOperationTypes/ModelAccessor indexing) from the ADR body into
implementation/subsystem docs. Keep only the decision-level statements: that
operations are authored as TypeScript functions, the descriptor carries minimal
dispatch metadata (method/self/impl), the codec-types map is bound at
factory-call time, and that predicate vs non-predicate is determined by the
return codec's boolean trait; move the step-by-step wrapping, AST construction,
descriptor indexing, and runtime/type-matching algorithms into a new
implementation note or subsystem document.
🪄 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: 2969b48b-ae8e-460a-86cd-f89b1a6f7645
📒 Files selected for processing (5)
docs/architecture docs/ADR-INDEX.mddocs/architecture docs/adrs/ADR 206 - Operations as TypeScript functions.mdexamples/prisma-next-demo/src/prisma/contract.d.tspackages/1-framework/1-core/operations/src/index.tspackages/1-framework/1-core/operations/test/operations-registry.test.ts
✅ Files skipped from review due to trivial changes (1)
- docs/architecture docs/ADR-INDEX.md
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/1-framework/1-core/operations/src/index.ts
Comment on lines
58
to
+65
| expect(() => | ||
| registry.register( | ||
| descriptor('bad', { | ||
| args: [{ nullable: false }], | ||
| }), | ||
| ), | ||
| ).toThrow('Operation "bad" arg[0] has neither codecId nor traits'); | ||
| registry.register({ | ||
| method: 'bad', | ||
| // @ts-expect-error — SelfSpec requires codecId or traits | ||
| self: {}, | ||
| impl: noopImpl, | ||
| }), | ||
| ).toThrow('Operation "bad" self has neither codecId nor traits'); |
There was a problem hiding this comment.
Avoid @ts-expect-error in this runtime test.
These cases are validating runtime behavior, so the compile-time suppression is out of place here. Use a narrow cast at the call boundary instead, or move the invalid-shape checks into a dedicated negative type test.
♻️ Suggested adjustment
registry.register({
method: 'bad',
- // `@ts-expect-error` — SelfSpec requires codecId or traits
- self: {},
+ self: {} as unknown as OperationEntry['self'],
impl: noopImpl,
}),Apply the same pattern to the codecId + traits case as well.
Also applies to: 83-90
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/1-framework/1-core/operations/test/operations-registry.test.ts`
around lines 58 - 65, Remove the compile-time suppression and instead cast the
invalid test payload at the call boundary so the runtime test still passes
TypeScript checks; replace the `// `@ts-expect-error`` usage before the
registry.register(...) call with an explicit narrow cast like `...register({
method: 'bad', self: {} } as unknown as any)` (or `as unknown as
SelfSpec`/`Partial<SelfSpec>` as appropriate) so the code compiles while
exercising the runtime validation in register; apply the same pattern to the
other negative case that checks the `codecId + traits` combination (the second
failing register call around lines 83-90).
Replaces the declarative `{args, returns, lowering}` operation record with a
TypeScript function `impl` paired with a minimal `self` dispatch hint. The
function's signature is the type-level surface; the function's body builds
the AST expression node via `toExpr` + `buildOperation`. Adapters and
extensions ship operation contributions as CT-generic factories whose impls
use `CodecExpression<CodecId, N, CT>` or `TraitExpression<Traits, N, CT>`
for argument types, letting authors use TypeScript's full expressivity
(generics, conditional returns, non-DB argument types).
Column handles on the ORM model accessor now implement `Expression<T>`
directly — carrying `returnType` and `buildAst()` on a single plain-object
literal — unlocking cross-column composition like
`db.a.embedding.cosineDistance(db.b.embedding)`. The old
`new ExpressionImpl(column, { codecId: '', nullable: false })` hack in the
extension-method factory is gone; `numericAgg`'s downcast
`(expr as ExpressionImpl).field.codecId` reads `expr.returnType.codecId`
off the Expression interface.
The ORM Proxy returns `undefined` for unknown fields (plain JS object
semantics). The relation-shorthand iterator throws explicitly on unknown
predicate keys so typos surface loudly instead of silently filtering
nothing.
Ported ops: `postgres/ilike`, `pgvector/cosineDistance`,
`pgvector/cosineSimilarity`, and `mongo-adapter/near`.
ADR 204 documents the decision.
Breaking change for extensions that shipped declarative operation records —
rewrite to a CT-generic factory returning `{method, self?, impl}`.
Replaces biome-ignore + `as any` casts in the two negative tests with `@ts-expect-error`. The assertion under test is specifically that `SelfSpec` rejects these shapes at the type level — @ts-expect-error makes that a compile-time check instead of a runtime-only check.
Add unit coverage for the new toExpr / buildOperation helpers in relational-core and invoke the mongoVectorNearOperation placeholder impl so both files reach their coverage thresholds.
SevInf
force-pushed
the
worktree/op-registry-ts
branch
from
46e7d79 to
afef53b
Compare
wmadden
pushed a commit
that referenced
this pull request
May 4, 2026
closes [TML-2307](https://linear.app/prisma-company/issue/TML-2307/author-sql-operations-as-typescript-functions) ## Intent Replace the closed declarative operation spec (`{args, returns, lowering}`) with a TypeScript function that *is* the operation: its signature is the type-level surface and its body builds the AST. A tiny `SqlOperationDescriptor` — `{method, self?, impl}` — carries only what the framework actually needs to dispatch, so authors get TypeScript's full expressivity (generics, conditional returns, non-DB argument types, overloads) on the authoring surface and the single surface cannot drift out of step with itself. ADR 204 records the decision. ## Change map - **Primitive** — `Expression<T>`, `CodecExpression`, `TraitExpression`, `toExpr`, `buildOperation` in relational-core: - [packages/2-sql/4-lanes/relational-core/src/expression.ts (L1–L117)](packages/2-sql/4-lanes/relational-core/src/expression.ts) - **Registry reshape** — descriptor is now `{method, self?, impl}`; registry validates only the `self` hint: - [packages/1-framework/1-core/operations/src/index.ts (L1–L60)](packages/1-framework/1-core/operations/src/index.ts) - [packages/2-sql/1-core/operations/src/index.ts (L1–L30)](packages/2-sql/1-core/operations/src/index.ts) - [packages/2-sql/1-core/contract/src/types.ts (L167–L200)](packages/2-sql/1-core/contract/src/types.ts) - **SQL builder `fns`** — flows authored `impl`s through unchanged (no wrapper); `numericAgg` reads `expr.returnType.codecId` structurally: - [packages/2-sql/4-lanes/sql-builder/src/expression.ts (L1–L125)](packages/2-sql/4-lanes/sql-builder/src/expression.ts) - [packages/2-sql/4-lanes/sql-builder/src/runtime/functions.ts (L1–L170)](packages/2-sql/4-lanes/sql-builder/src/runtime/functions.ts) - [packages/2-sql/4-lanes/sql-builder/src/runtime/expression-impl.ts (L1–L25)](packages/2-sql/4-lanes/sql-builder/src/runtime/expression-impl.ts) - **ORM column handles implement `Expression<T>`** — unlocks cross-column composition; Proxy returns `undefined` for unknown fields; shorthand predicate iterator throws on unknown keys: - [packages/3-extensions/sql-orm-client/src/model-accessor.ts (L60–L200)](packages/3-extensions/sql-orm-client/src/model-accessor.ts) - [packages/3-extensions/sql-orm-client/src/types.ts (L200–L420)](packages/3-extensions/sql-orm-client/src/types.ts) - **CT-generic factories** — adapters/extensions expose contributions as `QueryOperations<CT>()`: - [packages/3-targets/6-adapters/postgres/src/core/descriptor-meta.ts (L130–L160)](packages/3-targets/6-adapters/postgres/src/core/descriptor-meta.ts) - [packages/3-targets/6-adapters/postgres/src/types/operation-types.ts (L1–L22)](packages/3-targets/6-adapters/postgres/src/types/operation-types.ts) - [packages/3-extensions/pgvector/src/core/descriptor-meta.ts (L10–L58)](packages/3-extensions/pgvector/src/core/descriptor-meta.ts) - [packages/3-extensions/pgvector/src/types/operation-types.ts (L1–L45)](packages/3-extensions/pgvector/src/types/operation-types.ts) - [packages/3-mongo-target/2-mongo-adapter/src/core/operations.ts (L1–L12)](packages/3-mongo-target/2-mongo-adapter/src/core/operations.ts) - **Emitter** — emits a per-import intersection of `QueryOperationTypes<CodecTypes>`: - [packages/2-sql/3-tooling/emitter/src/index.ts (L305–L320)](packages/2-sql/3-tooling/emitter/src/index.ts) - **ADR 204**: - [docs/architecture docs/adrs/ADR 204 - Operations as TypeScript functions.md](docs/architecture%20docs/adrs/ADR%20204%20-%20Operations%20as%20TypeScript%20functions.md) **Tests (evidence):** - [packages/2-sql/4-lanes/sql-builder/test/runtime/functions.test.ts (L256–L320)](packages/2-sql/4-lanes/sql-builder/test/runtime/functions.test.ts) — extension function dispatch and `returnType` propagation - [packages/3-extensions/sql-orm-client/test/model-accessor.test.ts (L80–L135, L205–L230, L345–L390)](packages/3-extensions/sql-orm-client/test/model-accessor.test.ts) — raw value path, cross-column composition, unknown-field semantics, shorthand typo raises - [packages/3-extensions/sql-orm-client/test/extension-operations.test-d.ts (L40–L70)](packages/3-extensions/sql-orm-client/test/extension-operations.test-d.ts) — type-level assertions that `cosineDistance` accepts raw, `null`, and another column expression - [packages/3-extensions/pgvector/test/operations.test.ts (L24–L60)](packages/3-extensions/pgvector/test/operations.test.ts) — calling an op's `impl` yields an AST node carrying the lowering template - [packages/3-mongo-target/2-mongo-adapter/test/codecs.test.ts (L140–L170)](packages/3-mongo-target/2-mongo-adapter/test/codecs.test.ts) — `self` hint replaces `args[0]` on the descriptor - [packages/2-sql/5-runtime/test/execution-stack.test.ts, packages/2-sql/5-runtime/test/sql-context.test.ts](packages/2-sql/5-runtime/test) — registry fixtures updated to new shape ## The story 1. **Introduce the expression primitives.** A new module in relational-core defines `Expression<T>` (structural: `{returnType, buildAst}`), `CodecExpression<CodecId, Nullable, CT>` (exact codec), and `TraitExpression<Traits, Nullable, CT>` (codec gated by capability traits). `toExpr(value, codecId?)` wraps a raw JS value as a `ParamRef`, or threads through an existing Expression. `buildOperation({method, args, returns, lowering})` constructs the `OperationExpr` AST node and returns it as an `Expression<Returns>`. These are the building blocks operations use. 2. **Shrink the operation descriptor.** `OperationEntry` drops `args` and `returns` and becomes `{self?, impl}`. The framework-level registry validates only that `self` (when present) names either a codec identity *or* a trait set, not both — not the old per-argument check. Return codec and lowering live on the AST node the `impl` constructs; the registry no longer needs to see them. 3. **Flow authored impls through the SQL builder.** The builder's `fns` surface used to wrap each registered op in a generated function that coerced arguments by reading the declarative argument specs. That wrapper is deleted: `fns.someOp` now returns the authored `impl` directly. Any generics, conditional returns, or overloads the author wrote survive to the call site unchanged. Aggregate functions (`count`, `sum`, `avg`, …) drop the `ExpressionImpl.field` field and read `expr.returnType` structurally. 4. **Make columns Expressions.** The ORM field accessor used to be a bag of comparison/extension-method functions. It now *is* an `Expression<{codecId, nullable}>` — carrying `returnType` and `buildAst()` on the same object — with comparison and extension methods spread on top. Extension-method factories receive the column as a self Expression and forward it to `impl`. The `new ExpressionImpl(column, {codecId: '', nullable: false})` hack is gone. Cross-column composition — `db.a.embedding.cosineDistance(db.b.embedding)` — now works because the second argument *is* an `Expression`, which the `CodecExpression` union accepts. 5. **Tighten ORM accessor edges.** The Proxy used to synthesise a fail-closed accessor for unknown fields; it now returns `undefined` like any plain JS object would. The relation-shorthand iterator, which used to silently skip unknown predicate keys, now throws — because a typo'd filter key that silently matches every row is a sharp-edged footgun. 6. **Rewire contributors as factories.** `postgres/ilike`, `pgvector/{cosineDistance,cosineSimilarity}`, and `mongo-adapter/near` are ported: each adapter/extension exports a `QueryOperations<CT>()` factory generic over the contract's codec-types map and a matching `QueryOperationTypes<CT>` at the type level. The runtime-descriptor slots (`queryOperations: () => …`) call each factory at assembly time. 7. **Emit the intersection.** The contract emitter used to reach for a shared `generateCodecTypeIntersection` helper; it now inlines a per-import `QueryOperationTypes<CodecTypes>` alias list, producing `A<CodecTypes> & B<CodecTypes>` — the shape that reaches the SQL builder and ORM client is already specialised to the contract's concrete codec-types map. ## Behavior changes & evidence - **Operations are authored as TypeScript functions.** The descriptor is `{method, self?, impl}`; the impl's signature is the type surface; its body builds the AST via `toExpr` + `buildOperation`. Return codec, lowering template, and argument wrapping live inside the body. - **Why**: closed records cannot express generics, conditional return codecs, non-DB argument types, or method overloads; collapsing to a single authoring surface means the type-level view and the runtime view cannot drift. - **Implementation**: [packages/2-sql/4-lanes/relational-core/src/expression.ts](packages/2-sql/4-lanes/relational-core/src/expression.ts), [packages/1-framework/1-core/operations/src/index.ts](packages/1-framework/1-core/operations/src/index.ts), [packages/2-sql/1-core/contract/src/types.ts](packages/2-sql/1-core/contract/src/types.ts) - **Tests**: [packages/2-sql/4-lanes/sql-builder/test/runtime/functions.test.ts](packages/2-sql/4-lanes/sql-builder/test/runtime/functions.test.ts) — `extension functions produces OperationExpr from queryOperationTypes`; [packages/3-extensions/pgvector/test/operations.test.ts](packages/3-extensions/pgvector/test/operations.test.ts) — `descriptor provides query operations whose impls build AST with lowering` - **ORM column handles implement `Expression<T>` directly, enabling cross-column composition.** `db.a.embedding.cosineDistance(db.b.embedding)` compiles to a ColumnRef on `arg0`, not a ParamRef wrapping the accessor object. - **Why**: the authoring model treats a column as just another expression of the column's codec; the runtime must agree. - **Implementation**: [packages/3-extensions/sql-orm-client/src/model-accessor.ts](packages/3-extensions/sql-orm-client/src/model-accessor.ts) - **Tests**: [packages/3-extensions/sql-orm-client/test/model-accessor.test.ts](packages/3-extensions/sql-orm-client/test/model-accessor.test.ts) — `cosineDistance accepts another vector column and produces a ColumnRef on arg0 (cross-column composition)`; [packages/3-extensions/sql-orm-client/test/extension-operations.test-d.ts](packages/3-extensions/sql-orm-client/test/extension-operations.test-d.ts) — type-level union covers raw, `null`, and column - **Unknown fields on a model accessor return `undefined` (plain JS object semantics).** Before, the Proxy synthesised a fail-closed accessor whose comparison methods threw “does not support equality comparisons”. - **Why**: the `ModelAccessor<TContract, ModelName>` type already rejects typos at compile time for TS consumers, and programmatic consumers can detect missing fields with a plain `undefined` check. - **Implementation**: [packages/3-extensions/sql-orm-client/src/model-accessor.ts](packages/3-extensions/sql-orm-client/src/model-accessor.ts) - **Tests**: [packages/3-extensions/sql-orm-client/test/model-accessor.test.ts](packages/3-extensions/sql-orm-client/test/model-accessor.test.ts) — `returns undefined for fields whose storage table is not declared` - **Relation-shorthand predicates throw on unknown field keys.** Before, unknown keys were silently skipped. - **Why**: silently dropping a filter clause means a typo like `nmae: 'Alice'` matches every row instead of failing loudly. Prefer a sharp error at call time. - **Implementation**: [packages/3-extensions/sql-orm-client/src/model-accessor.ts](packages/3-extensions/sql-orm-client/src/model-accessor.ts) — `toRelationWhereExpr` - **Tests**: [packages/3-extensions/sql-orm-client/test/model-accessor.test.ts](packages/3-extensions/sql-orm-client/test/model-accessor.test.ts) — `Unknown fields in a shorthand predicate are surfaced loudly` - **Adapter / extension contributions ship as CT-generic factories.** Both the runtime descriptor (`() => readonly SqlOperationDescriptor[]`) and the type-level surface (`QueryOperationTypes<CT>`) are generic over the contract's codec-types map; the emitter instantiates `QueryOperationTypes<CodecTypes>` per import and intersects them. - **Why**: each `CodecExpression<CodecId, N, CT>` needs the concrete codec-types map to produce raw JS value unions on call sites; binding `CT` at factory-call time avoids a lane-level signature projection step. - **Implementation**: [packages/3-targets/6-adapters/postgres/src/core/descriptor-meta.ts](packages/3-targets/6-adapters/postgres/src/core/descriptor-meta.ts), [packages/3-extensions/pgvector/src/core/descriptor-meta.ts](packages/3-extensions/pgvector/src/core/descriptor-meta.ts), [packages/2-sql/3-tooling/emitter/src/index.ts](packages/2-sql/3-tooling/emitter/src/index.ts) - **Tests**: [packages/3-extensions/pgvector/test/operations.test.ts](packages/3-extensions/pgvector/test/operations.test.ts), [packages/3-mongo-target/2-mongo-adapter/test/codecs.test.ts](packages/3-mongo-target/2-mongo-adapter/test/codecs.test.ts) ## Compatibility / migration / risk - **Breaking change for extensions that shipped declarative operation records.** Rewrite to a CT-generic factory returning `{method, self?, impl}`; return codec and lowering move inside the `impl`'s `buildOperation({ returns, lowering })` call. ADR 017 (Extension Compatibility Policy) is honoured — no legacy-shape retention. - **ORM column method signatures widen.** `.cosineDistance(v: number[] | null)` becomes `.cosineDistance(CodecExpression<'pg/vector@1', Nullable, CT>)`, which is a superset (raw value, `null`, or another Expression of the same codec). Callers that previously passed only raw JS values continue to compile. - **Unknown accessor keys behave differently.** Programmatic code that relied on the old fail-closed Proxy accessor for unknown fields (e.g. catching a thrown error) must now detect `undefined`; relation-shorthand predicates must avoid typo'd keys or be prepared to catch the new error. ## Follow-ups / open questions - ADR 204 leaves one design question open: the adapter runtime descriptor's `queryOperations` slot has the shape `() => readonly SqlOperationDescriptor[]`, so each adapter's thunk currently calls its `QueryOperations<CT>()` factory *internally* with an unconstrained `CT`. Threading the contract's concrete codec-types map through the SPI slot is a separate change. ## Non-goals / intentionally out of scope - Not touching the built-in comparison methods (`eq`, `gt`, `like`, `isNull`, …) — they remain codec-trait-gated via `COMPARISON_METHODS_META` rather than registered operations. - Not deriving the `self` dispatch hint from the function's first parameter. The explicit `self` field keeps the dispatch key obvious to readers and keeps the type-level matcher identical to the runtime walk. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **New Features** * Introduced a typed expression system enabling operations to be authored as TypeScript functions with improved type safety and codec awareness. * Operations now support flexible dispatch via concrete codec targeting or trait-based capability selection. * **Refactor** * Simplified operation registration API to use minimal dispatch metadata and implementation functions instead of parameter/return specifications. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes TML-2307
Intent
Replace the closed declarative operation spec (
{args, returns, lowering}) with a TypeScript function that is the operation: its signature is the type-level surface and its body builds the AST. A tinySqlOperationDescriptor—{method, self?, impl}— carries only what the framework actually needs to dispatch, so authors get TypeScript's full expressivity (generics, conditional returns, non-DB argument types, overloads) on the authoring surface and the single surface cannot drift out of step with itself. ADR 204 records the decision.Change map
Expression<T>,CodecExpression,TraitExpression,toExpr,buildOperationin relational-core:{method, self?, impl}; registry validates only theselfhint:fns— flows authoredimpls through unchanged (no wrapper);numericAggreadsexpr.returnType.codecIdstructurally:Expression<T>— unlocks cross-column composition; Proxy returnsundefinedfor unknown fields; shorthand predicate iterator throws on unknown keys:QueryOperations<CT>():QueryOperationTypes<CodecTypes>:Tests (evidence):
returnTypepropagationcosineDistanceaccepts raw,null, and another column expressionimplyields an AST node carrying the lowering templateselfhint replacesargs[0]on the descriptorThe story
Introduce the expression primitives. A new module in relational-core defines
Expression<T>(structural:{returnType, buildAst}),CodecExpression<CodecId, Nullable, CT>(exact codec), andTraitExpression<Traits, Nullable, CT>(codec gated by capability traits).toExpr(value, codecId?)wraps a raw JS value as aParamRef, or threads through an existing Expression.buildOperation({method, args, returns, lowering})constructs theOperationExprAST node and returns it as anExpression<Returns>. These are the building blocks operations use.Shrink the operation descriptor.
OperationEntrydropsargsandreturnsand becomes{self?, impl}. The framework-level registry validates only thatself(when present) names either a codec identity or a trait set, not both — not the old per-argument check. Return codec and lowering live on the AST node theimplconstructs; the registry no longer needs to see them.Flow authored impls through the SQL builder. The builder's
fnssurface used to wrap each registered op in a generated function that coerced arguments by reading the declarative argument specs. That wrapper is deleted:fns.someOpnow returns the authoredimpldirectly. Any generics, conditional returns, or overloads the author wrote survive to the call site unchanged. Aggregate functions (count,sum,avg, …) drop theExpressionImpl.fieldfield and readexpr.returnTypestructurally.Make columns Expressions. The ORM field accessor used to be a bag of comparison/extension-method functions. It now is an
Expression<{codecId, nullable}>— carryingreturnTypeandbuildAst()on the same object — with comparison and extension methods spread on top. Extension-method factories receive the column as a self Expression and forward it toimpl. Thenew ExpressionImpl(column, {codecId: '', nullable: false})hack is gone. Cross-column composition —db.a.embedding.cosineDistance(db.b.embedding)— now works because the second argument is anExpression, which theCodecExpressionunion accepts.Tighten ORM accessor edges. The Proxy used to synthesise a fail-closed accessor for unknown fields; it now returns
undefinedlike any plain JS object would. The relation-shorthand iterator, which used to silently skip unknown predicate keys, now throws — because a typo'd filter key that silently matches every row is a sharp-edged footgun.Rewire contributors as factories.
postgres/ilike,pgvector/{cosineDistance,cosineSimilarity}, andmongo-adapter/nearare ported: each adapter/extension exports aQueryOperations<CT>()factory generic over the contract's codec-types map and a matchingQueryOperationTypes<CT>at the type level. The runtime-descriptor slots (queryOperations: () => …) call each factory at assembly time.Emit the intersection. The contract emitter used to reach for a shared
generateCodecTypeIntersectionhelper; it now inlines a per-importQueryOperationTypes<CodecTypes>alias list, producingA<CodecTypes> & B<CodecTypes>— the shape that reaches the SQL builder and ORM client is already specialised to the contract's concrete codec-types map.Behavior changes & evidence
Operations are authored as TypeScript functions. The descriptor is
{method, self?, impl}; the impl's signature is the type surface; its body builds the AST viatoExpr+buildOperation. Return codec, lowering template, and argument wrapping live inside the body.extension functions produces OperationExpr from queryOperationTypes; packages/3-extensions/pgvector/test/operations.test.ts —descriptor provides query operations whose impls build AST with loweringORM column handles implement
Expression<T>directly, enabling cross-column composition.db.a.embedding.cosineDistance(db.b.embedding)compiles to a ColumnRef onarg0, not a ParamRef wrapping the accessor object.cosineDistance accepts another vector column and produces a ColumnRef on arg0 (cross-column composition); packages/3-extensions/sql-orm-client/test/extension-operations.test-d.ts — type-level union covers raw,null, and columnUnknown fields on a model accessor return
undefined(plain JS object semantics). Before, the Proxy synthesised a fail-closed accessor whose comparison methods threw “does not support equality comparisons”.ModelAccessor<TContract, ModelName>type already rejects typos at compile time for TS consumers, and programmatic consumers can detect missing fields with a plainundefinedcheck.returns undefined for fields whose storage table is not declaredRelation-shorthand predicates throw on unknown field keys. Before, unknown keys were silently skipped.
nmae: 'Alice'matches every row instead of failing loudly. Prefer a sharp error at call time.toRelationWhereExprUnknown fields in a shorthand predicate are surfaced loudlyAdapter / extension contributions ship as CT-generic factories. Both the runtime descriptor (
() => readonly SqlOperationDescriptor[]) and the type-level surface (QueryOperationTypes<CT>) are generic over the contract's codec-types map; the emitter instantiatesQueryOperationTypes<CodecTypes>per import and intersects them.CodecExpression<CodecId, N, CT>needs the concrete codec-types map to produce raw JS value unions on call sites; bindingCTat factory-call time avoids a lane-level signature projection step.Compatibility / migration / risk
{method, self?, impl}; return codec and lowering move inside theimpl'sbuildOperation({ returns, lowering })call. ADR 017 (Extension Compatibility Policy) is honoured — no legacy-shape retention..cosineDistance(v: number[] | null)becomes.cosineDistance(CodecExpression<'pg/vector@1', Nullable, CT>), which is a superset (raw value,null, or another Expression of the same codec). Callers that previously passed only raw JS values continue to compile.undefined; relation-shorthand predicates must avoid typo'd keys or be prepared to catch the new error.Follow-ups / open questions
queryOperationsslot has the shape() => readonly SqlOperationDescriptor[], so each adapter's thunk currently calls itsQueryOperations<CT>()factory internally with an unconstrainedCT. Threading the contract's concrete codec-types map through the SPI slot is a separate change.Non-goals / intentionally out of scope
eq,gt,like,isNull, …) — they remain codec-trait-gated viaCOMPARISON_METHODS_METArather than registered operations.selfdispatch hint from the function's first parameter. The explicitselffield keeps the dispatch key obvious to readers and keeps the type-level matcher identical to the runtime walk.Summary by CodeRabbit
Release Notes
New Features
Refactor