Python: Isolate Anthropic response parsing state - #8242
Python: Isolate Anthropic response parsing state#8242Eduard van Valkenburg (eavanvalkenburg) wants to merge 1 commit into
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
The new dataclass uses a default_factory pattern that can break or confuse static type-checking, and a small guard is needed to avoid accumulating argument fragments under an empty call-id key.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR makes Anthropic streaming/tool parsing state response-local so concurrent responses from a reusable AnthropicClient don’t leak tool aliases, call identity, or argument-fragment parsing across streams.
Changes:
- Introduces
_AnthropicRequestStateand threads it through request preparation and both streaming/non-streaming response parsing paths. - Updates tool parsing to use request-local aliases and call/content-type tracking rather than instance attributes.
- Adds deterministic concurrency regression tests for overlapping call IDs, hosted/local tool mixes, approval requests, and sibling-stream failures.
File summaries
| File | Description |
|---|---|
| python/packages/anthropic/agent_framework_anthropic/_chat_client.py | Adds per-request parsing state and uses it throughout tool preparation and response/stream parsing. |
| python/packages/anthropic/tests/test_anthropic_client.py | Updates existing parsing tests to pass request state where needed and adds concurrent streaming regression coverage. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
| call_id = request_state.last_call_id_name[0] if request_state.last_call_id_name else "" | ||
| request_state.accumulated_tool_arguments[call_id] = ( | ||
| request_state.accumulated_tool_arguments.get(call_id, "") + content_block.partial_json | ||
| ) |
| tool_name_aliases: dict[str, str] = field(default_factory=dict[str, str]) | ||
| accumulated_tool_arguments: dict[str, str] = field(default_factory=dict[str, str]) |
There was a problem hiding this comment.
MAF Automated Review — Iteration 1
Result: Findings reported
Scope: full PR (1 commit(s)): d93df81c8feb
Model: gpt-5.6-sol-fast
Overview
The review found 1 verified inline finding(s).
Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
1 verified finding remained after source verification (1 medium) across 1 file. Details are attached to the affected lines below.
Affected areas: python/packages/anthropic/agent_framework_anthropic/_chat_client.py
| else: | ||
| call_id = self._last_call_id_name[0] if self._last_call_id_name else "" | ||
| call_id = request_state.last_call_id_name[0] if request_state.last_call_id_name else "" | ||
| request_state.accumulated_tool_arguments[call_id] = ( |
There was a problem hiding this comment.
Each local-tool input_json_delta at _chat_client.py:1608-1609 rebuilds the full accumulated string despite having no production reader, making endpoint-controlled fragmentation O(n²) and potentially blocking the event loop; any safe fix must preserve response-local, complete per-call arguments asserted by the regression test.
|
Closing as a duplicate of #8237. |
Motivation & Context
A reusable Anthropic client can serve multiple responses concurrently. Each response needs its own parsing context so streamed tool content is interpreted using the request that produced it.
Description & Review Guide
Related Issue
N/A — no matching public issue was found.
Contribution Checklist
breaking changelabel (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and the title prefix in sync automatically.