Skip to content

fix: raise a clear RuntimeError instead of a bare assert when streaming speaker selection yields no final result#7943

Open
roli-lpci wants to merge 1 commit into
microsoft:mainfrom
roli-lpci:fix/selector-group-chat-empty-stream-assert
Open

fix: raise a clear RuntimeError instead of a bare assert when streaming speaker selection yields no final result#7943
roli-lpci wants to merge 1 commit into
microsoft:mainfrom
roli-lpci:fix/selector-group-chat-empty-stream-assert

Conversation

@roli-lpci

Copy link
Copy Markdown

What

SelectorGroupChatManager._select_speaker (autogen_agentchat/teams/_group_chat/_selector_group_chat.py) seeds chunk: CreateResult | str = "" before iterating self._model_client.create_stream(...) for speaker selection. If the stream yields no final CreateResult (a legitimate edge case: a provider hiccup, a cancelled/truncated stream, or a custom ChatCompletionClient implementation), chunk is never a CreateResult, and the following line:

# The last chunk must be CreateResult.
assert isinstance(chunk, CreateResult)

raises a bare, message-less AssertionError. Under -O/PYTHONOPTIMIZE=1, assertions are stripped entirely, so the code instead falls through to response = "" and crashes moments later with an unrelated AttributeError: 'str' object has no attribute 'content'.

Fix

Seed chunk with None (a sentinel that can't be confused with real stream output) and replace the bare assert with an explicit if not isinstance(chunk, CreateResult): raise RuntimeError(...). This mirrors the sibling streaming call site, AssistantAgent._call_llm (autogen_agentchat/agents/_assistant_agent.py), which already handles this exact situation the same way. No behavior change on the happy path: when create_stream yields a final CreateResult, chunk ends up being that result exactly as before.

Test

Added test_selector_group_chat_streaming_no_final_result to tests/test_group_chat.py, parallel to the existing AssistantAgent regression test test_streaming_without_final_result. It overrides create_stream to yield chunks with no terminal CreateResult and asserts the clear RuntimeError. The existing selector suite (-k selector) passes unchanged.

Scope

Single-function fix, matching an idiom already established elsewhere in the same package. Does not touch AssistantAgent._call_llm or any other streaming call site.

Note

This package currently carries a "maintenance mode" banner (#7521) as the org consolidates into a newer Agent Framework. Submitting anyway since the fix is small, correct, and cheap to review; happy to close without hard feelings if maintainer bandwidth doesn't allow a look.

…ng speaker selection yields no final result

SelectorGroupChatManager._select_speaker seeds `chunk` with the string ""
before iterating create_stream() for speaker selection. If the stream yields
no final CreateResult, the following `assert isinstance(chunk, CreateResult)`
raises a bare, message-less AssertionError, and under `-O` (assertions stripped)
the code falls through to `response = ""` and later crashes with an unrelated
AttributeError. Seed `chunk` with None and raise an explicit RuntimeError,
matching AssistantAgent._call_llm which already handles this case the same way.

@ErenAta16 ErenAta16 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ran the two new tests on the branch — pass. The -O/PYTHONOPTIMIZE=1 angle is worth calling out on its own: even setting aside the message-less AssertionError, a bare assert silently vanishes under optimized mode, so this was not just "raises a worse-than-ideal error" but "fails an entirely different, more confusing way (or not at all, falling through to a later AttributeError) depending on how the interpreter is invoked." A real RuntimeError fixes both problems at once.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants