Skip to content

Python: tool role testing - #9323

Merged
Eduard van Valkenburg (eavanvalkenburg) merged 1 commit into
mainfrom
sramsey/python-tool-role
Oct 13, 2026
Merged

Eduard van Valkenburg (eavanvalkenburg) merged 1 commit into
mainfrom
sramsey/python-tool-role

Conversation

@sophia-ramsey

Copy link
Copy Markdown
Member

Motivation & Context

An issue reported that resuming a workflow with both Host tool results and approval decisions incorrectly placed the entire batch in a user-role message. Although #8751 fixed this behavior, existing tests do not directly assert the mixed-role messages passed from AgentExecutor to its wrapped agent. This PR adds focused regression coverage for content roles, adjacent grouping, and original request order across streaming and non-streaming resumes, including partial responses and checkpoint restoration.

Description & Review Guide

  • What are the major changes? An addition of test coverage for roles.
  • What is the impact of these changes? Regression testing to prevent the issue from occurring again and a clear resolution for the linked issue.
  • What do you want reviewers to focus on? Whether the testing suite is complete,

Related Issue

Fixes #8700

AI Assistance

  • No material AI assistance was used.
  • This is an AI-assisted contribution. I reviewed, understood, and verified all submitted content and accept responsibility for it.

AI assistance details: Used Copilot with GPT-6.1 Sol to find the PR that solved the issue.

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 links to an agreed issue with no competing open PR, or the Related Issue section documents a trivial-change or repository-automation exception.
  • 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.

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

The tests comprehensively cover the reported regression without production-code changes.

0 open findings

What changed in this PR

Adds regression coverage for mixed Host tool results and approval decisions during workflow resume.

Changes:

  • Verifies role grouping and request order across streaming, partial, and checkpointed resumes.
  • Verifies approved/rejected tools execute exactly once without replaying Host results.
File Description
python/​packages/​core/​tests/​workflow/​test_agent_executor_tool_calls.py Adds focused mixed-resume regression tests and a capturing test agent.

🧠 Review effort: Balanced


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

@github-code-quality

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: Python

Python / code-coverage/python

The overall line coverage in commit 34b8461 in the sramsey/python-tool-... branch remains at 92%, unchanged from commit 726dcff in the main branch.

@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)): 34b8461569d5
Model: gpt-5.6-sol

Overview

This test-only PR adds focused regression coverage for mixed Host-tool and approval responses across streaming, non-streaming, partial-response, and checkpoint-restoration paths. The tests deliberately reverse response submission, verify original request order and adjacent role grouping, exercise the real function loop, and assert exactly-once execution and input immutability. No publishable Critical, High, or Medium defect was established.

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.

Comment thread python/packages/core/tests/workflow/test_agent_executor_tool_calls.py Dismissed
Comment thread python/packages/core/tests/workflow/test_agent_executor_tool_calls.py Dismissed
Merged via the queue into main with commit a57b37d Oct 13, 2026
53 of 54 checks passed
@eavanvalkenburg
Eduard van Valkenburg (eavanvalkenburg) deleted the sramsey/python-tool-role branch October 13, 2026 11:47

This branch was previously deployed

1 inactive deployment
github-app-auth — 34b84615 Deployed Oct 13, 2026 by sophia-ramsey via team_check #6662
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.

Python: [Bug]: Mixed approval and Host results give a tool result the user role

3 participants