Skip to content

fix(conversation-manager): handle window_size=0 and reject negative values#2208

Open
SuperMarioYL wants to merge 1 commit intostrands-agents:mainfrom
SuperMarioYL:fix/sliding-window-zero-window-size
Open

fix(conversation-manager): handle window_size=0 and reject negative values#2208
SuperMarioYL wants to merge 1 commit intostrands-agents:mainfrom
SuperMarioYL:fix/sliding-window-zero-window-size

Conversation

@SuperMarioYL
Copy link
Copy Markdown

Problem

SlidingWindowConversationManager with window_size=0 silently fails to clear messages — the opposite of the intended behaviour documented in the TypeScript SDK.

Root cause in reduce_context:

trim_index = 2 if len(messages) <= self.window_size else len(messages) - self.window_size

With window_size=0 this sets trim_index = len(messages).
The while trim_index < len(messages) guard is immediately False, so the else branch fires:

  • During routine management (e is None): logs a warning and returns — messages unchanged.
  • During overflow recovery (e is not None): raises ContextWindowOverflowException — also wrong.

Negative window_size values weren't validated at construction time either.

Closes #2205.

Fix

  1. __init__ — raise ValueError for window_size < 0 (parallel to the existing per_turn guard).
  2. reduce_context — add a fast-path for window_size == 0: empty messages in-place, increment removed_message_count, and return. This matches the TypeScript SDK's explicit window_size=0 → "clear all messages" semantics (sdk-typescript#789).

Tests

Four new tests added to tests/strands/agent/test_conversation_manager.py:

Test What it checks
test_window_size_negative_raises_value_error window_size=-1 raises ValueError at construction
test_window_size_zero_clears_all_messages_on_apply_management apply_management with window_size=0 empties messages and updates removed_message_count
test_window_size_zero_clears_all_messages_on_reduce_context reduce_context with window_size=0 empties messages without overflow
test_window_size_zero_clears_on_overflow reduce_context with window_size=0 still clears messages even when e is provided

All 44 existing + new tests pass.

…alues

SlidingWindowConversationManager.reduce_context computed trim_index as
len(messages) - window_size. With window_size=0 that equals len(messages),
so the while-loop guard fired immediately and the else-branch either silently
returned (routine management) or raised ContextWindowOverflowException, leaving
messages unchanged instead of clearing them.

TypeScript SDK explicitly supports window_size=0 as "clear all messages" and
has a test for it. Align Python behaviour:

- Add a fast-path in reduce_context: if window_size == 0, empty the messages
  list in-place and update removed_message_count, then return.
- Add a ValueError guard in __init__ for negative window_size (parallel to the
  existing per_turn validation).

Fixes strands-agents#2205.
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 27, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

test_agent = Agent(messages=messages)
manager.reduce_context(test_agent, e=Exception("overflow"))

assert messages == []
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Issue: test_window_size_zero_clears_on_overflow is missing the removed_message_count assertion that the other two window_size=0 tests include. This means the test doesn't verify that the bookkeeping counter is updated in the overflow path.

Suggestion: Add assert manager.removed_message_count == 2 after assert messages == [] for consistency with the other tests and to fully verify the behavior.

@github-actions
Copy link
Copy Markdown

Assessment: Comment (leans Approve)

Clean, well-scoped fix that addresses a real bug with window_size=0 semantics and adds proper input validation. The PR description is excellent and the implementation is straightforward. One minor test gap noted inline — the overflow test is missing a removed_message_count assertion that the sibling tests include.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] SlidingWindowConversationManager: window_size=0 fails to clear messages

1 participant