Skip to content

fix(stream): preserve explicitly provided empty storage - #3498

Merged
vvlrff merged 5 commits into
ag2ai:mainfrom
fallintoplace:fix/preserve-empty-storage-20261024
Oct 25, 2026
Merged

vvlrff merged 5 commits into
ag2ai:mainfrom
fallintoplace:fix/preserve-empty-storage-20261024

Conversation

@fallintoplace

Copy link
Copy Markdown
Contributor

Why are these changes needed?

What: Keep an explicitly supplied storage backend, even when it is empty.

Why: A length-aware storage can be falsy before its first event. MemoryStream would silently replace it with a new MemoryStorage, so events went to the wrong backend.

Implementation: Check storage is not None rather than truthiness. Add a regression test with an initially empty storage that implements __len__.

Related issue number

None.

Checks

  • I've included any doc changes needed for https://docs.ag2.ai/. (None needed.)
  • I've added tests (if relevant) corresponding to the changes introduced in this PR.
  • I've made sure all auto checks have passed.

Local validation: reproduced the fallback behavior in isolation and syntax-checked the regression test. Full AG2 tests were not run because project dependencies are unavailable locally.

AI assistance

  • I understand the changes in this PR and can explain them in my own words.
  • I have verified that the PR description accurately reflects the actual diff.
  • If AI assistance was used, I reviewed, tested, and validated the generated code/text before submitting.

@github-actions github-actions Bot added the area:core Agent runtime core: ag2/*.py, ag2/events, ag2/response, ag2/streams label Oct 24, 2026

@BichengWang BichengWang 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.

The new regression test fails at this head: python -m pytest -q test/stream --ignore=test/stream/redis gives 1 failed, 48 passed. At test/stream/test_custom_storage.py:31, ModelMessage is transient, so the default stream intentionally filters it out before saving. The history assertion therefore gets [] even with the production fix.

Please use a persisted event such as ModelRequest([TextInput("hello")]), or explicitly set persist_all=True. I separately checked a persisted request with both persistence modes and history reuse: those checks fail on main and pass here, so the storage selection change itself addresses the reported bug.

@vvlrff

vvlrff commented Oct 25, 2026

Copy link
Copy Markdown
Collaborator

@fallintoplace Thanks!

@vvlrff
vvlrff enabled auto-merge October 25, 2026 12:36
@vvlrff
vvlrff added this pull request to the merge queue Oct 25, 2026
Merged via the queue into ag2ai:main with commit dded376 Oct 25, 2026
17 of 18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:core Agent runtime core: ag2/*.py, ag2/events, ag2/response, ag2/streams

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants