Skip to content

Python: Keep tool diagnostics internal by default - #8235

Open
Eduard van Valkenburg (eavanvalkenburg) wants to merge 3 commits into
microsoft:mainfrom
eavanvalkenburg:didactic-spoon
Open

Python: Keep tool diagnostics internal by default#8235
Eduard van Valkenburg (eavanvalkenburg) wants to merge 3 commits into
microsoft:mainfrom
eavanvalkenburg:didactic-spoon

Conversation

@eavanvalkenburg

@eavanvalkenburg Eduard van Valkenburg (eavanvalkenburg) commented Sep 10, 2026

Copy link
Copy Markdown
Member

Motivation & Context

Tool failures should return stable generic errors by default while retaining diagnostics for trusted local inspection. Generic response serialization and one Responses projection could include internal exception text instead of the channel-visible result.

Description & Review Guide

  • What are the major changes? Treat Content.exception as internal and serialize only a fixed non-sensitive failure marker, project visible function results in Responses payloads, and add cross-protocol regression coverage.
  • What is the impact of these changes? Default serialized and hosted responses retain stable public error output without including internal tool diagnostics. Explicit detailed-error behavior remains available.
  • What do you want reviewers to focus on? The Content serialization boundary and function-result projection behavior.

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 the 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

Some provider and compaction projections still expose internal exceptions, and Responses mishandles falsey visible results.

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

Pull request overview

Secures Python tool-error serialization while preserving diagnostics for trusted local inspection.

Changes:

  • Omits Content.exception from default serialization.
  • Uses channel-visible results in Responses projections.
  • Adds cross-protocol regression coverage and documentation.
File summaries
File Description
python/packages/core/agent_framework/_types.py Adds internal-diagnostic serialization controls.
python/packages/core/tests/core/test_types.py Tests default omission and explicit inclusion.
python/packages/core/tests/core/test_function_invocation_logic.py Tests detailed-error behavior.
python/packages/hosting-responses/agent_framework_hosting_responses/_parsing.py Projects visible function-result output.
python/packages/hosting-responses/tests/hosting_responses/test_parsing.py Tests streaming and non-streaming projections.
python/packages/foundry_hosting/tests/test_responses.py Tests hosted Responses behavior.
python/packages/hosting-mcp/tests/hosting_mcp/test_conversion.py Tests MCP diagnostic omission.
python/packages/hosting-a2a/tests/hosting_a2a/test_conversion.py Tests A2A diagnostic omission.
python/packages/ag-ui/tests/ag_ui/test_run_common.py Tests AG-UI diagnostic omission.
docs/specs/004-python-function-calling-loop.md Documents diagnostic visibility rules.
Review details
  • Files reviewed: 10/10 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/_types.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)): d71b113d2378
Model: gpt-5.6-sol-fast

Overview

The PR correctly separates channel-visible tool output from internal diagnostics, propagates the default omission through nested Content, and adds broad protocol regression coverage. The explicit local opt-in and equality handling preserve trusted inspection of live objects. However, default message persistence now removes the only failure-state marker, causing reloaded tool failures to be projected as successful by providers that derive status from exception.

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

Affected areas: python/packages/core/agent_framework/_types.py

Comment thread python/packages/core/agent_framework/_types.py Outdated
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

@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 2

Result: No findings
Scope: 1 net-new commit(s): a7330b0e1f03
Model: gpt-5.6-sol-fast

Overview

This incremental update makes exception diagnostics unconditionally absent from dictionary serialization, keeps them directly accessible to trusted local code, and adds coverage for both exclude_none modes and nested response serialization. The four-perspective review confirmed the redaction guardrails and found no new publishable issue after deduplication against the three unresolved existing review threads.

Reviewed the supplied incremental 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>
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.

2 participants