Skip to content

Python: Isolate Anthropic request parsing state - #8237

Open
Eduard van Valkenburg (eavanvalkenburg) wants to merge 2 commits into
microsoft:mainfrom
eavanvalkenburg:sturdy-pancake
Open

Python: Isolate Anthropic request parsing state#8237
Eduard van Valkenburg (eavanvalkenburg) wants to merge 2 commits into
microsoft:mainfrom
eavanvalkenburg:sturdy-pancake

Conversation

@eavanvalkenburg

Copy link
Copy Markdown
Member

Motivation & Context

Concurrent requests through a reusable Anthropic client should parse independently.

Description & Review Guide

  • What are the major changes? Move stream parsing metadata into per-request state and add deterministic concurrent-stream regressions.
  • What is the impact of these changes? Concurrent responses retain their own tool identities, argument deltas, hosted-tool handling, and approval requests.
  • What do you want reviewers to focus on? The request-state lifetime across streaming and non-streaming response processing.

Related Issue

None.

Contribution Checklist

  • The code builds clean without any errors or warnings
  • All unit tests pass, and I have added new tests where possible
  • The PR follows the Contribution Guidelines
  • This PR is linked to an issue and there is no other open PR for this issue (see Related Issue above).
  • This is not a breaking change. If it is a breaking change, add the breaking change label (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The modified non-streaming concurrency path lacks the regression coverage required by the function-loop specification.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Introduces per-request Anthropic parsing state to prevent concurrent responses from sharing tool metadata.

Changes:

  • Moves tool-call and alias metadata into _AnthropicRequestState.
  • Adds concurrent streaming regressions for failures and approvals.
File summaries
File Description
_chat_client.py Threads request-local state through preparation and response parsing.
test_anthropic_client.py Updates parsing tests and adds concurrency regressions.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Balanced

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

Comment thread python/packages/anthropic/agent_framework_anthropic/_chat_client.py

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MAF Automated Review — Iteration 1

Result: No findings
Scope: full PR (1 commit(s)): 905a605d25f2
Model: gpt-5.6-sol-fast

Overview

The PR replaces reusable-client parsing fields with state allocated and threaded per Anthropic request for both streaming and non-streaming paths. The same state follows tool preparation, event parsing, recursive MCP parsing, stream finalization, and approval generation, while deterministic concurrency tests cover interleaved failure and colliding call IDs. No publishable Critical, High, or Medium defect remains after reconciling the changed lifecycle against its callers and tests.

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.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

Request state is consistently propagated, and both streaming and non-streaming concurrency paths are covered.

Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

python Usage: [Issues, PRs], Target: Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants