-
Notifications
You must be signed in to change notification settings - Fork 5.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Tool call result summary message #4755
Tool call result summary message #4755
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- We recently updated the naming of toolcallmessage and toolcallresultmessage, look to pull from main and then add your changes, there is no real conflict
- The naming "ToolCallResultSummaryMessage" is a bit too long, I would just do "ToolCallSummaryMessage" perhaps, i'm sure @ekzhu has opinions
Code updated per request |
Thanks! See the failing tests, I believe ChatMessage should contain ToolCallSummaryMessage because of the Response type |
@victordibia This is related to #4725 I wonder what you think we should do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jspv you can read about how to run tests in CONTRIBUTING.md
In some group chat orchestrator classes, you need to update the handling of message thread:
Lines 426 to 427 in c74c2d6
def _thread_to_context(self) -> List[LLMMessage]: | |
"""Convert the message thread to a context for the model.""" |
Lines 103 to 104 in c74c2d6
if isinstance(msg, TextMessage | StopMessage | HandoffMessage): | |
message += f" {msg.content}" |
Bonus: would be great to add test code to verify the text content has been added to the prompt that gets sent to the monkey patched model client, for SelectorGroupChat.
Also declare message types for assistant agent, only when there are tools.
autogen/python/packages/autogen-agentchat/src/autogen_agentchat/agents/_assistant_agent.py
Lines 281 to 282 in c74c2d6
def produced_message_types(self) -> List[type[ChatMessage]]: | |
"""The types of messages that the assistant agent produces.""" |
...gentchat/src/autogen_agentchat/teams/_group_chat/_magentic_one/_magentic_one_orchestrator.py
Outdated
Show resolved
Hide resolved
...n/packages/autogen-agentchat/src/autogen_agentchat/teams/_group_chat/_selector_group_chat.py
Outdated
Show resolved
Hide resolved
@husseinmozannar I believe the recent changes to this PR has already resolved your requests. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@victordibia I believe this will break connection.py in AGS, specifically line 328 we should add ToolCallSummary
It is related, we might defer to a seperate PR. I feel #4725 might be better served by extending I'll try to see how I can address the ags breaks |
@husseinmozannar @victordibia please take over this PR from now on to address any changes necessary |
I have taken a look .. I'll address any changes needed in a seperate PR. (mostly to add checks for ToolCallSummaryMessage types). Its not exactly breaking for now (those messages may not show up in the UI) |
Why are these changes needed?
Creates a ToolCallResultSummaryMessage type more clearly differentiate when an agent is responding with a tool summary
vs. a TextMessage response from the LLM.
Related issue number
Closes #4754
Checks