Skip to content

Python: fix mixed function-call batch classification - #8428

Merged
Evan Mattson (moonbox3) merged 3 commits into
microsoft:mainfrom
eavanvalkenburg:issue-8079-pr-review
Sep 16, 2026
Merged

Evan Mattson (moonbox3) merged 3 commits into
microsoft:mainfrom
eavanvalkenburg:issue-8079-pr-review

Conversation

@eavanvalkenburg

Copy link
Copy Markdown
Member

Motivation & Context

Function-call batch classification stopped at the first approval-required, Host-owned, or fatal unknown call. Reordering the same model calls could therefore hide an approval requirement or a configured fatal error. Mixed approval and Host-owned batches also need to remain compatible with ToolApprovalMiddleware policies without executing cached siblings early.

Description & Review Guide

  • What are the major changes?
    • Scan and classify the complete actionable call batch before mutating approval state or executing a tool.
    • Raise configured fatal unknown-call errors before any pause or sibling execution.
    • Return approval-required and Host-owned calls as one ordered mixed batch, while retaining the existing session-backed cache for calls that do not require approval.
    • Keep ToolApprovalMiddleware automatic and standing approval policies compatible with mixed batches in streaming and non-streaming runs.
    • Reject incomplete mixed approval/Host responses before execution and reclassify approved Host tools before the local execution boundary.
    • Record the behavior and scenario-to-test mapping in the Python function-calling-loop specification.
  • What is the impact of these changes?
    Approval and fatal-error behavior no longer depends on model call order. Mixed batches remain fail-closed, and approved calls execute once with their original effective arguments. There is no public API change.
  • What do you want reviewers to focus on?
    The complete-batch precedence, the mixed-response correlation in _mixed_pause_batch_status, and preservation of ToolApprovalMiddleware behavior across streaming and non-streaming paths.

Related Issue

Fixes #8079

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.

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

Mixed-batch correlation misses prior Host requests without history and incorrectly includes historical requests when history is present.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Fixes order-dependent classification of mixed Python function-call batches and coordinates approval/Host-owned pauses.

Changes:

  • Classifies complete batches before execution.
  • Adds mixed-pause correlation and middleware handling.
  • Documents and tests streaming/non-streaming behavior.
File summaries
File Description
python/packages/core/agent_framework/_tools.py Implements classification and mixed-response handling.
python/packages/core/agent_framework/_harness/_tool_approval.py Preserves mixed batches through approval policies.
python/packages/core/tests/core/test_function_invocation_logic.py Tests classification and response completeness.
python/packages/core/tests/core/test_harness_tool_approval.py Tests middleware compatibility.
docs/specs/004-python-function-calling-loop.md Records invariants and test mapping.
Review details
  • Files reviewed: 5/5 changed files
  • Comments generated: 2
  • Review effort level: Balanced

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

Comment thread python/packages/core/agent_framework/_tools.py Outdated
Comment thread python/packages/core/agent_framework/_tools.py Outdated

@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: Findings reported
Scope: full PR (1 commit(s)): 7f90bef3b0b4
Model: gpt-5.6-sol-fast

Overview

The PR correctly classifies complete actionable batches before execution, binds approval decisions to session snapshots, and adds useful mixed-order and middleware regressions. However, mixed-response validation depends on replayed history instead of an active session-backed mixed batch, which creates both a service-history fail-open and a stale-history lockout. The streaming middleware also delays standalone user-input requests and duplicates terminal metadata when splitting mixed updates.

Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
4 verified findings remained after source verification (1 high, 3 medium) across 2 files. Details are attached to the affected lines below.

Affected areas: python/packages/core/agent_framework/_harness/_tool_approval.py, python/packages/core/agent_framework/_tools.py

Comment thread python/packages/core/agent_framework/_tools.py
Comment thread python/packages/core/agent_framework/_tools.py Outdated
Comment thread python/packages/core/agent_framework/_harness/_tool_approval.py Outdated
Comment thread python/packages/core/agent_framework/_harness/_tool_approval.py
@moonbox3
Evan Mattson (moonbox3) added this pull request to the merge queue Sep 16, 2026
Merged via the queue into microsoft:main with commit b83b0e3 Sep 16, 2026
43 checks passed

This branch was successfully deployed

1 active deployment
github-app-auth 8b7ea1fa Deployed Sep 16, 2026 by eavanvalkenburg via add_label #23015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Usage: [Issues, PRs], Target: documentation in the code base and learn docs python Usage: [Issues, PRs], Target: Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Python: always_require tool approval silently bypassed when preceded by a declaration-only or unknown call in the same batch

3 participants