Skip to content

Python: fix: prevent superlinear history growth by deduplicating messages in save_messages - #7242

Open
PratikWayase wants to merge 13 commits into
microsoft:mainfrom
PratikWayase:fix/per-service-history-duplicate-persistence
Open

Python: fix: prevent superlinear history growth by deduplicating messages in save_messages#7242
PratikWayase wants to merge 13 commits into
microsoft:mainfrom
PratikWayase:fix/per-service-history-duplicate-persistence

Conversation

@PratikWayase

Copy link
Copy Markdown
Contributor

Motivation & Context

  1. Why is this change required?
    The current history providers (InMemoryHistoryProvider, FileHistoryProvider, and RedisHistoryProvider) blindly append messages on every service call without checking if they already exist in the store.
  2. What problem does it solve?
    In looped runs (e.g., AG-UI stateless clients or harness todo loops), the transport passes the full accumulated conversation as input on every request. This caused the entire conversation history to be re-persisted every round, leading to superlinear store growth, corrupted context, repeated tool calls, and massive token bloat (up to 3× the real conversation size).
  3. What scenario does it contribute to?
    Multi-iteration flows where each service call's message list carries the accumulated conversation rather than a delta, particularly when using stateless chat clients over the AG-UI endpoint.
  4. Issue link: Fixes Python: [Bug]: per-service-call history persistence re-appends already-persisted messages #7211

Description & Review Guide

  • What are the major changes?

    • Added a new _get_message_identity helper function that generates a stable identity for a message (using message.id if available, otherwise falling back to a deterministic hash of role + serialized contents).
    • Updated save_messages in InMemoryHistoryProvider, FileHistoryProvider, and RedisHistoryProvider to build a set of existing message identities and filter out duplicates before appending/pushing new messages.
    • Added comprehensive regression tests for all three providers to ensure identical messages are ignored, mixed old/new messages only append the new ones, and messages with the same text but different roles are kept separate.
  • What is the impact of these changes?
    This hardens the system at the lowest storage level. It completely prevents duplicate message persistence regardless of how the middleware or transport layer constructs the message list. This stops token bloat, prevents context corruption, and ensures graceful degradation in long-running loops without requiring changes to the middleware routing logic.

  • What do you want reviewers to focus on?

    1. The stability and correctness of the _get_message_identity fallback logic (ensuring it handles missing IDs and serialization edge cases gracefully).
    2. The RedisHistoryProvider.save_messages implementation, specifically ensuring it correctly fetches existing messages via lrange to build the identity set before pushing only the new_messages via the pipeline.
    3. The new unit tests to ensure they adequately cover the deduplication edge cases.

Related Issue

Fixes #7211

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 review requested due to automatic review settings July 21, 2026 18:52
@giles17 giles17 added the python Usage: [Issues, PRs], Target: Python label Jul 21, 2026
@github-actions github-actions Bot changed the title fix: prevent superlinear history growth by deduplicating messages in save_messages Python: fix: prevent superlinear history growth by deduplicating messages in save_messages Jul 21, 2026

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

This PR fixes a Python history-persistence defect where per-service-call history providers re-persist the full accumulated conversation every round, causing superlinear growth and duplicated context. It introduces message-level deduplication in history providers (in-memory, file, Redis) and adds regression tests to ensure repeated inputs don’t bloat stored history.

Changes:

  • Added a message-identity helper and used it to filter out already-persisted messages before appending/pushing.
  • Updated save_messages behavior across in-memory, file (JSONL), and Redis history providers to skip duplicates.
  • Added unit/regression tests covering identical-message deduplication and mixed old/new message lists.

Reviewed changes

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

File Description
python/packages/core/agent_framework/_sessions.py Adds message identity + deduplication to in-memory and file history providers.
python/packages/core/tests/core/test_sessions.py Adds regression tests for deduplication behavior in core providers and looped runs.
python/packages/redis/agent_framework_redis/_history_provider.py Adds message identity + deduplication to Redis history persistence.
python/packages/redis/tests/test_providers.py Adds Redis-provider tests validating deduplication behavior.

Comment thread python/packages/core/agent_framework/_sessions.py Outdated
Comment thread python/packages/core/agent_framework/_sessions.py Outdated
Comment thread python/packages/core/agent_framework/_sessions.py Outdated
Comment thread python/packages/core/agent_framework/_sessions.py Outdated
Comment thread python/packages/redis/agent_framework_redis/_history_provider.py Outdated
@eavanvalkenburg

Copy link
Copy Markdown
Member

Please check the failing tests, and make sure the comments have a reply or are marked as resolved @PratikWayase

@PratikWayase
PratikWayase force-pushed the fix/per-service-history-duplicate-persistence branch from bf1f04f to ed50554 Compare July 24, 2026 09:39
@github-actions

github-actions Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Python Test Coverage

Python Test Coverage Report •
FileStmtsMissCoverMissing
packages/core/agent_framework
   _sessions.py9146493%161, 173–174, 198, 204–205, 243, 254, 268, 292, 337, 342, 344, 354, 386, 398, 408, 517, 586–587, 720, 1073–1077, 1092, 1122, 1159–1160, 1174, 1176, 1195, 1197, 1280, 1320, 1391, 1395, 1405, 1622, 1655–1656, 1661, 1676, 1754–1755, 1757, 1841, 1944, 2017, 2032, 2037–2039, 2063, 2082, 2085, 2093–2094, 2106–2107, 2119, 2129, 2159
packages/redis/agent_framework_redis
   _history_provider.py72198%205
TOTAL47389453590% 

Python Unit Test Overview

Tests Skipped Failures Errors Time
9754 34 💤 0 ❌ 0 🔥 2m 35s ⏱️

@eavanvalkenburg

Copy link
Copy Markdown
Member

Still some checks failing @PratikWayase

@eavanvalkenburg

Copy link
Copy Markdown
Member

and a new merge conflict @PratikWayase

@PratikWayase

Copy link
Copy Markdown
Contributor Author

@eavanvalkenburg, @moonbox3 @giles17 the merge conflict seems to be addressed now. Lmk if there's anything else

Comment thread python/packages/core/agent_framework/_sessions.py
Comment thread python/packages/redis/agent_framework_redis/_history_provider.py Outdated
@PratikWayase

Copy link
Copy Markdown
Contributor Author

@moonbox3 addressed your suggestions. Lmk if there's anything else.

if msg_id is not None:
return ("id", str(msg_id))

new_id = str(uuid.uuid4())

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 this fallback remain stable across requests? A stateless caller that reconstructs the same ID-less transcript gets a new UUID here on every call, so FileHistoryProvider and RedisHistoryProvider treat the replayed messages as new even though role and content are unchanged; only reusing the same Python object preserves the ID. That reintroduces the superlinear history growth this PR is meant to prevent. Could the identity be derived from stable request data or the generated ID be carried through the transport while still distinguishing separate identical turns?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've reverted the uuid4 approach entirely based on your feedback. Instead, I implemented a sequence-aware filter_new_messages() helper that uses stable content hashes. It correctly preserves legitimate duplicate turns within the same batch while detecting full transcript replays from stateless callers, preventing superlinear growth without requiring unstable IDs

await pipe.rpush(key, serialized) # type: ignore[misc]
for msg in new_messages:
identity = get_message_identity(msg)
await pipe.sadd(seen_key, json.dumps(list(identity))) # type: ignore[misc]

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 the :seen ledger be bounded along with max_messages? Every newly accepted message is added permanently, while only the list is trimmed at lines 194-197 and the set is removed only by clear(). A long-lived session configured with max_messages=N therefore retains an unbounded set and SMEMBERS scans it on every save, so Redis memory and save cost can grow without limit despite the documented retention bound. Could the seen state be pruned or otherwise kept proportional to retained history?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've removed the unbounded :seen SET entirely. Deduplication now uses the same sequence-aware filter_new_messages() logic against the bounded LRANGE result. This keeps Redis memory strictly proportional to max_messages and eliminates the SMEMBERS scan overhead

@PratikWayase
PratikWayase deployed to github-app-auth August 3, 2026 09:41 — with GitHub Actions Active
@PratikWayase

Copy link
Copy Markdown
Contributor Author

@moonbox3 addressed your recent suggestions. Lmk if there's anything else.

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: [Bug]: per-service-call history persistence re-appends already-persisted messages

5 participants