Skip to content

Python: fix(python): return call-level compaction summaries in agent responses - #8117

Draft
Vedant Sonkar (vedantsonkar) wants to merge 4 commits into
microsoft:mainfrom
vedantsonkar:fix/8099-compaction-summary-in-response
Draft

Python: fix(python): return call-level compaction summaries in agent responses#8117
Vedant Sonkar (vedantsonkar) wants to merge 4 commits into
microsoft:mainfrom
vedantsonkar:fix/8099-compaction-summary-in-response

Conversation

@vedantsonkar

Copy link
Copy Markdown

Motivation & Context

Call-level compaction (compaction_strategy on an Agent or client.as_agent()) works mid-run: it marks older self-generated tool-call/tool-result messages as excluded and inserts a summary the model sees. However, the summary messages never reached the final AgentResponse — only the exclusion flags did, because the flags are in-place mutations on Message objects shared between the model-input list (prepared_messages) and the returned transcript (function_call_messages), while the inserted summaries live only in the model-input list.

As a result, HistoryProvider.after_run() persisted that asymmetric response, and with InMemoryHistoryProvider(skip_excluded=True) — the documented way to combine compaction with history — the next get_messages() silently dropped the excluded tool groups with no summary replacing them. Any agent using tools + call-level compaction + a history provider hits this.

Fixes #8099

Description & Review Guide

  • What are the major changes?

    • New private helper _merge_compaction_summaries_into_transcript in _compaction.py, next to the existing _reconcile_compaction_summaries used by ChatMiddlewareLayer — this extends the same reconciliation pattern one layer up (model-input list → returned transcript).
    • Two call sites in FunctionInvocationLayer._get_response_with_function_invocation (_tools.py): after each Phase-2 model call and after the Phase-3 final no-tools call, both before the transcript is prepended to a terminal response.
    • The helper inserts each summary before the first transcript message whose SUMMARIZED_BY_SUMMARY_ID_KEY back-link matches the summary id. Back-link matching is deliberate: it works even when run-generated messages carry no message_id. Nested summary-of-summary resolves naturally (inner summaries precede outer ones in the working list). Summaries of caller-owned input messages are skipped — those sources are not part of the returned transcript, so persisting them would duplicate content.
    • Regression tests: test_function_loop_returns_compaction_summaries_in_final_response (Phase-2 terminal path), test_function_loop_returns_compaction_summaries_when_iteration_budget_exhausted (max-iterations path), and test_agent_run_returns_and_persists_compaction_summaries (agent-level, with InMemoryHistoryProvider(skip_excluded=True) asserting the summary survives a simulated turn-2 load). All three failed before the fix.
    • Spec traceability (normative bullet, matrix row, related-issue entry) in docs/specs/004-python-function-calling-loop.md, per python/AGENTS.md.
    • A separate chore commit applies the one-line ruff formatting to test_agent_executor.py that the locked formatter / pre-commit hook now requires (unrelated to the fix, isolated for clarity).
  • What is the impact of these changes?

    • Non-streaming runs that compact tool groups in-run now return the inserted summaries in ChatResponse.messages / AgentResponse.messages, positioned before the group each summary replaces — so history loaded with skip_excluded keeps the summarized content instead of losing it silently.
    • Streaming is untouched (its final response is assembled from accumulated updates, a mechanism that never carried compaction summaries or exclusion flags; existing streaming compaction tests remain green).
    • Scope is limited to the non-streaming function-invocation loop; no public API changes, no breaking change.
  • What do you want reviewers to focus on?

    • The matching logic in _merge_compaction_summaries_into_transcript (back-link matching, nested summaries, and the rationale for skipping summaries of caller-owned input messages).
    • Why the merge call is unconditional rather than gated on the compaction_strategy parameter: compaction may come from the inner client's own default strategy, which the invocation layer does not see as a parameter (this was caught during development — the naive gate silently skipped the merge for client-level defaults).

Validated with uv run poe test -P core (4434 passed, 35 skipped, 2 xfailed), poe syntax -P core, poe pyright -P core, and poe test-typing -P core — all green.

Related Issue

Fixes #8099

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.

Removes a blank line between stdlib import groups that the locked ruff
version (and therefore the pre-commit format hook) now requires.
In-run compaction excluded tool groups via flags on Message objects shared
with the function-invocation transcript, but inserted summary messages only
into the model-input list. The final ChatResponse (and the history persisted
from it) therefore kept only the exclusion flags, so loading history with
skip_excluded silently dropped the summarized tool results (microsoft#8099).

Merge summaries back into the transcript after each model call, matching
each summary to its excluded group via the SUMMARIZED_BY_SUMMARY_ID_KEY
back-link. Summaries of caller-owned input messages stay out of the
returned transcript.

Adds regression tests for the Phase-2 terminal path, the max-iterations
terminal path, and agent-level persistence with InMemoryHistoryProvider.
…oop spec

Adds the normative invariant that non-streaming runs return inserted
compaction summaries in the final response transcript, a traceability
matrix row pointing at the new regression tests, and a related-issue
entry for microsoft#8099.
Copilot AI balanced review requested due to automatic review settings September 7, 2026 10:02

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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@agent-framework-automation agent-framework-automation Bot added documentation Usage: [Issues, PRs], Target: documentation in the code base and learn docs python Usage: [Issues, PRs], Target: Python labels Sep 7, 2026
@github-actions github-actions Bot changed the title fix(python): return call-level compaction summaries in agent responses Python: fix(python): return call-level compaction summaries in agent responses Sep 7, 2026
@vedantsonkar

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree

Comment on lines +491 to +493
if insertion_index is None:
continue
transcript_messages.insert(insertion_index, message)

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.

What happens when compaction creates a summary of an earlier summary? working_messages places the outer summary before the inner one, so this pass skips the outer summary before its anchor has been inserted. The inner summary is then inserted while still excluded, and skip_excluded=True removes it along with the original tool group, recreating the silent data loss this PR is meant to prevent. Could this resolve the summary dependencies to a fixed point, like _reconcile_compaction_summaries does?

# back before the transcript is prepended to a terminal response (issue #8099).
# The merge is unconditional: compaction may also come from the inner client's
# own default strategy, which this layer does not see as a parameter.
_merge_compaction_summaries_into_transcript(function_call_messages, prepared_messages)

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.

Could we make sure this merged transcript is included on every terminal path? When the next model turn produces an approval request, user-input request, or middleware termination, _process_model_function_calls returns action="return" and _get_response_with_function_invocation returns response directly at _tools.py:3369. The merged summary and the earlier tool group remain only in function_call_messages, so the returned and persisted transcript still loses the compacted history on those supported exits.

Comment on lines +483 to +493
insertion_index = next(
(
index
for index, transcript_message in enumerate(transcript_messages)
if _summarized_by_summary_id(transcript_message) == message.message_id
),
None,
)
if insertion_index is None:
continue
transcript_messages.insert(insertion_index, message)

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.

Would it make sense to share the existing summary-reconciliation logic here? _reconcile_compaction_summaries already resolves nested dependencies and rejects summaries that include messages outside the owning list, while this new path accepts the first matching back-link in one forward pass. Generalizing that helper to take the transcript's message identities would keep ordering and caller-owned-input filtering in one implementation instead of requiring future compaction changes to update two different algorithms.

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: [Bug]: Call-level compaction summary dropped from AgentResponse (only exclusion flags persist)

3 participants