feat(orm): add result plugin extension point#2442
Open
genu wants to merge 10 commits intozenstackhq:devfrom
Open
feat(orm): add result plugin extension point#2442genu wants to merge 10 commits intozenstackhq:devfrom
genu wants to merge 10 commits intozenstackhq:devfrom
Conversation
Implement a new `result` extension point that allows plugins to declare computed fields on query results with automatic type safety and select/omit awareness. Changes: - Add ExtResultFieldDef and ExtResultBase types to plugin.ts - Add ExtResult generic parameter to RuntimePlugin, AnyPlugin, definePlugin - Thread ExtResult through ClientContract and all CRUD return types - Add ExtractExtResult, ExtResultSelectOmitFields, SelectAwareExtResult type helpers - Implement recursive runtime computation in client-impl.ts with needs injection/stripping - Add ext result fields to Zod validation for select/omit schemas - Support nested relation ext result computation (both runtime and types) - Add 26 comprehensive tests covering single-model and nested relation scenarios Features: - Type-safe computed fields on query results - Select/omit-aware types (only include selected fields in result type) - Automatic field dependency (needs) injection and stripping - Multi-plugin composition support - Full support for nested relations (include/select with ext results) - Works with $transaction, $setAuth, $setOptions, etc. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughAdds plugin-driven "ext result" computed fields: new ExtResult types and plugin result definitions; threads ExtResult through public client and CRUD typings; runtime prepares args and post-processes rows to compute ext fields; zod schemas expose ext selection flags; comprehensive e2e tests and schema fixtures added. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 |
When a user omits a field that is a `needs` dependency of an active ext result field, the computed field was silently missing from results. Ensure needs dependencies are un-omitted for the DB query and stripped from the final result, mirroring the existing `select` path behavior. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
tests/e2e/orm/plugin-infra/ext-result.test.ts (1)
601-603: Avoid hard-coded foreign keys in relation setup.These cases use
authorId: 1directly. Prefer the ID from the created user to keep tests independent from sequence/reset behavior.Suggested patch pattern
-await extDb.user.create({ data: { name: 'Alice' } }); -await extDb.post.create({ data: { title: 'Hello', authorId: 1 } }); +const user = await extDb.user.create({ data: { name: 'Alice' } }); +await extDb.post.create({ data: { title: 'Hello', authorId: user.id } });Also applies to: 624-626, 655-657, 680-682, 705-707, 734-736
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/e2e/orm/plugin-infra/ext-result.test.ts` around lines 601 - 603, The test uses hard-coded foreign keys (authorId: 1) when creating posts; change each extDb.user.create call to capture the returned user (e.g., const user = await extDb.user.create(...)) and use that user's id in subsequent extDb.post.create calls (authorId: user.id) so tests don't rely on sequence/reset behavior; update all occurrences that currently pass authorId: 1 (the blocks around extDb.user.create / extDb.post.create) accordingly.
🤖 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-types.ts`:
- Around line 2458-2469: SelectAwareExtResult currently treats any key present
in select/omit as active regardless of its boolean value; update the key
extraction to only include keys whose value is truthy (true). Replace the
Extract<keyof S & string, ...> expression with a conditional-key expression like
({ [K in keyof S & string]: S[K] extends true ? K : never }[keyof S & string]) &
keyof ExtractExtResult<ExtResult, Model>, and similarly for omit use ({ [K in
keyof O & string]: O[K] extends true ? K : never }[keyof O & string]) & keyof
ExtractExtResult<ExtResult, Model> so SelectAwareExtResult (and the omit branch)
matches the runtime truthiness checks; refer to SelectAwareExtResult and
ExtractExtResult to locate and update the two key-extraction spots.
In `@tests/e2e/orm/plugin-infra/ext-result.test.ts`:
- Around line 36-40: The test relies on array index ordering from
extDb.user.findMany() which is non-deterministic; update the call in the test to
request a deterministic order (e.g., add an orderBy clause such as orderBy: {
id: 'asc' } or createdAt: 'asc') so the assertions expecting users[0].greeting
and users[1].greeting are stable, or alternatively fetch and sort the returned
array by a stable key before asserting; locate the call to extDb.user.findMany
in the test and apply the orderBy or explicit sort to produce deterministic
ordering.
---
Nitpick comments:
In `@tests/e2e/orm/plugin-infra/ext-result.test.ts`:
- Around line 601-603: The test uses hard-coded foreign keys (authorId: 1) when
creating posts; change each extDb.user.create call to capture the returned user
(e.g., const user = await extDb.user.create(...)) and use that user's id in
subsequent extDb.post.create calls (authorId: user.id) so tests don't rely on
sequence/reset behavior; update all occurrences that currently pass authorId: 1
(the blocks around extDb.user.create / extDb.post.create) accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 340e2411-a2f8-4e9b-9c37-c0c7f30db3c4
📒 Files selected for processing (8)
packages/orm/src/client/client-impl.tspackages/orm/src/client/contract.tspackages/orm/src/client/crud-types.tspackages/orm/src/client/plugin.tspackages/orm/src/client/zod/factory.tspackages/plugins/policy/src/plugin.tstests/e2e/orm/plugin-infra/ext-result.test.tstests/e2e/orm/plugin-infra/ext-result/schema.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
tests/e2e/orm/plugin-infra/ext-result.test.ts (1)
36-39:⚠️ Potential issue | 🟡 MinorAdd deterministic ordering for index-based assertions.
At Line 36,
findMany()is used withoutorderBy, but Lines 38-39 assertusers[0]andusers[1]. This can be flaky depending on DB return order (same pattern appears elsewhere in this file).Suggested patch pattern
-const users = await extDb.user.findMany(); +const users = await extDb.user.findMany({ orderBy: { id: 'asc' } });#!/bin/bash # Verify index-based assertions that may need explicit ordering in this file rg -n -C2 'findMany\(|createManyAndReturn\(|\[[0-9]+\]!\.' tests/e2e/orm/plugin-infra/ext-result.test.ts🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/e2e/orm/plugin-infra/ext-result.test.ts` around lines 36 - 39, The test uses extDb.user.findMany() and then asserts users[0] and users[1] which is flaky because DB return order is not guaranteed; modify the test to call extDb.user.findMany({ orderBy: { id: 'asc' } }) (or another stable column like createdAt/name) before making index-based assertions, and apply the same explicit orderBy change to any other occurrences in this file that assert by array index (e.g., places referencing users[N] or similar results from createManyAndReturn/findMany).packages/orm/src/client/client-impl.ts (1)
569-571:⚠️ Potential issue | 🟠 MajorGate ext-result processing to row-returning operations only.
At Line 569 and Line 586, ext-result logic is enabled for all
postProcessoperations;groupBystill passespostProcess = trueat Line 799. This can apply model ext fields to aggregate rows whose runtime/type shape doesn’t model ext results.Suggested fix
+ const shouldApplyExtResult = + hasAnyExtResult && + [ + 'findUnique', + 'findFirst', + 'findMany', + 'create', + 'createManyAndReturn', + 'update', + 'updateManyAndReturn', + 'upsert', + 'delete', + ].includes(operation); const processedArgs = - postProcess && hasAnyExtResult + shouldApplyExtResult ? prepareArgsForExtResult(_args, model, schema, plugins) : _args; ... - if (result && postProcess && hasAnyExtResult) { + if (result && shouldApplyExtResult) { result = applyExtResult(result, model, _args, schema, plugins); }Also applies to: 586-588, 793-800
🤖 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 569 - 571, postProcess currently gates ext-result handling for all operations, which incorrectly applies model extension fields to non-row-returning ops like groupBy; update the logic around prepareArgsForExtResult (used where postProcess && hasAnyExtResult) to additionally verify the operation returns rows (e.g., check the resolver/method type or a flag indicating row-returning operations) before calling prepareArgsForExtResult, and apply this same guard wherever similar code appears (references: prepareArgsForExtResult, hasAnyExtResult, postProcess, and the groupBy call site) so ext-result processing runs only for row-returning operations.
🧹 Nitpick comments (2)
tests/e2e/orm/plugin-infra/ext-result.test.ts (1)
626-627: Avoid hard-coded relation IDs in fixtures.At Line 627, using
authorId: 1assumes a fixed PK value. Prefer using the created user’s id from Line 626 to keep tests resilient. This same pattern appears again (e.g., Lines 650, 681, 706, 731, 760).Suggested patch pattern
-await extDb.user.create({ data: { name: 'Alice' } }); -await extDb.post.create({ data: { title: 'Hello', authorId: 1 } }); +const user = await extDb.user.create({ data: { name: 'Alice' } }); +await extDb.post.create({ data: { title: 'Hello', authorId: user.id } });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/e2e/orm/plugin-infra/ext-result.test.ts` around lines 626 - 627, The test uses a hard-coded foreign key (authorId: 1) when creating posts; capture the created user's id from extDb.user.create (e.g., const user = await extDb.user.create(...); const userId = user.id) and pass that variable to extDb.post.create (authorId: userId) everywhere it appears (references around extDb.user.create and extDb.post.create) to avoid assuming a fixed primary key; update all occurrences noted (lines near the subsequent post creations) to use the created user's id instead of 1.packages/orm/src/client/client-impl.ts (1)
1002-1005: Cache ext-result defs perapplyExtResultcall to reduce row-wise overhead.
collectExtResultDefsis invoked again for every row inapplyExtResultToRow, which is avoidable work for large result sets.Refactor sketch
function applyExtResult( result: unknown, model: string, originalArgs: unknown, schema: SchemaDef, plugins: AnyPlugin[], ): unknown { + const extResultDefs = collectExtResultDefs(model, plugins); if (Array.isArray(result)) { for (let i = 0; i < result.length; i++) { - result[i] = applyExtResultToRow(result[i], model, originalArgs, schema, plugins); + result[i] = applyExtResultToRow(result[i], model, originalArgs, schema, plugins, extResultDefs); } return result; } else { - return applyExtResultToRow(result, model, originalArgs, schema, plugins); + return applyExtResultToRow(result, model, originalArgs, schema, plugins, extResultDefs); } } function applyExtResultToRow( row: unknown, model: string, originalArgs: unknown, schema: SchemaDef, plugins: AnyPlugin[], + extResultDefs: Map<string, ExtResultDef>, ): unknown { ... - const extResultDefs = collectExtResultDefs(model, plugins);Also applies to: 1024-1025
🤖 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 1002 - 1005, The loop calls applyExtResultToRow for each row which internally calls collectExtResultDefs repeatedly causing per-row overhead; change applyExtResult (or the block where the Array.isArray(result) is handled) to call collectExtResultDefs once, store the returned ext-result definitions in a local variable, and pass that cached defs into applyExtResultToRow (or modify applyExtResultToRow to accept defs as an extra parameter) so the per-row iteration reuses the same defs instead of recomputing them for every row; apply the same change to the analogous code at the 1024-1025 site.
🤖 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/plugin-infra/ext-result.test.ts`:
- Around line 167-205: The test name mentions "aggregate" but never calls
extDb.user.aggregate; add an aggregate call and assertion to match the test
intent: invoke extDb.user.aggregate with a simple aggregate (e.g., _count or
_avg) via the extDb.user.aggregate(...) API, assert the expected aggregate value
shape (e.g., presence of _count) and then assert that the returned aggregate
object does not have the virtual field (e.g., (aggregateResult as any).upperName
is undefined); alternatively, if you prefer to change the test name, update the
it(...) description to remove "aggregate" to reflect the current assertions.
---
Duplicate comments:
In `@packages/orm/src/client/client-impl.ts`:
- Around line 569-571: postProcess currently gates ext-result handling for all
operations, which incorrectly applies model extension fields to
non-row-returning ops like groupBy; update the logic around
prepareArgsForExtResult (used where postProcess && hasAnyExtResult) to
additionally verify the operation returns rows (e.g., check the resolver/method
type or a flag indicating row-returning operations) before calling
prepareArgsForExtResult, and apply this same guard wherever similar code appears
(references: prepareArgsForExtResult, hasAnyExtResult, postProcess, and the
groupBy call site) so ext-result processing runs only for row-returning
operations.
In `@tests/e2e/orm/plugin-infra/ext-result.test.ts`:
- Around line 36-39: The test uses extDb.user.findMany() and then asserts
users[0] and users[1] which is flaky because DB return order is not guaranteed;
modify the test to call extDb.user.findMany({ orderBy: { id: 'asc' } }) (or
another stable column like createdAt/name) before making index-based assertions,
and apply the same explicit orderBy change to any other occurrences in this file
that assert by array index (e.g., places referencing users[N] or similar results
from createManyAndReturn/findMany).
---
Nitpick comments:
In `@packages/orm/src/client/client-impl.ts`:
- Around line 1002-1005: The loop calls applyExtResultToRow for each row which
internally calls collectExtResultDefs repeatedly causing per-row overhead;
change applyExtResult (or the block where the Array.isArray(result) is handled)
to call collectExtResultDefs once, store the returned ext-result definitions in
a local variable, and pass that cached defs into applyExtResultToRow (or modify
applyExtResultToRow to accept defs as an extra parameter) so the per-row
iteration reuses the same defs instead of recomputing them for every row; apply
the same change to the analogous code at the 1024-1025 site.
In `@tests/e2e/orm/plugin-infra/ext-result.test.ts`:
- Around line 626-627: The test uses a hard-coded foreign key (authorId: 1) when
creating posts; capture the created user's id from extDb.user.create (e.g.,
const user = await extDb.user.create(...); const userId = user.id) and pass that
variable to extDb.post.create (authorId: userId) everywhere it appears
(references around extDb.user.create and extDb.post.create) to avoid assuming a
fixed primary key; update all occurrences noted (lines near the subsequent post
creations) to use the created user's id instead of 1.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 646e0c67-c9c1-483e-8217-faea28a8e0bd
📒 Files selected for processing (2)
packages/orm/src/client/client-impl.tstests/e2e/orm/plugin-infra/ext-result.test.ts
- Gate ext-result computation to model-row operations only, excluding
groupBy which uses postProcess but returns aggregated rows
- Fix SelectAwareExtResult type to respect boolean values in select/omit
(e.g. `{ select: { field: false } }` no longer type-includes the field)
- Add orderBy to findMany test to ensure deterministic ordering
- Remove "aggregate" from test name that didn't test aggregate
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/orm/src/client/client-impl.ts (1)
551-554: Consider renaming shadowedpluginsvariable for clarity.The outer
plugins(line 552) is captured by theproceedclosure for ext result processing, while the innerplugins(line 607) is a spread copy used for plugin iteration. Both work correctly, but the shadowing can be confusing when reading the code.Suggested rename
// apply plugins - const plugins = [...(client.$options.plugins ?? [])]; - for (const plugin of plugins) { + const pluginList = [...(client.$options.plugins ?? [])]; + for (const plugin of pluginList) {Also applies to: 606-607
🤖 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 551 - 554, The outer variable plugins (from client.$options.plugins) is shadowed by a later local spread copy, which is confusing; rename the outer binding (e.g., clientPlugins or outerPlugins) and update all uses (including the hasExtResultDefs(plugins) call and the proceed closure that captures it) so the closure references the renamed clientPlugins, and rename the inner spread copy (e.g., pluginsCopy or pluginList) where iteration happens (the local spread near the plugin iteration block) to avoid shadowing and clarify intent.
🤖 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/plugin-infra/ext-result.test.ts`:
- Around line 626-627: The test hardcodes authorId: 1 when creating posts which
can break if auto-increment IDs are not 1; change the pattern so you capture the
created user's id from extDb.user.create and use that id when creating the post
(e.g., assign const user = await extDb.user.create(...); then use user.id for
extDb.post.create). Apply the same fix for the other occurrences of
extDb.post.create that pass authorId (the repeated pattern around the other test
cases).
---
Nitpick comments:
In `@packages/orm/src/client/client-impl.ts`:
- Around line 551-554: The outer variable plugins (from client.$options.plugins)
is shadowed by a later local spread copy, which is confusing; rename the outer
binding (e.g., clientPlugins or outerPlugins) and update all uses (including the
hasExtResultDefs(plugins) call and the proceed closure that captures it) so the
closure references the renamed clientPlugins, and rename the inner spread copy
(e.g., pluginsCopy or pluginList) where iteration happens (the local spread near
the plugin iteration block) to avoid shadowing and clarify intent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 368a023a-086b-4802-aad7-dde0743a87bf
📒 Files selected for processing (3)
packages/orm/src/client/client-impl.tspackages/orm/src/client/crud-types.tstests/e2e/orm/plugin-infra/ext-result.test.ts
Ext result fields in nested relation select/omit are not yet reflected in the type system. Use `as any` casts to fix TypeScript compilation errors in the nested relation test cases while still testing runtime behavior. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
MySQL does not support createManyAndReturn. Add an early return guard matching the pattern used in other test files. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
tests/e2e/orm/plugin-infra/ext-result.test.ts (1)
631-632:⚠️ Potential issue | 🟡 MinorHardcoded
authorId: 1causes flaky tests across database providers.This pattern appears in multiple tests (lines 632, 655, 686, 713, 738, 767). Auto-increment counters don't reset on
deleteMany(), so the first user created might haveid > 1. Use the created user's actual ID instead.Suggested fix pattern
-await extDb.user.create({ data: { name: 'Alice' } }); -await extDb.post.create({ data: { title: 'Hello', authorId: 1 } }); +const user = await extDb.user.create({ data: { name: 'Alice' } }); +await extDb.post.create({ data: { title: 'Hello', authorId: user.id } });,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/e2e/orm/plugin-infra/ext-result.test.ts` around lines 631 - 632, The test uses a hardcoded authorId (authorId: 1) when creating posts which causes flakes because auto-increment IDs may not start at 1 after deleteMany(); update the test to capture the created user result from extDb.user.create (e.g., const user = await extDb.user.create(...)) and use user.id for extDb.post.create authorId (and similarly replace other hardcoded authorId occurrences in this file at the locations noted) so the post references the actual created user's ID.
🤖 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/plugin-infra/ext-result.test.ts`:
- Around line 164-169: The test assumes a deterministic order from
extDb.user.createManyAndReturn; instead make assertions order-independent by
verifying the returned array contains both expected records regardless of
position—e.g., extract names or upperName from the users array (from the users
variable returned by extDb.user.createManyAndReturn) and assert that the
set/array includes 'ALICE' and 'BOB' (or compare sorted arrays) rather than
checking users[0] and users[1].
- Around line 193-194: The test uses a hardcoded id (1) in extDb.user.exists
which makes it flaky; capture the created user's id from the create call (e.g.,
save the result of extDb.user.create into a variable like createdUser or user)
and use createdUser.id in the exists check (extDb.user.exists({ where: { id:
createdUser.id } })). Update the assertion to use that dynamic id so it reliably
checks existence regardless of auto-increment state.
---
Duplicate comments:
In `@tests/e2e/orm/plugin-infra/ext-result.test.ts`:
- Around line 631-632: The test uses a hardcoded authorId (authorId: 1) when
creating posts which causes flakes because auto-increment IDs may not start at 1
after deleteMany(); update the test to capture the created user result from
extDb.user.create (e.g., const user = await extDb.user.create(...)) and use
user.id for extDb.post.create authorId (and similarly replace other hardcoded
authorId occurrences in this file at the locations noted) so the post references
the actual created user's ID.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4514f3a1-875d-419e-9b2f-908ef03d9668
📒 Files selected for processing (1)
tests/e2e/orm/plugin-infra/ext-result.test.ts
…ultField helper - Add ExtResult parameter to IncludeInput and SelectInput, and pass it through SelectIncludeOmit, so nested relation select/omit inputs recognize ext result field names without requiring `as any` casts - Add resultField() helper function that provides typed compute parameters based on needs keys via correlated type inference - Remove as-any casts from nested relation tests now that types work - Add test for resultField helper Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Import ExtResultFieldDef from plugin.ts instead of redeclaring a local duplicate type, and extract duplicated Zod factory loop into a shared addExtResultFields helper method. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add the source .zmodel file so the generated schema.ts can be regenerated, matching the convention used by sibling test directories. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add input.ts and models.ts generated alongside schema.ts, matching the convention used by sibling test directories. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Move EXT_RESULT_OPERATIONS to a module-level constant to avoid reallocating on every Proxy get trap, and compute collectExtResultFieldDefs once per query instead of once per row. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
Author
|
@coderabbitai resume |
Contributor
✅ Actions performedReviews resumed. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
tests/e2e/orm/plugin-infra/ext-result.test.ts (2)
193-193:⚠️ Potential issue | 🟡 MinorRemove hardcoded PK assumptions in setup and assertions.
Several cases assume IDs start at
1(or thatidDoubled === 2), which makes this suite flaky across DB providers and run order.Suggested fix pattern
- await extDb.user.create({ data: { name: 'Alice' } }); - await extDb.post.create({ data: { title: 'Hello', authorId: 1 } }); + const user = await extDb.user.create({ data: { name: 'Alice' } }); + await extDb.post.create({ data: { title: 'Hello', authorId: user.id } }); - const exists = await extDb.user.exists({ where: { id: 1 } }); + const exists = await extDb.user.exists({ where: { id: user.id } }); - expect(users[0]!.idDoubled).toBe(2); + expect(users[0]!.idDoubled).toBe(users[0]!.id * 2);Also applies to: 343-343, 404-404, 570-570, 581-581, 632-632, 655-655, 684-684, 709-709, 734-734, 763-763, 808-808
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/e2e/orm/plugin-infra/ext-result.test.ts` at line 193, Tests assume primary keys equal specific values (e.g. id === 1 or idDoubled === 2) which is flaky; update the setup and assertions in ext-result.test.ts to capture and use the actual IDs (or other stable identifiers) returned by create/insert operations instead of hardcoding numeric PKs: replace direct checks like extDb.user.exists({ where: { id: 1 } }) and comparisons against literal IDs with checks using the inserted record's id variable (or query by unique fields), and update any related assertions (e.g. idDoubled comparisons) to compute using those captured values so the tests no longer rely on DB-specific starting IDs.
164-169:⚠️ Potential issue | 🟡 MinorMake
createManyAndReturnassertions order-independent.This still depends on returned row order, which is not guaranteed across providers.
Suggested patch
const users = await extDb.user.createManyAndReturn({ data: [{ name: 'Alice' }, { name: 'Bob' }], }); expect(users).toHaveLength(2); -expect(users[0]!.upperName).toBe('ALICE'); -expect(users[1]!.upperName).toBe('BOB'); +const upperNames = users.map((u) => u.upperName).sort(); +expect(upperNames).toEqual(['ALICE', 'BOB']);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/e2e/orm/plugin-infra/ext-result.test.ts` around lines 164 - 169, The test assumes a specific row order from extDb.user.createManyAndReturn; make it order-independent by extracting the returned users' upperName values (from the createManyAndReturn result) and asserting the set/array contains both 'ALICE' and 'BOB' (e.g., compare sorted arrays or use an unordered containment assertion such as expect(...).toEqual(expect.arrayContaining([...]))), rather than checking users[0] and users[1] directly.
🤖 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/plugin-infra/ext-result.test.ts`:
- Around line 603-608: The test currently asserts posts by position after
calling extDb.user.findMany({ include: { posts: true } }) causing flakiness;
update the assertion to be content-based or deterministic by either sorting the
returned posts array (e.g., by id or title) before asserting or finding each
post by its upperTitle/value and asserting their contents, targeting the posts
collection on users[0] (users[0].posts) and the upperTitle property (upperTitle)
rather than relying on posts[0]/posts[1] positions.
---
Duplicate comments:
In `@tests/e2e/orm/plugin-infra/ext-result.test.ts`:
- Line 193: Tests assume primary keys equal specific values (e.g. id === 1 or
idDoubled === 2) which is flaky; update the setup and assertions in
ext-result.test.ts to capture and use the actual IDs (or other stable
identifiers) returned by create/insert operations instead of hardcoding numeric
PKs: replace direct checks like extDb.user.exists({ where: { id: 1 } }) and
comparisons against literal IDs with checks using the inserted record's id
variable (or query by unique fields), and update any related assertions (e.g.
idDoubled comparisons) to compute using those captured values so the tests no
longer rely on DB-specific starting IDs.
- Around line 164-169: The test assumes a specific row order from
extDb.user.createManyAndReturn; make it order-independent by extracting the
returned users' upperName values (from the createManyAndReturn result) and
asserting the set/array contains both 'ALICE' and 'BOB' (e.g., compare sorted
arrays or use an unordered containment assertion such as
expect(...).toEqual(expect.arrayContaining([...]))), rather than checking
users[0] and users[1] directly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2fb575b0-4834-494a-997a-374be229765f
📒 Files selected for processing (8)
packages/orm/src/client/client-impl.tspackages/orm/src/client/crud-types.tspackages/orm/src/client/plugin.tspackages/orm/src/client/zod/factory.tstests/e2e/orm/plugin-infra/ext-result.test.tstests/e2e/orm/plugin-infra/ext-result/input.tstests/e2e/orm/plugin-infra/ext-result/models.tstests/e2e/orm/plugin-infra/ext-result/schema.zmodel
✅ Files skipped from review due to trivial changes (1)
- packages/orm/src/client/crud-types.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/orm/src/client/zod/factory.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implement a new
resultextension point for plugins to declare computed fields on query results with full type safety and select/omit awareness. The feature includes recursive runtime computation, automatic dependency (needs) injection/stripping, and complete support for nested relations.Implementation
ExtResultFieldDefandExtResultBasetypes to plugin definitionsExtResultgeneric throughClientContract,ModelResult, and all CRUD return typesSelectAwareExtResulttype that respects select/omit clausesprepareArgsForExtResultandapplyExtResultruntime functions for nested relations$use,$unuse,$transaction, etc.)Testing
Usage Example
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Refactor
Tests