Skip to content

Python: Reuse registered tools in Monty callbacks - #8395

Merged
Eduard van Valkenburg (eavanvalkenburg) merged 1 commit into
microsoft:mainfrom
eavanvalkenburg:monty-invocation-limit-assessment
Sep 15, 2026
Merged

Eduard van Valkenburg (eavanvalkenburg) merged 1 commit into
microsoft:mainfrom
eavanvalkenburg:monty-invocation-limit-assessment

Conversation

@eavanvalkenburg

@eavanvalkenburg Eduard van Valkenburg (eavanvalkenburg) commented Sep 15, 2026 •

Copy link
Copy Markdown
Member

Motivation & Context

Keep Monty host-tool invocation accounting consistent with direct invocation of registered FunctionTool instances.

Description & Review Guide

  • What are the major changes? Bind Monty callbacks to the registered tool's invoke method, remove the unused copy import, document host-tool lifetime, and add focused regression coverage.
  • What is the impact of these changes? Monty callbacks use the registered tool's invocation and exception counters across code executions and provider runs. Raw return values and bound-instance behavior are preserved. Core tool lifecycle and concurrency semantics are unchanged.
  • What do you want reviewers to focus on? Callback binding and lifetime accounting across repeated executions, provider snapshots, and asynchronous calls.

Related Issue

No linked 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 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.

🟢 Approval recommended

The focused implementation is consistent with FunctionTool lifecycle semantics and has comprehensive regression coverage.

Pull request overview

Reuses registered Monty host tools so invocation and exception accounting persists consistently.

Changes:

  • Bind callbacks directly to registered FunctionTool.invoke.
  • Document host-tool counter lifetime.
  • Add regression and concurrency coverage.
File summaries
File Description
python/packages/monty/agent_framework_monty/_execute_code_tool.py Reuses registered tool instances.
python/packages/monty/README.md Documents counter lifetime.
python/packages/monty/tests/monty/test_monty_codeact.py Adds focused callback and provider tests.
python/packages/monty/tests/monty/test_monty_codeact_integration.py Tests execution and concurrency behavior.
Review details
  • Files reviewed: 4/4 changed files
  • Comments generated: 0
  • Review effort level: Balanced (auto)

Note

Copilot is running an experiment and ran this review at Balanced.


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

@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)): da284fb8a4dd
Model: gpt-5.6-sol-fast

Overview

The change makes Monty callbacks invoke the exact registered FunctionTool, bringing host-tool counters in line with the core instance-lifetime contract while continuing to return raw Python values. The strongest guardrails are focused coverage for invocation and exception limits, bound methods, concurrent async calls, repeated executions, and provider snapshots, plus explicit lifetime documentation. No publishable defect remains after reconciling the shared-quota concern against the existing opt-in contract.

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.

Merged via the queue into microsoft:main with commit 0e27716 Sep 15, 2026
50 checks passed

This branch was previously deployed

1 inactive deployment
github-app-auth — da284fb8 Deployed Sep 15, 2026 by eavanvalkenburg via add_label #22907
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.

3 participants