Repository navigation
Python: fix tools from context providers - #9403
sophia-ramsey wants to merge 2 commits into
Conversation
agent-framework-automation
Bot
added
documentation
There was a problem hiding this comment.
🟡 Changes recommended
Serialization and supported streaming/client paths can still lose or misrepresent tool definitions.
4 open findings
What changed in this PR
Addresses #9160 by capturing run-specific tool definitions for evaluation, including tools supplied by context providers.
Changes:
- Captures schema-only tool snapshots and preserves them through serialization.
- Adds an explicit
tools=evaluation override and loop aggregation support. - Adds regression tests and documents snapshot behavior.
| File | Description |
|---|---|
python/packages/core/tests/core/test_evaluation.py |
Tests tool discovery, overrides, and serialization. |
python/packages/core/AGENTS.md |
Documents evaluation snapshots. |
python/packages/core/agent_framework/_types.py |
Stores and restores tool schemas. |
python/packages/core/agent_framework/_harness/_loop.py |
Aggregates definitions across loop iterations. |
python/packages/core/agent_framework/_evaluation.py |
Uses captured definitions and explicit overrides. |
python/packages/core/agent_framework/_agents.py |
Captures tools during execution. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
| session_context = context["session_context"] | ||
| if context["suppress_response_id"]: | ||
| response.response_id = None | ||
| response._set_evaluation_tools(context["evaluation_tools"]) # pyright: ignore[reportPrivateUsage] |
| for tool in _normalize_tools(co.get("tools")) | ||
| if isinstance(tool, FunctionTool) | ||
| ] | ||
| if isinstance(self.client, ChatMiddlewareLayer): |
| schema = deepcopy(tool.parameters()) | ||
| json.dumps(schema, allow_nan=False) |
| if definitions: | ||
| self.additional_properties[_EVALUATION_TOOLS_METADATA_KEY] = {"version": 1, "tools": definitions} | ||
| else: | ||
| self.additional_properties.pop(_EVALUATION_TOOLS_METADATA_KEY, None) |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
MAF Automated Review — Iteration 1
Result: Findings reported
Scope: full PR (2 commit(s)): 813d7c0994f1, 36a84a8608a6
Model: gpt-5.6-sol
Overview
The change captures run-specific tool declarations with deep-copied, JSON-validated schemas, restores them as non-executable tools, and carries them through normal response serialization and loop aggregation. Per-run capture, malformed-metadata validation, explicit overrides, and broad streaming/non-streaming tests are strong guardrails. Three serialization and lifecycle gaps remain: empty snapshots are erased, same-name progressive replacements retain stale schemas, and update-only streaming round trips omit the snapshots.
Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
3 verified findings remained after source verification (3 medium) across 2 files. Details are attached to the affected lines below.
Affected areas: python/packages/core/agent_framework/_agents.py, python/packages/core/agent_framework/_types.py
| if definitions: | ||
| self.additional_properties[_EVALUATION_TOOLS_METADATA_KEY] = {"version": 1, "tools": definitions} | ||
| else: | ||
| self.additional_properties.pop(_EVALUATION_TOOLS_METADATA_KEY, None) |
There was a problem hiding this comment.
An empty capture is still a meaningful snapshot, but this branch deletes its serialized marker. After a JSON/dict/pickle round trip, _get_evaluation_tools() returns None, so evaluation with an agent falls back to the agent's current tools and evaluates a no-tool run against unrelated definitions. Please persist {"version": 1, "tools": []} so serialized responses distinguish "captured empty" from legacy responses with no snapshot.
| from ._tools import FunctionTool | ||
|
|
||
| for tool in _normalize_tools((context.options or {}).get("tools")): | ||
| if isinstance(tool, FunctionTool) and tool.name not in self._seen: |
There was a problem hiding this comment.
remove_tools() followed by add_tools() can validly replace a tool with the same name on the next function-loop iteration, but this name-only gate keeps the original declaration. The model can then invoke the replacement schema while evaluation sees the obsolete one. Please update the captured declaration when a same-name description or schema changes, or reject that replacement explicitly instead of silently retaining stale metadata.
| session_context = context["session_context"] | ||
| if context["suppress_response_id"]: | ||
| response.response_id = None | ||
| response._set_evaluation_tools(context["evaluation_tools"]) # pyright: ignore[reportPrivateUsage] |
There was a problem hiding this comment.
Attaching the snapshot only in the result hook means none of the already-emitted AgentResponseUpdates carries it. Consumers that persist streaming updates and rebuild with the public AgentResponse.from_updates() API therefore lose the definitions and cannot perform the post-hoc evaluation this change enables. Please carry the versioned schema metadata on the terminal update, or otherwise make update-only streaming round trips preserve it.
Motivation & Context
The agent evaluation previously omitted tools injected by context providers, causing Foundry tool-selection and tool-input-accuracy evaluators to return null scores because tool definitions were missing. This change captures the tool definitions available during execution and preserves schema-only snapshots across response serialization, so they remain available for evaluation. It supports evaluating agents with dynamically provided tools, both immediately after execution and from saved responses.
Description & Review Guide
tools=override.tools=was not explicitly required to resolve this issue, but it seemed like it would improve the developer experience.Related Issue
Fixes #9160
AI Assistance
AI assistance details: Used copilot with GPT-6.1 Sol for solution planning, design evaluation, implementation, and testing.
Contribution Checklist
breaking changelabel (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.