Conversation
… double validation Refactors validation handling in tool creation by setting input/output validation indices to NaN, replacing the previous proxy-based approach. This prevents validation from occurring twice - once at the tool level and once at the procedure call level - which was causing issues when schemas transform data into different shapes. Includes test coverage for the new validation disabling behavior and updates related tRPC integration to use the same approach.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
dosubot
bot
added
the
size:L
|
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. WalkthroughRefactors tool execution to accept calling options (including an abort signal), wraps calls with a temporary Procedure configured to disable oRPC input/output validation via NaN indices, updates related imports and tests, and updates docs to remove a duplicate-validation warning. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant Tool
participant Wrapper as Procedure (NaN indices)
participant CallClient as oRPC call
participant Handler
Caller->>Tool: execute(input, { abortSignal })
Tool->>Wrapper: create Procedure with inputValidationIndex=NaN / outputValidationIndex=NaN
Tool->>Wrapper: call(input, { abortSignal })
Wrapper->>CallClient: invoke underlying call (propagate abortSignal)
CallClient->>Handler: execute handler with abortSignal
Handler-->>CallClient: result
CallClient-->>Wrapper: result
Wrapper-->>Tool: result
Tool-->>Caller: Promise<result>
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)packages/ai-sdk/src/tool.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 (2)
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 |
Summary of ChangesHello @unnoq, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request streamlines the validation process for tools integrated with oRPC by eliminating redundant validation steps. Previously, tools could undergo validation twice, leading to potential issues with data transformation. The changes ensure that validation occurs only where intended, improving consistency and preventing unexpected failures, particularly in scenarios involving complex schema transformations. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a significant improvement by disabling validation at the oRPC level in createTool to prevent double validation, which is a cleaner approach than the previous proxy-based method. The changes are well-implemented across the ai-sdk and trpc packages, with corresponding updates to tests and documentation. However, I've identified a critical typing issue in packages/ai-sdk/src/tool.ts where a type cast incorrectly removes the Promise type from a return value, which could lead to runtime errors.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
More templates
@orpc/ai-sdk
@orpc/arktype
@orpc/client
@orpc/contract
@orpc/experimental-durable-iterator
@orpc/hey-api
@orpc/interop
@orpc/json-schema
@orpc/nest
@orpc/openapi
@orpc/openapi-client
@orpc/otel
@orpc/experimental-publisher
@orpc/react
@orpc/react-query
@orpc/experimental-react-swr
@orpc/server
@orpc/shared
@orpc/solid-query
@orpc/standard-server
@orpc/standard-server-aws-lambda
@orpc/standard-server-fastify
@orpc/standard-server-fetch
@orpc/standard-server-node
@orpc/standard-server-peer
@orpc/svelte-query
@orpc/tanstack-query
@orpc/trpc
@orpc/valibot
@orpc/vue-colada
@orpc/vue-query
@orpc/zod
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/content/docs/integrations/ai-sdk.md(2 hunks)packages/ai-sdk/src/tool.test.ts(3 hunks)packages/ai-sdk/src/tool.ts(2 hunks)packages/server/src/procedure-client.test.ts(1 hunks)packages/trpc/src/to-orpc-router.test.ts(2 hunks)packages/trpc/src/to-orpc-router.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
packages/server/src/procedure-client.test.ts (3)
packages/server/src/procedure.ts (1)
Procedure(55-71)packages/server/src/builder.ts (1)
input(243-251)packages/server/src/procedure-client.ts (1)
createProcedureClient(72-165)
packages/ai-sdk/src/tool.test.ts (1)
packages/ai-sdk/src/tool.ts (1)
createTool(131-163)
packages/trpc/src/to-orpc-router.test.ts (2)
packages/contract/tests/shared.ts (2)
inputSchema(6-6)outputSchema(8-8)packages/server/src/procedure.ts (1)
Procedure(55-71)
packages/trpc/src/to-orpc-router.ts (1)
packages/shared/src/object.ts (1)
isTypescriptObject(57-59)
packages/ai-sdk/src/tool.ts (1)
packages/server/src/procedure.ts (1)
Procedure(55-71)
⏰ 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: publish-commit
- GitHub Check: lint
- GitHub Check: test
🔇 Additional comments (8)
packages/trpc/src/to-orpc-router.ts (1)
94-98: NaN sentinel cleanly disables duplicate validationSwapping the proxy for NaN indices keeps schema metadata intact while letting tRPC remain authoritative on parsing.
apps/content/docs/integrations/ai-sdk.md (1)
163-165: Helpful inline hintThe placeholder makes future overrides clearer without altering current behavior.
packages/server/src/procedure-client.test.ts (1)
656-672: Great coverage for the NaN sentinelNice regression test confirming that bypassing validation via NaN pipes raw input through exactly as designed.
packages/ai-sdk/src/tool.test.ts (1)
84-129: Tool tests capture abort + no-validation flowAsync execute plus the invalid-input assertion line up perfectly with the new createTool behavior.
packages/trpc/src/to-orpc-router.test.ts (1)
34-50: Test proves TRPC keeps validation while oRPC skipsAsserting both the NaN indices and the divergent call outcomes gives great confidence in the bridge.
packages/ai-sdk/src/tool.ts (3)
157-159: Type casts are necessary but verify AI SDK performs validation.The type casts on lines 157 and 159 bypass TypeScript's type safety. This is necessary because disabling oRPC validation creates a type mismatch—the AI SDK validates the input/output using schemas, producing types that don't perfectly align with oRPC's expectations. Ensure that the AI SDK properly validates both input and output, as oRPC validation is now disabled.
160-160: Good use ofsatisfiesfor type safety.The
satisfiesconstraint ensures the execute function matches the expected Tool signature while preserving type inference. This is a robust approach to maintain type safety.
149-153: I need to examine how thenextfunction is called to verify whether the NaN approach actually works correctly:Now let me check how the
nextfunction is invoked to understand the full middleware loop:Review comment needs clarification: NaN-based disablement technically works but relies on unintuitive JavaScript behavior.
The implementation does disable validation, but the mechanism is fragile. When comparing numbers, they're considered equal only if both are not NaN, and NaN compares unequal to any other value — including to another NaN value. When
inputValidationIndexandoutputValidationIndexare set toNumber.NaN, the equality checks at lines 234 and 268 ofprocedure-client.ts(if (index === inputValidationIndex)) will never match any numeric index, thus skipping validation.However, this approach relies on NaN's implicit comparison behavior rather than an explicit sentinel pattern. Math.min and Math.max return NaN if any parameter is or is converted into NaN, so the clamping operation preserves NaN correctly. The pattern works as intended but lacks clarity—using an explicit value like
-1with a dedicated check would be more maintainable and self-documenting.No actual issues were found with the implementation, but the review comment's verification request has been satisfied: the NaN-based approach does prevent validation execution as designed.
1 participant
Refactors validation handling in tool creation by setting input/output validation indices to NaN, replacing the previous proxy-based approach. This prevents validation from occurring twice - once at the tool level and once at the procedure call level - which was causing issues when schemas transform data into different shapes. Includes test coverage for the new validation disabling behavior and updates related tRPC integration to use the same approach.
Summary by CodeRabbit
New Features
Documentation
Tests