Repository navigation
Python: Fix: surface error_code for Anthropic text-editor tool errors (empty message) - #9334
Conversation
…message) In the text_editor_code_execution_tool_result_error branch, the message was `content_block.content.error_code and getattr(..., "error_message", "")`. error_code is a required, always-truthy Literal, so `error_code and error_message` always evaluates to error_message — which is Optional[str] and typically None for these errors — so the ErrorContent ended up with no message at all, and error_code was discarded. The sibling branches (code_execution_tool_result_error, bash_code_execution_tool_result_error) use error_code correctly. Use `error_message or error_code` so the human-readable message is preferred when present and the always-present code is the fallback; the message is never empty. Add a regression test (error_code set, error_message None) asserting the error item carries the code.
|
/review |
There was a problem hiding this comment.
MAF Automated Review — Iteration 1
Result: No findings
Scope: full PR (1 commit(s)): 830cf33dee9b
Model: gpt-5.6-sol
Overview
The change makes Anthropic text-editor execution errors prefer a human-readable message and reliably fall back to the provider's required error code. The supported SDK contract guarantees both attributes, and the regression test covers the previously empty-message path while the shared parser keeps streaming and non-streaming behavior aligned. No Critical, High, or Medium issue remains after reconciliation.
Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
No publishable findings remained after source verification for this scope.
|
|
||
| assert len(result) == 1 | ||
| assert result[0].type == "function_result" | ||
| error_out = result[0].items[0] |
There was a problem hiding this comment.
PratikDhanave (@PratikDhanave) result[0].items is typed as Sequence[Content] | None, but the new test indexes it without first narrowing away None. The current head reports this exact line in pyrefly, ty, zuban, pyright, and mypy. Add a guard such as assert result[0].items is not None before indexing, or otherwise narrow the value before reading items[0].
|
Thank you for the contribution. This pull request has had outstanding feedback about the required |
Motivation & Context
In the Anthropic chat client, a
text_editor_code_execution_tool_result_errorproduced an empty error message and silently dropped theerror_code. This hides the actual failure (e.g.file_not_found,execution_time_exceeded) from anything consuming the tool result.Description & Review Guide
_chat_client.pybuilt the error content as:error_codeis a required, always-truthyLiteral, soerror_code and error_messagealways evaluates to the right operanderror_message— which isOptional[str]and is typicallyNonefor these errors. Result: theErrorContenthas no message and theerror_codeis discarded. The two sibling error branches in the same function useerror_codecorrectly (code_execution_tool_result_error→message=...error_code;bash_code_execution_tool_result_error→stderr=...error_code).message=content_block.content.error_message or content_block.content.error_code— prefer the human-readable message when present, fall back to the always-present code; never empty.error_codeset,error_message=None) asserting the parsed error item's message equals the code. Fails before the fix (None), passes after. Full anthropic text-editor suite green.Related Issue
N/A - trivial change: a localized, two-token correctness fix in one parser branch, matching the behavior of its sibling branches, covered by a new unit test.
AI Assistance
AI assistance details: This change was developed with Claude Code (an AI agent) — it located the bug, wrote the one-line fix and the regression test, and ran verification (pytest). PratikDhanave (@PratikDhanave) to review and tick the AI-assisted attestation above before merge.
Contribution Checklist