Skip to content

Conversation

@ekzhu
Copy link
Contributor

@ekzhu ekzhu commented Mar 29, 2025

Rename the ChatMessage and AgentEvent base classes to BaseChatMessage and BaseAgentEvent.

Bring back the ChatMessage and AgentEvent as union of built-in concrete types to avoid breaking existing applications that depends on Pydantic serialization.

Why?

Many existing code uses containers like this:

class AppMessage(BaseModel):
   name: str
   message: ChatMessage 

# Serialization is this:
m = AppMessage(...)
m.model_dump_json()

# Fields like HandoffMessage.target will be lost because it is now treated as a base class without content or target fields.

The assumption on ChatMessage or AgentEvent to be a union of concrete types could be in many existing code bases. So this PR brings back the union types, while keep method type hints such as those on on_messages to use the BaseChatMessage and BaseAgentEvent base classes for flexibility.

@codecov
Copy link

codecov bot commented Mar 29, 2025

Codecov Report

Attention: Patch coverage is 96.00000% with 6 lines in your changes missing coverage. Please review.

Project coverage is 76.98%. Comparing base (e686342) to head (c9df21e).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...utogen-agentchat/src/autogen_agentchat/messages.py 93.61% 3 Missing ⚠️
...es/autogen-ext/src/autogen_ext/ui/_rich_console.py 0.00% 2 Missing ⚠️
...en_agentchat/teams/_group_chat/_base_group_chat.py 85.71% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6144      +/-   ##
==========================================
+ Coverage   76.97%   76.98%   +0.01%     
==========================================
  Files         192      192              
  Lines       13487    13493       +6     
==========================================
+ Hits        10381    10387       +6     
  Misses       3106     3106              
Flag Coverage Δ
unittests 76.98% <96.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@a-holm
Copy link

a-holm commented Mar 30, 2025

I took the freedom to have a look for the practice. LGTM! This PR effectively addresses the backward compatibility concern for Pydantic serialization by renaming the base classes to BaseChatMessage and BaseAgentEvent and reintroducing the original names as discriminated union types. The changes are applied consistently across the codebase, including necessary updates to type hints, the MessageFactory, tests, and documentation. This refactoring improves type clarity for future extensions while maintaining compatibility for existing users.

The renames and type hint updates appear consistent. No instances were found where the old names ChatMessage/AgentEvent were used when BaseChatMessage/BaseAgentEvent were intended after the refactor, or vice versa within the diff's scope. The introduction of BaseTextChatMessage as an intermediate base class for text-based chat messages is a logical addition to share implementation (to_text, to_model_text, to_model_message) and doesn't introduce conflicts.

Test files (test_messages.py, test_assistant_agent.py, test_group_chat.py, etc.) have been updated to reflect the renames and correctly use BaseChatMessage/BaseAgentEvent in type hints where applicable. test_messages.py includes tests for the new union types and MessageFactory.

@ekzhu ekzhu merged commit 7615c7b into main Mar 30, 2025
57 checks passed
@ekzhu ekzhu deleted the ekzhu-renaming branch March 30, 2025 16:34
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.

4 participants