fix: validate participants type in BaseGroupChat constructor - #8145
MOHAMMED WASIM KHAN (wasim-builds) wants to merge 2 commits into
Conversation
Previously, passing None, a non-sequence, or non-agent objects as participants would raise internal AttributeError/TypeError. Now raises clear TypeError with descriptive message. Fixes microsoft#7580
There was a problem hiding this comment.
🟡 Changes recommended
The new error message currently says “sequence” while the code enforces list/tuple (and the type annotation still implies list), so the message/tests should be aligned with the actual accepted types.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR improves input validation for BaseGroupChat (and therefore RoundRobinGroupChat) by turning previously-leaked internal AttributeError/TypeError failures into explicit, descriptive constructor errors when participants is invalid, and adds regression tests to lock in that behavior.
Changes:
- Added constructor validation in
BaseGroupChat.__init__forparticipants(None / non-container / wrong element types). - Added regression tests covering
participants=None, non-sequence input, and a mixed valid/invalid participants list.
File summaries
| File | Description |
|---|---|
| python/packages/autogen-agentchat/src/autogen_agentchat/teams/_group_chat/_base_group_chat.py | Adds participants type validation and clearer error messages in the base group chat constructor. |
| python/packages/autogen-agentchat/tests/test_group_chat.py | Adds regression tests asserting clearer constructor errors for invalid participants. |
Review details
Suppressed comments (1)
python/packages/autogen-agentchat/tests/test_group_chat.py:1960
- Same as above: the asserted error message substring should match the enforced input types (list/tuple) to keep the test accurate and stable.
"""Test that non-sequence participants raises a clear TypeError."""
with pytest.raises(TypeError, match="participants must be a non-empty sequence"):
RoundRobinGroupChat(participants="not a list") # type: ignore
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if participants is None: | ||
| raise TypeError("participants must be a non-empty sequence of ChatAgent or Team instances, got None") | ||
| if not isinstance(participants, (list, tuple)): | ||
| raise TypeError( | ||
| f"participants must be a non-empty sequence of ChatAgent or Team instances, got {type(participants).__name__}" |
| """Test that participants=None raises a clear TypeError.""" | ||
| with pytest.raises(TypeError, match="participants must be a non-empty sequence"): | ||
| RoundRobinGroupChat(participants=None) # type: ignore |
|
Hi team, Following up on this PR regarding fix: validate participants type in BaseGroupChat constructor in microsoft/autogen. Working on multi-agent orchestration, agent communication, and tool execution in AutoGen has been a great experience, and I am very keen to continue contributing to microsoft/autogen. I am also open to freelance/contract work, full-time engineering roles, or joining the organization as an active contributor or team member. Please feel free to check out my GitHub profile at https://github.com/wasim-builds. |
Summary
Fixes #7580
Previously, passing None, a non-sequence, or non-agent objects as participants would raise internal AttributeError/TypeError. Now raises clear TypeError with descriptive message.
Changes
Test plan