feat(postgres): make ilike adapter-specific via trait-targeted operations#330
Open
feat(postgres): make ilike adapter-specific via trait-targeted operations#330
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughDecouples return metadata from ParamSpec; introduces trait-based Field/Arg specs; removes AST-level Changes
Sequence DiagramsequenceDiagram
participant User as User Code
participant MA as Model Accessor
participant Ops as Operation Index
participant RE as ResolveExtArg
participant Expr as Expression Builder
participant Adapter as Adapter Query Ops
User->>MA: Access field (e.g., post.title)
MA->>Ops: Lookup ops by codecId
MA->>Ops: Lookup ops by trait(s)
Ops-->>MA: Return combined ops list
User->>MA: Call extension method (e.g., ilike(pattern))
MA->>RE: Resolve arg spec (codecId or traits)
RE-->>MA: Return resolved arg/expression types
MA->>Adapter: Select query operation descriptor (ilike)
MA->>Expr: Build OperationExpr node
Expr-->>User: Return Boolean Expression
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~55 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/3-extensions/sql-orm-client/src/model-accessor.ts`:
- Around line 157-160: The code in model-accessor.ts currently maps userArgSpecs
to ParamRef.of(args[i], ...) but only forwards codecId, losing trait-based
metadata; update the astArgs mapping so ParamRef.of receives the trait metadata
when codecId is absent (e.g., include argSpec.traits or traitTarget in the
options object you pass to ParamRef.of), and add a minimal fail-fast guard in
the same mapping (inside the userArgSpecs.map) to throw a clear error if an
argSpec has neither codecId nor traits so unsupported untyped params fail early;
refer to the symbols userArgSpecs, astArgs, ParamRef.of, and
argSpec.traits/traitTarget when making the change.
In `@packages/3-extensions/sql-orm-client/src/types.ts`:
- Around line 236-247: QueryOperationMethod currently maps user args via
MapArgsToJsTypes which only uses CodecArgJsType and therefore yields unknown for
args that specify traits; update the mapping so trait-based args resolve to
their JS types by reusing the trait-aware resolution logic used in ResolveExtArg
(as in sql-builder) and/or OpMatchesField. Specifically, modify MapArgsToJsTypes
(and/or CodecArgJsType) to detect ext-arg shapes with traits and apply the same
ResolveExtArg/OpMatchesField resolution path so QueryOperationMethod's parameter
types become the concrete JS types rather than unknown; keep function/type names
QueryOperationMethod, MapArgsToJsTypes, CodecArgJsType, ResolveExtArg and
OpMatchesField as reference points when implementing the fix.
🪄 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: 6319e707-06b1-4abd-bffa-e05389f7bcea
⛔ Files ignored due to path filters (2)
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/**
📒 Files selected for processing (28)
packages/1-framework/1-core/operations/src/index.tspackages/2-sql/1-core/contract/src/exports/types.tspackages/2-sql/1-core/contract/src/types.tspackages/2-sql/2-authoring/contract-ts/package.jsonpackages/2-sql/2-authoring/contract-ts/tsdown.config.tspackages/2-sql/4-lanes/relational-core/src/ast/types.tspackages/2-sql/4-lanes/relational-core/test/ast/predicate.test.tspackages/2-sql/4-lanes/sql-builder/src/expression.tspackages/2-sql/4-lanes/sql-builder/src/scope.tspackages/2-sql/4-lanes/sql-builder/test/integration/extension-functions.test.tspackages/2-sql/4-lanes/sql-builder/test/playground/extension-functions.test-d.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/filters.test.tspackages/3-extensions/sql-orm-client/test/generated-contract-types.test-d.tspackages/3-extensions/sql-orm-client/test/model-accessor.test.tspackages/3-extensions/sql-orm-client/test/orm.types.test-d.tspackages/3-extensions/sql-orm-client/test/where-binding.test.tspackages/3-targets/6-adapters/postgres/package.jsonpackages/3-targets/6-adapters/postgres/src/core/adapter.tspackages/3-targets/6-adapters/postgres/src/core/descriptor-meta.tspackages/3-targets/6-adapters/postgres/src/exports/operation-types.tspackages/3-targets/6-adapters/postgres/src/exports/runtime.tspackages/3-targets/6-adapters/postgres/src/types/operation-types.tspackages/3-targets/6-adapters/postgres/tsdown.config.tspackages/3-targets/6-adapters/sqlite/src/core/adapter.tspackages/3-targets/6-adapters/sqlite/test/adapter.test.ts
💤 Files with no reviewable changes (6)
- packages/3-targets/6-adapters/postgres/src/core/adapter.ts
- packages/2-sql/4-lanes/relational-core/test/ast/predicate.test.ts
- packages/3-extensions/sql-orm-client/test/where-binding.test.ts
- packages/3-targets/6-adapters/sqlite/src/core/adapter.ts
- packages/3-targets/6-adapters/sqlite/test/adapter.test.ts
- packages/3-extensions/sql-orm-client/test/model-accessor.test.ts
SevInf
force-pushed
the
worktree/fix-ilike
branch
from
e22d894 to
128c098
Compare
@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/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/runtime-executor
@prisma-next/mongo-codec
@prisma-next/mongo-contract
@prisma-next/mongo-value
@prisma-next/mongo-contract-psl
@prisma-next/mongo-contract-ts
@prisma-next/mongo-emitter
@prisma-next/mongo-schema-ir
@prisma-next/mongo-query-ast
@prisma-next/mongo-orm
@prisma-next/mongo-pipeline-builder
@prisma-next/mongo-lowering
@prisma-next/mongo-wire
@prisma-next/sql-contract
@prisma-next/sql-errors
@prisma-next/sql-operations
@prisma-next/sql-schema-ir
@prisma-next/sql-contract-psl
@prisma-next/sql-contract-ts
@prisma-next/sql-contract-emitter
@prisma-next/sql-lane-query-builder
@prisma-next/sql-relational-core
@prisma-next/sql-builder
@prisma-next/target-postgres
@prisma-next/target-sqlite
@prisma-next/adapter-postgres
@prisma-next/adapter-sqlite
@prisma-next/driver-postgres
@prisma-next/driver-sqlite
commit: |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/3-extensions/sql-orm-client/src/model-accessor.ts (1)
157-160:⚠️ Potential issue | 🟠 MajorTrait-targeted non-self args are still erased during AST binding.
Line 159 only forwards
codecIdintoParamRef. If an extension operation defines a user arg viatraits, that metadata is dropped here and the ORM runtime builds an untyped param, so later lowering/binding cannot distinguish the trait-targeted contract anymore. This leaves trait-based args only partially implemented outside theselfposition.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/3-extensions/sql-orm-client/src/model-accessor.ts` around lines 157 - 160, The mapping that builds astArgs from userArgSpecs drops trait metadata (only codecId is forwarded), so update the ParamRef construction in the userArgSpecs -> astArgs code path to include argSpec.traits (and any other metadata needed for trait-targeted non-self args) in the options passed to ParamRef.of; ensure you read from entry.args / argSpec and pass through codecId plus traits (or a combined metadata object) so trait-targeted args remain typed and available for later lowering/binding.
🤖 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/1-core/contract/src/types.ts`:
- Around line 167-170: The QueryOperationArgSpec type currently declares traits
as string which widens the API; change the traits field in QueryOperationArgSpec
to use the canonical codec trait union type used elsewhere (e.g., CodecTrait or
CodecTraits) instead of plain string, preserving readonly and optional semantics
(readonly traits?: CanonicalTraitUnion) so only valid trait members type-check
and typos are caught at compile time; update any imports to reference that union
where QueryOperationArgSpec is declared.
---
Duplicate comments:
In `@packages/3-extensions/sql-orm-client/src/model-accessor.ts`:
- Around line 157-160: The mapping that builds astArgs from userArgSpecs drops
trait metadata (only codecId is forwarded), so update the ParamRef construction
in the userArgSpecs -> astArgs code path to include argSpec.traits (and any
other metadata needed for trait-targeted non-self args) in the options passed to
ParamRef.of; ensure you read from entry.args / argSpec and pass through codecId
plus traits (or a combined metadata object) so trait-targeted args remain typed
and available for later lowering/binding.
🪄 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: cde64526-2ab9-40f9-b10e-82443440065f
⛔ Files ignored due to path filters (2)
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/**
📒 Files selected for processing (26)
packages/1-framework/1-core/operations/src/index.tspackages/2-sql/1-core/contract/src/exports/types.tspackages/2-sql/1-core/contract/src/types.tspackages/2-sql/4-lanes/relational-core/src/ast/types.tspackages/2-sql/4-lanes/relational-core/test/ast/predicate.test.tspackages/2-sql/4-lanes/sql-builder/src/expression.tspackages/2-sql/4-lanes/sql-builder/src/scope.tspackages/2-sql/4-lanes/sql-builder/test/integration/extension-functions.test.tspackages/2-sql/4-lanes/sql-builder/test/playground/extension-functions.test-d.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/filters.test.tspackages/3-extensions/sql-orm-client/test/generated-contract-types.test-d.tspackages/3-extensions/sql-orm-client/test/model-accessor.test.tspackages/3-extensions/sql-orm-client/test/orm.types.test-d.tspackages/3-extensions/sql-orm-client/test/where-binding.test.tspackages/3-targets/6-adapters/postgres/package.jsonpackages/3-targets/6-adapters/postgres/src/core/adapter.tspackages/3-targets/6-adapters/postgres/src/core/descriptor-meta.tspackages/3-targets/6-adapters/postgres/src/exports/operation-types.tspackages/3-targets/6-adapters/postgres/src/exports/runtime.tspackages/3-targets/6-adapters/postgres/src/types/operation-types.tspackages/3-targets/6-adapters/postgres/tsdown.config.tspackages/3-targets/6-adapters/sqlite/src/core/adapter.tspackages/3-targets/6-adapters/sqlite/test/adapter.test.ts
💤 Files with no reviewable changes (6)
- packages/3-targets/6-adapters/sqlite/src/core/adapter.ts
- packages/3-targets/6-adapters/postgres/src/core/adapter.ts
- packages/2-sql/4-lanes/relational-core/test/ast/predicate.test.ts
- packages/3-targets/6-adapters/sqlite/test/adapter.test.ts
- packages/3-extensions/sql-orm-client/test/model-accessor.test.ts
- packages/3-extensions/sql-orm-client/test/where-binding.test.ts
✅ Files skipped from review due to trivial changes (8)
- packages/3-extensions/sql-orm-client/test/orm.types.test-d.ts
- packages/2-sql/1-core/contract/src/exports/types.ts
- packages/3-extensions/sql-orm-client/test/generated-contract-types.test-d.ts
- packages/3-targets/6-adapters/postgres/tsdown.config.ts
- packages/2-sql/4-lanes/sql-builder/test/integration/extension-functions.test.ts
- packages/3-targets/6-adapters/postgres/package.json
- packages/3-targets/6-adapters/postgres/src/exports/operation-types.ts
- packages/3-targets/6-adapters/postgres/src/core/descriptor-meta.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- packages/3-extensions/sql-orm-client/test/filters.test.ts
- packages/3-targets/6-adapters/postgres/src/exports/runtime.ts
- packages/2-sql/4-lanes/sql-builder/src/scope.ts
- packages/2-sql/4-lanes/sql-builder/test/playground/extension-functions.test-d.ts
- packages/3-extensions/sql-orm-client/test/extension-operations.test-d.ts
- packages/2-sql/4-lanes/relational-core/src/ast/types.ts
- packages/1-framework/1-core/operations/src/index.ts
- packages/3-targets/6-adapters/postgres/src/types/operation-types.ts
- packages/2-sql/4-lanes/sql-builder/src/expression.ts
- packages/3-extensions/sql-orm-client/src/types.ts
SevInf
force-pushed
the
worktree/fix-ilike
branch
from
128c098 to
4ee59bb
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
packages/3-extensions/sql-orm-client/src/types.ts (1)
242-245:⚠️ Potential issue | 🟠 MajorTrait-based operation args still degrade to
unknown.
QueryOperationMethodnow exposes the new trait-targeted operations, but its parameter tuple still comes fromMapArgsToJsTypes -> CodecArgJsType, which only understands{ codecId: ... }. Any non-self arg declared with{ traits: ... }will therefore surface here asunknown, so the ORM method signatures still lose the concrete JS types this PR is trying to preserve.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/3-extensions/sql-orm-client/src/types.ts` around lines 242 - 245, The method parameter types degrade to unknown because MapArgsToJsTypes and CodecArgJsType only handle { codecId: ... } shapes and do not resolve args declared with { traits: ... }; update the type pipeline so QueryOperationMethod receives resolved JS types for trait-based args by extending CodecArgJsType (or MapArgsToJsTypes) to detect a { traits: T } shape and map it to the corresponding codec/JS type via the existing trait-to-codec lookup helper (or add a small type-level resolver that converts trait names to their codec types), ensuring MapArgsToJsTypes<...> returns concrete types for args with { traits: ... } instead of unknown so QueryOperationMethod signatures preserve the correct JS types.
🤖 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/sql-builder/src/expression.ts`:
- Around line 81-90: ResolveExtArg currently maps trait-based arguments to
Expression<...> while codecId-based arguments use ExpressionOrValue<...>,
causing asymmetric API surface; change the trait branch in ResolveExtArg to
allow both expressions and raw values by returning ExpressionOrValue<{ codecId:
CodecIdsWithTrait<CT, T>; nullable: N }, CT> instead of Expression<{ codecId:
CodecIdsWithTrait<CT, T>; nullable: N }>, so both codecId and trait paths accept
the same input range; update the ResolveExtArg conditional that references
"traits: infer T" and "nullable: infer N" to use ExpressionOrValue and keep
CT/CodecIdsWithTrait references intact.
In `@packages/3-extensions/sql-orm-client/src/types.ts`:
- Around line 255-264: The conditional currently treats `codecId` and `traits`
as alternatives, so a matching `codecId` bypasses trait checks; change the
branching in the type conditional so it first checks for the combined shape
(e.g., `Self extends { readonly codecId: CodecId; readonly traits: infer
RequiredTraits extends string }`) and if so ensure `CodecId extends keyof CT ?
CT[CodecId] extends { readonly traits: infer FieldTraits } ? RequiredTraits
extends FieldTraits ? true : false : false : false`; otherwise fall back to the
single-property branches for `codecId` or `traits`. This preserves existing
`CT`, `CodecId`, `Self`, `RequiredTraits`, and `FieldTraits` symbols while
enforcing both constraints when both fields are present.
In `@packages/3-targets/6-adapters/postgres/src/core/descriptor-meta.ts`:
- Around line 131-136: Update the operation type for the ilike operator so its
first argument is non-nullable to match the runtime descriptor: change the
nullable flag for the first ilike arg in the operation-types declaration (the
ilike entry in packages/.../operation-types.ts) from nullable: true/boolean to
nullable: false so it aligns with the runtime postgresQueryOperations descriptor
(ilike entry) and prevents nullable textual fields from being incorrectly typed
as supporting .ilike().
---
Duplicate comments:
In `@packages/3-extensions/sql-orm-client/src/types.ts`:
- Around line 242-245: The method parameter types degrade to unknown because
MapArgsToJsTypes and CodecArgJsType only handle { codecId: ... } shapes and do
not resolve args declared with { traits: ... }; update the type pipeline so
QueryOperationMethod receives resolved JS types for trait-based args by
extending CodecArgJsType (or MapArgsToJsTypes) to detect a { traits: T } shape
and map it to the corresponding codec/JS type via the existing trait-to-codec
lookup helper (or add a small type-level resolver that converts trait names to
their codec types), ensuring MapArgsToJsTypes<...> returns concrete types for
args with { traits: ... } instead of unknown so QueryOperationMethod signatures
preserve the correct JS types.
🪄 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: bfe58c26-5a99-49ef-b0a2-46183837bc3b
⛔ Files ignored due to path filters (3)
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/**test/e2e/framework/test/fixtures/generated/contract.d.tsis excluded by!**/generated/**
📒 Files selected for processing (28)
examples/prisma-next-demo/src/prisma/contract.d.tspackages/1-framework/1-core/operations/src/index.tspackages/2-sql/1-core/contract/src/exports/types.tspackages/2-sql/1-core/contract/src/types.tspackages/2-sql/4-lanes/relational-core/src/ast/types.tspackages/2-sql/4-lanes/relational-core/test/ast/predicate.test.tspackages/2-sql/4-lanes/sql-builder/src/expression.tspackages/2-sql/4-lanes/sql-builder/src/scope.tspackages/2-sql/4-lanes/sql-builder/test/integration/extension-functions.test.tspackages/2-sql/4-lanes/sql-builder/test/playground/extension-functions.test-d.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/filters.test.tspackages/3-extensions/sql-orm-client/test/generated-contract-types.test-d.tspackages/3-extensions/sql-orm-client/test/model-accessor.test.tspackages/3-extensions/sql-orm-client/test/orm.types.test-d.tspackages/3-extensions/sql-orm-client/test/where-binding.test.tspackages/3-targets/6-adapters/postgres/package.jsonpackages/3-targets/6-adapters/postgres/src/core/adapter.tspackages/3-targets/6-adapters/postgres/src/core/descriptor-meta.tspackages/3-targets/6-adapters/postgres/src/exports/operation-types.tspackages/3-targets/6-adapters/postgres/src/exports/runtime.tspackages/3-targets/6-adapters/postgres/src/types/operation-types.tspackages/3-targets/6-adapters/postgres/tsdown.config.tspackages/3-targets/6-adapters/sqlite/src/core/adapter.tspackages/3-targets/6-adapters/sqlite/test/adapter.test.tstest/integration/test/fixtures/contract.d.ts
💤 Files with no reviewable changes (6)
- packages/3-targets/6-adapters/sqlite/test/adapter.test.ts
- packages/3-targets/6-adapters/postgres/src/core/adapter.ts
- packages/3-targets/6-adapters/sqlite/src/core/adapter.ts
- packages/3-extensions/sql-orm-client/test/where-binding.test.ts
- packages/2-sql/4-lanes/relational-core/test/ast/predicate.test.ts
- packages/3-extensions/sql-orm-client/test/model-accessor.test.ts
✅ Files skipped from review due to trivial changes (10)
- packages/3-targets/6-adapters/postgres/src/exports/operation-types.ts
- packages/3-extensions/sql-orm-client/test/generated-contract-types.test-d.ts
- packages/2-sql/4-lanes/sql-builder/test/integration/extension-functions.test.ts
- packages/3-targets/6-adapters/postgres/tsdown.config.ts
- packages/3-extensions/sql-orm-client/test/orm.types.test-d.ts
- packages/3-targets/6-adapters/postgres/package.json
- packages/2-sql/4-lanes/sql-builder/src/scope.ts
- packages/2-sql/4-lanes/sql-builder/test/playground/extension-functions.test-d.ts
- packages/3-targets/6-adapters/postgres/src/types/operation-types.ts
- packages/2-sql/1-core/contract/src/exports/types.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/3-extensions/sql-orm-client/test/filters.test.ts
- packages/3-targets/6-adapters/postgres/src/exports/runtime.ts
- packages/3-extensions/sql-orm-client/test/extension-operations.test-d.ts
- packages/2-sql/1-core/contract/src/types.ts
- packages/1-framework/1-core/operations/src/index.ts
- packages/2-sql/4-lanes/relational-core/src/ast/types.ts
- packages/3-extensions/sql-orm-client/src/model-accessor.ts
SevInf
force-pushed
the
worktree/fix-ilike
branch
from
4ee59bb to
5195631
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
packages/2-sql/1-core/contract/src/types.ts (1)
167-170:⚠️ Potential issue | 🟠 MajorKeep
traitson the canonical codec-trait union.
readonly traits?: stringmakes misspellings type-check and weakens the compile-time gating this PR is adding for trait-targeted operations. This should stay aligned with the shared codec-trait type instead of widening to arbitrary strings.Verify which canonical trait alias already exists before changing this field:
#!/bin/bash set -euo pipefail echo "== current QueryOperationArgSpec ==" sed -n '167,171p' packages/2-sql/1-core/contract/src/types.ts echo echo "== candidate canonical trait aliases in repo ==" rg -n --type=ts '\b(CodecTrait|CodecTraits)\b' packages🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-sql/1-core/contract/src/types.ts` around lines 167 - 170, The QueryOperationArgSpec currently declares readonly traits?: string which weakens type safety; change this field to use the existing canonical codec-trait alias (e.g. CodecTrait or CodecTraits) used across the repo instead of plain string. Locate the type QueryOperationArgSpec in types.ts and replace traits?: string with the correct canonical type (nullable/optional as appropriate), but first confirm which alias name is present in the codebase (search for CodecTrait or CodecTraits) and import or reference that alias so the field aligns with the shared codec-trait union.packages/2-sql/4-lanes/sql-builder/src/expression.ts (1)
81-90:⚠️ Potential issue | 🟡 MinorTrait-targeted args should accept literals too.
The codecId branch returns
ExpressionOrValue, but the trait branch only acceptsExpression. That makes trait-only args stricter than codecId-targeted args and blocks raw literals for the same logical operation surface.Suggested type fix
type ResolveExtArg<Arg, CT extends Record<string, { readonly input: unknown }>> = Arg extends { readonly codecId: infer CId extends string; readonly nullable: infer N extends boolean; } ? ExpressionOrValue<{ codecId: CId; nullable: N }, CT> : Arg extends { readonly traits: infer T extends string; readonly nullable: infer N extends boolean; } - ? Expression<{ codecId: CodecIdsWithTrait<CT, T>; nullable: N }> + ? ExpressionOrValue<{ codecId: CodecIdsWithTrait<CT, T>; nullable: N }, CT> : never;Verify whether any current operation types rely on trait-only args:
#!/bin/bash set -euo pipefail echo "== current ResolveExtArg trait branch ==" sed -n '81,91p' packages/2-sql/4-lanes/sql-builder/src/expression.ts echo echo "== trait-based operation arg specs ==" rg -n -C2 'traits:' packages/3-targets packages/2-sql test --type=ts🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-sql/4-lanes/sql-builder/src/expression.ts` around lines 81 - 90, The trait-only branch of the ResolveExtArg type is too strict and disallows raw literals; update the second conditional branch in ResolveExtArg so it returns the same ExpressionOrValue wrapper as the codecId branch (using CodecIdsWithTrait<CT, T> and the inferred N) instead of plain Expression, so trait-targeted args accept literals; locate ResolveExtArg in expression.ts and replace the trait branch's return type to ExpressionOrValue<{ codecId: CodecIdsWithTrait<CT, T>; nullable: N }, CT> (keeping the existing inference of T and N).
🤖 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/3-extensions/sql-orm-client/src/model-accessor.ts`:
- Around line 59-75: The current indexing and lookup widen trait matching by
indexing ops under each individual trait and later unioning them, and the else
if prevents trait indexing when a self arg has both codecId and traits; update
the code so codecId and trait paths run independently (remove the else if
branching) and, when selecting trait-based candidates from opsByTrait for a
field, only include operations whose self.traits are a subset of the field’s
traits by filtering with self.traits.every(t => field.traits.includes(t))
(reference opsByCodecId, opsByTrait, and the self.traits usage and the matching
logic that currently unions trait candidates).
---
Duplicate comments:
In `@packages/2-sql/1-core/contract/src/types.ts`:
- Around line 167-170: The QueryOperationArgSpec currently declares readonly
traits?: string which weakens type safety; change this field to use the existing
canonical codec-trait alias (e.g. CodecTrait or CodecTraits) used across the
repo instead of plain string. Locate the type QueryOperationArgSpec in types.ts
and replace traits?: string with the correct canonical type (nullable/optional
as appropriate), but first confirm which alias name is present in the codebase
(search for CodecTrait or CodecTraits) and import or reference that alias so the
field aligns with the shared codec-trait union.
In `@packages/2-sql/4-lanes/sql-builder/src/expression.ts`:
- Around line 81-90: The trait-only branch of the ResolveExtArg type is too
strict and disallows raw literals; update the second conditional branch in
ResolveExtArg so it returns the same ExpressionOrValue wrapper as the codecId
branch (using CodecIdsWithTrait<CT, T> and the inferred N) instead of plain
Expression, so trait-targeted args accept literals; locate ResolveExtArg in
expression.ts and replace the trait branch's return type to ExpressionOrValue<{
codecId: CodecIdsWithTrait<CT, T>; nullable: N }, CT> (keeping the existing
inference of T and N).
🪄 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: 5496665e-609a-486b-b3fa-d7bafa07ab92
⛔ Files ignored due to path filters (3)
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/**test/e2e/framework/test/fixtures/generated/contract.d.tsis excluded by!**/generated/**
📒 Files selected for processing (28)
examples/prisma-next-demo/src/prisma/contract.d.tspackages/1-framework/1-core/operations/src/index.tspackages/2-sql/1-core/contract/src/exports/types.tspackages/2-sql/1-core/contract/src/types.tspackages/2-sql/4-lanes/relational-core/src/ast/types.tspackages/2-sql/4-lanes/relational-core/test/ast/predicate.test.tspackages/2-sql/4-lanes/sql-builder/src/expression.tspackages/2-sql/4-lanes/sql-builder/src/scope.tspackages/2-sql/4-lanes/sql-builder/test/integration/extension-functions.test.tspackages/2-sql/4-lanes/sql-builder/test/playground/extension-functions.test-d.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/filters.test.tspackages/3-extensions/sql-orm-client/test/generated-contract-types.test-d.tspackages/3-extensions/sql-orm-client/test/model-accessor.test.tspackages/3-extensions/sql-orm-client/test/orm.types.test-d.tspackages/3-extensions/sql-orm-client/test/where-binding.test.tspackages/3-targets/6-adapters/postgres/package.jsonpackages/3-targets/6-adapters/postgres/src/core/adapter.tspackages/3-targets/6-adapters/postgres/src/core/descriptor-meta.tspackages/3-targets/6-adapters/postgres/src/exports/operation-types.tspackages/3-targets/6-adapters/postgres/src/exports/runtime.tspackages/3-targets/6-adapters/postgres/src/types/operation-types.tspackages/3-targets/6-adapters/postgres/tsdown.config.tspackages/3-targets/6-adapters/sqlite/src/core/adapter.tspackages/3-targets/6-adapters/sqlite/test/adapter.test.tstest/integration/test/fixtures/contract.d.ts
💤 Files with no reviewable changes (5)
- packages/3-extensions/sql-orm-client/test/where-binding.test.ts
- packages/3-targets/6-adapters/sqlite/test/adapter.test.ts
- packages/3-targets/6-adapters/sqlite/src/core/adapter.ts
- packages/2-sql/4-lanes/relational-core/test/ast/predicate.test.ts
- packages/3-targets/6-adapters/postgres/src/core/adapter.ts
✅ Files skipped from review due to trivial changes (6)
- packages/3-targets/6-adapters/postgres/src/exports/operation-types.ts
- packages/3-targets/6-adapters/postgres/package.json
- packages/2-sql/1-core/contract/src/exports/types.ts
- packages/3-extensions/sql-orm-client/test/generated-contract-types.test-d.ts
- packages/2-sql/4-lanes/sql-builder/test/integration/extension-functions.test.ts
- packages/3-targets/6-adapters/postgres/src/core/descriptor-meta.ts
🚧 Files skipped from review as they are similar to previous changes (12)
- packages/3-targets/6-adapters/postgres/tsdown.config.ts
- packages/3-extensions/sql-orm-client/test/orm.types.test-d.ts
- test/integration/test/fixtures/contract.d.ts
- packages/2-sql/4-lanes/sql-builder/src/scope.ts
- packages/3-targets/6-adapters/postgres/src/types/operation-types.ts
- packages/3-extensions/sql-orm-client/test/model-accessor.test.ts
- examples/prisma-next-demo/src/prisma/contract.d.ts
- packages/3-extensions/sql-orm-client/test/filters.test.ts
- packages/3-extensions/sql-orm-client/test/extension-operations.test-d.ts
- packages/2-sql/4-lanes/sql-builder/test/playground/extension-functions.test-d.ts
- packages/1-framework/1-core/operations/src/index.ts
- packages/3-extensions/sql-orm-client/src/types.ts
SevInf
force-pushed
the
worktree/fix-ilike
branch
from
5195631 to
b57097f
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/2-sql/4-lanes/sql-builder/src/expression.ts (1)
81-90:⚠️ Potential issue | 🟡 MinorKeep trait-targeted args symmetric with codecId-targeted args.
Line 90 only accepts
Expression<...>for trait-gated params, while line 85 accepts raw inputs viaExpressionOrValue. That makes trait-targeted operation args stricter than codecId-targeted ones and can block valid literal inputs for adapter ops.Suggested fix
- ? Expression<{ codecId: CodecIdsWithTrait<CT, T>; nullable: N }> + ? ExpressionOrValue<{ codecId: CodecIdsWithTrait<CT, T>; nullable: N }, CT>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-sql/4-lanes/sql-builder/src/expression.ts` around lines 81 - 90, ResolveExtArg is asymmetric: the codecId branch returns ExpressionOrValue but the traits branch returns only Expression, disallowing literal inputs for trait-gated params; change the traits branch to return ExpressionOrValue as well (i.e., replace Expression<{ codecId: CodecIdsWithTrait<CT, T>; nullable: N }> with ExpressionOrValue<{ codecId: CodecIdsWithTrait<CT, T>; nullable: N }, CT>) so trait-targeted args accept both Expressions and raw values consistent with the codecId path.
🤖 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/3-extensions/sql-orm-client/test/extension-operations.test-d.ts`:
- Around line 96-101: Update the type assertion in the 'ilike returns
AnyExpression (predicate)' test to use the non-deprecated matcher: replace the
expectTypeOf<ReturnType<IlikeFn>>().toMatchTypeOf<import('@prisma-next/sql-relational-core/ast').AnyExpression>()
call with
expectTypeOf<ReturnType<IlikeFn>>().toExtend<import('@prisma-next/sql-relational-core/ast').AnyExpression>();
locate the test by the IlikeFn alias (PostAccessor['title']['ilike']) and adjust
only the matcher name so the test verifies the return type extends
AnyExpression.
---
Duplicate comments:
In `@packages/2-sql/4-lanes/sql-builder/src/expression.ts`:
- Around line 81-90: ResolveExtArg is asymmetric: the codecId branch returns
ExpressionOrValue but the traits branch returns only Expression, disallowing
literal inputs for trait-gated params; change the traits branch to return
ExpressionOrValue as well (i.e., replace Expression<{ codecId:
CodecIdsWithTrait<CT, T>; nullable: N }> with ExpressionOrValue<{ codecId:
CodecIdsWithTrait<CT, T>; nullable: N }, CT>) so trait-targeted args accept both
Expressions and raw values consistent with the codecId path.
🪄 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: 04848f22-85e2-4aac-a0c3-4dd505ccf86f
⛔ Files ignored due to path filters (3)
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/**test/e2e/framework/test/fixtures/generated/contract.d.tsis excluded by!**/generated/**
📒 Files selected for processing (28)
examples/prisma-next-demo/src/prisma/contract.d.tspackages/1-framework/1-core/operations/src/index.tspackages/2-sql/1-core/contract/src/exports/types.tspackages/2-sql/1-core/contract/src/types.tspackages/2-sql/4-lanes/relational-core/src/ast/types.tspackages/2-sql/4-lanes/relational-core/test/ast/predicate.test.tspackages/2-sql/4-lanes/sql-builder/src/expression.tspackages/2-sql/4-lanes/sql-builder/src/scope.tspackages/2-sql/4-lanes/sql-builder/test/integration/extension-functions.test.tspackages/2-sql/4-lanes/sql-builder/test/playground/extension-functions.test-d.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/filters.test.tspackages/3-extensions/sql-orm-client/test/generated-contract-types.test-d.tspackages/3-extensions/sql-orm-client/test/model-accessor.test.tspackages/3-extensions/sql-orm-client/test/orm.types.test-d.tspackages/3-extensions/sql-orm-client/test/where-binding.test.tspackages/3-targets/6-adapters/postgres/package.jsonpackages/3-targets/6-adapters/postgres/src/core/adapter.tspackages/3-targets/6-adapters/postgres/src/core/descriptor-meta.tspackages/3-targets/6-adapters/postgres/src/exports/operation-types.tspackages/3-targets/6-adapters/postgres/src/exports/runtime.tspackages/3-targets/6-adapters/postgres/src/types/operation-types.tspackages/3-targets/6-adapters/postgres/tsdown.config.tspackages/3-targets/6-adapters/sqlite/src/core/adapter.tspackages/3-targets/6-adapters/sqlite/test/adapter.test.tstest/integration/test/fixtures/contract.d.ts
💤 Files with no reviewable changes (5)
- packages/3-targets/6-adapters/postgres/src/core/adapter.ts
- packages/3-targets/6-adapters/sqlite/test/adapter.test.ts
- packages/3-extensions/sql-orm-client/test/where-binding.test.ts
- packages/3-targets/6-adapters/sqlite/src/core/adapter.ts
- packages/2-sql/4-lanes/relational-core/test/ast/predicate.test.ts
✅ Files skipped from review due to trivial changes (9)
- packages/3-extensions/sql-orm-client/test/orm.types.test-d.ts
- packages/3-extensions/sql-orm-client/test/generated-contract-types.test-d.ts
- packages/3-targets/6-adapters/postgres/tsdown.config.ts
- packages/2-sql/4-lanes/sql-builder/test/integration/extension-functions.test.ts
- packages/3-targets/6-adapters/postgres/src/exports/operation-types.ts
- packages/2-sql/1-core/contract/src/exports/types.ts
- packages/3-targets/6-adapters/postgres/package.json
- packages/2-sql/4-lanes/relational-core/src/ast/types.ts
- packages/3-extensions/sql-orm-client/src/types.ts
🚧 Files skipped from review as they are similar to previous changes (11)
- packages/3-extensions/sql-orm-client/test/filters.test.ts
- packages/2-sql/4-lanes/sql-builder/test/playground/extension-functions.test-d.ts
- packages/3-targets/6-adapters/postgres/src/core/descriptor-meta.ts
- packages/2-sql/4-lanes/sql-builder/src/scope.ts
- packages/3-extensions/sql-orm-client/test/model-accessor.test.ts
- packages/3-targets/6-adapters/postgres/src/exports/runtime.ts
- packages/3-targets/6-adapters/postgres/src/types/operation-types.ts
- packages/1-framework/1-core/operations/src/index.ts
- packages/2-sql/1-core/contract/src/types.ts
- packages/3-extensions/sql-orm-client/src/model-accessor.ts
- examples/prisma-next-demo/src/prisma/contract.d.ts
SevInf
force-pushed
the
worktree/fix-ilike
branch
5 times, most recently
from
65d5bf2 to
bf38492
Compare
wmadden
approved these changes
Apr 20, 2026
Contributor
wmadden
left a comment
wmadden
left a comment
There was a problem hiding this comment.
Approving preemptively — the architectural direction (moving ilike out of core, introducing trait-targeted extension-operation args) is right and the implementation is clean. Findings below are concentrated around the type/runtime representation mismatch for traits, validation gaps for the new optional-arg shape, and one missing emitter test. Please address or respond to the inline comments before merging — no re-review needed.
Full artifacts (local): wip/review-code/pr-330/{spec,system-design-review,code-review,walkthrough}.md.
What looks solid:
- Layering is correct —
ilikeis no longer a coreBinaryOp, and adapters now own their operators via the samequeryOperationTypesimport wiring as pgvector. - Predicate detection (
IsBooleanReturn+ thecreateExtensionMethodFactoryshort-circuit) is a real correctness win — boolean-returning ops now form WHERE-clause predicates instead of accidentally proxyingComparisonMethods. - Deletions are clean and in lockstep — SQLite throw guard, the unreachable operator-map entry, and the corresponding adapter test all go together with the core AST removal.
- Type-level negative tests (
@ts-expect-erroronmetadata.ilike,views.ilike) lock in the gating.
Deferred items (D01–D04 in code-review.md) are intentionally out of scope.
| @@ -1,11 +1,17 @@ | |||
| export interface ParamSpec { | |||
| readonly codecId?: string; | |||
| readonly traits?: readonly string[]; | |||
Contributor
There was a problem hiding this comment.
F01 — traits shape diverges between runtime and contract-type layers
Runtime ParamSpec.traits: readonly string[] here disagrees with contract-type QueryOperationArgSpec.traits: CodecTrait (single literal) in packages/2-sql/1-core/contract/src/types.ts:170. At runtime the model-accessor uses self.traits.every((t) => codecTraits.includes(t)) — AND over a list. At the type level, only a single trait is exposed. Today every registered op is single-trait, so the asymmetry is latent, but:
OpMatchesField'sRequiredTraits extends FieldTraitsarm distributes ifRequiredTraitsis ever a union, producingtrue | false, which fails theextends truecheck upstream — the type-level semantics for a multi-trait arg become "no match" even when all traits are present.- Authors writing a future adapter op must remember the type-level shape is fundamentally narrower than what the runtime registrar supports.
Suggestion: Commit to a single representation at both layers, then encode that in the types. Either:
- (a) Narrow the runtime to a single trait (simplest; matches current usage and the contract-type shape), or
- (b) Generalize both to a tuple/array with explicit AND semantics (
readonly traits?: readonly CodecTrait[]) and updateOpMatchesField/CodecIdsWithTraitto intersect over the tuple.
Option (a) example:
export interface ParamSpec {
readonly codecId?: string;
readonly traits?: string;
readonly nullable: boolean;
}// postgres descriptor-meta.ts
{ traits: 'textual', nullable: false },// model-accessor.ts trait branch
} else if (self?.traits) {
for (const codec of context.codecs.values()) {
if (codec.traits?.includes(self.traits as CodecTrait)) {
registerOp(codec.id, op);
}
}
}| @@ -1,11 +1,17 @@ | |||
| export interface ParamSpec { | |||
| readonly codecId?: string; | |||
Contributor
There was a problem hiding this comment.
F02 — No validation that a ParamSpec has at least one of codecId / traits
Both fields are now optional, but the registry (createOperationRegistry.register, lines 26–45 in this file) accepts any shape. A descriptor with neither codecId nor traits on its self arg will silently register against nothing — the op exists in the contract's queryOperations but never attaches to any field. Easy author error to introduce (copy-paste, delete a field) and hard to diagnose from the outside.
Suggestion: Assert at registration time that every arg has at least one of codecId / traits. Throw in register so the error surfaces at descriptor load time:
register(descriptor: OperationDescriptor<T>) {
if (descriptor.method in operations) {
throw new Error(`Operation "${descriptor.method}" is already registered`);
}
for (const [i, arg] of descriptor.args.entries()) {
if (!arg.codecId && !arg.traits?.length) {
throw new Error(
`Operation "${descriptor.method}" arg[${i}] has neither codecId nor traits`,
);
}
}
// ... existing code
}Pairs naturally with F06 (the silent codecId/traits precedence in model-accessor).
| export type QueryOperationTypes = SqlQueryOperationTypes<{ | ||
| readonly ilike: { | ||
| readonly args: readonly [ | ||
| { readonly traits: 'textual'; readonly nullable: boolean }, |
Contributor
There was a problem hiding this comment.
F03 — Type-level first-arg nullable is boolean but runtime sets false
This hand-authored type declares { readonly traits: 'textual'; readonly nullable: boolean } — boolean means "either" — yet the runtime descriptor in postgres/src/core/descriptor-meta.ts:135 fixes nullable: false. Small drift, but real: the type is more permissive than the runtime, future authors may copy this and hit mismatches, and the SQL builder cannot rely on the self arg being non-null at the type level.
Also worth noting: OpMatchesField currently doesn't consult nullable at all when matching, so this flag on the self arg has no effect on field surfacing either way. That may be intentional (ilike handles nulls fine in SQL), but a one-line comment would help a future reader distinguish "intentionally ignored" from "forgot to check".
Suggestion: Make the type match the runtime (nullable: false for the self arg). If boolean is principled, add a one-line comment explaining why.
Comment on lines
+252
to
+266
| type OpMatchesField<Op, CodecId extends string, CT extends Record<string, unknown>> = Op extends { | ||
| readonly args: readonly [infer Self, ...(readonly unknown[])]; | ||
| } | ||
| ? Self extends { readonly codecId: CodecId } | ||
| ? true | ||
| : Self extends { readonly traits: infer RequiredTraits extends string } | ||
| ? CodecId extends keyof CT | ||
| ? CT[CodecId] extends { readonly traits: infer FieldTraits } | ||
| ? RequiredTraits extends FieldTraits | ||
| ? true | ||
| : false | ||
| : false | ||
| : false | ||
| : false | ||
| : false; |
Contributor
There was a problem hiding this comment.
F04 — OpMatchesField multi-trait match has distribution hazard
The conditional RequiredTraits extends FieldTraits distributes over RequiredTraits. For single-trait self args (today's only case) this is fine. For a hypothetical traits: 'textual' | 'numeric', TS distributes the check, producing boolean instead of true/false, and the outer ... extends true ? OpName : never evaluates to never — the op silently disappears from the field's method surface. The symmetric hazard exists in CodecIdsWithTrait in packages/2-sql/4-lanes/sql-builder/src/expression.ts (~lines 70–80).
Suggestion: Either lock the contract-type representation to single traits only (see F01 option a), or wrap trait checks in tuple form ([RequiredTraits] extends [FieldTraits]) and switch to tuple-array AND semantics. The [T] extends [U] trick disables distribution and would match the runtime's AND intent for arrays of traits if the shape is generalized later.
Comment on lines
+271
to
+277
| queryOperationTypes: { | ||
| import: { | ||
| package: '@prisma-next/adapter-postgres/operation-types', | ||
| named: 'QueryOperationTypes', | ||
| alias: 'PgAdapterQueryOps', | ||
| }, | ||
| }, |
Contributor
There was a problem hiding this comment.
F05 — No emitter end-to-end test for adapter-owned QueryOperationTypes
The PR description concedes: "The emitter pipeline should be validated end-to-end … but no e2e emit test exists yet." Five contract.d.ts fixtures were updated to include PgAdapterQueryOps. If they were regenerated from the emitter, the test plan is implicit; if hand-edited, the emitter never ran over these shapes. Either way, there is no guard against a regression where this queryOperationTypes.import block is dropped by the emitter. Given the whole surface gating depends on this import appearing in the generated contract.d.ts, the coverage gap is significant.
Suggestion: Add a focused emitter test that runs the real generator on a minimal postgres-adapter contract and asserts the presence of:
- the
import type { QueryOperationTypes as PgAdapterQueryOps } …line, and - the
PgAdapterQueryOps & …intersection in the emittedQueryOperationTypesalias.
packages/2-sql/3-tooling/emitter/test/emitter-hook.generation.basic.test.ts already exercises queryOperationTypeImports; the new test can mirror that pattern.
Comment on lines
+70
to
+77
| if (self?.codecId) { | ||
| registerOp(self.codecId, op); | ||
| } else if (self?.traits) { | ||
| for (const codec of context.codecs.values()) { | ||
| const codecTraits: readonly string[] = codec.traits ?? []; | ||
| if (self.traits.every((t) => codecTraits.includes(t))) { | ||
| registerOp(codec.id, op); | ||
| } |
Contributor
There was a problem hiding this comment.
F06 — Trait-registration loop silently ignores ops whose self arg sets both codecId and traits
The branch is if (self?.codecId) { … } else if (self?.traits) { … }. An op declaring both fields gets registered only by codecId, dropping the trait path. Probably correct semantically (codec-id is strictly more specific) but a silent precedence rule that deserves either a comment or an explicit guard.
Suggestion: Add a short comment to the effect of "codecId takes precedence when both are set", or — if "both is illegal" is the intended contract — assert it (if (self.codecId && self.traits) throw …). Pairs naturally with F02.
Comment on lines
+58
to
+65
| function registerOp(codecId: string, op: [string, SqlOperationEntry]) { | ||
| let existing = opsByCodecId.get(codecId); | ||
| if (!existing) { | ||
| existing = []; | ||
| opsByCodecId.set(codecId, existing); | ||
| } | ||
| existing.push(op); | ||
| } |
Contributor
There was a problem hiding this comment.
F07 — [string, SqlOperationEntry] tuple rebuilt per op; minor but worth a named alias
The [string, SqlOperationEntry] shape is used as both the Map value element and a per-op tuple, with an inline type annotation at the call site (line 69). This reads awkwardly and obscures the intent (the tuple is really "op name + entry"). Not a correctness issue — readability only.
Suggestion: A local type alias, e.g.
type NamedOp = readonly [name: string, entry: SqlOperationEntry];would name the pair and communicate that it's positional. registerOp then becomes registerOp(codecId, op: NamedOp).
Comment on lines
+67
to
+80
| it('creates ilike as trait-matched extension operation returning predicate', () => { | ||
| const user = createModelAccessor(context, 'User'); | ||
| const ilike = user['name']!.ilike; | ||
| const result = ilike('%ali%'); | ||
| expect(result).toBeInstanceOf(OperationExpr); | ||
| const op = result as OperationExpr; | ||
| expect(op.method).toBe('ilike'); | ||
| expect(op.self).toEqual(ColumnRef.of('users', 'name')); | ||
| }); | ||
|
|
||
| it('does not expose ilike on non-textual fields', () => { | ||
| const post = createModelAccessor(context, 'Post'); | ||
| const field = post['views'] as unknown as Record<string, unknown>; | ||
| expect(field['ilike']).toBeUndefined(); |
Contributor
There was a problem hiding this comment.
F10 — Consider one multi-trait-subset regression test
These new tests validate the happy path (single-trait ilike on users.name) and the negative path (not on views). They don't pin the trait-subset AND semantics at the runtime layer — a single-trait test doesn't distinguish every from some. A tiny regression test with a synthetic op requiring two traits (e.g. ['equality', 'textual']) and fields with one vs both would lock in the runtime contract and make any future change to the registration loop a conscious edit.
Suggestion: Add one unit test that constructs a synthetic context with a multi-trait op and asserts it attaches only to codecs whose traits are a superset. Pairs naturally with either direction of F01.
…tions Move ilike from a core comparison method to a postgres adapter-provided extension operation. Operation args now support targeting by trait (e.g., `traits: ['textual']`) in addition to codecId, allowing adapter operations to match any codec with the required traits. - Extend ParamSpec and QueryOperationArgSpec with optional `traits` field - Add FieldSpec/TraitField to sql-builder scope for trait-targeted expressions - Update FieldOperations type matching to support trait-based operations - Boolean-trait return detection makes predicate ops return AnyExpression - Register ilike in postgres adapter's queryOperations() - Remove ilike from BinaryOp union, BinaryExpr, ComparisonMethodFns - Remove ilike runtime guard from SQLite adapter - Fix tsdown exports stripping ./schema-sql in sql-contract-ts
SevInf
force-pushed
the
worktree/fix-ilike
branch
from
bf38492 to
bc433ef
Compare
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-2238
Intent
ilike(case-insensitive LIKE) is a PostgreSQL-specific operator that has no SQLite equivalent. Until now it lived in core as aBinaryOp/ComparisonMethod, available on every textual field regardless of target. SQLite threw a runtime error when it encountered anilikeAST node. This PR movesiliketo the postgres adapter so it is only available at the type level for postgres contracts — SQLite contracts never see it.The deeper change: operation args can now target by codec trait (e.g.
traits: 'textual') in addition to exactcodecId. This lets a single adapter-provided operation match every codec that carries a given trait, without enumerating codec IDs.Change map
ParamSpecgains optionaltraits(runtime:readonly string[]). NewReturnSpec(requiredcodecId) split fromParamSpecforOperationEntry.returns.QueryOperationArgSpecwith optionaltraits: string(type-level union).QueryOperationTypeEntry.argsuses it.TraitField/FieldSpecscope types.Expression<T>broadened toFieldSpec.ResolveExtArg+CodecIdsWithTraitresolve trait args to the union of matching codec IDs.ilikeremoved fromComparisonMethodFnsandCOMPARISON_METHODS_META. NewOpMatchesFieldhelper does codecId-exact OR trait-subset matching.IsBooleanReturndetects predicate ops → returnAnyExpressioninstead ofComparisonMethods.opsByTraitindex alongsideopsByCodecId.createExtensionMethodFactoryreturnsOperationExprdirectly for boolean-returning ops.postgresQueryOperationsarray with ilike descriptor (traits: ['textual'], template{{self}} ILIKE {{arg0}}).queryOperations()wired into runtime descriptor. Newoperation-typesexport.ilikeremoved fromoperatorMap.ilikethrow guard andoperatorMapentry removed.'ilike'removed fromBinaryOpunion andBinaryExpr.ilike()static method.customExportshook preserves./schema-sqlJSON export that tsdown's export-sync was stripping.The story
Trait-targeted args —
ParamSpecandQueryOperationArgSpecgain an optionaltraitsfield. At the type level it's a string union ('textual'); at runtime it's an array (['textual']). This is the foundational primitive that lets operations match by codec capability rather than identity.ilike leaves core — Removed from
BinaryOp,BinaryExpr,ComparisonMethodFns, andCOMPARISON_METHODS_META. The AST, adapters, and ORM no longer treat it as a built-in comparison.Postgres adapter registers ilike — A new
postgresQueryOperationsarray defines ilike with{ traits: ['textual'] }as self arg,pg/text@1as pattern arg,pg/bool@1as return, and infix template{{self}} ILIKE {{arg0}}. The adapter's runtime descriptor exposesqueryOperations(). Contract emission picks it up viatypes.queryOperationTypesin descriptor meta.Type-level matching —
OpMatchesFieldchecks whether an operation's self arg matches a field: either exactcodecIdmatch (existing pgvector path) or trait-subset match (new).FieldOperationsuses this to surface ilike on text fields in postgres contracts and hide it from sqlite contracts.Predicate detection —
IsBooleanReturnchecks whether the return codec has the'boolean'trait. If so,QueryOperationMethodreturnsAnyExpressiondirectly (a WHERE predicate), notComparisonMethods<...>. At runtime,createExtensionMethodFactoryreturns theOperationExprdirectly for boolean-returning ops. This meansuser.name.ilike('%alice%')is a predicate expression, not a chainable value.SQL builder support —
Expression<T>now acceptsFieldSpec(=ScopeField | TraitField).ResolveExtArgmaps trait-targeted args toExpression<{ codecId: CodecIdsWithTrait<CT, T> }>, sofns.ilike(f.name, '%test%')accepts any textual column expression.Behavior changes & evidence
ilike: extension-operations.test-d.ts —PostAccessor['title']hasilikeproperty, returnsAnyExpression.ilike: Same file —PostAccessor['views']andPostAccessor['embedding']do not exposeilike.ilike: NoQueryOperationTypesfrom the adapter →FieldOperationsfinds nothing → field type has noilike.fns.ilike(f.name, '%alice%')typechecks and returnsExpression<BooleanCodecType>.Compatibility / migration / risk
.ilike()on a SQLite contract field will get a compile error instead of a runtime error. This is the intended improvement.ParamSpec.codecIdis always present needs adjustment.ReturnSpec(new) guaranteescodecIdfor return types.pg/bool@1codec will automatically be treated as a predicate (returnsAnyExpression). This is the correct behavior for boolean-returning operations.Follow-ups / open questions
QueryOperationTypesshould includePgAdapterQueryOpsautomatically (the descriptor meta is wired, but no e2e emit test exists yet).~,~*,SIMILAR TO) can follow the same pattern.Non-goals / intentionally out of scope
OperationTypes(codec-keyed) for the postgres adapter — onlyQueryOperationTypes(flat) is needed since trait matching handles the codec-to-operation mapping.Summary by CodeRabbit
New Features
Improvements
Behavioral Changes
Tests