fix(ai): bound agent tool-call fan-out (#964)#965
Conversation
…allsPerTurn maxIterations only counts model turns, so one turn can emit unbounded parallel tool calls. Expose toolCallCount on AgentLoopState, add a maxToolCalls strategy, and cap per-turn execution with maxToolCallsPerTurn. Closes #964
📝 WalkthroughWalkthroughThe agent loop now tracks cumulative and per-turn tool calls, exports a ChangesTool Call Budgeting
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant Chat
participant TextEngine
participant Adapter
participant Tool
Client->>Chat: start chat with tool-call limits
Chat->>TextEngine: configure maxToolCallsPerTurn
TextEngine->>Adapter: request model turn
Adapter-->>TextEngine: return parallel tool calls
TextEngine->>Tool: execute allowed calls
TextEngine-->>Chat: emit executed and skipped tool results
Chat-->>Client: stream completed response
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
testing/e2e/tests/max-tool-calls.spec.tsParsing error: "parserOptions.project" has been provided for 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 |
🚀 Changeset Version Preview1 package(s) bumped directly, 44 bumped as dependents. 🟥 Major bumps
🟨 Minor bumps
🟩 Patch bumps
|
|
View your CI Pipeline Execution ↗ for commit f9858a6
☁️ Nx Cloud last updated this comment at |
@tanstack/ai
@tanstack/ai-acp
@tanstack/ai-angular
@tanstack/ai-anthropic
@tanstack/ai-bedrock
@tanstack/ai-claude-code
@tanstack/ai-client
@tanstack/ai-code-mode
@tanstack/ai-code-mode-skills
@tanstack/ai-codex
@tanstack/ai-devtools-core
@tanstack/ai-elevenlabs
@tanstack/ai-event-client
@tanstack/ai-fal
@tanstack/ai-gemini
@tanstack/ai-grok
@tanstack/ai-grok-build
@tanstack/ai-groq
@tanstack/ai-isolate-cloudflare
@tanstack/ai-isolate-node
@tanstack/ai-isolate-quickjs
@tanstack/ai-mcp
@tanstack/ai-mistral
@tanstack/ai-ollama
@tanstack/ai-openai
@tanstack/ai-opencode
@tanstack/ai-openrouter
@tanstack/ai-preact
@tanstack/ai-react
@tanstack/ai-react-ui
@tanstack/ai-sandbox
@tanstack/ai-sandbox-cloudflare
@tanstack/ai-sandbox-daytona
@tanstack/ai-sandbox-docker
@tanstack/ai-sandbox-local-process
@tanstack/ai-sandbox-sprites
@tanstack/ai-sandbox-vercel
@tanstack/ai-solid
@tanstack/ai-solid-ui
@tanstack/ai-svelte
@tanstack/ai-utils
@tanstack/ai-vue
@tanstack/ai-vue-ui
@tanstack/openai-base
@tanstack/preact-ai-devtools
@tanstack/react-ai-devtools
@tanstack/solid-ai-devtools
commit: |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/ai/src/activities/chat/index.ts`:
- Around line 1979-2013: Clamp maxToolCallsPerTurn to a non-negative value in
applyToolCallBudget before using it for comparisons, slicing, and skipped-result
generation. Ensure negative configuration values execute zero tool calls and
mark the entire batch as skipped, while preserving the existing behavior for
null, zero, and positive caps.
In `@testing/e2e/tests/max-tool-calls.spec.ts`:
- Around line 1-7: Update the header comment in the
maxToolCalls/maxToolCallsPerTurn spec to explicitly state that it uses the fully
synthetic in-process createFatParallelAdapter, never reaches a real LLM
provider, and is therefore exempt from aimock wiring under the policy for
runtime/infrastructure-only tests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 15f0b90b-96c1-457e-95c4-7dc9557d3d34
📒 Files selected for processing (14)
.changeset/max-tool-calls-budget.mddocs/api/ai.mddocs/chat/agentic-cycle.mddocs/config.jsonpackages/ai/skills/ai-core/tool-calling/SKILL.mdpackages/ai/src/activities/chat/agent-loop-strategies.tspackages/ai/src/activities/chat/index.tspackages/ai/src/index.tspackages/ai/src/types.tspackages/ai/tests/agent-loop-strategies.test.tspackages/ai/tests/chat.test.tstesting/e2e/src/routeTree.gen.tstesting/e2e/src/routes/api.max-tool-calls-wire.tstesting/e2e/tests/max-tool-calls.spec.ts
| private applyToolCallBudget(toolCalls: Array<ToolCall>): { | ||
| toExecute: Array<ToolCall> | ||
| skippedResults: Array<ToolResult> | ||
| } { | ||
| this.lastTurnToolCallCount = toolCalls.length | ||
| this.toolCallCount += toolCalls.length | ||
|
|
||
| const cap = this.maxToolCallsPerTurn | ||
| if (cap == null || toolCalls.length <= cap) { | ||
| return { toExecute: toolCalls, skippedResults: [] } | ||
| } | ||
|
|
||
| this.logger.agentLoop( | ||
| `maxToolCallsPerTurn=${cap} skipped=${toolCalls.length - cap}`, | ||
| { | ||
| maxToolCallsPerTurn: cap, | ||
| emitted: toolCalls.length, | ||
| skipped: toolCalls.length - cap, | ||
| }, | ||
| ) | ||
|
|
||
| const toExecute = toolCalls.slice(0, cap) | ||
| const skippedResults: Array<ToolResult> = toolCalls | ||
| .slice(cap) | ||
| .map((tc) => ({ | ||
| toolCallId: tc.id, | ||
| toolName: tc.function.name, | ||
| result: { | ||
| error: `Skipped: exceeded maxToolCallsPerTurn (${cap})`, | ||
| }, | ||
| state: 'output-error' as const, | ||
| })) | ||
|
|
||
| return { toExecute, skippedResults } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
node -e "console.log([0,1,2,3,4,5,6,7].slice(0,-1))"Repository: TanStack/ai
Length of output: 177
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the option wiring and any validation/defaulting for maxToolCallsPerTurn.
rg -n "maxToolCallsPerTurn" packages/ai/src -S
# Inspect the relevant implementation area around applyToolCallBudget.
sed -n '1920,2035p' packages/ai/src/activities/chat/index.ts
# Inspect the option/type definition if present.
fd -a "types.ts" packages/ai/srcRepository: TanStack/ai
Length of output: 5672
Clamp maxToolCallsPerTurn to 0 or greater before slicing. Negative values make slice(0, cap) execute most of the batch and only skip the tail, so a bad config defeats the intended per-turn cap.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/ai/src/activities/chat/index.ts` around lines 1979 - 2013, Clamp
maxToolCallsPerTurn to a non-negative value in applyToolCallBudget before using
it for comparisons, slicing, and skipped-result generation. Ensure negative
configuration values execute zero tool calls and mark the entire batch as
skipped, while preserving the existing behavior for null, zero, and positive
caps.
| import { expect, test } from '@playwright/test' | ||
|
|
||
| /** | ||
| * Regression for issue #964: maxIterations counts model turns, not tool calls. | ||
| * maxToolCalls + maxToolCallsPerTurn must bound fan-out from a single fat turn. | ||
| */ | ||
| test.describe('chat() maxToolCalls / maxToolCallsPerTurn (#964)', () => { |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Document the aimock exemption in the header comment.
This spec exercises a fully synthetic in-process adapter (createFatParallelAdapter in api.max-tool-calls-wire.ts) that never reaches a real LLM provider, so it's exempt from the aimock-wiring policy — but that rationale isn't stated here.
Based on learnings, "if the E2E spec only tests runtime/infrastructure behavior ... and the code path under test never reaches the provider HTTP layer, do not add aimock wiring ... document the policy exception in the spec's header comment".
📝 Suggested header addition
/**
* Regression for issue `#964`: maxIterations counts model turns, not tool calls.
* maxToolCalls + maxToolCallsPerTurn must bound fan-out from a single fat turn.
+ *
+ * Uses a synthetic in-process adapter (no real provider HTTP call), so aimock
+ * wiring is not applicable here.
*/📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { expect, test } from '@playwright/test' | |
| /** | |
| * Regression for issue #964: maxIterations counts model turns, not tool calls. | |
| * maxToolCalls + maxToolCallsPerTurn must bound fan-out from a single fat turn. | |
| */ | |
| test.describe('chat() maxToolCalls / maxToolCallsPerTurn (#964)', () => { | |
| import { expect, test } from '`@playwright/test`' | |
| /** | |
| * Regression for issue `#964`: maxIterations counts model turns, not tool calls. | |
| * maxToolCalls + maxToolCallsPerTurn must bound fan-out from a single fat turn. | |
| * | |
| * Uses a synthetic in-process adapter (no real provider HTTP call), so aimock | |
| * wiring is not applicable here. | |
| */ | |
| test.describe('chat() maxToolCalls / maxToolCallsPerTurn (`#964`)', () => { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@testing/e2e/tests/max-tool-calls.spec.ts` around lines 1 - 7, Update the
header comment in the maxToolCalls/maxToolCallsPerTurn spec to explicitly state
that it uses the fully synthetic in-process createFatParallelAdapter, never
reaches a real LLM provider, and is therefore exempt from aimock wiring under
the policy for runtime/infrastructure-only tests.
Source: Learnings
Apply maxToolCallsPerTurn to pending/resume batches, reject negative caps, dedupe toolCallCount on wait→resume, and lock skipped-emission counting with unit tests. Clarify post-turn / emitted-budget docs.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/ai/tests/chat.test.ts (1)
1302-1341: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate skip-filter assertion logic across tests.
The
chunks.filter(...)block checking forTOOL_CALL_RESULTevents whose content includes'exceeded maxToolCallsPerTurn'is repeated near-verbatim at Lines 1330-1337 and 1422-1429 (and likely elsewhere per the earlier maxToolCallsPerTurn tests). Extracting a small helper (e.g.filterSkippedBudgetResults(chunks)) would reduce the risk of these assertions drifting out of sync if the skip-error wording ever changes.♻️ Proposed helper extraction
+function filterSkippedBudgetResults(chunks: Array<StreamChunk>) { + return chunks.filter((c) => { + if (c.type !== 'TOOL_CALL_RESULT') return false + const content = (c as { content?: unknown }).content + return ( + typeof content === 'string' && + content.includes('exceeded maxToolCallsPerTurn') + ) + }) +}Then in each test:
- const skipped = chunks.filter((c) => { - if (c.type !== 'TOOL_CALL_RESULT') return false - const content = (c as { content?: unknown }).content - return ( - typeof content === 'string' && - content.includes('exceeded maxToolCallsPerTurn') - ) - }) + const skipped = filterSkippedBudgetResults(chunks)Also applies to: 1422-1429
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ai/tests/chat.test.ts` around lines 1302 - 1341, Extract the repeated TOOL_CALL_RESULT filtering logic into a shared helper such as filterSkippedBudgetResults near the related test utilities, matching content containing “exceeded maxToolCallsPerTurn”. Replace the inline chunks.filter assertions in the maxToolCallsPerTurn tests, including the cases around both referenced sections, with this helper while preserving their existing length assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/ai/tests/chat.test.ts`:
- Around line 1302-1341: Extract the repeated TOOL_CALL_RESULT filtering logic
into a shared helper such as filterSkippedBudgetResults near the related test
utilities, matching content containing “exceeded maxToolCallsPerTurn”. Replace
the inline chunks.filter assertions in the maxToolCallsPerTurn tests, including
the cases around both referenced sections, with this helper while preserving
their existing length assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 46b84fcb-140a-4c14-a409-32a1a0c76420
📒 Files selected for processing (7)
docs/api/ai.mddocs/chat/agentic-cycle.mdpackages/ai/src/activities/chat/agent-loop-strategies.tspackages/ai/src/activities/chat/index.tspackages/ai/src/types.tspackages/ai/tests/chat.test.tstesting/e2e/tests/max-tool-calls.spec.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- testing/e2e/tests/max-tool-calls.spec.ts
- packages/ai/src/types.ts
- docs/api/ai.md
- packages/ai/src/activities/chat/agent-loop-strategies.ts
- docs/chat/agentic-cycle.md
- packages/ai/src/activities/chat/index.ts
Summary
AgentLoopStatenow includestoolCallCountandlastTurnToolCallCountso strategies can budget tools, not just model turnsmaxToolCalls(n)strategy stops the agent loop once cumulative tool calls hit the limitchat({ maxToolCallsPerTurn })option caps how many parallel tool calls execute in a single turn (excess get error results so history stays consistent)Fixes the report where
maxIterations(20)still allowed ~160 tool calls in a few fat turns and blew serverless timeouts.Usage
Test plan
maxToolCallsstrategy behaviortoolCallCount/lastTurnToolCallCountmaxToolCallsPerTurnexecutes only the first N calls and emits skip errors for the resttesting/e2e/tests/max-tool-calls.spec.ts(fat 8-call turn → 3 executed, 5 skipped)@tanstack/aiCloses #964
Summary by CodeRabbit
New Features
maxToolCallsPerTurnlimit.Documentation
maxIterationsbounds model turns (not tool calls).maxToolCallsand combining strategies with per-turn tool-call caps.