Conversation
|
Claude finished @ymc9's task —— View job PR Review: ORM API SlicingI'll provide a comprehensive review of this PR that introduces ORM API slicing functionality. Todo List:
Review SummaryThis PR introduces a powerful API slicing feature that allows fine-grained control over ORM client capabilities. The implementation is comprehensive and well-architected. ✅ Strengths1. Excellent Type Safety ImplementationThe TypeScript implementation in
2. Comprehensive Test CoverageThe test suite in
3. Well-Architected Core Implementation
4. Client Library IntegrationTanStack Query integration properly respects slicing configuration:
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
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:
📝 WalkthroughWalkthroughIntroduce comprehensive slicing (models, operations, procedures, filter kinds) with type-level utilities and runtime guards, thread Options through ORM and TanStack Query typings/hooks, rename aggregate operator constant, relax some internal handler typings, and add extensive type tests and e2e slicing tests. Changes
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/orm/src/client/crud/validator/index.ts (2)
901-924:⚠️ Potential issue | 🟠 MajorJSON filters incorrectly require
Jsonkind even for equality.
Type-levelJsonFilterallowsEquality-only usage, but runtime validation rejects all JSON filters unlessJsonis included. This creates a type/runtime mismatch and blocks validequals/notfilters.🛠️ Suggested fix
- // Check if Json filter kind is allowed - if (allowedFilterKinds && !allowedFilterKinds.has('Json')) { - // Return a never schema if Json filters are not allowed - return z.never(); - } - - const valueSchema = this.makeJsonValueSchema(optional, true); - return z.strictObject({ - path: z.string().optional(), - equals: valueSchema.optional(), - not: valueSchema.optional(), - string_contains: z.string().optional(), - string_starts_with: z.string().optional(), - string_ends_with: z.string().optional(), - mode: this.makeStringModeSchema().optional(), - array_contains: valueSchema.optional(), - array_starts_with: valueSchema.optional(), - array_ends_with: valueSchema.optional(), - }); + const valueSchema = this.makeJsonValueSchema(optional, true); + const operators = { + path: z.string().optional(), + equals: valueSchema.optional(), + not: valueSchema.optional(), + string_contains: z.string().optional(), + string_starts_with: z.string().optional(), + string_ends_with: z.string().optional(), + mode: this.makeStringModeSchema().optional(), + array_contains: valueSchema.optional(), + array_starts_with: valueSchema.optional(), + array_ends_with: valueSchema.optional(), + }; + const filtered = this.trimFilterOperators(operators, allowedFilterKinds); + return Object.keys(filtered).length === 0 ? z.never() : z.strictObject(filtered);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/orm/src/client/crud/validator/index.ts` around lines 901 - 924, The runtime check in makeJsonFilterSchema currently rejects all JSON filters unless the 'Json' kind is present; update the conditional that inspects allowedFilterKinds (from getEffectiveFilterKinds) to permit equality-only JSON filters by allowing the schema when allowedFilterKinds has 'Equality' even if it lacks 'Json'. In practice, change the logic that returns z.never() so it only returns never when allowedFilterKinds exists and contains neither 'Json' nor 'Equality'; keep the rest of makeJsonFilterSchema (valueSchema, equals/not, string_/array_ predicates, mode) unchanged.
578-613:⚠️ Potential issue | 🟠 MajorRelation filters can still target excluded models.
Model exclusion is enforced in select/include/create/update, butmakeWhereSchemastill permits relation filters to excluded models. This leaks access to excluded models viawhereclauses.🛠️ Suggested fix
if (fieldDef.relation) { if (withoutRelationFields) { continue; } - // Check if Relation filter kind is allowed - const allowedFilterKinds = this.getEffectiveFilterKinds(model, field); - if (allowedFilterKinds && !allowedFilterKinds.has('Relation')) { - // Relation filters are not allowed for this field - use z.never() - fieldSchema = z.never(); - } else { + // Disallow relation filters to excluded models + if (!this.isModelAllowed(fieldDef.type)) { + fieldSchema = z.never(); + } else { + // Check if Relation filter kind is allowed + const allowedFilterKinds = this.getEffectiveFilterKinds(model, field); + if (allowedFilterKinds && !allowedFilterKinds.has('Relation')) { + fieldSchema = z.never(); + } else { fieldSchema = z.lazy(() => this.makeWhereSchema(fieldDef.type, false).optional()); // optional to-one relation allows null fieldSchema = this.nullableIf(fieldSchema, !fieldDef.array && !!fieldDef.optional); if (fieldDef.array) { // to-many relation fieldSchema = z.union([ fieldSchema, z.strictObject({ some: fieldSchema.optional(), every: fieldSchema.optional(), none: fieldSchema.optional(), }), ]); } else { // to-one relation fieldSchema = z.union([ fieldSchema, z.strictObject({ is: fieldSchema.optional(), isNot: fieldSchema.optional(), }), ]); } + } } } else {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/orm/src/client/crud/validator/index.ts` around lines 578 - 613, When building relation filter schemas, ensure relation targets that are excluded by the model-exclusion rules are rejected: before calling this.makeWhereSchema(fieldDef.type, false) check the same exclusion predicate used elsewhere (the function used for select/include/create/update exclusion — e.g. a method like this.isModelExcluded or the existing exclusion lookup) using the related model identifier fieldDef.type; if the related model is excluded, set fieldSchema = z.never() and skip the lazy schema creation, otherwise proceed with the current lazy makeWhereSchema + nullableIf + array/non-array union logic so excluded models cannot be referenced in where filters.
🧹 Nitpick comments (5)
packages/clients/tanstack-query/test/react-sliced-client.test-d.ts (1)
9-18: ConflictingincludedOperationsandexcludedOperationsfor User — intentional?The
Usermodel config specifies'update'in bothincludedOperationsandexcludedOperations. While this config is only used in the "works with sliced models" test (which doesn't check operation-level typing), the overlap is confusing. Consider either:
- Adding an explicit test that validates the precedence behavior (exclude wins over include), or
- Removing the conflict to keep the test fixture clear.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/clients/tanstack-query/test/react-sliced-client.test-d.ts` around lines 9 - 18, The User model in the slicing config has a conflicting operation listed in both includedOperations and excludedOperations; remove the ambiguity by deleting 'update' from the excludedOperations array (i.e., update the slicing -> models -> User config to keep includedOperations: ['findUnique','findMany','update'] and either make excludedOperations empty or remove the 'update' entry) so the test fixture is clear, or alternatively add a focused test that asserts precedence (exclude wins) using the slicing config and operation-level type checks for User.packages/orm/src/client/client-impl.ts (2)
485-538:isModelIncludedandisProcedureIncludedshare identical structure — consider extracting a generic helper.Both functions follow the exact same
includedX/excludedXcheck pattern. A single genericisIncluded(included, excluded, name)helper would reduce duplication.♻️ Example consolidation
+function isSliceIncluded( + included: readonly string[] | undefined, + excluded: readonly string[] | undefined, + name: string, +): boolean { + if (included !== undefined && !included.includes(name)) { + return false; + } + if (excluded && excluded.length > 0 && excluded.includes(name)) { + return false; + } + return true; +} + function isModelIncluded(options: ClientOptions<SchemaDef>, model: string): boolean { const slicing = options.slicing; if (!slicing) return true; - const { includedModels, excludedModels } = slicing; - if (includedModels !== undefined) { - if (!includedModels.includes(model as any)) { - return false; - } - } - if (excludedModels && excludedModels.length > 0) { - if (excludedModels.includes(model as any)) { - return false; - } - } - return true; + return isSliceIncluded( + slicing.includedModels as readonly string[] | undefined, + slicing.excludedModels as readonly string[] | undefined, + model, + ); } function isProcedureIncluded(options: ClientOptions<SchemaDef>, procedureName: string): boolean { const slicing = options.slicing; if (!slicing) return true; - const { includedProcedures, excludedProcedures } = slicing; - if (includedProcedures !== undefined) { - if (!(includedProcedures as readonly string[]).includes(procedureName)) { - return false; - } - } - if (excludedProcedures && excludedProcedures.length > 0) { - if ((excludedProcedures as readonly string[]).includes(procedureName)) { - return false; - } - } - return true; + return isSliceIncluded( + slicing.includedProcedures as readonly string[] | undefined, + slicing.excludedProcedures as readonly string[] | undefined, + procedureName, + ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/orm/src/client/client-impl.ts` around lines 485 - 538, Extract a generic helper like isIncluded(included?: readonly string[] | undefined, excluded?: readonly string[] | undefined, name: string): boolean that implements the existing logic (if included is !== undefined then require included.includes(name); then if excluded present and non-empty return false if excluded.includes(name); otherwise true). Replace the body of isModelIncluded to call isIncluded(includedModels, excludedModels, model) and isProcedureIncluded to call isIncluded(includedProcedures as readonly string[] | undefined, excludedProcedures as readonly string[] | undefined, procedureName) so behavior (including the semantic that an empty included array means include none) remains identical and duplication is removed.
466-475: Excluded models silently returnundefined— consider a descriptive runtime error.When a model is excluded via slicing, the proxy returns
undefined(line 471). A JavaScript consumer (or one ignoring types) callingclient.user.findMany(...)will get a crypticTypeError: Cannot read properties of undefined. A small guarded throw with a clear message would improve the developer experience.💡 Proposed improvement
if (model) { // Check if model is allowed by slicing configuration if (!isModelIncluded(client.$options, model)) { - return undefined; + throw new Error( + `Model "${model}" is not available. It has been excluded by the client's slicing configuration.` + ); } return createModelCrudHandler(client as any, model, client.inputValidator, resultProcessor); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/orm/src/client/client-impl.ts` around lines 466 - 475, The proxy currently returns undefined for excluded models which leads to cryptic runtime errors; update the block in the string-prop handling (the branch that finds model via Object.keys(client.$schema.models).find(...)) so that after isModelIncluded(client.$options, model) returns false you throw a clear Error instead of returning undefined; include the model name and a hint about slicing/selection in the message and reference the involved symbols (prop, model, client.$options, isModelIncluded, createModelCrudHandler) so callers know the model is intentionally excluded.packages/clients/tanstack-query/test/svelte-sliced-client.test-d.ts (1)
9-15: Contradictory slicing config:updateis both included and excluded.The User model specifies
includedOperations: ['findUnique', 'findMany', 'update']andexcludedOperations: ['update']. PerGetSlicedOperationsintype-utils.ts, exclusions win (Exclude<IO, EO>), soupdateis effectively removed. If this is intentional to test precedence semantics, a brief comment would clarify the intent. Otherwise, remove'update'from one of the lists to avoid confusion.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/clients/tanstack-query/test/svelte-sliced-client.test-d.ts` around lines 9 - 15, The User model's slicing config is contradictory—'update' appears in both includedOperations and excludedOperations—so adjust it: either remove 'update' from includedOperations or from excludedOperations, or add a clarifying comment that this test intentionally verifies precedence (GetSlicedOperations uses Exclude<IO, EO> so exclusions win). Locate the User model object (slicing.models.User) and update the includedOperations/excludedOperations arrays accordingly or insert the explanatory comment next to them.packages/orm/src/client/crud/validator/index.ts (1)
2128-2149: Avoid hard-coded filter-kind list.
If new kinds are added toFILTER_PROPERTY_TO_KIND, the exclusion-only path will silently allow the wrong set. Consider deriving from the mapping instead.♻️ Suggested refactor
- if (excluded !== undefined) { - if (!result) { - // If no inclusion list, start with all filter kinds - result = new Set(['Equality', 'Range', 'Like', 'Json', 'List', 'Relation']); - } + if (excluded !== undefined) { + if (!result) { + // If no inclusion list, start with all filter kinds + result = new Set(Object.values(FILTER_PROPERTY_TO_KIND)); + } // Remove excluded kinds for (const kind of excluded) { result.delete(kind); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/orm/src/client/crud/validator/index.ts` around lines 2128 - 2149, computeFilterKinds currently seeds the exclusion-only path with a hard-coded list ('Equality','Range','Like','Json','List','Relation'), which will break when FILTER_PROPERTY_TO_KIND gains new kinds; update computeFilterKinds to derive the initial full set from the keys/values of FILTER_PROPERTY_TO_KIND (e.g., iterate its values or keys to build the Set) so excluded-only behavior uses the authoritative mapping; ensure you reference FILTER_PROPERTY_TO_KIND when constructing the default Set and keep the existing logic for applying included/excluded arrays intact.
🤖 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/orm/src/client/constants.ts`:
- Around line 29-31: You renamed the exported AggregateOperators constant/type
which breaks downstream code; restore a deprecated alias by re-exporting the old
names (e.g., export const AGGREGATE_OPERATORS = AggregateOperators and export
type AGGREGATE_OPERATORS = AggregateOperators) alongside the new
AggregateOperators export and add a brief deprecation comment so consumers see
it’s legacy. Ensure the alias references the existing AggregateOperators symbol
(both the const and the type) rather than duplicating the array.
In `@packages/orm/src/client/crud/validator/index.ts`:
- Around line 731-746: Nested typed-JSON filters are using the inner type's
field name for slicing instead of the outer model field's slice configuration;
update the call site in validator index.ts (inside isTypeDefType branch) so
makeTypedJsonFilterSchema receives the outer model field identifier (e.g., pass
a parentField or rootName property set to the outer field variable used for
fieldSchemas key) rather than (or in addition to) name: field, and then update
makeTypedJsonFilterSchema to use that parentField/rootName when computing slice
info; ensure the symbol names to change are the call in
this.makeTypedJsonFilterSchema(...) and the makeTypedJsonFilterSchema
signature/implementation to read the parentField/rootName and prefer it for
slicing decisions.
In `@tests/e2e/orm/client-api/slicing.test.ts`:
- Around line 1706-1708: The test currently calls db.user.findMany({ where: {
name: { gte: 'A' } } }) but claims Range filters are excluded; update the
assertion to expect the call to reject instead of succeed: wrap the
db.user.findMany invocation in an assertion that it throws/rejects (e.g., use
your test runner's expect(...).rejects/throws pattern) and ensure the failure is
specifically tied to the Range filter on name (referencing the db.user.findMany
call and the where: { name: { gte: 'A' } } clause).
---
Outside diff comments:
In `@packages/orm/src/client/crud/validator/index.ts`:
- Around line 901-924: The runtime check in makeJsonFilterSchema currently
rejects all JSON filters unless the 'Json' kind is present; update the
conditional that inspects allowedFilterKinds (from getEffectiveFilterKinds) to
permit equality-only JSON filters by allowing the schema when allowedFilterKinds
has 'Equality' even if it lacks 'Json'. In practice, change the logic that
returns z.never() so it only returns never when allowedFilterKinds exists and
contains neither 'Json' nor 'Equality'; keep the rest of makeJsonFilterSchema
(valueSchema, equals/not, string_/array_ predicates, mode) unchanged.
- Around line 578-613: When building relation filter schemas, ensure relation
targets that are excluded by the model-exclusion rules are rejected: before
calling this.makeWhereSchema(fieldDef.type, false) check the same exclusion
predicate used elsewhere (the function used for select/include/create/update
exclusion — e.g. a method like this.isModelExcluded or the existing exclusion
lookup) using the related model identifier fieldDef.type; if the related model
is excluded, set fieldSchema = z.never() and skip the lazy schema creation,
otherwise proceed with the current lazy makeWhereSchema + nullableIf +
array/non-array union logic so excluded models cannot be referenced in where
filters.
---
Duplicate comments:
In `@packages/clients/tanstack-query/test/vue-sliced-client.test-d.ts`:
- Around line 9-15: The test config for slicing has a contradictory entry where
the User model lists 'update' in both includedOperations and excludedOperations;
update the test to remove the contradiction by either removing 'update' from
excludedOperations or from includedOperations (or add an explicit comment in the
test clarifying that this duplication is intentional to test precedence
semantics), targeting the slicing -> models -> User block and the
includedOperations/excludedOperations arrays to make the intent consistent with
the other framework tests.
---
Nitpick comments:
In `@packages/clients/tanstack-query/test/react-sliced-client.test-d.ts`:
- Around line 9-18: The User model in the slicing config has a conflicting
operation listed in both includedOperations and excludedOperations; remove the
ambiguity by deleting 'update' from the excludedOperations array (i.e., update
the slicing -> models -> User config to keep includedOperations:
['findUnique','findMany','update'] and either make excludedOperations empty or
remove the 'update' entry) so the test fixture is clear, or alternatively add a
focused test that asserts precedence (exclude wins) using the slicing config and
operation-level type checks for User.
In `@packages/clients/tanstack-query/test/svelte-sliced-client.test-d.ts`:
- Around line 9-15: The User model's slicing config is contradictory—'update'
appears in both includedOperations and excludedOperations—so adjust it: either
remove 'update' from includedOperations or from excludedOperations, or add a
clarifying comment that this test intentionally verifies precedence
(GetSlicedOperations uses Exclude<IO, EO> so exclusions win). Locate the User
model object (slicing.models.User) and update the
includedOperations/excludedOperations arrays accordingly or insert the
explanatory comment next to them.
In `@packages/orm/src/client/client-impl.ts`:
- Around line 485-538: Extract a generic helper like isIncluded(included?:
readonly string[] | undefined, excluded?: readonly string[] | undefined, name:
string): boolean that implements the existing logic (if included is !==
undefined then require included.includes(name); then if excluded present and
non-empty return false if excluded.includes(name); otherwise true). Replace the
body of isModelIncluded to call isIncluded(includedModels, excludedModels,
model) and isProcedureIncluded to call isIncluded(includedProcedures as readonly
string[] | undefined, excludedProcedures as readonly string[] | undefined,
procedureName) so behavior (including the semantic that an empty included array
means include none) remains identical and duplication is removed.
- Around line 466-475: The proxy currently returns undefined for excluded models
which leads to cryptic runtime errors; update the block in the string-prop
handling (the branch that finds model via
Object.keys(client.$schema.models).find(...)) so that after
isModelIncluded(client.$options, model) returns false you throw a clear Error
instead of returning undefined; include the model name and a hint about
slicing/selection in the message and reference the involved symbols (prop,
model, client.$options, isModelIncluded, createModelCrudHandler) so callers know
the model is intentionally excluded.
In `@packages/orm/src/client/crud/validator/index.ts`:
- Around line 2128-2149: computeFilterKinds currently seeds the exclusion-only
path with a hard-coded list
('Equality','Range','Like','Json','List','Relation'), which will break when
FILTER_PROPERTY_TO_KIND gains new kinds; update computeFilterKinds to derive the
initial full set from the keys/values of FILTER_PROPERTY_TO_KIND (e.g., iterate
its values or keys to build the Set) so excluded-only behavior uses the
authoritative mapping; ensure you reference FILTER_PROPERTY_TO_KIND when
constructing the default Set and keep the existing logic for applying
included/excluded arrays intact.
|
Claude finished @ymc9's task —— View job PR Review: ORM API Slicing ImplementationI'll provide a comprehensive review of this PR that introduces ORM API slicing functionality. Review Tasks:
Review SummaryThis PR introduces a comprehensive and well-architected API slicing feature that provides fine-grained control over ORM client capabilities. The implementation demonstrates high-quality TypeScript engineering with extensive test coverage. ✅ Strengths1. Excellent TypeScript ImplementationThe type system in
2. Comprehensive Test CoverageThe test suite in
3. Clean Architecture & Implementation
4. Powerful Feature Set
🐛 Critical Bugs Found1. JSON Filter Validation Bug (
|
- type simplification - addressing PR comments
|
Claude finished @ymc9's task —— View job PR Review: ORM API Slicing ImplementationI'll provide a comprehensive review of this PR that introduces ORM API slicing functionality. Review Tasks:
Review SummaryThis PR introduces a comprehensive and well-architected API slicing feature that provides fine-grained control over ORM client capabilities. The implementation demonstrates high-quality TypeScript engineering with extensive test coverage. ✅ Strengths1. Excellent TypeScript ImplementationThe type system in
2. Comprehensive Test CoverageThe test suite in
3. Well-Architected Core Implementation
4. Client Library IntegrationTanStack Query integration properly respects slicing configuration:
|
There was a problem hiding this comment.
Actionable comments posted: 1
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-types.ts (1)
299-839:⚠️ Potential issue | 🟠 MajorAdd type-level guard to prevent empty filter objects when
AllowedKindsisnever.When
includedFilterKinds: []results inAllowedKinds = never, the filter types currently accept{}because all filter conditionals collapse to empty objects and thenotproperty is optional. This creates a type-safety gap where invalid filters compile successfully (caught only at runtime). Add the suggested guard at theFieldFilterentry point to ensure the type itself resolves toneverwhen no filter kinds are allowed, preventing even empty objects from being assignable.Suggested fix
type FieldFilter< Schema extends SchemaDef, Model extends GetModels<Schema>, Field extends GetModelFields<Schema, Model>, Options extends QueryOptions<Schema>, WithAggregations extends boolean, AllowedKinds extends FilterKind = GetSlicedFilterKindsForField<Schema, Model, Field, Options>, > = + [AllowedKinds] extends [never] + ? never + : Field extends RelationFields<Schema, Model> ? // relation RelationFilter<Schema, Model, Field, Options, AllowedKinds> : FieldIsArray<Schema, Model, Field> extends true🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/orm/src/client/crud-types.ts` around lines 299 - 839, The FieldFilter type allows an empty object when AllowedKinds is never; update the FieldFilter declaration to short-circuit to never when AllowedKinds is never (i.e., add a top-level conditional like "AllowedKinds extends never ? never : ..." around the current union/conditional logic) so that FieldFilter<..., AllowedKinds = never> resolves to never and prevents empty-filter assignment; apply this change to the existing FieldFilter type (referencing FieldFilter and AllowedKinds) so all downstream filters inherit the guard.
🧹 Nitpick comments (4)
packages/orm/src/client/crud/validator/index.ts (4)
831-845: Minor:internalMakeArrayFilterSchemareturnsz.strictObject({})when all operators are trimmed, accepting only{}.When both List and Equality filter kinds are excluded, all operators are stripped and the schema becomes
z.strictObject({}). This accepts only an empty object{}, which is technically valid but semantically useless. Other filter methods usez.never()for the fully-excluded case (e.g.,makeJsonFilterSchemaat line 911). For consistency, you could short-circuit toz.never()whenfilteredOperatorsis empty.♻️ Optional consistency fix
// Filter operators based on allowed filter kinds const filteredOperators = this.trimFilterOperators(operators, allowedFilterKinds); + if (Object.keys(filteredOperators).length === 0) { + return z.never(); + } + return z.strictObject(filteredOperators);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/orm/src/client/crud/validator/index.ts` around lines 831 - 845, internalMakeArrayFilterSchema currently builds operators then trims them via trimFilterOperators and returns z.strictObject(filteredOperators); if all operators are removed this yields z.strictObject({}) which accepts only {} — change the function (internalMakeArrayFilterSchema) to detect when filteredOperators is empty and return z.never() instead (matching makeJsonFilterSchema behavior); use the existing trimFilterOperators result to decide the short-circuit so callers get a never schema for the fully-excluded case.
2086-2132:getEffectiveFilterKinds: Fallback chain looks correct but$allmodel doesn't check field-specific overrides.The lookup precedence is:
slicing.models[model].fields[field]slicing.models[model].fields.$allslicing.models.$all.fields.$allStep 3 doesn't check
slicing.models.$all.fields[field]. This is likely intentional (field names across models vary, so a global field-specific override is unusual), but worth confirming.Also, extensive
as anycasts onslicing.modelssuggest the type definitions for the slicing config might benefit from stronger typing so these lookups are type-safe.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/orm/src/client/crud/validator/index.ts` around lines 2086 - 2132, The fallback chain in getEffectiveFilterKinds misses checking for a global model-level field override: before using allModelsConfig.fields.$all, add a branch that checks allModelsConfig.fields[field] and, if present, return computeFilterKinds(allModelsConfig.fields[field].includedFilterKinds, allModelsConfig.fields[field].excludedFilterKinds); keep the existing $all fallback after that; also remove or narrow the (slicing.models as any) casts by using the typed slicing.models where available to make these lookups type-safe (adjust types for models and fields if necessary).
2148-2157: Hardcoded set of all filter kinds may drift out of sync withFILTER_PROPERTY_TO_KIND.Line 2151 hardcodes
['Equality', 'Range', 'Like', 'Json', 'List', 'Relation']. If a new filter kind is added toFILTER_PROPERTY_TO_KINDinconstants.ts, this list must be updated manually.Consider deriving it from the constants:
♻️ Derive from FILTER_PROPERTY_TO_KIND
- result = new Set(['Equality', 'Range', 'Like', 'Json', 'List', 'Relation']); + result = new Set(Object.values(FILTER_PROPERTY_TO_KIND));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/orm/src/client/crud/validator/index.ts` around lines 2148 - 2157, The code hardcodes the full list of filter kinds when building the initial `result` Set (inside the block that checks `if (excluded !== undefined)`), which can drift from `FILTER_PROPERTY_TO_KIND`; change this to derive the full set from the constant `FILTER_PROPERTY_TO_KIND` (imported from constants.ts) and use its values to populate `result` (e.g., create a Set from the values of `FILTER_PROPERTY_TO_KIND`) instead of the literal array so future additions remain in sync with `FILTER_PROPERTY_TO_KIND`.
793-820: Cache inefficiency:@cache()methods receive fullFieldDefobjects but only use 4 properties.The
@cache()decorator onmakeEnumFilterSchema,makeArrayFilterSchema,makePrimitiveFilterSchema, andmakeTypedJsonFilterSchemausesjson-stable-stringifyto generate cache keys from all method arguments. These methods have signatures expectingfieldInfo: FieldInfo(with onlyname,type,optional,array) but actual callers throughoutmakeWhereSchema(lines 622, 729, 760) pass completeFieldDefobjects containing 14 additional properties:id,unique,updatedAt,attributes,default,omit,relation,foreignKeyFor,computed,originModel,isDiscriminator, etc.When the cache decorator serializes these full objects, two FieldDef instances with identical filter-relevant properties but different
attributes,default, orrelationvalues generate different cache keys, causing unnecessary cache misses for semantically identical schemas.Extract the minimal
FieldInfosubset before calling these methods to keep cache keys minimal and improve hit rates:Example extraction at call site
// In makeWhereSchema, before calling filter schema methods: +const fi: FieldInfo = { name: fieldDef.name, type: fieldDef.type, optional: fieldDef.optional, array: fieldDef.array }; if (enumDef) { if (Object.keys(enumDef.values).length > 0) { - fieldSchema = this.makeEnumFilterSchema(model, fieldDef, withAggregations, ignoreSlicing); + fieldSchema = this.makeEnumFilterSchema(model, fi, withAggregations, ignoreSlicing); } } else if (fieldDef.array) { - fieldSchema = this.makeArrayFilterSchema(model, fieldDef); + fieldSchema = this.makeArrayFilterSchema(model, fi); } else if (this.isTypeDefType(fieldDef.type)) { - fieldSchema = this.makeTypedJsonFilterSchema(model, fieldDef); + fieldSchema = this.makeTypedJsonFilterSchema(model, fi); } else { - fieldSchema = this.makePrimitiveFilterSchema(model, fieldDef, withAggregations, ignoreSlicing); + fieldSchema = this.makePrimitiveFilterSchema(model, fi, withAggregations, ignoreSlicing); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/orm/src/client/crud/validator/index.ts` around lines 793 - 820, Call sites should pass only the minimal FieldInfo ({name, type, optional, array}) instead of full FieldDef so the `@cache`() keys are stable; update calls that currently forward a FieldDef into makeEnumFilterSchema, makeArrayFilterSchema, makePrimitiveFilterSchema, and makeTypedJsonFilterSchema to first construct a small object const fieldInfo = { name: field.name, type: field.type, optional: !!field.optional, array: !!field.array } and pass that. Ensure all references use the new FieldInfo variable (not the original FieldDef) so json-stable-stringify used by the `@cache`() decorator serializes only those four properties and avoids spurious cache misses.
🤖 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/orm/src/client/crud/validator/index.ts`:
- Around line 930-978: The cache collision stems from passing a Set to
`@cache`()-decorated methods (makeBooleanFilterSchema, makeBytesFilterSchema,
makeDateTimeFilterSchema, makeStringFilterSchema, makeNumberFilterSchema)
because stableStringify serializes Set as {} — convert the Set to a
deterministic array before it's passed into these methods. Update all call sites
that pass allowedFilterKinds to use allowedFilterKinds ?
[...allowedFilterKinds].sort() : undefined (or otherwise compute a sorted
string[] inside the caller) and, if desired, adjust the parameter types on the
above methods from Set<string>|undefined to string[]|undefined to reflect the
new input shape so the cache keys are stable and distinct per set contents.
---
Outside diff comments:
In `@packages/orm/src/client/crud-types.ts`:
- Around line 299-839: The FieldFilter type allows an empty object when
AllowedKinds is never; update the FieldFilter declaration to short-circuit to
never when AllowedKinds is never (i.e., add a top-level conditional like
"AllowedKinds extends never ? never : ..." around the current union/conditional
logic) so that FieldFilter<..., AllowedKinds = never> resolves to never and
prevents empty-filter assignment; apply this change to the existing FieldFilter
type (referencing FieldFilter and AllowedKinds) so all downstream filters
inherit the guard.
---
Duplicate comments:
In `@packages/orm/src/client/crud/validator/index.ts`:
- Around line 731-748: The nested typed-JSON branch is using the inner fieldDef
(which carries the nested field name) for filter-kind lookups and schema
creation, causing slicing rules tied to the outer field to be ignored; update
the typed-JSON handling so that calls which compute filter kinds or slicing
(e.g., getEffectiveFilterKinds) and schema factories (e.g.,
makeTypedJsonFilterSchema) use the outer field identifier (the local variable
fieldName / the outer fieldDef) instead of fieldDef.name — either pass fieldName
down to makeTypedJsonFilterSchema or have makeTypedJsonFilterSchema accept an
explicit outerFieldName parameter and use that for
getEffectiveFilterKinds(contextModel, ...); ensure all lookups that determine
slice-specific behavior reference the outer field name so nested filters honor
the parent field's slicing configuration.
---
Nitpick comments:
In `@packages/orm/src/client/crud/validator/index.ts`:
- Around line 831-845: internalMakeArrayFilterSchema currently builds operators
then trims them via trimFilterOperators and returns
z.strictObject(filteredOperators); if all operators are removed this yields
z.strictObject({}) which accepts only {} — change the function
(internalMakeArrayFilterSchema) to detect when filteredOperators is empty and
return z.never() instead (matching makeJsonFilterSchema behavior); use the
existing trimFilterOperators result to decide the short-circuit so callers get a
never schema for the fully-excluded case.
- Around line 2086-2132: The fallback chain in getEffectiveFilterKinds misses
checking for a global model-level field override: before using
allModelsConfig.fields.$all, add a branch that checks
allModelsConfig.fields[field] and, if present, return
computeFilterKinds(allModelsConfig.fields[field].includedFilterKinds,
allModelsConfig.fields[field].excludedFilterKinds); keep the existing $all
fallback after that; also remove or narrow the (slicing.models as any) casts by
using the typed slicing.models where available to make these lookups type-safe
(adjust types for models and fields if necessary).
- Around line 2148-2157: The code hardcodes the full list of filter kinds when
building the initial `result` Set (inside the block that checks `if (excluded
!== undefined)`), which can drift from `FILTER_PROPERTY_TO_KIND`; change this to
derive the full set from the constant `FILTER_PROPERTY_TO_KIND` (imported from
constants.ts) and use its values to populate `result` (e.g., create a Set from
the values of `FILTER_PROPERTY_TO_KIND`) instead of the literal array so future
additions remain in sync with `FILTER_PROPERTY_TO_KIND`.
- Around line 793-820: Call sites should pass only the minimal FieldInfo ({name,
type, optional, array}) instead of full FieldDef so the `@cache`() keys are
stable; update calls that currently forward a FieldDef into
makeEnumFilterSchema, makeArrayFilterSchema, makePrimitiveFilterSchema, and
makeTypedJsonFilterSchema to first construct a small object const fieldInfo = {
name: field.name, type: field.type, optional: !!field.optional, array:
!!field.array } and pass that. Ensure all references use the new FieldInfo
variable (not the original FieldDef) so json-stable-stringify used by the
`@cache`() decorator serializes only those four properties and avoids spurious
cache misses.
|
Claude finished @ymc9's task —— View job PR Review: ORM API Slicing ImplementationI'll provide a comprehensive review of this PR that introduces ORM API slicing functionality. Review Tasks:
Review SummaryThis PR introduces a comprehensive and well-architected API slicing feature that provides fine-grained control over ORM client capabilities. The implementation demonstrates high-quality TypeScript engineering with extensive test coverage. ✅ Strengths1. Excellent TypeScript ImplementationThe type system in
2. Comprehensive Test CoverageThe test suite in
3. Well-Architected Core Implementation
4. Client Library IntegrationTanStack Query integration properly respects slicing configuration:
🐛 Critical Bugs Found1. JSON Filter Validation Bug (
|
There was a problem hiding this comment.
Actionable comments posted: 1
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)
579-614:⚠️ Potential issue | 🟠 Major
makeWhereSchemaskips theisModelAllowedcheck for relation field targets — slicing bypassEvery other schema builder gates relation fields with
isModelAllowed(fieldDef.type):
Method Guard makeSelectSchemaif (this.isModelAllowed(fieldDef.type))makeIncludeSchemaif (this.isModelAllowed(fieldDef.type))makeCreateDataSchemaif (!this.isModelAllowed(fieldDef.type)) returnmakeUpdateDataSchemaif (!this.isModelAllowed(fieldDef.type)) return
makeWhereSchemaonly checks the'Relation'filter kind (line 586) but never callsisModelAllowed. If model B is excluded viaexcludedModelsor absent fromincludedModels, a user can still write:await client.modelA.findMany({ where: { relationToB: { id: 1 } } })This forces the database to join an excluded model's table, defeating model slicing entirely. The impact extends to every internal
makeWhereSchemacall site (cursor, connect, update filter, delete filter, etc.).🛠️ Proposed fix
if (fieldDef.relation) { if (withoutRelationFields) { continue; } - // Check if Relation filter kind is allowed - const allowedFilterKinds = this.getEffectiveFilterKinds(model, field); - if (allowedFilterKinds && !allowedFilterKinds.includes('Relation')) { - // Relation filters are not allowed for this field - use z.never() + // Check if the target model is allowed by slicing configuration + if (!this.isModelAllowed(fieldDef.type)) { fieldSchema = z.never(); } else { - fieldSchema = z.lazy(() => this.makeWhereSchema(fieldDef.type, false).optional()); - - // optional to-one relation allows null - fieldSchema = this.nullableIf(fieldSchema, !fieldDef.array && !!fieldDef.optional); - - if (fieldDef.array) { - // to-many relation - fieldSchema = z.union([ - fieldSchema, - z.strictObject({ - some: fieldSchema.optional(), - every: fieldSchema.optional(), - none: fieldSchema.optional(), - }), - ]); - } else { - // to-one relation - fieldSchema = z.union([ - fieldSchema, - z.strictObject({ - is: fieldSchema.optional(), - isNot: fieldSchema.optional(), - }), - ]); - } + // Check if Relation filter kind is allowed + const allowedFilterKinds = this.getEffectiveFilterKinds(model, field); + if (allowedFilterKinds && !allowedFilterKinds.includes('Relation')) { + fieldSchema = z.never(); + } else { + fieldSchema = z.lazy(() => this.makeWhereSchema(fieldDef.type, false).optional()); + fieldSchema = this.nullableIf(fieldSchema, !fieldDef.array && !!fieldDef.optional); + + if (fieldDef.array) { + fieldSchema = z.union([ + fieldSchema, + z.strictObject({ + some: fieldSchema.optional(), + every: fieldSchema.optional(), + none: fieldSchema.optional(), + }), + ]); + } else { + fieldSchema = z.union([ + fieldSchema, + z.strictObject({ + is: fieldSchema.optional(), + isNot: fieldSchema.optional(), + }), + ]); + } + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/orm/src/client/crud/validator/index.ts` around lines 579 - 614, The relation branch in makeWhereSchema does not check isModelAllowed(fieldDef.type), allowing filters into excluded models; add a guard like the other builders: if (!this.isModelAllowed(fieldDef.type)) { fieldSchema = z.never(); } (placed before computing allowedFilterKinds/creating the lazy schema and respecting withoutRelationFields) so any relation targeting an excluded model yields z.never() and prevents joins into sliced models.
🧹 Nitpick comments (1)
packages/orm/src/client/crud/validator/index.ts (1)
905-928:makeJsonFilterSchemauses all-or-nothing'Json'-kind gating; Equality-kind operators (equals,not) are also rejectedWhen
allowedFilterKindsexcludes'Json', the method returnsz.never()(line 912) — even thoughequalsandnotare mapped to'Equality'inFILTER_PROPERTY_TO_KIND. Every other primitive filter type (string, number, boolean, etc.) routes throughtrimFilterOperators/createUnionFilterSchemafor per-operator control. A user who configuresallowedFilterKinds = ['Equality']on a Json field currently getsz.never()for the entire filter instead of just the Json-specific operators being stripped.Consider applying
trimFilterOperatorsto the JSON filter operators and falling back toz.never()only when all operators are removed:💡 Suggested approach
`@cache`() private makeJsonFilterSchema(contextModel: string | undefined, field: string, optional: boolean) { const allowedFilterKinds = this.getEffectiveFilterKinds(contextModel, field); - // Check if Json filter kind is allowed - if (allowedFilterKinds && !allowedFilterKinds.includes('Json')) { - return z.never(); - } - const valueSchema = this.makeJsonValueSchema(optional, true); - return z.strictObject({ + const allOperators = { path: z.string().optional(), equals: valueSchema.optional(), not: valueSchema.optional(), string_contains: z.string().optional(), string_starts_with: z.string().optional(), string_ends_with: z.string().optional(), mode: this.makeStringModeSchema().optional(), array_contains: valueSchema.optional(), array_starts_with: valueSchema.optional(), array_ends_with: valueSchema.optional(), - }); + }; + const filtered = this.trimFilterOperators(allOperators, allowedFilterKinds); + if (Object.keys(filtered).length === 0) { + return z.never(); + } + return z.strictObject(filtered); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/orm/src/client/crud/validator/index.ts` around lines 905 - 928, The current makeJsonFilterSchema returns z.never() whenever getEffectiveFilterKinds says 'Json' is disallowed, which blocks Equality-kind operators (equals/not) that map to FILTER_PROPERTY_TO_KIND; instead build the JSON filter schema as you already do (using makeJsonValueSchema/valueSchema and the operator fields), then call trimFilterOperators (the same helper used for other primitives) with FILTER_PROPERTY_TO_KIND and the allowed kinds to strip disallowed JSON-specific operators, and only return z.never() if trimFilterOperators removes all operator fields; update makeJsonFilterSchema to perform this per-operator trimming rather than an all-or-nothing check.
🤖 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/orm/src/client/crud/validator/index.ts`:
- Around line 2087-2133: In getEffectiveFilterKinds, add the missing precedence
check for slicing.models.$all.fields[field] between the model-specific field
fallback and the final $all.$all fallback so that
slicing.models.$all.fields[specificField] is evaluated and passed to
computeFilterKinds before falling back to allModelsConfig.fields.$all; also
remove or replace the unsafe "as any" casts on slicing.models accesses by using
proper typing or a runtime type guard for slicing.models to preserve type safety
when reading modelConfig and allModelsConfig.
---
Outside diff comments:
In `@packages/orm/src/client/crud/validator/index.ts`:
- Around line 579-614: The relation branch in makeWhereSchema does not check
isModelAllowed(fieldDef.type), allowing filters into excluded models; add a
guard like the other builders: if (!this.isModelAllowed(fieldDef.type)) {
fieldSchema = z.never(); } (placed before computing allowedFilterKinds/creating
the lazy schema and respecting withoutRelationFields) so any relation targeting
an excluded model yields z.never() and prevents joins into sliced models.
---
Duplicate comments:
In `@packages/orm/src/client/crud/validator/index.ts`:
- Around line 731-749: The loop over typeDef.fields is passing the inner
typedef's fieldDef (whose name is the inner field, e.g., "street") into
makeEnumFilterSchema, makeArrayFilterSchema, and makePrimitiveFilterSchema,
causing getEffectiveFilterKinds(contextModel, fieldDef.name) to resolve the
wrong field; fix by creating a shallow copy of the fieldDef with the outer loop
key as the name (e.g., const outerFieldDef = { ...fieldDef, name: fieldName })
and pass outerFieldDef into makeEnumFilterSchema, makeArrayFilterSchema, and
makePrimitiveFilterSchema so slicing/filter-kind resolution uses the actual
owning model field; leave makeTypedJsonFilterSchema recursion as-is.
---
Nitpick comments:
In `@packages/orm/src/client/crud/validator/index.ts`:
- Around line 905-928: The current makeJsonFilterSchema returns z.never()
whenever getEffectiveFilterKinds says 'Json' is disallowed, which blocks
Equality-kind operators (equals/not) that map to FILTER_PROPERTY_TO_KIND;
instead build the JSON filter schema as you already do (using
makeJsonValueSchema/valueSchema and the operator fields), then call
trimFilterOperators (the same helper used for other primitives) with
FILTER_PROPERTY_TO_KIND and the allowed kinds to strip disallowed JSON-specific
operators, and only return z.never() if trimFilterOperators removes all operator
fields; update makeJsonFilterSchema to perform this per-operator trimming rather
than an all-or-nothing check.
|
Claude finished @ymc9's task —— View job PR Review: ORM API Slicing ImplementationI'll provide a comprehensive review of this PR that introduces ORM API slicing functionality. Review Tasks:
Review SummaryThis PR introduces a comprehensive and well-architected API slicing feature that provides fine-grained control over ORM client capabilities. The implementation demonstrates high-quality TypeScript engineering with extensive test coverage. ✅ Strengths1. Excellent TypeScript ImplementationThe type system in
2. Comprehensive Test CoverageThe test suite in
3. Well-Architected Core Implementation
4. Client Library IntegrationTanStack Query integration properly respects slicing configuration:
🐛 Critical Bugs Found1. JSON Filter Validation Bug (
|
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
packages/orm/src/client/crud/validator/index.ts (1)
2156-2176: Minor inefficiency incomputeFilterKindsexclusion loop.Each excluded kind triggers a full
filter()over the result array, creating a new array each time: O(n·m). Since these sets are very small (typically ≤6 kinds), this is negligible. A single pass would be cleaner though:♻️ Optional single-pass alternative
if (excluded !== undefined) { if (!result) { // If no inclusion list, start with all filter kinds result = [...this.allFilterKinds]; } // Remove excluded kinds - for (const kind of excluded) { - result = result.filter((k) => k !== kind); - } + const excludedSet = new Set(excluded); + result = result.filter((k) => !excludedSet.has(k)); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/orm/src/client/crud/validator/index.ts` around lines 2156 - 2176, The exclusion loop creates a new array per excluded kind (O(n·m)); modify computeFilterKinds to do exclusion in a single pass by building an exclusion set from the excluded parameter and then applying a single filter over result (or this.allFilterKinds when included is undefined) to remove any kind present in that set; update references in computeFilterKinds to use the temporary Set and a single result.filter((k) => !excludeSet.has(k)) so the behavior and return value remain identical.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/e2e/orm/client-api/slicing.test.ts`:
- Line 1015: The validation expectation string in the test call to
toBeRejectedByValidation is missing the closing double-quote; update the
argument passed to toBeRejectedByValidation (the array element currently '"gte')
to include the closing quote so it matches the other occurrences (i.e., '"gte"')
to keep consistency and correct string syntax in slicing.test.ts.
- Around line 1862-1864: The test comment and variable are misleading: update
the comment that currently reads "Level 2 – User.email: Equality + Range" to
reference User.age instead, and rename the variable emailRange to ageRange where
the query const emailRange = await db.user.findMany({ where: { age: { gte: 20 }
} }); is used (and update any corresponding expect(...) assertions) so the
variable name and comment correctly reflect that db.user.findMany is filtering
on age.
- Around line 970-974: The "Range filters" block is misleading: db.user.findMany
is called with an empty where (where: {}) so no range is tested; update the test
by either changing the comment to reflect it's a generic findMany, or add an
actual range filter (e.g., replace where: {} with where: { age: { gte: 20 } } or
another appropriate field) so rangeResult actually exercises range filtering and
the expect remains valid; locate the db.user.findMany call and the rangeResult
variable to apply the fix.
- Around line 1371-1376: The test calls db.post.findMany with a non-existent
relation filter "user"; change the where clause to use the actual relation name
"author" (e.g., where: { author: { is: { email: 'test' } } }) and update the
expected validation error from '"user"' to '"author"' in the
toBeRejectedByValidation assertion so the test references db.post.findMany and
the Post->User relation correctly.
---
Duplicate comments:
In `@packages/orm/src/client/crud/validator/index.ts`:
- Around line 731-748: The nested typed-JSON branch is passing the inner
sub-field's FieldInfo (fieldDef) to filter-schema builders, so
getEffectiveFilterKinds(model, fieldDef.name) looks up the wrong slice key;
instead, when iterating Object.entries(typeDef.fields) create an adjusted
FieldInfo that preserves the original fieldDef properties but sets name =
fieldName (the outer model field) and pass that adjusted FieldInfo into
makeTypedJsonFilterSchema, makeEnumFilterSchema, makeArrayFilterSchema, and
makePrimitiveFilterSchema (still using contextModel and the same optional()
wrapping) so slicing uses the outer field name for lookups.
In `@tests/e2e/orm/client-api/slicing.test.ts`:
- Around line 1693-1708: The comment above the tests is incorrect and the Range
assertion is missing: update the comment to reflect that the name field allows
Range filters because name: { excludedFilterKinds: ['Like'] } overrides $all
excluding Range, and replace the discarded db.user.findMany({ where: { name: {
gte: 'A' } } }) call with an explicit assertion (e.g., assign result from
db.user.findMany or use expect(...) to check length/contents) so the test
verifies that gte succeeds; references: db.user.findMany, name field,
excludedFilterKinds, contains, gte.
---
Nitpick comments:
In `@packages/orm/src/client/crud/validator/index.ts`:
- Around line 2156-2176: The exclusion loop creates a new array per excluded
kind (O(n·m)); modify computeFilterKinds to do exclusion in a single pass by
building an exclusion set from the excluded parameter and then applying a single
filter over result (or this.allFilterKinds when included is undefined) to remove
any kind present in that set; update references in computeFilterKinds to use the
temporary Set and a single result.filter((k) => !excludeSet.has(k)) so the
behavior and return value remain identical.
|
Claude finished @ymc9's task —— View job PR Review: ORM API Slicing ImplementationI'll provide a comprehensive review of this PR that introduces ORM API slicing functionality. Review Tasks:
Review SummaryThis PR introduces a comprehensive and well-architected API slicing feature that provides fine-grained control over ORM client capabilities. The implementation demonstrates high-quality TypeScript engineering with extensive test coverage. ✅ Strengths1. Excellent TypeScript ImplementationThe type system in
2. Comprehensive Test CoverageThe test suite in
3. Well-Architected Core Implementation
4. Client Library IntegrationTanStack Query integration properly respects slicing configuration:
🐛 Critical Bugs Found1. JSON Filter Validation Bug (
|
|
Claude finished @ymc9's task —— View job PR Review: ORM API Slicing ImplementationI'll provide a comprehensive review of this PR that introduces ORM API slicing functionality. Review Tasks:
Compiling comprehensive review findings... Review SummaryThis PR introduces a comprehensive and well-architected API slicing feature that provides fine-grained control over ORM client capabilities. The implementation demonstrates high-quality TypeScript engineering with extensive test coverage. ✅ Strengths1. Excellent TypeScript ImplementationThe type system in
2. Comprehensive Test CoverageThe test suite in
3. Well-Architected Core Implementation
4. Client Library IntegrationTanStack Query integration properly respects slicing configuration:
🐛 Critical Bugs Found1. JSON Filter Validation Bug (
|
1 participant
Summary by CodeRabbit
New Features
Refactor
Tests
Other