Skip to content

Python: deduplicate MessagePack FileHistoryProvider writes - #8224

Merged
Eduard van Valkenburg (eavanvalkenburg) merged 1 commit into
microsoft:mainfrom
CoralGarden52:fix/file-history-msgpack-dedup
Sep 10, 2026
Merged

Eduard van Valkenburg (eavanvalkenburg) merged 1 commit into
microsoft:mainfrom
CoralGarden52:fix/file-history-msgpack-dedup

Conversation

@CoralGarden52

Copy link
Copy Markdown
Contributor

Motivation & Context

FileHistoryProvider supports JSONL by default and length-prefixed MessagePack as an opt-in format. The existing deduplication fix in #7242 applies filter_new_messages() to the JSONL write path, but the MessagePack write path appends every supplied message. As a result, a client that replays a complete transcript on every turn gets different persistence and replay behavior depending only on the serialization format.

This is a follow-up to #7242 and fixes a real, deterministic persistence bug. On upstream main at 018056a52, I reproduced it with a real Agent / AgentSession / FileHistoryProvider lifecycle using a fixed session, ten turns, unique assistant responses, and a complete transcript supplied on every turn. Before this change, the stored history was:

scenario JSONL MessagePack
Agent lifecycle: 10 full-transcript turns 20 records / 3,000 bytes 110 records / 13,530 bytes
Direct provider: 10 cumulative transcript writes 20 records / 2,840 bytes 110 records / 12,870 bytes

The baseline Agent client-message counts were [1, 5, 9, 13, 17, 21, 25, 29, 33, 37] for JSONL and [1, 5, 11, 19, 29, 41, 55, 71, 89, 109] for MessagePack. No exception was raised; loading the session returned repeated turns, increasing storage and later history/model-context size. A delta-only control stored 20 records in both formats, isolating the defect to full-transcript replay.

Description & Review Guide

  • What are the major changes?

    • Read existing length-prefixed MessagePack records while holding the existing per-session write lock.
    • Apply the same filter_new_messages(existing_messages, messages) logic already used by the JSONL path before appending.
    • Skip the write when the incoming transcript contains no new messages.
    • Parameterize FileHistoryProvider deduplication tests over json and msgpack, including replayed transcripts, repeated writes, suffix-only appends, role-sensitive identity, and legitimate identical messages within one batch.
    • No public API, file format, or existing MessagePack record compatibility changes.
  • What is the impact of these changes?

    • After the change, the same real Agent lifecycle stores 20 records in both formats. The post-fix run measured JSONL 20 records / 3,061 bytes and MessagePack 20 records / 2,521 bytes; client-message counts matched in both runs: [1, 5, 9, 13, 17, 21, 25, 29, 33, 37].
    • The direct cumulative-transcript control also measured JSONL 20 records / 2,770 bytes and MessagePack 20 records / 2,270 bytes after the fix.
    • MessagePack remains length-prefixed and readable by the existing loader. The write path now has the same read-before-append behavior as JSONL, so the two formats have consistent transcript persistence semantics.
    • The change is intentionally limited to FileHistoryProvider persistence. It does not address the separate open AG-UI context-composition issue in Python: AG-UI + HistoryProvider: compose history+input without duplicating turns (UI vs LLM context authorities) #8135 or AG-UI snapshot-resume issue in Python: fix(ag-ui): dedupe client-replayed transcripts on resume #8149.
  • What do you want reviewers to focus on?

    • Whether reading existing MessagePack records under the existing lock preserves the intended append-only and corruption handling behavior.
    • Whether filter_new_messages() preserves legitimate duplicate turns within one incoming batch while removing replayed prefixes.
    • Whether the format-parameterized regression coverage is sufficient for existing and full-transcript callers.

The following checks were run on Python 3.12.14:

  • uv run pytest packages/core/tests/core/test_sessions.py -q passed on the focused session suite.
  • uv run poe test --package core passed: 5023 passed, 131 skipped, 2 xfailed (the run reported existing warnings).
  • uv run poe syntax --package core passed both core formatting and lint tasks.
  • uv build --package agent-framework-core completed successfully.
  • git diff --check and git show --check passed.
  • mypy :: core and zuban :: core passed through the test-typing task. The aggregate ty / pyrefly sweeps were blocked by the environment missing the optional agent_hooks package, and Pyright could not start because the environment provides Node 12 while the installed Pyright bundle requires newer JavaScript syntax; neither failure reported a diagnostic in the changed implementation.

Related Issue

Fixes #8223

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.

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.

🟢 Approval recommended

The focused implementation preserves locking and file-format behavior with adequate regression coverage.

Pull request overview

Aligns MessagePack history persistence with JSONL transcript deduplication.

Changes:

  • Deduplicates MessagePack records before appending.
  • Extends regression tests across both formats.
File summaries
File Description
python/packages/core/agent_framework/_sessions.py Filters replayed MessagePack messages under the session lock.
python/packages/core/tests/core/test_sessions.py Adds format-parameterized deduplication coverage.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Balanced

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

@CoralGarden52
CoralGarden52 force-pushed the fix/file-history-msgpack-dedup branch from a932561 to f39fde2 Compare September 10, 2026 09:34
Merged via the queue into microsoft:main with commit 5be4c78 Sep 10, 2026
40 checks passed
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: FileHistoryProvider MessagePack persistence bypasses transcript deduplication

3 participants