Make model tool call errors recoverable - #7452
Conversation
🦋 Changeset detectedLatest commit: e16a01f The changes in this PR will be included in the next version bump. This PR includes changesets to release 30 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 |
5025d54 to
c9ca41a
Compare
Bundle Size AnalysisGenerated from PR build output; treat the content below as untrusted.
|
c9ca41a to
89a5342
Compare
89a5342 to
c93e6ec
Compare
c93e6ec to
8a7151d
Compare
8a7151d to
1226f8d
Compare
1226f8d to
2d8ecc8
Compare
2d8ecc8 to
43bc5b9
Compare
43bc5b9 to
3937a48
Compare
3937a48 to
1cbfb23
Compare
IMax153
left a comment
IMax153
left a comment
There was a problem hiding this comment.
I've done a cursory review and noted some significant bugs that could result from this proposal.
However, I'm not too sure in general about the changes proposed by this PR because they would materially change how invalid model-generated calls are handled by LanguageModel.
Instead of being surfaced in the Effect error channel, operations like generateText would succeed, which may result in existing callers silently missing the fact that you now need to interrogate response.toolCallErrors to get at invalid tool calls.
I think the API needs to be revisited. It may be better to have an opt-in option on tool call declarations to enable this behavior (like we have with failureMode), or a different API altogether.
I don't think I can merge this proposal in its current form.
There was a problem hiding this comment.
At this point, toolCall.params would have already been decoded by normalizeToolCall. This decodes it a second time.
For tool call schemas that contain transformations (i.e. Schema.FiniteFromString), the second decode fails and orElseSucceed(false) would disable approval.
Please pass the decoded parameters directly to needsApproval and add a transformed-schema regression test to ensure this case is covered moving forward.
|
|
||
| const tool = toolkit.tools[toolCall.name] | ||
| const parametersSchema = tool.parametersSchema | ||
| const codec = toolCall.providerExecuted === true || Tool.isDynamic(tool) |
There was a problem hiding this comment.
Dynamic tools can use either raw JSON Schema OR an Effect Schema for defining parameters.
Tool params defined with a Schema still need to use the provider codec. This bypass should really only be used if tool.jsonSchema is defined.
| assistantParts.push(makePart("tool-call", { | ||
| id: part.id, | ||
| name: part.name, | ||
| params: part.params, |
There was a problem hiding this comment.
This would copy unchecked parameters into chat history.
For example, a value like 1n becomes a model-visible error with these changes, but would then cause Chat.exportJson to fail. The preserved parameters need the same JSON-safe treatment as error.toolParams.
| type: "function_call", | ||
| name: toolName, | ||
| call_id: part.id, | ||
| arguments: JSON.stringify(part.params), |
There was a problem hiding this comment.
For malformed calls, part.params is already the original argument string. Stringifying it again here would change "{" into ""{"", so history no longer reproduces the call that failed. Please preserve the original provider argument representation.
Approval was decided by decoding tool parameters a second time, after `normalizeToolCalls` had already decoded them. For any schema with a transformation the second decode fails, and the surrounding `orElseSucceed(constFalse)` turned that failure into "no approval needed", so a tool that required approval ran without it. Approval now reads the decoded parameters, which is what `NeedsApprovalFunction` already declares it receives, and the function can no longer fail. Dynamic tools bypassed the provider codec whenever they were dynamic. Only a dynamic tool that declares raw JSON Schema should bypass it; one whose parameters are an Effect `Schema` still needs it. The guard now matches the one `Tool.getJsonSchema` already uses. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Superseded by #7574. Correcting this comment: it previously said that PR followed the review by making recovery opt-in per tool through #7574 has since been rebuilt on top of #7588 and does now follow the review where it can: a call naming a tool in the toolkit follows that tool's Thanks for the review — it was right, and the corrected shape is much smaller. |
2 participants
Summary
LanguageModelResponse.ToolCallErrorPartvalues instead of failing the whole model operationHandlerResultvalues withisFailureProblem
Given a normal toolkit:
if the provider returns a complete tool call like this:
the current implementation fails
generateTextwith aToolParameterValidationError. The model never receives that retryable error,so it cannot correct the call. An invented tool name has the same problem, and
several providers fail even earlier when tool arguments contain malformed JSON.
With this change, the operation succeeds and exposes the failure directly:
When response parts are added to a
Chat, the error becomes an assistant toolcall followed by a failed tool result. Direct
Toolkit.handlecalls remainstrict; only model-originated calls are converted into model-visible feedback.
Design
Providers translate their wire formats but leave toolkit lookup and Schema
decoding to the shared
LanguageModellayer. The shared layer normalizes toolcalls and validates the complete response before any handler can perform side
effects. Invalid and unknown calls become error parts; valid calls are decoded
once and executed through the Toolkit's decoded-handler boundary.
For streaming responses, only valid decoded calls enter the one-chunk lookahead
buffer introduced in #7486. Invalid calls are emitted immediately and never
execute, while incomplete responses still interrupt or suppress valid calls and
produce their existing synthesized failure results. Mixed responses preserve
and execute valid sibling calls.
Toolkit.makeWithHandlerexposes the same encoded and decoded boundaries usedinternally so integrations can compose resolved toolkits without rebuilding
them or weakening their types.
HandlerResultis discriminated byisFailure,which preserves success and failure narrowing through those compositions.
The manual-resolution path additionally re-encodes valid calls through the
original tool Schema, preserving its documented encoded parameter type.
ToolParameterValidationError.toolParamsaccepts the original runtime valuebut uses Effect's canonical defect encoder for its JSON representation. This
keeps diagnostic construction total without weakening the encoded contract or
changing ordinary JSON objects during decoding.
OpenAI, OpenAI-compatible, Anthropic, and OpenRouter use one shared secure JSON
constructor for streamed and non-streamed calls. A small leaf module for HTTP
detail schemas keeps the new
Response/AiErrorrelationship free of importcycles; the existing public
Response.HttpRequestDetailsandResponse.HttpResponseDetailsexports are unchanged.Related issues and work
Toolkit.handlecalls remain strict; recovery happens at the model boundary for calls produced by a provider.effect/unstable/aidisableToolCallResolution results with decoded params while toolkit.handle expects encoded ones #7353, the v4 counterpart of@effect/ai:toolkit.handledouble-decodes params whendisableToolCallResolution: true#6119).Prior art
This follows the recovery boundary used by other agent runtimes:
repairToolCallfor custom repair before falling back to that representation.Validation
pnpm lint-fixpnpm checkpnpm build