Skip to content

Python: Fix compaction persistence, thresholds, and observability - #7912

Open
Eduard van Valkenburg (eavanvalkenburg) wants to merge 2 commits into
microsoft:mainfrom
eavanvalkenburg:eavanvalkenburg-fix-compaction-behavior
Open

Python: Fix compaction persistence, thresholds, and observability#7912
Eduard van Valkenburg (eavanvalkenburg) wants to merge 2 commits into
microsoft:mainfrom
eavanvalkenburg:eavanvalkenburg-fix-compaction-behavior

Conversation

@eavanvalkenburg

@eavanvalkenburg Eduard van Valkenburg (eavanvalkenburg) commented Aug 27, 2026

Copy link
Copy Markdown
Member

Motivation & Context

Compaction can both mark existing messages as excluded and insert replacement summary messages. Those two mutations must remain together across every model call in a function-invocation loop. When chat middleware is present, ChatMiddlewareLayer operates on a copied message list; exclusions persist through shared Message objects, but inserted summaries previously remained only in the copy. Later tool-loop iterations therefore lost both the original content and its replacement summary.

The default context-window strategy also used TokenBudgetComposedStrategy for its 50% tool-eviction phase. That composer's strict fallback can exclude ordinary conversation groups when tool-result compaction cannot meet the budget, which made destructive truncation happen below the separately documented 80% truncation threshold.

Finally, compaction had no positive operational signal when it changed context, and truncation had no opt-in way to retain the user's opening request.

Description & Review Guide

  • What are the major changes?
    • Preserve replacement summaries across chat middleware boundaries. The middleware layer captures the durable baseline before middleware runs, normalizes valid message-sequence replacements to a list before the downstream client call, and reconciles summaries in a pipeline-level finally so streaming, non-streaming, termination, and failure paths behave consistently.
    • Reconcile nested summaries by dependency rather than list order. Only summaries transitively backed by durable source messages cross the middleware copy boundary; ordinary middleware rewrites stay call-local, and exclusions are rolled back when a summary depends on middleware-only content.
    • Replace the nested token-budget composers in ContextWindowCompactionStrategy with explicit phase gates. Tool-result compaction may run above tool_eviction_threshold; destructive truncation can run only when the post-eviction context still exceeds truncation_threshold.
    • Emit one structured INFO log when a compaction strategy reports a change. The LogRecord receives compaction_phase, compaction_strategy, and before/after included message and token counts through extra; message content is never logged. OpenTelemetry events and callbacks remain out of scope.
    • Add preserve_first_user_group=False to TruncationStrategy and surface the same opt-in on ContextWindowCompactionStrategy. Existing behavior remains the default. If protected groups cannot fit within the actual input budget, the strategy preserves them and emits a structured warning with the remaining token count and input budget.
    • Add regression coverage for middleware-backed tool loops in both response modes, sequence replacement, middleware-side and nested compaction, short-circuit termination, service-managed conversation behavior, exception paths, middleware-local content, threshold separation, structured logs, and first-user preservation.
  • What is the impact of these changes?
    • Function-invocation loops retain summarized history consistently whether or not chat middleware is configured or rewrites the message sequence.
    • Non-tool conversation history between the tool-eviction and truncation thresholds is no longer destructively shortened.
    • Operators can observe when compaction changes context, inspect aggregate effects without exposing conversation content, and detect when protected content cannot fit within the configured input budget.
    • Applications that need to retain the opening task can opt in without changing defaults for existing users.
  • What do you want reviewers to focus on?
    • The summary synchronization invariant in ChatMiddlewareLayer: only summaries transitively linked to durable source messages should cross the middleware copy boundary, and middleware rewrites must remain call-local.
    • The separation between tool eviction and destructive truncation thresholds.
    • The structured log field names and the default-off semantics of preserve_first_user_group.

Related Issue

Fixes #7744
Fixes #7224
Fixes #7216

There are no other open pull requests linked to these issues.

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.

Preserve compaction summaries across chat middleware boundaries, keep destructive truncation behind its documented threshold, add structured INFO logs, and support preserving the first user group.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 726d66ea-6cff-4cb8-9090-8d919392271d
@github-actions

github-actions Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Python Test Coverage

Python Test Coverage Report •
FileStmtsMissCoverMissing
packages/core/agent_framework
   _compaction.py9076492%138–139, 147, 174, 265–266, 284–285, 302, 337, 351, 358, 373–374, 428, 435, 451, 501, 537, 539, 559, 601, 657, 663, 665, 684, 728–733, 745, 832, 834, 902, 964, 1110, 1117, 1123–1127, 1130–1132, 1157, 1270, 1272, 1274, 1311, 1318, 1323, 1338, 1352–1353, 1356–1357, 1459, 1482, 1538, 1670, 1760
   _middleware.py4981696%514, 887, 894–895, 1077, 1148, 1166, 1168, 1301, 1304, 1331, 1333, 1479, 1483, 1675, 1751
TOTAL48314448190% 

Python Unit Test Overview

Tests Skipped Failures Errors Time
9772 36 💤 0 ❌ 0 🔥 2m 35s ⏱️

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.

Pull request overview

Fixes Python compaction persistence across middleware, separates eviction/truncation thresholds, adds observability, and supports preserving the opening user group.

Changes:

  • Synchronizes compaction summaries across middleware-backed model calls.
  • Adds explicit threshold gates and optional first-user preservation.
  • Emits structured compaction logs with regression coverage.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
_middleware.py Propagates linked summaries to durable history.
_compaction.py Adds logging, threshold gates, and preservation support.
test_clients.py Covers middleware and tool-loop persistence.
test_compaction.py Covers logs, thresholds, and preservation.

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

Comment thread python/packages/core/agent_framework/_middleware.py Outdated
Comment thread python/packages/core/tests/core/test_compaction.py
Comment thread python/packages/core/agent_framework/_compaction.py

@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)): 241ed4acac9d
Model: gpt-5.6-sol

Overview

The review found 3 verified inline finding(s).

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/_compaction.py, python/packages/core/agent_framework/_middleware.py

Comment thread python/packages/core/agent_framework/_middleware.py Outdated
Comment thread python/packages/core/agent_framework/_middleware.py Outdated
Comment thread python/packages/core/agent_framework/_compaction.py
Reconcile summaries at the middleware pipeline boundary, support sequence replacement and nested summaries, strengthen logging coverage, refresh threshold documentation, and warn when protected groups exceed the input budget.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 726d66ea-6cff-4cb8-9090-8d919392271d
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

2 participants