Send the error text (not null) in the Anthropic tool_result when FunctionResultContent has an Error and nil Result - #756
Conversation
There was a problem hiding this comment.
Pull request overview
Updates Anthropic provider message construction so tool failures surface diagnostic text in tool_result content (instead of serializing a nil result to null), aligning Anthropic behavior with other providers and improving model/tool debugging.
Changes:
- In
buildMessageParam, short-circuit tool result serialization to send"Error: ..."when a tool result has an error and would otherwise serialize tonull. - Add black-box tests asserting Anthropic requests include error text for failed tool results and preserve the success path behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| provider/anthropicprovider/agent.go | Adjusts Anthropic tool_result content construction to include error text instead of null on failures. |
| provider/anthropicprovider/agent_test.go | Adds request-capture tests ensuring tool_result error/success content is serialized as intended. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This comment has been minimized.
This comment has been minimized.
The FunctionResultContent branch derived the tool_result content solely
from c.Result, so a failed tool result carrying an Error but a nil Result
serialized to the literal "null" while is_error was set to true. The model
never saw why the tool failed.
Short-circuit on c.Error and send "Error: <text>" instead, matching the
other providers: OpenAI Responses sends "Error: %v", OpenAI Chat sets the
content to the error, and Gemini sends {"error": ...}. This also aligns
with the .NET/Python SDKs, which surface the failure diagnostic in the
tool result rather than dropping it.
19f9db3 to
572318b
Compare
Parity Review — ✅ ApprovedThis PR fixes a behavior bug in the Anthropic provider ( Scope AssessmentNo exported API surface changed. The fix is confined to the unexported Cross-SDK ParityThis change closes a pre-existing divergence rather than introducing one:
The PR description notes this aligns with .NET and Python SDK semantics, which surface the failure text in the tool result rather than sending VerdictThe fix brings the Anthropic provider to parity with all other Go providers and with the upstream .NET/Python behavioral expectation. No cross-repo consistency issues found. Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
|
@PratikDhanave conflicts |
What
In
provider/anthropicprovider/agent.go, the*message.FunctionResultContentbranch ofbuildMessageParamderived thetool_resultcontent solely fromc.Result. When a tool result carried anErrorbut a nilResult, the default arm'sjson.Marshal(c.Result)produced the literalnull, andc.Errorwas only used as theis_errorflag:So Anthropic received
tool_resultcontentnullwithis_error: true, and the model never saw why the tool failed.This change short-circuits on the error: when
c.Error != nil, the content is set to"Error: " + c.Error.Error()before building the block. The normal (success) path is unchanged.Why
Every other Go provider surfaces the error text for the same content:
openaiprovider/chat.gosets the content tofuncResult.Erroropenaiprovider/responses.gosends"Error: %v"geminiprovidersends{"error": c.Error.Error()}Anthropic was the only provider dropping the diagnostic. This also matches the .NET/Python SDK semantics, which put the failure text into the tool result rather than sending
null, keeping cross-SDK behavior aligned. This is reachable from in-process failed tool results (e.g. the tool-autocall loop) that setErrorwith an emptyResult.Testing
Added two black-box tests in the canonical
agent_test.go, driven through the existing fake-transport harness (a.Run(...).Collect()):TestToolResultErrorSendsErrorText: aRoleToolmessage whoseFunctionResultContenthasErrorset and a nilResult. Asserts the outgoingtool_resultblock hasis_error: trueand content containing the error text, and not the literalnull. This test fails before the fix (content isnull) and passes after.TestToolResultSuccessSendsResult: a companion case withErrornil andResultset, confirming the success path is unchanged (is_errorfalse, content carries the result).