Python: fix(redis): honour a max_messages retention limit of zero

[chinmayv095]

Motivation & Context

RedisHistoryProvider.__init__ documents max_messages as "Maximum number of messages to
retain per session ... None means unlimited storage". Because None is the documented
sentinel for unlimited, 0 has to mean "retain nothing". Today it means "retain everything".

save_messages enforces the limit by trimming to -max_messages:

await self._redis_client.ltrim(key, -self.max_messages, -1)

For max_messages=0 that is LTRIM key 0 -1, which is Redis's "keep the whole list". The guard
current_count > self.max_messages is true for any non-empty list, so the trim runs on every
save and does nothing. The one setting that asks for no retention is the only non-None setting
that never bounds the list, and it does so silently — no error, no warning, just unbounded growth
of chat_messages:<session_id>.

Negative values are worse than a no-op: max_messages=-5 issues LTRIM key 5 -1, which deletes
the five oldest messages on every save while the list still grows without bound. That destroys
history and still does not enforce a limit.

Description & Review Guide

What are the major changes?

Two, both in _history_provider.py:

1. save_messages short-circuits when the limit is zero: it drops any existing history with
   delete(key) — the call clear() in this same class already uses — and returns before
   serializing anything. LTRIM cannot express "keep nothing", and writing first would put the
   payload into Redis, and into any AOF or replica stream, before deleting it. The positive-limit
   trim path is untouched by the diff.
2. __init__ rejects a negative max_messages with a ValueError, alongside the three
   ValueErrors it already raises for invalid configuration.

The docstring now states the 0 case explicitly.

What is the impact of these changes?

max_messages=None and max_messages=N for N > 0 are byte-for-byte unchanged — the existing
ltrim(key, -N, -1) path is untouched, and the existing tests covering it pass unmodified. Only
0 and negatives change, and both are currently broken. No key format, serialization or wire
change, so existing deployments are unaffected.

What do you want reviewers to focus on?

Whether rejecting negatives at construction is preferred over clamping them to 0. I chose to
raise because the current behaviour deletes data, so silently reinterpreting the value seemed
worse than refusing it, and because __init__ already validates its other arguments this way.
Happy to switch to a clamp if you'd rather not add a new raise.

Deliberately out of scope — RedisHistoryProvider still writes to
{key_prefix}:{session_id} without source_id, so two providers with different source_ids
but the same prefix share one list, where CosmosHistoryProvider isolates by source_id in
get_messages, clear and list_sessions. That is a key-format change with migration
implications, not a bug fix, so it is not in this PR. Raised separately as #7471, with the
migration options laid out there.

Related Issue

Fixes #7469

Contribution Checklist

• The code builds clean without any errors or warnings
• All unit tests pass, and I have added new tests where possible

Verification, all on python/packages/redis:

• Baseline on main: 45 passed, 0 failed.
• Both new tests run against unpatched code first and fail —
  test_max_messages_zero_retains_nothing (delete called 0 times) and
  test_negative_max_messages_raises (no ValueError). With the fix: 47 passed, 0 failed,
  zero regressions.
• ruff check clean, ruff format --check reports both files already formatted.
• mypy --config-file python/pyproject.toml agent_framework_redis — no issues in 4 source files.
• Second commit addresses the automated review: the zero path no longer writes before deleting,
  the new ValueError is documented under Raises:, and the test now asserts the pipeline is
  never used. All checks re-run green.

[Copilot]

Pull request overview

Fixes RedisHistoryProvider retention semantics so max_messages=0 does not behave like “unlimited”, and adds validation to reject negative max_messages values.

Changes:

• Update RedisHistoryProvider.__init__ documentation and configuration validation for max_messages (explicitly document 0, reject negatives).
• Adjust retention enforcement logic in save_messages for the max_messages=0 case.
• Add unit tests covering negative max_messages and max_messages=0 behavior.

Reviewed changes

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

File	Description
python/packages/redis/agent_framework_redis/_history_provider.py	Adds max_messages validation and implements the special-case retention handling for 0, plus docstring update.
python/packages/redis/tests/test_providers.py	Adds tests for negative max_messages and for max_messages=0 retention behavior.

[chinmayv095]

@microsoft-github-policy-service agree

[moonbox3]

Could this zero-retention path avoid deleting history owned by another provider? _redis_key() omits source_id, so two RedisHistoryProviders with the default prefix share {key_prefix}:{session_id}; because providers persist in reverse order, a zero-limit provider placed before a positive-limit provider deletes the latter's newly written history. Could the delete be scoped to this provider, or deferred until keys include source_id, so enabling zero retention cannot cause cross-provider data loss?