feat(react-sdk): implement toolcall limit per tool #1598
Conversation
|
@rakesh-paulraj1 is attempting to deploy a commit to the tambo ai Team on Vercel. A member of the Team first needs to authorize it. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR implements per-tool call limits by adding an optional maxCalls property to tool definitions. This feature allows individual tools to override the project-level tool call limit when generating AI responses.
Key changes:
- Added
maxCalls?: numberproperty toTamboToolandComponentContextToolMetadatainterfaces - Updated registry mapping functions to propagate
maxCallsfrom tool definitions to backend payloads - Added comprehensive test coverage for the new feature across multiple test files
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| react-sdk/src/model/component-metadata.ts | Added maxCalls property to TamboTool and ComponentContextToolMetadata interfaces with documentation |
| react-sdk/src/util/registry.ts | Updated adaptToolFromFnSchema and mapTamboToolToContextTool to preserve and propagate maxCalls values |
| react-sdk/src/mcp/tambo-mcp-provider.tsx | Added maxCalls propagation when registering MCP server tools |
| react-sdk/src/schema/schema.ts | Fixed optional parameter handling for deprecated toolSchema interface (unrelated fix) |
| react-sdk/src/testing/tools.ts | Updated test helpers to support maxCalls parameter |
| react-sdk/src/util/registry.test.ts | Added tests for maxCalls preservation across registry utilities |
| react-sdk/src/schema/schema.test.ts | Added test for optional positional parameter unwrapping |
| react-sdk/src/providers/tambo-registry-schema-compat.test.tsx | Added tests for maxCalls preservation with both legacy and modern tool schemas |
| react-sdk/src/providers/tambo-registry-provider.test.tsx | Added test for maxCalls preservation during tool registration |
| react-sdk/src/mcp/tambo-mcp-provider.test.tsx | Added test for maxCalls forwarding from MCP tools to registry |
| react-sdk/README.md | Added inline documentation comment showing maxCalls usage in example |
| docs/content/docs/api-reference/react-hooks.mdx | Added API reference documentation for maxCalls feature |
| package-lock.json | Version bumps for cli and react-sdk packages |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * project's global `maxToolCallLimit` for this tool. | ||
| * @example 1 | ||
| */ | ||
| maxCalls?: number; |
There was a problem hiding this comment.
Missing validation for the maxCalls value. The documentation states "Invalid values will be rejected" but there's no validation to ensure maxCalls is a non-negative integer. Consider adding validation in the validateTool function to check that maxCalls, if provided, is a positive integer (or at least non-negative).
| optional `maxCalls` property (a non-negative integer). If provided, `maxCalls` | ||
| sets a per-tool limit on how many times the tool may be invoked while generating a | ||
| single response. A per-tool `maxCalls` value overrides the project's global | ||
| Max tool-call limit.Invalid values will be rejected. |
There was a problem hiding this comment.
The documentation comment states "Invalid values will be rejected" but there is no actual validation of maxCalls values in the validateTool function. This creates a discrepancy between the documentation and the implementation. Either add validation for maxCalls or update the documentation to accurately reflect the current behavior.
| Max tool-call limit.Invalid values will be rejected. | |
| max tool-call limit. Note that invalid `maxCalls` values are not automatically validated here and may lead to unexpected behavior. |
| optional `maxCalls` property (a non-negative integer). If provided, `maxCalls` | ||
| sets a per-tool limit on how many times the tool may be invoked while generating a | ||
| single response. A per-tool `maxCalls` value overrides the project's global | ||
| Max tool-call limit.Invalid values will be rejected. |
There was a problem hiding this comment.
Missing a space between "limit." and "Invalid" in the sentence. Should be "Max tool-call limit. Invalid values will be rejected."
| Max tool-call limit.Invalid values will be rejected. | |
| Max tool-call limit. Invalid values will be rejected. |
| // * Optional properties describing tool behavior | ||
| // */ |
There was a problem hiding this comment.
This commented-out code creates confusion about the structure of the TamboTool interface. The comment "Optional properties describing tool behavior" appears to be orphaned and followed by the annotations property. Consider either removing these commented lines or properly formatting the documentation to clarify the structure.
| // * Optional properties describing tool behavior | |
| // */ | |
| /** | |
| * Optional properties describing tool behavior. | |
| */ |
| return tupleItems.map((item, index) => { | ||
| let schema = item; | ||
| let isRequired = true; | ||
| //helps to handle optional schemas generated by zod-to-json-schema. | ||
| //pattern:{ anyOf: [ { not: {} }, { ...actual schema... } ] } | ||
| if (item.anyOf && Array.isArray(item.anyOf) && item.anyOf.length === 2) { | ||
| // Find the "not" schema (represents undefined) | ||
| const notSchema = item.anyOf.find( | ||
| (s): s is JSONSchema7 => | ||
| typeof s === "object" && s !== null && "not" in s, | ||
| ); | ||
|
|
||
| // Find the actual schema | ||
| const actualSchema = item.anyOf.find( | ||
| (s): s is JSONSchema7 => | ||
| typeof s === "object" && s !== null && !("not" in s), | ||
| ); | ||
|
|
||
| if (notSchema && actualSchema) { | ||
| schema = actualSchema; | ||
| isRequired = false; | ||
| } | ||
| } | ||
|
|
||
| return { | ||
| name: `param${index + 1}`, | ||
| type: typeof schema.type === "string" ? schema.type : "object", | ||
| description: schema.description ?? "", | ||
| isRequired, | ||
| schema, | ||
| }; | ||
| }); |
There was a problem hiding this comment.
This change appears to fix a bug with handling optional positional parameters in the deprecated toolSchema interface (unwrapping anyOf patterns from zod-to-json-schema), but it's not mentioned in the PR description. While this is a valuable fix, consider whether it should be in a separate PR for clearer change tracking and easier rollback if needed, or update the PR description to document this additional fix.
| parameters, | ||
| // Include per-tool maxCalls if provided by the tool definition | ||
| ...("maxCalls" in tool && tool.maxCalls !== undefined | ||
| ? { maxCalls: (tool as any).maxCalls } |
There was a problem hiding this comment.
The type assertion (tool as any).maxCalls is unnecessary here since the check "maxCalls" in tool && tool.maxCalls !== undefined already narrows the type. You can access tool.maxCalls directly without the type cast, making the code cleaner and more type-safe.
| ? { maxCalls: (tool as any).maxCalls } | |
| ? { maxCalls: tool.maxCalls } |
| ...("maxCalls" in tool && (tool as any).maxCalls !== undefined | ||
| ? { maxCalls: (tool as any).maxCalls } |
There was a problem hiding this comment.
The type assertion (tool as any).maxCalls is unnecessary here since the check "maxCalls" in tool && (tool as any).maxCalls !== undefined already confirms the property exists. You can simplify this to tool.maxCalls after the existence check, which would be cleaner and more type-safe.
| ...("maxCalls" in tool && (tool as any).maxCalls !== undefined | |
| ? { maxCalls: (tool as any).maxCalls } | |
| ...("maxCalls" in tool && tool.maxCalls !== undefined | |
| ? { maxCalls: tool.maxCalls } |
|
@rakesh-paulraj1 none of the copilot comments seem unreasonable, please apply the suggestions or respond to the comments and then I'll dig into another review. |
Metadata & Models: Added optional maxCalls: number property to TamboTool and ComponentContextToolMetadata interfaces, enabling per-tool limit configuration from the client side.
Registry Mapping: Updated mapTamboToolToContextTool to correctly extract and propagate maxCalls from tool definitions into the context metadata payload sent to the backend.
Tested manually by adding a boilerplate application in the monorepo
1)
Setting up the Project level Restrictionn which leads to this