feat: plugin-extended query args #599
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughRefactors CRUD operation types into const-backed categorized sets, threads per-operation ExtQueryArgs through client/plugin/validator APIs, moves input validation to a per-client InputValidator, updates handler and plugin hook signatures to be operation-aware, and adds E2E tests and generated types for ext-query-args. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
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
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (1)tests/e2e/**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (8)📓 Common learnings📚 Learning: 2025-11-26T01:55:04.540ZApplied to files:
📚 Learning: 2025-11-26T01:55:04.540ZApplied to files:
📚 Learning: 2025-11-26T01:55:04.540ZApplied to files:
📚 Learning: 2025-11-26T01:55:04.540ZApplied to files:
📚 Learning: 2025-11-26T01:55:04.540ZApplied to files:
📚 Learning: 2025-11-26T01:55:04.540ZApplied to files:
📚 Learning: 2025-12-30T15:07:06.254ZApplied to files:
🧬 Code graph analysis (1)packages/orm/src/client/crud/validator/index.ts (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🔇 Additional comments (12)
✏️ Tip: You can disable this entire section by setting 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 |
|
Claude finished @ymc9's task —— View job Code Review CompleteI've conducted a thorough review of the "WIP: plugin custom options" PR. This feature implements extended query arguments for plugins, allowing them to add custom options to ORM operations while maintaining type safety and validation. Todo List:
📋 SummaryThis PR introduces a powerful plugin extension mechanism that allows plugins to add custom query arguments to any ORM operation. The implementation is well-architected with proper type safety, validation, and comprehensive test coverage. ✅ Strengths🏗️ Excellent Architecture
🔒 Robust Input Validation
🧪 Comprehensive Test Coverage
📊 Performance Conscious
|
There was a problem hiding this comment.
Pull request overview
This PR introduces a plugin infrastructure feature that allows plugins to extend query arguments for CRUD operations. The changes enable plugins to define custom options (like cache TTL settings) that can be passed to database operations and validated using Zod schemas.
Changes:
- Added
ExtQueryArgstype system to allow plugins to extend query arguments for specific or all CRUD operations - Refactored CRUD operation types from string unions to const arrays with exported values
- Updated the input validator to merge plugin-provided Zod schemas with base operation schemas
- Added comprehensive test coverage for the extended query args feature
Reviewed changes
Copilot reviewed 18 out of 20 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/e2e/package.json | Added zod dependency for testing plugin validation schemas |
| tests/e2e/orm/plugin-infra/ext-query-args/* | New test schema, models, and input types for testing extended query args |
| tests/e2e/orm/plugin-infra/ext-query-args.test.ts | Comprehensive tests for plugin extended query args feature |
| samples/orm/package.json | Added zod dependency to sample project |
| samples/orm/main.ts | Example usage of definePlugin with custom options (incomplete implementation) |
| pnpm-lock.yaml | Updated lock file with new dependencies including zod 4.1.12 |
| packages/plugins/policy/src/plugin.ts | Updated PolicyPlugin to match new RuntimePlugin signature |
| packages/orm/src/utils/type-utils.ts | Added Exact utility type for type-level validation |
| packages/orm/src/client/plugin.ts | Added ExtQueryArgsBase type and updated RuntimePlugin to support extended query args |
| packages/orm/src/client/options.ts | Renamed OmitOptions to OmitConfig and updated plugin type to accept any ExtQueryArgs |
| packages/orm/src/client/index.ts | Exported CRUD operation constant arrays for external use |
| packages/orm/src/client/crud/validator/index.ts | Added mergePluginArgsSchema method to integrate plugin validation schemas |
| packages/orm/src/client/crud/operations/find.ts | Updated to pass operation name to validator instead of options object |
| packages/orm/src/client/crud/operations/base.ts | Converted operation types to const arrays with exported values |
| packages/orm/src/client/crud-types.ts | Minor formatting changes to ModelResult type |
| packages/orm/src/client/contract.ts | Added ExtractExtQueryArgs utility and updated all operation signatures to accept extended args |
| packages/orm/src/client/client-impl.ts | Updated findMany to accept plugin options parameter (incomplete for other operations) |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/orm/src/client/crud/operations/find.ts (1)
6-21: Don’t clobber caller-providedtakefor find-first semantics.
Right nowtakeis always forced to1, which discards explicittakevalues (e.g., negativetaketo reverse ordering). Consider defaulting only when it’s undefined.🔧 Suggested fix
- if (findOne) { - // ensure "limit 1" - parsedArgs = parsedArgs ?? {}; - parsedArgs.take = 1; - } + if (findOne) { + // ensure "limit 1" when not explicitly set + parsedArgs = parsedArgs ?? {}; + if (parsedArgs.take === undefined) { + parsedArgs.take = 1; + } + }
🤖 Fix all issues with AI agents
In `@samples/orm/main.ts`:
- Line 47: The call to db.user.findMany is returning a promise that is not
awaited and its result is discarded; either await the promise (e.g., prepend
await to db.user.findMany in an async context) or remove the call, or if this is
intentional demo code add a clear inline comment explaining the discarded
result/extended-args demonstration; locate the invocation of db.user.findMany in
samples/orm/main.ts and update it accordingly.
- Line 28: The anonymous resolver listPublicPosts currently uses an empty object
destructuring pattern "({})" which triggers the linter; change its parameter to
either no params "()" or a single unused placeholder like "(_)" in the
listPublicPosts arrow function to silence the linter and indicate the parameter
is intentionally unused.
🧹 Nitpick comments (4)
samples/orm/main.ts (1)
32-40: Remove or document commented-out code.This commented block appears to be leftover from a previous implementation. Consider removing it or adding a TODO comment explaining why it's kept.
packages/orm/src/client/client-impl.ts (1)
452-467: Context object typed asanybypasses type safety.The
ctxobject is explicitly typed asanyon line 453, which bypasses TypeScript's type checking. Based on learnings,ts-expect-errorand loose typing should be avoided in production code. Consider using a proper interface for the context object.Suggested approach
Define a typed context interface that includes the optional
optionsproperty:interface OnQueryContext { client: ClientContract<any>; model: string; operation: AllCrudOperations; args: unknown; proceed: (nextArgs: unknown) => Promise<unknown>; options?: unknown; }Then use this type instead of
any:- proceed = (_args: unknown) => { - const ctx: any = { + proceed = (_args: unknown) => { + const ctx: OnQueryContext = {packages/orm/src/client/plugin.ts (2)
75-83: Consider documenting the second overload or removing dead code path.The
definePluginimplementation acceptspluginOrSchemaandpluginparameters, but:
- Only one overload signature is declared (lines 75-77)
- The implementation checks
plugin ?? pluginOrSchema, suggesting a second overload pattern that isn't exposedIf a second overload (schema, plugin) was removed, the
pluginparameter and conditional logic may be dead code. If it's intended for future use, consider adding the overload signature or a comment explaining the intent.If the second overload is needed, add the signature:
export function definePlugin<ExtQueryArgs extends ExtQueryArgsBase = {}, Schema extends SchemaDef = SchemaDef>( plugin: RuntimePlugin<Exact<ExtQueryArgs, ExtQueryArgsBase>, Schema>, ): RuntimePlugin<ExtQueryArgs, Schema>; + +export function definePlugin<ExtQueryArgs extends ExtQueryArgsBase = {}, Schema extends SchemaDef = SchemaDef>( + schema: Schema, + plugin: RuntimePlugin<Exact<ExtQueryArgs, ExtQueryArgsBase>, Schema>, +): RuntimePlugin<ExtQueryArgs, Schema>; export function definePlugin(pluginOrSchema: unknown, plugin?: unknown) { // If plugin is provided, it's the second overload (schema, plugin) // Otherwise, it's the first overload (plugin only) return plugin ?? pluginOrSchema; }
2-2: Use explicit generic parameters forZodObjecttype.The
ZodObjecttype from Zod is generic and should specify type parameters for the object shape and optional unknown-key behavior (e.g.,ZodObject<any>orZodObject<Record<string, any>>). BareZodObjectusage without parameters reduces type specificity on line 68.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (19)
packages/orm/src/client/client-impl.tspackages/orm/src/client/contract.tspackages/orm/src/client/crud-types.tspackages/orm/src/client/crud/operations/base.tspackages/orm/src/client/crud/operations/find.tspackages/orm/src/client/crud/validator/index.tspackages/orm/src/client/index.tspackages/orm/src/client/options.tspackages/orm/src/client/plugin.tspackages/orm/src/utils/type-utils.tspackages/plugins/policy/src/plugin.tssamples/orm/main.tssamples/orm/package.jsontests/e2e/orm/plugin-infra/ext-query-args.test.tstests/e2e/orm/plugin-infra/ext-query-args/input.tstests/e2e/orm/plugin-infra/ext-query-args/models.tstests/e2e/orm/plugin-infra/ext-query-args/schema.tstests/e2e/orm/plugin-infra/ext-query-args/schema.zmodeltests/e2e/package.json
🧰 Additional context used
📓 Path-based instructions (2)
tests/e2e/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
E2E tests should validate real-world schema compatibility with established projects
Files:
tests/e2e/orm/plugin-infra/ext-query-args/models.tstests/e2e/orm/plugin-infra/ext-query-args.test.tstests/e2e/orm/plugin-infra/ext-query-args/schema.tstests/e2e/orm/plugin-infra/ext-query-args/input.ts
**/*.zmodel
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.zmodel: Always runzenstack generateafter modifying ZModel schemas
ZModel schema files should define database structure and policies that compile to TypeScript viazenstack generate
Files:
tests/e2e/orm/plugin-infra/ext-query-args/schema.zmodel
🧠 Learnings (10)
📓 Common learnings
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/orm/**/*.{ts,tsx} : Implement plugin hooks at ORM, Kysely, and entity mutation levels for query interception and customization
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/orm/**/*.{ts,tsx} : Implement plugin hooks at ORM, Kysely, and entity mutation levels for query interception and customization
Applied to files:
tests/e2e/orm/plugin-infra/ext-query-args/models.tssamples/orm/package.jsontests/e2e/orm/plugin-infra/ext-query-args.test.tspackages/orm/src/client/index.tstests/e2e/orm/plugin-infra/ext-query-args/schema.tspackages/orm/src/client/options.tstests/e2e/orm/plugin-infra/ext-query-args/schema.zmodelpackages/orm/src/client/crud/operations/find.tspackages/orm/src/client/crud/operations/base.tspackages/orm/src/client/crud/validator/index.tssamples/orm/main.tstests/e2e/orm/plugin-infra/ext-query-args/input.tspackages/orm/src/client/plugin.tspackages/orm/src/client/client-impl.tspackages/orm/src/client/contract.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to **/*.zmodel : ZModel schema files should define database structure and policies that compile to TypeScript via `zenstack generate`
Applied to files:
tests/e2e/orm/plugin-infra/ext-query-args/models.tssamples/orm/package.jsontests/e2e/orm/plugin-infra/ext-query-args/schema.tstests/e2e/orm/plugin-infra/ext-query-args/schema.zmodelpackages/orm/src/client/crud/validator/index.tssamples/orm/main.tstests/e2e/orm/plugin-infra/ext-query-args/input.tspackages/orm/src/client/plugin.tspackages/orm/src/client/contract.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to tests/e2e/**/*.{ts,tsx} : E2E tests should validate real-world schema compatibility with established projects
Applied to files:
tests/e2e/orm/plugin-infra/ext-query-args/models.tstests/e2e/orm/plugin-infra/ext-query-args.test.tstests/e2e/orm/plugin-infra/ext-query-args/schema.tstests/e2e/orm/plugin-infra/ext-query-args/schema.zmodeltests/e2e/package.jsontests/e2e/orm/plugin-infra/ext-query-args/input.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/orm/**/*.test.{ts,tsx} : ORM package tests should include comprehensive client API tests and policy tests
Applied to files:
tests/e2e/orm/plugin-infra/ext-query-args/models.tssamples/orm/package.jsontests/e2e/orm/plugin-infra/ext-query-args.test.tspackages/orm/src/client/index.tspackages/orm/src/client/options.tspackages/orm/src/client/crud/operations/base.tspackages/orm/src/client/crud/validator/index.tssamples/orm/main.tstests/e2e/orm/plugin-infra/ext-query-args/input.tspackages/orm/src/client/plugin.tspackages/orm/src/client/client-impl.tspackages/orm/src/client/contract.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/orm/**/*.{ts,tsx} : Use Kysely as the query builder interface for low-level database queries, avoiding raw SQL when possible
Applied to files:
tests/e2e/orm/plugin-infra/ext-query-args/models.tssamples/orm/package.jsontests/e2e/orm/plugin-infra/ext-query-args.test.tspackages/orm/src/client/crud/operations/find.tspackages/orm/src/client/crud/validator/index.tssamples/orm/main.tstests/e2e/orm/plugin-infra/ext-query-args/input.tspackages/orm/src/client/plugin.tspackages/orm/src/client/client-impl.tspackages/orm/src/client/contract.ts
📚 Learning: 2025-12-30T15:07:06.254Z
Learnt from: mwillbanks
Repo: zenstackhq/zenstack-v3 PR: 550
File: packages/orm/src/client/crud/operations/base.ts:158-159
Timestamp: 2025-12-30T15:07:06.254Z
Learning: Do not use ts-expect-error in production code within the zenstackhq/zenstack-v3 repo (e.g., packages/*). Use explicit type annotations, targeted type assertions, or refactor to resolve the type error. ts-expect-error may be acceptable only in test files for stubbing or temporary silencing. Ensure production code is type-safe and maintainable.
Applied to files:
packages/orm/src/utils/type-utils.tspackages/orm/src/client/crud-types.tspackages/orm/src/client/index.tspackages/plugins/policy/src/plugin.tspackages/orm/src/client/options.tspackages/orm/src/client/crud/operations/find.tspackages/orm/src/client/crud/operations/base.tspackages/orm/src/client/crud/validator/index.tspackages/orm/src/client/plugin.tspackages/orm/src/client/client-impl.tspackages/orm/src/client/contract.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to tests/**/type*.{ts,tsx} : Ensure TypeScript inference and type coverage are validated through type coverage tests
Applied to files:
tests/e2e/orm/plugin-infra/ext-query-args.test.ts
📚 Learning: 2025-10-21T16:09:31.218Z
Learnt from: ymc9
Repo: zenstackhq/zenstack-v3 PR: 319
File: packages/runtime/src/client/executor/zenstack-query-executor.ts:63-72
Timestamp: 2025-10-21T16:09:31.218Z
Learning: In ZenStack, TypeDefs can be inherited by models. When a TypeDef contains fields with `map` attributes, those mapped field names need to be processed by the QueryNameMapper since they become part of the inheriting model's schema. Therefore, when checking if a schema has mapped names (e.g., in `schemaHasMappedNames`), both `schema.models` and `schema.typeDefs` must be inspected for `@map` and `map` attributes.
Applied to files:
tests/e2e/orm/plugin-infra/ext-query-args/schema.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Database migrations should use Prisma CLI under the hood via ZenStack commands
Applied to files:
samples/orm/main.ts
🧬 Code graph analysis (6)
tests/e2e/orm/plugin-infra/ext-query-args.test.ts (5)
packages/orm/src/client/contract.ts (1)
ClientContract(74-217)packages/orm/src/client/crud/validator/index.ts (1)
schema(66-68)packages/testtools/src/client.ts (1)
createTestClient(101-248)packages/orm/src/client/plugin.ts (1)
definePlugin(79-83)packages/orm/src/client/index.ts (2)
CoreReadOperations(10-10)CoreWriteOperations(11-11)
packages/orm/src/client/options.ts (2)
packages/orm/src/client/plugin.ts (1)
RuntimePlugin(16-70)packages/schema/src/schema.ts (1)
SchemaDef(11-19)
packages/orm/src/client/crud/operations/find.ts (3)
packages/orm/src/client/crud/operations/base.ts (2)
CoreCrudOperations(60-77)CoreCrudOperations(82-82)packages/orm/src/client/index.ts (1)
CoreCrudOperations(9-9)packages/language/src/validators/function-invocation-validator.ts (1)
validateArgs(129-150)
samples/orm/main.ts (4)
packages/orm/src/client/plugin.ts (1)
definePlugin(79-83)samples/next.js/zenstack/schema-lite.ts (1)
schema(120-120)samples/orm/zenstack/schema.ts (1)
schema(260-260)samples/next.js/lib/db.ts (1)
db(6-25)
packages/orm/src/client/client-impl.ts (3)
packages/orm/src/client/plugin.ts (1)
RuntimePlugin(16-70)packages/orm/src/client/crud/operations/base.ts (4)
CoreCrudOperations(60-77)CoreCrudOperations(82-82)AllCrudOperations(125-125)AllCrudOperations(130-130)packages/orm/src/client/crud/operations/find.ts (1)
FindOperationHandler(5-29)
packages/orm/src/client/contract.ts (5)
packages/orm/src/client/constants.ts (1)
TRANSACTION_UNSUPPORTED_METHODS(14-14)packages/orm/src/client/crud/operations/base.ts (2)
CoreCrudOperations(60-77)CoreCrudOperations(82-82)packages/schema/src/schema.ts (2)
SchemaDef(11-19)GetModels(122-122)packages/orm/src/client/plugin.ts (1)
RuntimePlugin(16-70)packages/orm/src/client/crud-types.ts (2)
Subset(905-907)ExistsArgs(1091-1091)
🪛 Biome (2.1.2)
samples/orm/main.ts
[error] 28-28: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Agent
- GitHub Check: CodeQL analysis (javascript-typescript)
- GitHub Check: claude-review
🔇 Additional comments (33)
samples/orm/package.json (1)
21-22: Bothzodandkyselyare properly declared in the workspace catalog (pnpm-workspace.yaml), so the"catalog:"references in this file are valid and require no changes.packages/orm/src/client/crud-types.ts (1)
217-221: LGTM!The reformatting of the
_countconditional type branch improves readability while preserving the original type logic.tests/e2e/package.json (1)
28-29: LGTM!The
zoddependency addition is appropriate for the new extended query args validation tests, and thecatalog:version specifier is consistent with other dependencies in this workspace.tests/e2e/orm/plugin-infra/ext-query-args/schema.zmodel (1)
1-8: LGTM!The minimal ZModel schema is well-suited for testing extended query args functionality. The corresponding
schema.tsfile appears to be the generated output, confirming the schema has been processed.packages/orm/src/utils/type-utils.ts (1)
91-92: LGTM!The
Exact<T, Shape>utility type follows the standard TypeScript pattern for enforcing exact structural matches, preventing excess properties at the type level.tests/e2e/orm/plugin-infra/ext-query-args/schema.ts (1)
1-38: LGTM!The generated schema file correctly reflects the ZModel definition with proper typing. The auto-generation header appropriately warns against manual modifications.
packages/orm/src/client/crud/operations/base.ts (2)
57-140: Clean operation-group exports for API clarity.
The grouped const/type pairs look consistent and should be helpful for consumer typing and plugin extensibility.
177-177: Handler signature matches the new core-op taxonomy.tests/e2e/orm/plugin-infra/ext-query-args/models.ts (1)
1-10: Generated model alias looks good.packages/orm/src/client/index.ts (1)
6-12: Re-exports look consistent and tidy.packages/plugins/policy/src/plugin.ts (2)
6-9: Non-generic plugin declaration is clean.
25-27: Kysely hook typing update looks consistent.tests/e2e/orm/plugin-infra/ext-query-args.test.ts (2)
1-6: Well-structured test suite for extended query args.The test file provides comprehensive coverage of the plugin ExtQueryArgs feature, including:
- Extending all operations with validation
- Extending specific operations (read-only)
- Different extensions for different operation types
- Proper negative tests with
@ts-expect-errorfor type safety validation
18-22: Consider makingcacheoptional incacheBustSchemafor consistency.Unlike
cacheSchemawherecacheis optional (line 15),cacheBustSchemarequirescacheto be present. This means write operations will fail validation ifcacheis omitted. Verify this is the intended behavior for the test, or align withcacheSchemaby adding.optional().packages/orm/src/client/options.ts (2)
62-62: Type loosening fromRuntimePlugin<Schema>toRuntimePlugin<any>.This change allows plugins with arbitrary
ExtQueryArgsto be registered. While this provides flexibility for the new plugin options feature, it reduces type safety at theClientOptionslevel. The actual type checking now relies on the$usemethod's return type inference.
114-120: API rename fromOmitOptionstoOmitConfigis complete. All internal usages have been updated; no legacy references toOmitOptionsremain in the codebase. However, no backward compatibility alias was created, which means external consumers would experience a breaking change if this is a published API.packages/orm/src/client/client-impl.ts (2)
295-303: LGTM on plugin management methods.The
$use,$unuse, and$unuseAllmethods are correctly updated to useRuntimePlugin<any>to accommodate the new ExtQueryArgs generic parameter.
519-530: Extended query args are already supported across all operations, not justfindMany.The test file
ext-query-args.test.tsvalidates that all operations—includingfindUnique,findUniqueOrThrow,findFirst,findFirstOrThrow,create,createMany,update,updateMany,upsert,delete,count,aggregate,groupBy, and others—successfully accept extended query args. The implementation pattern varies (onlyfindManyuses rest args), but this is an internal detail; extended args reachcreatePromisethrough the plugin validation layer and work uniformly across all operations.Likely an incorrect or invalid review comment.
packages/orm/src/client/crud/validator/index.ts (3)
349-373: Schema caching now includes operation in the cache key.The cache key now properly includes the
operationfield, ensuring that per-operation plugin extensions are cached separately. This is important for correctness since different operations may have different extended argument schemas.
390-426: Operation-driven schema construction looks correct.The
makeFindSchemamethod now accepts an operation parameter and correctly determines behavior (unique vs non-unique) based on the operation. The plugin args are properly merged before applying mutual exclusivity refinements.
375-386: No issues found. ThepluginSchema.shapeaccess is type-safe due to the type guard on line 380 that narrowsZodObject | undefinedtoZodObjectbefore property access.packages/orm/src/client/plugin.ts (3)
8-11: LGTM!The
ExtQueryArgsBasetype correctly models per-operation extended query args using a mapped type overCoreCrudOperations | 'all', providing a clean extension point for plugins.
58-70: LGTM!The conditional intersection type cleanly adds the
extQueryArgsproperty only whenExtQueryArgshas keys, avoiding unnecessary properties on plugins that don't extend query args. ThegetValidationSchemacallback provides a proper hook for runtime validation.
134-139: LGTM!The
OnQueryHookContextupdates improve type safety:
operation: AllCrudOperationsproperly reflects all possible CRUD operationsargs: objectis more specific thanunknownwhile remaining flexible for various operation arg shapestests/e2e/orm/plugin-infra/ext-query-args/input.ts (1)
1-31: LGTM!This auto-generated file correctly exports strongly-typed ORM argument aliases for the
Userentity. The type aliases consistently follow the patternUserXxxArgs = $XxxArgs<$Schema, "User">and properly leverage the ORM's generic type system for per-operation typing.packages/orm/src/client/contract.ts (8)
51-58: LGTM!The
ExtractExtQueryArgshelper type correctly implements the fallback chain: operation-specific →'all'→ empty object. The use ofNonNullableensures undefined values in the extended args don't propagate.
74-78: LGTM!Adding
ExtQueryArgsas the third generic parameter with a default of{}maintains backward compatibility while enabling the new plugin extension mechanism.
181-183: LGTM!The
$usemethod correctly accumulates plugin-provided extended query args via intersection (ExtQueryArgs & PluginExtQueryArgs), enabling multiple plugins to contribute their own extension points.
186-193: Consider whether$unuseand$unuseAllshould resetExtQueryArgs.When a plugin is removed via
$unuseor$unuseAll, the return type still carries the currentExtQueryArgs. This could lead to type-level inconsistencies where the types suggest extended args are available, but the runtime plugin providing them has been removed.However, since TypeScript can't "subtract" from intersection types, this may be an intentional limitation. If so, consider documenting this behavior.
211-216: LGTM!The model operations mapping correctly threads
ExtQueryArgstoModelOperations, ensuring per-model CRUD operations receive the extended query args.
271-275: LGTM!Adding
ExtQueryArgstoAllModelOperationscompletes the type threading, allowing extended query args to flow to all operation signatures.
358-360: Consistent pattern applied across all operations.The intersection pattern
FindManyArgs<Schema, Model> & ExtractExtQueryArgs<ExtQueryArgs, 'findMany'>is correctly applied to both the generic constraint and theSelectSubsetparameter, ensuring type safety for extended args.
864-873: LGTM!
ModelOperationscorrectly passesExtQueryArgstoAllModelOperations, completing the type propagation chain for all model CRUD operations.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
|
Claude finished @ymc9's task —— View job Claude finished reviewing the pull request Code Review CompleteI've conducted a thorough review of the "feat: plugin-extended query args" PR. This feature implements extended query arguments for plugins, allowing them to add custom options to ORM operations while maintaining type safety and validation. Todo List:
📋 SummaryThis PR introduces a powerful plugin extension mechanism that allows plugins to add custom query arguments to any ORM operation. The implementation is well-architected with proper type safety, validation, and comprehensive test coverage. ✅ Strengths🏗️ Excellent Architecture
🔒 Robust Input Validation
🧪 Comprehensive Test Coverage
📊 Performance Conscious
|
|
Claude finished @ymc9's task —— View job Claude finished @ymc9's task —— View job Code Review CompleteI've conducted a thorough review of the "WIP: plugin custom options" PR. This feature implements extended query arguments for plugins, allowing them to add custom options to ORM operations while maintaining type safety and validation. Todo List:
📋 SummaryThis PR introduces a powerful plugin extension mechanism that allows plugins to add custom query arguments to any ORM operation. The implementation is well-architected with proper type safety, validation, and comprehensive test coverage. ✅ Strengths🏗️ Excellent Architecture
🔒 Robust Input Validation
🧪 Comprehensive Test Coverage
📊 Performance Conscious
|
|
Claude finished @ymc9's task —— View job Code Review CompleteI've conducted a thorough review of the "feat: plugin-extended query args" PR. This feature implements extended query arguments for plugins, allowing them to add custom options to ORM operations while maintaining type safety and validation. Todo List:
📋 SummaryThis PR introduces a powerful plugin extension mechanism that allows plugins to add custom query arguments to any ORM operation. The implementation is well-architected with proper type safety, validation, and comprehensive test coverage. ✅ Strengths🏗️ Excellent Architecture
🔒 Robust Input Validation
🧪 Comprehensive Test Coverage
📊 Performance Conscious
|
|
Claude finished @ymc9's task —— View job Code Review CompleteI've conducted a thorough review of the "feat: plugin-extended query args" PR. This feature implements extended query arguments for plugins, allowing them to add custom options to ORM operations while maintaining type safety and validation. Todo List:
📋 SummaryThis PR introduces a powerful plugin extension mechanism that allows plugins to add custom query arguments to any ORM operation. The implementation is well-architected with proper type safety, validation, and comprehensive test coverage. ✅ Strengths🏗️ Excellent Architecture
🔒 Robust Input Validation
🧪 Comprehensive Test Coverage
📊 Performance Conscious
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@packages/orm/src/client/contract.ts`:
- Around line 74-78: The methods on ClientContract ($setAuth, $setOptions,
$setInputValidation, $transaction, $unuse, $unuseAll) currently drop the third
generic ExtQueryArgs causing plugin-extended args to be lost; update each method
signature to preserve and propagate ExtQueryArgs in their return types (e.g.,
return ClientContract<Schema, NewOptionsOrSameOptions, ExtQueryArgs> or make the
method generic over the new Options while carrying ExtQueryArgs through) so the
third type parameter is not removed after calling these APIs; locate and change
the return-type declarations in the ClientContract definition to include the
existing ExtQueryArgs generic.
In `@packages/orm/src/client/plugin.ts`:
- Around line 130-135: The OnQueryHookContext type currently requires args:
object which fails when operations like findMany/findFirst are called with no
args; update the OnQueryHookContext definition (the args property) to be
optional (e.g., args?: object | undefined or args?: Record<string, any>) and
audit usages of OnQueryHookContext in plugin.ts and related plugins to add safe
null/undefined checks before accessing args so plugins won't assume non-null
args at runtime.
♻️ Duplicate comments (2)
samples/orm/main.ts (2)
24-24: Fix empty object pattern inlistPublicPosts.Biome flags the empty destructuring pattern; use no params or a placeholder parameter.
♻️ Suggested fix
- listPublicPosts: ({}) => [], + listPublicPosts: () => [],
41-41: Await or document the fire‑and‑forget query.Unawaited promises can surface unhandled rejections. If this is just a demo call, add a comment; otherwise await it.
💡 Suggested fix
- db.user.findMany({ where: { id: '1' } }); + await db.user.findMany({ where: { id: '1' } });
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
packages/orm/src/client/client-impl.tspackages/orm/src/client/contract.tspackages/orm/src/client/options.tspackages/orm/src/client/plugin.tssamples/orm/main.tstests/e2e/orm/plugin-infra/ext-query-args.test.tstests/e2e/orm/policy/basic-schema-read.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/e2e/orm/plugin-infra/ext-query-args.test.ts
- packages/orm/src/client/options.ts
🧰 Additional context used
📓 Path-based instructions (1)
tests/e2e/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
E2E tests should validate real-world schema compatibility with established projects
Files:
tests/e2e/orm/policy/basic-schema-read.test.ts
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/orm/**/*.{ts,tsx} : Implement plugin hooks at ORM, Kysely, and entity mutation levels for query interception and customization
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/orm/**/*.{ts,tsx} : Use Kysely as the query builder interface for low-level database queries, avoiding raw SQL when possible
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/orm/**/*.test.{ts,tsx} : ORM package tests should include comprehensive client API tests and policy tests
Applied to files:
tests/e2e/orm/policy/basic-schema-read.test.tspackages/orm/src/client/client-impl.tspackages/orm/src/client/plugin.tspackages/orm/src/client/contract.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to tests/e2e/**/*.{ts,tsx} : E2E tests should validate real-world schema compatibility with established projects
Applied to files:
tests/e2e/orm/policy/basic-schema-read.test.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/orm/**/*.{ts,tsx} : Implement plugin hooks at ORM, Kysely, and entity mutation levels for query interception and customization
Applied to files:
tests/e2e/orm/policy/basic-schema-read.test.tssamples/orm/main.tspackages/orm/src/client/client-impl.tspackages/orm/src/client/plugin.tspackages/orm/src/client/contract.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/orm/**/*.{ts,tsx} : Use Kysely as the query builder interface for low-level database queries, avoiding raw SQL when possible
Applied to files:
packages/orm/src/client/client-impl.tspackages/orm/src/client/plugin.tspackages/orm/src/client/contract.ts
📚 Learning: 2025-12-30T15:07:06.254Z
Learnt from: mwillbanks
Repo: zenstackhq/zenstack-v3 PR: 550
File: packages/orm/src/client/crud/operations/base.ts:158-159
Timestamp: 2025-12-30T15:07:06.254Z
Learning: Do not use ts-expect-error in production code within the zenstackhq/zenstack-v3 repo (e.g., packages/*). Use explicit type annotations, targeted type assertions, or refactor to resolve the type error. ts-expect-error may be acceptable only in test files for stubbing or temporary silencing. Ensure production code is type-safe and maintainable.
Applied to files:
packages/orm/src/client/client-impl.tspackages/orm/src/client/plugin.tspackages/orm/src/client/contract.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to **/*.zmodel : ZModel schema files should define database structure and policies that compile to TypeScript via `zenstack generate`
Applied to files:
packages/orm/src/client/plugin.tspackages/orm/src/client/contract.ts
🧬 Code graph analysis (4)
tests/e2e/orm/policy/basic-schema-read.test.ts (1)
packages/plugins/policy/src/plugin.ts (1)
PolicyPlugin(6-29)
samples/orm/main.ts (1)
samples/next.js/lib/db.ts (1)
db(6-25)
packages/orm/src/client/plugin.ts (2)
packages/orm/src/client/crud/operations/base.ts (4)
CoreCrudOperations(60-77)CoreCrudOperations(82-82)AllCrudOperations(125-125)AllCrudOperations(130-130)packages/orm/src/utils/type-utils.ts (1)
Exact(92-92)
packages/orm/src/client/contract.ts (3)
packages/orm/src/client/index.ts (1)
CoreCrudOperations(9-9)packages/orm/src/client/crud/operations/base.ts (2)
CoreCrudOperations(60-77)CoreCrudOperations(82-82)packages/orm/src/client/plugin.ts (2)
ExtQueryArgsBase(11-11)RuntimePlugin(16-70)
🪛 Biome (2.1.2)
samples/orm/main.ts
[error] 24-24: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build-test (22.x, sqlite)
- GitHub Check: build-test (22.x, postgresql)
- GitHub Check: claude-review
🔇 Additional comments (7)
tests/e2e/orm/policy/basic-schema-read.test.ts (1)
26-26: PolicyPlugin instantiation aligns with new typing.Dropping the schema generic matches the updated PolicyPlugin signature. Please verify the test still typechecks/runs under the new plugin typings.
samples/orm/main.ts (1)
30-30: Proceed cast matches current sample typing.Using
as anyhere is fine for the sample’s hook plumbing; please verify typecheck remains clean.packages/orm/src/client/plugin.ts (1)
1-78: ExtQueryArgs scaffolding is coherent.The conditional
extQueryArgsshape and updateddefinePlugintyping align cleanly withCoreCrudOperations.packages/orm/src/client/contract.ts (2)
43-58: ExtQueryArgs extraction + fallback looks good.The per‑operation extraction with an
'all'fallback is clear and predictable.
181-216: Per‑operation ExtQueryArgs propagation is thorough.Threading
ExtractExtQueryArgsthrough$useand all model operations keeps the extension surface consistent.Also applies to: 358-859
packages/orm/src/client/client-impl.ts (2)
16-31: CRUD handler wiring matches new operation groups.Using
CoreCrudOperationsfor handlers andAllCrudOperationsfor the nominal operation keeps plugin context accurate while preserving existing behavior.Also applies to: 422-462, 514-522
295-325: Plugin list handling remains clean after typing broadened.Cloning plugin arrays for
$use/$unuse/$unuseAllmaintains immutability and keeps the client options consistent.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
|
Claude finished @ymc9's task —— View job Code Review CompleteI've conducted a thorough review of the "feat: plugin-extended query args" PR. This feature implements extended query arguments for plugins, allowing them to add custom options to ORM operations while maintaining type safety and validation. Todo List:
📋 SummaryThis PR introduces a powerful plugin extension mechanism that allows plugins to add custom query arguments to any ORM operation. The implementation is well-architected with proper type safety, validation, and comprehensive test coverage. ✅ Strengths🏗️ Excellent Architecture
🔒 Robust Input Validation
🧪 Comprehensive Test Coverage
📊 Performance Conscious
|
|
Claude finished @ymc9's task —— View job Code Review CompleteI've conducted a thorough review of the "feat: plugin-extended query args" PR. This feature implements extended query arguments for plugins, allowing them to add custom options to ORM operations while maintaining type safety and validation. Todo List:
📋 SummaryThis PR introduces a powerful plugin extension mechanism that allows plugins to add custom query arguments to any ORM operation. The implementation is well-architected with proper type safety, validation, and comprehensive test coverage. ✅ Strengths🏗️ Excellent Architecture
🔒 Robust Input Validation
🧪 Comprehensive Test Coverage
📊 Performance Conscious
|
|
Claude finished @ymc9's task —— View job Code Review CompleteI've conducted a thorough review of the "feat: plugin-extended query args" PR. This feature implements extended query arguments for plugins, allowing them to add custom options to ORM operations while maintaining type safety and validation. Todo List:
📋 SummaryThis PR introduces a powerful plugin extension mechanism that allows plugins to add custom query arguments to any ORM operation. The implementation is well-architected with proper type safety, validation, and comprehensive test coverage. ✅ Strengths🏗️ Excellent Architecture
🔒 Robust Input Validation
🧪 Comprehensive Test Coverage
📊 Performance Conscious
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/orm/src/client/crud/validator/index.ts`:
- Around line 1458-1467: makeUpdateManyAndReturnSchema currently calls
makeUpdateManySchema (which already calls mergePluginArgsSchema for
'updateMany') and then calls mergePluginArgsSchema again for
'updateManyAndReturn', causing double/incorrect plugin augmentation; change this
by creating or using a base-maker that returns the un-augmented update-many
schema (e.g., introduce makeUpdateManyBaseSchema(model) or add a skipPlugin
boolean parameter to makeUpdateManySchema) then in makeUpdateManyAndReturnSchema
build the base with that un-augmented helper, extend with select/omit, and call
mergePluginArgsSchema exactly once with 'updateManyAndReturn'; target symbols:
makeUpdateManyAndReturnSchema, makeUpdateManySchema (or new
makeUpdateManyBaseSchema), and mergePluginArgsSchema.
🧹 Nitpick comments (2)
packages/orm/src/client/client-impl.ts (1)
62-62: Consider makinginputValidatorreadonly or private with accessor.The
inputValidatorproperty is now public withoutreadonly, which allows external code to potentially reassign it. Since the validator is tightly coupled to the client's options and schema, external mutation could lead to inconsistent state.♻️ Suggested change
- inputValidator: InputValidator<SchemaDef>; + readonly inputValidator: InputValidator<SchemaDef>;packages/orm/src/client/crud/validator/index.ts (1)
365-376: Document plugin schema compatibility expectations.When multiple plugins extend the schema, Zod's
.extend()method will replace earlier field definitions if plugins define the same extended field names. While the final.strict()call ensures proper validation, there's no safeguard or documentation about this behavior. Consider documenting the expected contract—that plugin schemas should not conflict—or adding validation to detect and warn about field name collisions between plugins.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/orm/src/client/client-impl.tspackages/orm/src/client/crud/validator/index.tstests/e2e/orm/plugin-infra/ext-query-args.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/e2e/orm/plugin-infra/ext-query-args.test.ts
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/orm/**/*.{ts,tsx} : Implement plugin hooks at ORM, Kysely, and entity mutation levels for query interception and customization
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/orm/**/*.{ts,tsx} : Use Kysely as the query builder interface for low-level database queries, avoiding raw SQL when possible
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/orm/**/*.{ts,tsx} : Implement plugin hooks at ORM, Kysely, and entity mutation levels for query interception and customization
Applied to files:
packages/orm/src/client/client-impl.tspackages/orm/src/client/crud/validator/index.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/orm/**/*.test.{ts,tsx} : ORM package tests should include comprehensive client API tests and policy tests
Applied to files:
packages/orm/src/client/client-impl.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/orm/**/*.{ts,tsx} : Use Kysely as the query builder interface for low-level database queries, avoiding raw SQL when possible
Applied to files:
packages/orm/src/client/client-impl.tspackages/orm/src/client/crud/validator/index.ts
📚 Learning: 2025-12-30T15:07:06.254Z
Learnt from: mwillbanks
Repo: zenstackhq/zenstack-v3 PR: 550
File: packages/orm/src/client/crud/operations/base.ts:158-159
Timestamp: 2025-12-30T15:07:06.254Z
Learning: Do not use ts-expect-error in production code within the zenstackhq/zenstack-v3 repo (e.g., packages/*). Use explicit type annotations, targeted type assertions, or refactor to resolve the type error. ts-expect-error may be acceptable only in test files for stubbing or temporary silencing. Ensure production code is type-safe and maintainable.
Applied to files:
packages/orm/src/client/client-impl.tspackages/orm/src/client/crud/validator/index.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to **/*.zmodel : ZModel schema files should define database structure and policies that compile to TypeScript via `zenstack generate`
Applied to files:
packages/orm/src/client/crud/validator/index.ts
🧬 Code graph analysis (1)
packages/orm/src/client/crud/validator/index.ts (1)
packages/orm/src/client/crud/operations/base.ts (2)
CoreCrudOperations(60-77)CoreCrudOperations(82-82)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: claude-review
- GitHub Check: build-test (22.x, sqlite)
- GitHub Check: build-test (22.x, postgresql)
🔇 Additional comments (12)
packages/orm/src/client/crud/validator/index.ts (4)
62-63: LGTM on the caching refactor.Switching from a global/WeakMap-based cache to an instance-level
Map<string, ZodType>is appropriate here since the cache keys now incorporate the operation context, and schemas may vary per-client due to plugin configurations.
192-203: LGTM on the operation-aware validation signature.The updated
validateFindArgsmethod correctly threads the operation through tomakeFindSchema, enabling per-operation plugin augmentation and schema behavior (unique/findOne distinctions).
380-416: LGTM on the refactoredmakeFindSchemamethod.The logic correctly derives
uniqueandfindOnebehavior from the operation parameter, applies appropriate field constraints (skip/take/orderBy/cursor/distinct for non-unique operations), and properly chains plugin augmentation before applying mutual exclusivity refinements.
1447-1456: Consider extracting the repeated strict object pattern.The
makeUpdateManySchemanow directly returns the merged schema without intermediate assignment. While correct, this pattern (construct base → merge plugin → return) is repeated across many methods. This is acceptable as-is, but could be a candidate for a helper if the pattern grows.packages/orm/src/client/client-impl.ts (8)
296-307: LGTM on$usevalidator recreation.Correctly creates a fresh
InputValidatorafter adding a plugin, ensuring the schema cache reflects the new plugin's extended query args. The comment explains the rationale well.
309-326: LGTM on$unusevalidator recreation.Consistently creates a fresh validator when removing a plugin, maintaining cache integrity.
328-339: LGTM on$unuseAllvalidator recreation.Correctly handles the case of removing all plugins with appropriate validator recreation.
354-359: LGTM on$setOptionsvalidator recreation.Correctly creates a fresh validator since options may affect validation settings.
432-434: LGTM on thecreatePromisesignature update.The distinction between
operation(CoreCrudOperations) andnominalOperation(AllCrudOperations) correctly separates the underlying operation from the user-facing operation name (e.g.,findUniquevsfindUniqueOrThrow).
417-417: LGTM on passing shared validator to handler.Passing
client.inputValidatorinstead of creating a new one per-model access improves performance and ensures consistent caching behavior.
462-473: No changes needed—OnQueryHookContextalready includes all the properties (client,model,operation,args, andproceed) being constructed in thectxobject. The type definition is complete and accurate.Likely an incorrect or invalid review comment.
118-118: No action needed. The code properly handles validator cache invalidation.All methods that change validation-affecting options (
$setOptions,$setInputValidation,$use,$unuse,$unuseAll) explicitly create new validators with fresh schema caches. The validator's cache key includesextraValidationsEnabled, which depends on thevalidateInputoption, providing an additional safety layer. The code includes explicit comments documenting this intentional design: "create a new validator to have a fresh schema cache, because options may change validation settings."Transactions intentionally share the parent client's validator since they don't change options—only query scope—making the cache sharing safe and beneficial for performance.
Likely an incorrect or invalid review comment.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
|
Claude finished @ymc9's task —— View job Code Review CompleteI've conducted a thorough review of the "feat: plugin-extended query args" PR. This feature implements extended query arguments for plugins, allowing them to add custom options to ORM operations while maintaining type safety and validation. Todo List:
📋 SummaryThis PR introduces a powerful plugin extension mechanism that allows plugins to add custom query arguments to any ORM operation. The implementation is well-architected with proper type safety, validation, and comprehensive test coverage. ✅ Strengths🏗️ Excellent Architecture
🔒 Robust Input Validation
🧪 Comprehensive Test Coverage
📊 Performance Conscious
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@packages/orm/src/client/contract.ts`:
- Around line 146-150: Restore the JSDoc `@deprecated` tag on the
$setInputValidation declaration so the comment above the method reads that it is
deprecated and directs users to use $setOptions instead; update the docblock for
$setInputValidation (the ClientContract<Schema, Options, ExtQueryArgs> method)
to include a line like "@deprecated Use `$setOptions` instead." without changing
the signature or behavior.
In `@tests/e2e/orm/plugin-infra/ext-query-args.test.ts`:
- Around line 18-22: The current cacheBustSchema makes the write-side `cache`
property required which causes writes without `cache` to fail; update the schema
`cacheBustSchema` so that the `cache` property is optional by adding
`.optional()` to the `cache` field (i.e., make `cache:
z.strictObject({...}).optional()`), or alternatively add a new test asserting
that `cache` is required for write operations if that is the intended behavior;
reference `cacheBustSchema`, the `cache` field and its `bust` boolean to locate
the change.
🧹 Nitpick comments (2)
tests/e2e/orm/plugin-infra/ext-query-args.test.ts (2)
70-83: Avoid brittle exact-match error strings from Zod.Line 70-83 assert exact messages (e.g., “Too small”, “Unrecognized key”), which are Zod defaults and may change across versions. Prefer regex or asserting on structured error data to keep tests stable.
♻️ Example adjustment (apply similarly to other assertions)
-await expect(extDb.user.findMany({ cache: { ttl: -1 } })).rejects.toThrow('Too small'); +await expect(extDb.user.findMany({ cache: { ttl: -1 } })).rejects.toThrow(/too small/i);
7-9: Add a real-world schema compatibility fixture.This E2E test uses a bespoke schema; consider adding a fixture based on a known real-world schema (or project sample) to validate compatibility beyond synthetic models. As per coding guidelines, E2E tests should validate real-world schema compatibility with established projects.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/orm/src/client/contract.tstests/e2e/orm/plugin-infra/ext-query-args.test.ts
🧰 Additional context used
📓 Path-based instructions (1)
tests/e2e/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
E2E tests should validate real-world schema compatibility with established projects
Files:
tests/e2e/orm/plugin-infra/ext-query-args.test.ts
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/orm/**/*.{ts,tsx} : Implement plugin hooks at ORM, Kysely, and entity mutation levels for query interception and customization
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/orm/**/*.test.{ts,tsx} : ORM package tests should include comprehensive client API tests and policy tests
Applied to files:
tests/e2e/orm/plugin-infra/ext-query-args.test.tspackages/orm/src/client/contract.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/orm/**/*.{ts,tsx} : Implement plugin hooks at ORM, Kysely, and entity mutation levels for query interception and customization
Applied to files:
tests/e2e/orm/plugin-infra/ext-query-args.test.tspackages/orm/src/client/contract.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to tests/e2e/**/*.{ts,tsx} : E2E tests should validate real-world schema compatibility with established projects
Applied to files:
tests/e2e/orm/plugin-infra/ext-query-args.test.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to tests/**/type*.{ts,tsx} : Ensure TypeScript inference and type coverage are validated through type coverage tests
Applied to files:
tests/e2e/orm/plugin-infra/ext-query-args.test.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/orm/**/*.{ts,tsx} : Use Kysely as the query builder interface for low-level database queries, avoiding raw SQL when possible
Applied to files:
tests/e2e/orm/plugin-infra/ext-query-args.test.tspackages/orm/src/client/contract.ts
📚 Learning: 2025-12-30T15:07:06.254Z
Learnt from: mwillbanks
Repo: zenstackhq/zenstack-v3 PR: 550
File: packages/orm/src/client/crud/operations/base.ts:158-159
Timestamp: 2025-12-30T15:07:06.254Z
Learning: Do not use ts-expect-error in production code within the zenstackhq/zenstack-v3 repo (e.g., packages/*). Use explicit type annotations, targeted type assertions, or refactor to resolve the type error. ts-expect-error may be acceptable only in test files for stubbing or temporary silencing. Ensure production code is type-safe and maintainable.
Applied to files:
packages/orm/src/client/contract.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to **/*.zmodel : ZModel schema files should define database structure and policies that compile to TypeScript via `zenstack generate`
Applied to files:
packages/orm/src/client/contract.ts
🧬 Code graph analysis (1)
tests/e2e/orm/plugin-infra/ext-query-args.test.ts (2)
packages/orm/src/client/crud/operations/base.ts (4)
CoreReadOperations(87-95)CoreReadOperations(100-100)CoreWriteOperations(105-115)CoreWriteOperations(120-120)packages/orm/src/client/plugin.ts (1)
definePlugin(72-76)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build-test (22.x, postgresql)
- GitHub Check: claude-review
- GitHub Check: build-test (22.x, sqlite)
🔇 Additional comments (17)
packages/orm/src/client/contract.ts (17)
43-45: LGTM!The new imports are correctly structured to support the
ExtQueryArgspropagation feature.
51-58: LGTM!The
ExtractExtQueryArgshelper type correctly implements the fallback chain (operation-specific → 'all' → empty object) with appropriate use ofNonNullablefor optional properties.
74-78: LGTM!The
ExtQueryArgsgeneric parameter with its default value{}maintains backward compatibility while enabling plugin-extended query args.
165-168: LGTM!
ExtQueryArgsis correctly propagated toTransactionClientContract, ensuring plugin-extended args remain available within transactions.
181-183: LGTM!The intersection type
ExtQueryArgs & PluginExtQueryArgscorrectly accumulates extended query args from multiple plugins, enabling proper plugin composition.
188-193: Verify the$unusebehavior withExtQueryArgs.
$unusepreservesExtQueryArgswhile$unuseAlldrops them. This asymmetry may be intentional (since tracking which plugin contributed which args is complex at the type level), but worth confirming this is the intended design.
211-216: LGTM!
ExtQueryArgsis correctly threaded through toModelOperations, enabling per-operation extension at the model level.
222-226: LGTM!
TransactionClientContractcorrectly includesExtQueryArgswith the proper type constraint.
272-276: LGTM!
ExtQueryArgsis correctly added. The type constraint is appropriately applied at theClientContractlevel, which flows down here.
359-361: LGTM!The
findManysignature correctly appliesExtractExtQueryArgsfor operation-specific extended args.
369-381: LGTM!
findUniqueandfindUniqueOrThrowcorrectly share the'findUnique'operation key for extended args, which is appropriate since they have the same input structure.
389-401: LGTM!
findFirstandfindFirstOrThrowcorrectly share the'findFirst'operation key.
455-513: LGTM!Create operations (
create,createMany,createManyAndReturn) correctly apply operation-specific extended args.
632-689: LGTM!Update operations (
update,updateMany,updateManyAndReturn) correctly apply operation-specific extended args.
711-759: LGTM!
upsert,delete, anddeleteManyoperations correctly apply operation-specific extended args.
779-860: LGTM!Aggregation operations (
count,aggregate,groupBy,exists) correctly apply operation-specific extended args.
865-874: LGTM!
ModelOperationscorrectly defaultsExtQueryArgsto{}for backward compatibility and propagates it toAllModelOperations.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
|
Claude finished @ymc9's task —— View job Code Review CompleteI've conducted a thorough review of the "feat: plugin-extended query args" PR. This feature implements extended query arguments for plugins, allowing them to add custom options to ORM operations while maintaining type safety and validation. Todo List:
📋 SummaryThis PR introduces a powerful plugin extension mechanism that allows plugins to add custom query arguments to any ORM operation. The implementation is well-architected with proper type safety, validation, and comprehensive test coverage. ✅ Strengths🏗️ Excellent Architecture
🔒 Robust Input Validation
🧪 Comprehensive Test Coverage
📊 Performance Conscious
|
|
Claude finished @ymc9's task —— View job Code Review CompleteI've conducted a thorough review of the "feat: plugin-extended query args" PR. This feature implements extended query arguments for plugins, allowing them to add custom options to ORM operations while maintaining type safety and validation. Todo List:
📋 SummaryThis PR introduces a powerful plugin extension mechanism that allows plugins to add custom query arguments to any ORM operation. The implementation is well-architected with proper type safety, validation, and comprehensive test coverage. ✅ Strengths🏗️ Excellent Architecture
🔒 Robust Input Validation
🧪 Comprehensive Test Coverage
📊 Performance Conscious
|
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/orm/src/client/crud/validator/index.ts (1)
1458-1466: Fix: updateManyAndReturn loses its operation-specific plugin args.
makeUpdateManyAndReturnSchemacurrently reusesmakeUpdateManySchema, so onlyupdateManyplugin args are merged andupdateManyAndReturn-specific ext args are ignored (and the wrong ones are accepted). This breaks per-operation ext args validation.🛠️ Proposed fix (merge plugin args for updateManyAndReturn)
private makeUpdateManyAndReturnSchema(model: string) { - // plugin extended args schema is merged in `makeUpdateManySchema` - const baseSchema: ZodObject = this.makeUpdateManySchema(model); - let schema: ZodType = baseSchema.extend({ + const baseSchema = z.strictObject({ + where: this.makeWhereSchema(model, false).optional(), + data: this.makeUpdateDataSchema(model, [], true), + limit: z.number().int().nonnegative().optional(), + }).extend({ select: this.makeSelectSchema(model).optional().nullable(), omit: this.makeOmitSchema(model).optional().nullable(), }); + let schema: ZodType = this.mergePluginArgsSchema(baseSchema, 'updateManyAndReturn'); schema = this.refineForSelectOmitMutuallyExclusive(schema); return schema; }
♻️ Duplicate comments (1)
tests/e2e/orm/plugin-infra/ext-query-args.test.ts (1)
18-22: Write-sidecacheproperty is required, unlike the read-side schema.The
cacheBustSchemamakescacherequired, whereascacheSchema(line 11-15) makes it optional. This asymmetry means write operations must always includecacheto pass validation once the extension is applied. If this is intentional, consider adding a test case that explicitly verifies writes withoutcachefail validation.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/orm/src/client/contract.tspackages/orm/src/client/crud/validator/index.tspackages/orm/src/client/plugin.tstests/e2e/orm/plugin-infra/ext-query-args.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/orm/src/client/plugin.ts
🧰 Additional context used
📓 Path-based instructions (1)
tests/e2e/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
E2E tests should validate real-world schema compatibility with established projects
Files:
tests/e2e/orm/plugin-infra/ext-query-args.test.ts
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/orm/**/*.{ts,tsx} : Implement plugin hooks at ORM, Kysely, and entity mutation levels for query interception and customization
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/orm/**/*.test.{ts,tsx} : ORM package tests should include comprehensive client API tests and policy tests
Applied to files:
tests/e2e/orm/plugin-infra/ext-query-args.test.tspackages/orm/src/client/crud/validator/index.tspackages/orm/src/client/contract.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/orm/**/*.{ts,tsx} : Implement plugin hooks at ORM, Kysely, and entity mutation levels for query interception and customization
Applied to files:
tests/e2e/orm/plugin-infra/ext-query-args.test.tspackages/orm/src/client/crud/validator/index.tspackages/orm/src/client/contract.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to tests/e2e/**/*.{ts,tsx} : E2E tests should validate real-world schema compatibility with established projects
Applied to files:
tests/e2e/orm/plugin-infra/ext-query-args.test.tspackages/orm/src/client/contract.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to tests/**/type*.{ts,tsx} : Ensure TypeScript inference and type coverage are validated through type coverage tests
Applied to files:
tests/e2e/orm/plugin-infra/ext-query-args.test.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/orm/**/*.{ts,tsx} : Use Kysely as the query builder interface for low-level database queries, avoiding raw SQL when possible
Applied to files:
tests/e2e/orm/plugin-infra/ext-query-args.test.tspackages/orm/src/client/crud/validator/index.tspackages/orm/src/client/contract.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to **/*.zmodel : ZModel schema files should define database structure and policies that compile to TypeScript via `zenstack generate`
Applied to files:
tests/e2e/orm/plugin-infra/ext-query-args.test.tspackages/orm/src/client/crud/validator/index.tspackages/orm/src/client/contract.ts
📚 Learning: 2025-12-30T15:07:06.254Z
Learnt from: mwillbanks
Repo: zenstackhq/zenstack-v3 PR: 550
File: packages/orm/src/client/crud/operations/base.ts:158-159
Timestamp: 2025-12-30T15:07:06.254Z
Learning: Do not use ts-expect-error in production code within the zenstackhq/zenstack-v3 repo (e.g., packages/*). Use explicit type annotations, targeted type assertions, or refactor to resolve the type error. ts-expect-error may be acceptable only in test files for stubbing or temporary silencing. Ensure production code is type-safe and maintainable.
Applied to files:
packages/orm/src/client/crud/validator/index.tspackages/orm/src/client/contract.ts
🧬 Code graph analysis (1)
packages/orm/src/client/crud/validator/index.ts (1)
packages/orm/src/client/crud/operations/base.ts (2)
CoreCrudOperations(60-77)CoreCrudOperations(82-82)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build-test (22.x, postgresql)
- GitHub Check: build-test (22.x, sqlite)
- GitHub Check: claude-review
🔇 Additional comments (12)
tests/e2e/orm/plugin-infra/ext-query-args.test.ts (6)
1-6: LGTM!Imports are well-organized, bringing in the necessary types (
CoreReadOperations,CoreWriteOperations,definePlugin,ClientContract) from the ORM package along with test utilities and Zod for schema validation.
27-34: LGTM!Proper test lifecycle management with database cleanup before each test and safe disconnection after.
36-163: Comprehensive test coverage for extending all operations.This test thoroughly validates the plugin-extended query args feature:
- Optional cache and ttl parameters
- TTL propagation through
onQueryhook- Validation errors for invalid values and unrecognized keys
- Coverage across all CRUD operation types (read, create, update, delete)
- Transactional and plugin boundary usage (
$transaction,$use,$setOptions,$setAuth)This aligns well with the coding guidelines for E2E tests validating real-world schema compatibility. Based on learnings, the comprehensive client API coverage is appropriate for ORM tests.
165-193: LGTM!Good test for operation-specific extensions. Validates that only read operations accept the cache extension while write operations correctly reject it. The runtime check using
(CoreReadOperations as readonly string[]).includes(operation)properly aligns compile-time types with runtime behavior.
195-247: LGTM!Excellent test for per-operation schema differentiation. The test correctly validates:
- Read operations accept
ttlbut rejectbust- Write operations accept
bustbut rejectttl- Proper propagation of extended args through the
onQueryhookThis demonstrates the flexibility of the plugin system for operation-aware extensions.
249-272: LGTM!Critical test for validating schema isolation between clients. The double verification (lines 264-266 and 268-271) ensures that plugin extensions don't leak across client instances and that internal caching doesn't cause cross-contamination.
packages/orm/src/client/crud/validator/index.ts (3)
59-63: Solid: per-operation schema caching and validation flow.Per-client
schemaCacheplus operation-aware cache keys/validation avoids cross-op schema collisions and keeps validation deterministic.Also applies to: 192-203, 331-363
365-415: Nice: operation-aware find schema with plugin extensions.The operation-derived
findUnique/findFirstsemantics combined with plugin arg merging keeps validation aligned with the new per-op ext args model.
1148-1174: Good: consistent plugin-arg merging across CRUD schemas.The per-operation
mergePluginArgsSchemaintegration is applied uniformly across create/update/delete/count/aggregate/groupBy, which should reduce drift in validation behavior.Also applies to: 1433-1482, 1594-1670, 1705-1771
packages/orm/src/client/contract.ts (3)
51-58: Nice: ExtractExtQueryArgs adds a clear per-operation fallback.This keeps ext args scoped to each operation while still honoring a shared
allshape.
74-218: Good: ExtQueryArgs is preserved across client APIs and plugin installs.The updated signatures for
$setAuth,$setOptions,$setInputValidation,$transaction,$use, and$unusekeep extended args flowing through client instances.
274-863: Good: per-operation ext args are threaded through model operations.The operation signatures consistently intersect core args with
ExtractExtQueryArgs, matching the new per-op ext args contract.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Summary by CodeRabbit
New Features
API
Types
Tests
Samples
✏️ Tip: You can customize this high-level summary in your review settings.