feat: replace tool routers with searchTools + callTool tool routing#991
Conversation
🦋 Changeset detectedLatest commit: 02aa85a The changes in this PR will be included in the next version bump. This PR includes changesets to release 24 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
This comment has been minimized.
This comment has been minimized.
📝 WalkthroughWalkthroughThis PR replaces router-based tool routing with a search-first workflow: agents expose two system tools— Changes
Sequence Diagram(s)sequenceDiagram
participant Model as Model/Agent
participant SearchTool as searchTools
participant CallTool as callTool
participant ToolPool as Tool Pool
participant Tool as Target Tool
Model->>SearchTool: searchTools(user query)
activate SearchTool
Note over SearchTool: embedding-based ranking (topK)\nreturns ToolSearchResult with schemas
SearchTool-->>Model: ranked results
deactivate SearchTool
Note over Model: record searched tools in context
Model->>CallTool: callTool(toolName, validatedArgs)
activate CallTool
Note over CallTool: validate args against schema
opt enforceSearchBeforeCall
CallTool->>CallTool: assert tool was in prior search
end
CallTool->>ToolPool: locate tool by name
ToolPool-->>CallTool: tool reference
CallTool->>Tool: execute(args)
Tool-->>CallTool: result
CallTool-->>Model: execution result
deactivate CallTool
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
Deploying voltagent with
|
| Latest commit: |
02aa85a
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://da90182d.voltagent.pages.dev |
| Branch Preview URL: | https://feat-improvement-tool-routin.voltagent.pages.dev |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/src/tool/routing/embedding.ts (1)
40-65: Add a safe fallback if schema normalization throws.
normalizeParametersForTextassumes schema conversion will always succeed; a thrown error here would break tool routing. A small try/catch keeps routing resilient.🛠️ Proposed fix
const normalizeParametersForText = (value: unknown): unknown => { if (!value || typeof value !== "object") { return value; } const candidate = value as { _def?: unknown; def?: unknown }; if (candidate._def || candidate.def) { - return zodSchemaToJsonUI(value); + try { + return zodSchemaToJsonUI(value); + } catch { + return value; + } } return value; };
🤖 Fix all issues with AI agents
In `@docs/tool-routing-plan.md`:
- Around line 54-59: Update the "### 6) Tests" checklist to reflect the current
implementation status: for the header "### 6) Tests" edit the four items
("Search selection tests (embedding + fallback)", "callTool validation + error
path tests", "Provider tool fallback tests", "Embedding cache tests") to mark
completed tests with checked boxes or remove/annotate items that are not
applicable, and add brief references to the corresponding test files or test
suites (e.g., unit/integration test names) if available so the checklist is no
longer stale.
In `@packages/core/src/agent/agent.ts`:
- Around line 4791-4800: When toolRouting is false, do not silently filter out
tools named by getToolRoutingSupportToolNames; instead detect conflicts between
supportNames and keys of preparedStaticTools/preparedDynamicTools and surface a
clear error. In the block after resolving toolRouting (resolveToolRouting and
setting TOOL_ROUTING_CONTEXT_KEY), compute the intersection of supportNames
(from getToolRoutingSupportToolNames) with Object.keys(preparedStaticTools) and
Object.keys(preparedDynamicTools); if any conflicts exist, throw an Error (or
use the agent’s logger) listing the conflicting tool names and referencing
toolRouting so callers can fix naming, rather than silently removing entries.
- Around line 5549-5562: The code currently throws a generic Error when
needsApproval === true which bypasses the ToolDeniedError handling; remove the
throw and always delegate to ensureToolApproval so approval failures follow the
existing abort flow. Replace the throw in the needsApproval === true branch with
a call to this.ensureToolApproval(tool, args, executionOptions,
executionOptions.toolContext?.callId ?? randomUUID()), ensuring
ensureToolApproval (and ToolDeniedError) handles denials for provider tools and
uses tool.name/args/executionOptions for context.
- Around line 5597-5604: The current comparison uses safeStringify(requested)
!== safeStringify(received) which can fail for equivalent objects with different
key orders; replace this stringified check with a deep object equality check
(e.g., use a reliable utility like deepEqual or lodash.isEqual) to compare args
and toolCall.input (callInput) directly and keep the existing error throw
referencing tool.name if the deep equality fails; update imports and tests
accordingly so the equality check uses the chosen deep comparer rather than
safeStringify.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/core/src/agent/agent.ts`:
- Around line 5160-5176: The search candidates should exclude tools that cannot
be executed by callTool: in buildToolSearchCandidates, after the existing
isToolRoutingSupportTool filter, also filter out tools that are non-executable
by checking for an executable signature (e.g., typeof (tool as any).call !==
"function" or an explicit flag like (tool as any).callable === false) so only
runnable tools reach the map; reference buildToolSearchCandidates,
toolPoolManager.getAllTools, isToolRoutingSupportTool, isProviderTool and
callTool when making the change.
| toolCallId, | ||
| error: `Tool ${tool.name} requires approval.`, | ||
| }; | ||
| private recordSearchedTools(options: ToolExecuteOptions | undefined, toolNames: string[]): void { |
There was a problem hiding this comment.
There's a minor issue here: the context only exists in a single agent call. This means that multiple calls to the same conversation will require the agent to repeatedly use the search tool, even if the context has already been searched. A better approach would be to store the records in the conversation.
what about do you think? @omeraplak
There was a problem hiding this comment.
hmmm the conversation table might not be a perfect fit for this. We could consider creating a separate table instead.
As a temporary workaround, something like this could work. You keep a per-conversation context in memory and pass the same context object across calls so the agent can maintain state between messages.
// simple in-memory store (replace with Redis/DB in production)
const conversationContexts = new Map<string, Map<string | symbol, unknown>>();
function getConversationContext(conversationId: string) {
let ctx = conversationContexts.get(conversationId);
if (!ctx) {
ctx = new Map();
conversationContexts.set(conversationId, ctx);
}
return ctx;
}
// usage
const conversationId = "conv_123";
const context = getConversationContext(conversationId);
await agent.streamText("first message", { conversationId, context });
await agent.streamText("second message", { conversationId, context });There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/src/agent/agent.ts (1)
5567-5659: Force deterministic temperature for provider-tool proxy calls.Using the agent’s temperature can make the proxy call non-deterministic and trigger false arg-mismatch errors. This should be forced to 0 to keep callTool stable.
🔧 Proposed fix
- temperature: this.temperature ?? 0, + temperature: 0,
PR Checklist
Please check if your PR fulfills the following requirements:
Bugs / Features
What is the current behavior?
What is the new behavior?
fixes (issue)
Notes for reviewers
Summary by cubic
Replaces router tools with searchTools and callTool for tool routing. Simplifies the model surface, adds embedding-based search, validates tool calls, and updates docs and examples. Fixes #989.
New Features
Migration
Written for commit 02aa85a. Summary will update on new commits.
Summary by CodeRabbit
Breaking Changes
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.