Skip to content

Python: Preserve compaction through middleware rewrites - #8671

Merged
westey (westey-m) merged 3 commits into
microsoft:mainfrom
eavanvalkenburg:fix-compaction-rewrites
Sep 24, 2026
Merged

westey (westey-m) merged 3 commits into
microsoft:mainfrom
eavanvalkenburg:fix-compaction-rewrites

Conversation

@eavanvalkenburg

Copy link
Copy Markdown
Member

Motivation & Context

Chat middleware may need to replace messages for a provider-specific request shape. When downstream compaction summarizes those replacements, the ownership-aware reconciliation cannot connect the replacement IDs to caller-owned history, so it rejects the summary, removes exclusions, and silently leaves the original history unchanged.

This adds an explicit opt-in provenance contract so middleware replacements remain call-local while compaction derived from them can become durable.

Description & Review Guide

  • What are the major changes?
    • Add ChatContext.record_message_replacement(...) to record invocation-local replacement provenance without persisting replacement messages.
    • Resolve replacement lineage transitively during summary reconciliation, transfer accepted exclusion metadata to rooted source messages, and reject ambiguous or unrooted dependencies.
    • Emit a content-free warning when chat-middleware summary reconciliation is rejected.
    • Preserve replacement-backed summaries across short-circuit and message-restore middleware paths by retaining fallback message snapshots when the final handler is not reached.
    • Add streaming, non-streaming, transitive, termination, rejection, and API validation coverage.
  • What is the impact of these changes?
    • Middleware can explicitly allow downstream compaction to update caller-owned history without making provider-only rewrites durable.
    • Unregistered rewrites remain conservative and call-local, while rejected compaction is no longer silent.
  • What do you want reviewers to focus on?
    • The ownership boundary in _reconcile_compaction_summaries, especially transitive replacement roots and exclusion transfer.
    • The termination fallback in ChatMiddlewareLayer, which uses middleware snapshots only when no downstream list was captured.

Related Issue

Fixes #8313

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 a language prefix) — a workflow keeps the label and title prefix in sync automatically.

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 review overview

🟡 Changes recommended

Partial replacement coverage can lose content, and duplicate summary IDs can be persisted ambiguously.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 2 High severity

Open (2)
What changed in this PR

Adds opt-in provenance tracking so middleware-generated replacements can participate in durable compaction without persisting provider-local rewrites.

Changes:

  • Adds replacement provenance recording and transitive reconciliation.
  • Preserves summaries across termination/restoration paths and logs rejections.
  • Adds streaming, validation, and reconciliation tests.
File Description
python/​packages/​core/​agent_framework/​_middleware.py Adds provenance API and fallback snapshots.
python/​packages/​core/​agent_framework/​_compaction.py Resolves replacement lineage and transfers exclusions.
python/​packages/​core/​tests/​core/​test_middleware.py Tests provenance API validation.
python/​packages/​core/​tests/​core/​test_clients.py Tests reconciliation scenarios.
python/​packages/​core/​AGENTS.md Documents replacement registration.

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

@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)): 0a0d0c0a966f
Model: gpt-5.6-sol-fast

Overview

The review found 2 verified inline finding(s).

Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
2 verified findings remained after source verification (1 high, 1 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

@westey-m
westey (westey-m) added this pull request to the merge queue Sep 24, 2026
Merged via the queue into microsoft:main with commit 2191c60 Sep 24, 2026
45 checks passed

This branch was previously deployed

1 inactive deployment
github-app-auth — d781d967 Deployed Sep 24, 2026 by eavanvalkenburg via add_label #23698
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

3 participants