Skip to content

Python: fix(core): include tool trajectory details in summarizer input - #8087

Open
JHf0912 wants to merge 4 commits into
microsoft:mainfrom
JHf0912:fix/summarization-tool-trajectory-input
Open

Python: fix(core): include tool trajectory details in summarizer input#8087
JHf0912 wants to merge 4 commits into
microsoft:mainfrom
JHf0912:fix/summarization-tool-trajectory-input

Conversation

@JHf0912

@JHf0912 JHf0912 commented Sep 5, 2026

Copy link
Copy Markdown

Problem

SummarizationStrategy feeds the summarizer LLM a transcript rendered by _format_summary_message (python/packages/core/agent_framework/_compaction.py), which only uses Message.text. Since Message.text (python/packages/core/agent_framework/_types.py) concatenates TextContent only, tool rounds collapse to placeholders like 2. [assistant] function_call and 3. [tool] function_result. The summarizer therefore never sees the tool name, arguments, results, exceptions, or call_id, and produces summaries that silently discard the tool trajectory.

Fixes #8086

Solution

Add a per-content renderer _format_summary_content that serializes tool trajectory contents into the summary input transcript, and route _format_summary_message through it:

  • function_callfunction_call <name>(<arguments>) [call_id=<id>]
  • function_resultfunction_result: <result> [call_id=<id>] (with error(<exception>) prefix when the call failed)
  • mcp_server_tool_call / mcp_server_tool_result → same shape with the MCP tool name and output
  • function_approval_request / function_approval_response → nested call name, approval id, and decision

Design decisions:

  • Zero public API change. Only private formatting helpers in _compaction.py change; SummarizationStrategy's trigger conditions, summary message shape, and trace links are untouched.
  • Group-atomic input selection unchanged. _select_summary_input_groups still selects whole groups by token budget; enriched groups simply carry their true payload, so the budget now reflects what the summarizer actually receives.
  • Legacy rendering preserved for text-only messages (byte-identical, locked in by a regression test).

Before → after (real output from the end-to-end reproduction):

# before
2. [assistant] function_call
3. [tool] function_result

# after
2. [assistant] function_call get_weather({"city":"Seattle"}) [call_id=call_1]
3. [tool] function_result: sunny, 22C [call_id=call_1]

Changes

  • python/packages/core/agent_framework/_compaction.py
    • New _format_summary_content dispatch over tool-call / result / MCP / approval content types (reuses existing _tool_result_text).
    • _format_summary_message now combines structured renderings with Message.text, falling back to the legacy content-type list only when nothing else is available.
  • python/packages/core/tests/core/test_compaction.py
    • 8 new tests: function call details, results with exceptions, results without call_id, mixed text/tool messages, text-only golden rendering, MCP tool details, approval request, approval response.

Testing

cd python/packages/core
uv run pytest tests/core/test_compaction.py -q   # 87 passed
uv run pytest tests/core -q                       # full core suite: passed
uv run ruff check agent_framework/_compaction.py tests/core/test_compaction.py
uv run poe pyright                               # no new errors in touched files
  • End-to-end: a SummarizationStrategy run over a 6-message conversation with two tool rounds shows the summarizer input containing names, arguments, results, and call_ids, and the summary message replacing the excluded originals with trace links intact.
  • Regression anchors: existing budget/selection tests (test_summarization_strategy_bounds_summary_input_to_complete_groups, test_summary_input_selection_does_not_retokenize_selected_transcript) pass unchanged; group-atomic selection semantics are preserved.

Notes for Reviewer

  • Token budget: enriched groups are larger, so a budgeted summarization round may select fewer groups than before. This is intentional — the payload was always part of the context; it was just invisible to the summarizer. Flagged for review in case maintainers prefer an include_tool_details opt-in/opt-out.
  • Trust boundary: tool arguments/results now reach the summarizer LLM. The class docstring already warns that the summarizer must be trusted as much as the primary model; this PR widens the data surface sent to it. A redaction hook could be a follow-up.
  • Out of scope (deliberately): text_reasoning rendering, structured JSON summary output, and no-call_id adjacency pairing in group_messages are left unchanged; the last touches pairing semantics covered by spec docs/specs/004-python-function-calling-loop.md and is proposed separately.
  • Orthogonal to open PR Python: fix: preserve synthetic compaction summaries #7944 (synthetic summary reconciliation in before_run).

Introduce _format_summary_content as the dispatch point for per-content
summary rendering and rewrite _format_summary_message to combine structured
renderings with Message.text, keeping the legacy fallback to content types.
Behavior is byte-identical for existing inputs; tool-call and tool-result
branches follow in the next commit.
Render function_call contents with name, arguments, and call id, and
function_result contents with result text, exception, and call id in
_format_summary_content, so the summarizer LLM sees the tool trajectory
instead of a bare content type. Text-only messages keep their legacy
rendering byte-for-byte; messages mixing text and tool contents now include
both.
Extend _format_summary_content with mcp_server_tool_call /
mcp_server_tool_result (tool name, arguments, output, call id) and
function_approval_request / function_approval_response (nested call name,
approval id, decision) branches, completing the tool trajectory visible to
the summarizer LLM.

Fixes microsoft#8086

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

A critical crash path and two moderate rendering defects remain unresolved.

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

Pull request overview

Adds structured function, MCP, and approval trajectory details to Python summarizer transcripts.

Changes:

  • Renders tool calls, results, exceptions, IDs, and approval decisions.
  • Preserves text-only formatting.
  • Adds focused regression tests.

Required fixes:

  • Critical (1 vote): Safely stringify non-JSON-serializable MCP result mappings to prevent compaction crashes.
  • Moderate (2 votes): Preserve chronological ordering of text and structured content.
  • Moderate (1 vote): Use tool_name when rendering approvals wrapping MCP calls.
File summaries
File Description
python/packages/core/tests/core/test_compaction.py Adds formatter regression coverage.
python/packages/core/agent_framework/_compaction.py Adds structured summary rendering; contains the unresolved findings above.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 3
  • Review effort level: Balanced

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

Comment on lines +1232 to +1235
if content.type == "mcp_server_tool_result":
result_text = _tool_result_text(content.output)
if content.exception:
result_text = f"error({content.exception}): {result_text}"
return f"mcp_tool_result: {result_text}{call_id_suffix}"
if content.type in ("function_approval_request", "function_approval_response"):
nested_call = content.function_call
name = nested_call.name if nested_call is not None else ""
Comment on lines +1250 to +1253
parts = [_format_summary_content(content) for content in message.contents]
if message.text:
parts.append(message.text)
content_text = "; ".join(part for part in parts if part)
- Stringify non-JSON-serializable MCP result mappings with the same
  json.dumps(default=str) fallback used by Content.from_function_result,
  preventing compaction crashes inside _select_summary_input_groups.
- Preserve time order of text and structured contents in
  _format_summary_message by emitting consecutive text blocks in place.
- Use tool_name as the fallback identity for MCP tool calls nested in
  approval contents.

Addresses review feedback on PR microsoft#8087; adds three regression tests.
@JHf0912
JHf0912 deployed to github-app-auth September 5, 2026 09:22 — with GitHub Actions Active
@JHf0912

JHf0912 commented Sep 5, 2026

Copy link
Copy Markdown
Author

Addressed the three findings in 6b5360d:

  1. Crash path: non-JSON-serializable MCP result mappings now use the same \json.dumps(..., default=str)\ with \str()\ fallback as \Content.from_function_result, so _select_summary_input_groups\ can no longer raise.
  2. Time order: _format_summary_message\ now emits consecutive text blocks in place while iterating \message.contents, preserving temporal order for mixed messages; pure-text rendering stays byte-identical.
  3. MCP approval identity: nested calls in approval contents fall back to \ ool_name\ when
    ame\ is absent.

Added three regression tests (non-JSON MCP output, mixed-content ordering, MCP approval tool name); all 90 tests in \ ests/core/test_compaction.py\ pass, ruff clean.

copilot-pull-request-reviewer please re-review.

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]: Summarizer input drops tool call parameters and results

2 participants