Python: fix(python): return call-level compaction summaries in agent responses - #8117
Conversation
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.
|
@microsoft-github-policy-service agree |
| if insertion_index is None: | ||
| continue | ||
| transcript_messages.insert(insertion_index, message) |
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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.
| 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) |
There was a problem hiding this comment.
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.
Motivation & Context
Call-level compaction (
compaction_strategyon anAgentorclient.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 finalAgentResponse— only the exclusion flags did, because the flags are in-place mutations onMessageobjects 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 withInMemoryHistoryProvider(skip_excluded=True)— the documented way to combine compaction with history — the nextget_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?
_merge_compaction_summaries_into_transcriptin_compaction.py, next to the existing_reconcile_compaction_summariesused byChatMiddlewareLayer— this extends the same reconciliation pattern one layer up (model-input list → returned transcript).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.SUMMARIZED_BY_SUMMARY_ID_KEYback-link matches the summary id. Back-link matching is deliberate: it works even when run-generated messages carry nomessage_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.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), andtest_agent_run_returns_and_persists_compaction_summaries(agent-level, withInMemoryHistoryProvider(skip_excluded=True)asserting the summary survives a simulated turn-2 load). All three failed before the fix.docs/specs/004-python-function-calling-loop.md, perpython/AGENTS.md.test_agent_executor.pythat the locked formatter / pre-commit hook now requires (unrelated to the fix, isolated for clarity).What is the impact of these changes?
ChatResponse.messages/AgentResponse.messages, positioned before the group each summary replaces — so history loaded withskip_excludedkeeps the summarized content instead of losing it silently.What do you want reviewers to focus on?
_merge_compaction_summaries_into_transcript(back-link matching, nested summaries, and the rationale for skipping summaries of caller-owned input messages).compaction_strategyparameter: 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, andpoe test-typing -P core— all green.Related Issue
Fixes #8099
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.