Skip to content

Commit

Permalink
Ensure 'name' on initial message (microsoft#2635)
Browse files Browse the repository at this point in the history 
* Update to ensure name on initial messages

* Corrected test cases for messages now including names.

* Added name to messages within select speaker nested chat

* Corrected select speaker group chat tests for name field

---------

Co-authored-by: Chi Wang <[email protected]>
  • Loading branch information
@marklysze @sonichi @open-autogen
2 people authored and open-autogen committed Aug 21, 2024
1 parent 5a406e4 commit 522ef49
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 11 deletions.
17 changes: 13 additions & 4 deletions autogen/agentchat/conversable_agent.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -621,7 +621,7 @@ def _assert_valid_name(name):
raise ValueError(f"Invalid name: {name}. Name must be less than 64 characters.")
return name

def _append_oai_message(self, message: Union[Dict, str], role, conversation_id: Agent) -> bool:
def _append_oai_message(self, message: Union[Dict, str], role, conversation_id: Agent, is_sending: bool) -> bool:
"""Append a message to the ChatCompletion conversation.
If the message received is a string, it will be put in the "content" field of the new dictionary.
Expand All @@ -633,6 +633,7 @@ def _append_oai_message(self, message: Union[Dict, str], role, conversation_id:
message (dict or str): message to be appended to the ChatCompletion conversation.
role (str): role of the message, can be "assistant" or "function".
conversation_id (Agent): id of the conversation, should be the recipient or sender.
is_sending (bool): If the agent (aka self) is sending to the conversation_id agent, otherwise receiving.
Returns:
bool: whether the message is appended to the ChatCompletion conversation.
Expand Down Expand Up @@ -662,7 +663,15 @@ def _append_oai_message(self, message: Union[Dict, str], role, conversation_id:

if oai_message.get("function_call", False) or oai_message.get("tool_calls", False):
oai_message["role"] = "assistant" # only messages with role 'assistant' can have a function call.
elif "name" not in oai_message:
# If we don't have a name field, append it
if is_sending:
oai_message["name"] = self.name
else:
oai_message["name"] = conversation_id.name

self._oai_messages[conversation_id].append(oai_message)

return True

def _process_message_before_send(
Expand Down Expand Up @@ -718,7 +727,7 @@ def send(
message = self._process_message_before_send(message, recipient, ConversableAgent._is_silent(self, silent))
# When the agent composes and sends the message, the role of the message is "assistant"
# unless it's "function".
valid = self._append_oai_message(message, "assistant", recipient)
valid = self._append_oai_message(message, "assistant", recipient, is_sending=True)
if valid:
recipient.receive(message, self, request_reply, silent)
else:
Expand Down Expand Up @@ -768,7 +777,7 @@ async def a_send(
message = self._process_message_before_send(message, recipient, ConversableAgent._is_silent(self, silent))
# When the agent composes and sends the message, the role of the message is "assistant"
# unless it's "function".
valid = self._append_oai_message(message, "assistant", recipient)
valid = self._append_oai_message(message, "assistant", recipient, is_sending=True)
if valid:
await recipient.a_receive(message, self, request_reply, silent)
else:
Expand Down Expand Up @@ -839,7 +848,7 @@ def _print_received_message(self, message: Union[Dict, str], sender: Agent):

def _process_received_message(self, message: Union[Dict, str], sender: Agent, silent: bool):
# When the agent receives a message, the role of the message is "user". (If 'role' exists and is 'function', it will remain unchanged.)
valid = self._append_oai_message(message, "user", sender)
valid = self._append_oai_message(message, "user", sender, is_sending=False)
if logging_enabled():
log_event(self, "received_message", message=message, sender=sender.name, valid=valid)

Expand Down
3 changes: 3 additions & 0 deletions autogen/agentchat/groupchat.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -649,6 +649,7 @@ def validate_speaker_name(recipient, messages, sender, config) -> Tuple[bool, Un
if self.select_speaker_prompt_template is not None:
start_message = {
"content": self.select_speaker_prompt(agents),
"name": "checking_agent",
"override_role": self.role_for_select_speaker_messages,
}
else:
Expand Down Expand Up @@ -813,6 +814,7 @@ def _validate_speaker_name(

return True, {
"content": self.select_speaker_auto_multiple_template.format(agentlist=agentlist),
"name": "checking_agent",
"override_role": self.role_for_select_speaker_messages,
}
else:
Expand Down Expand Up @@ -842,6 +844,7 @@ def _validate_speaker_name(

return True, {
"content": self.select_speaker_auto_none_template.format(agentlist=agentlist),
"name": "checking_agent",
"override_role": self.role_for_select_speaker_messages,
}
else:
Expand Down
19 changes: 12 additions & 7 deletions test/agentchat/test_groupchat.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -724,7 +724,7 @@ def test_clear_agents_history():
agent1_history = list(agent1._oai_messages.values())[0]
agent2_history = list(agent2._oai_messages.values())[0]
assert agent1_history == [
{"content": "hello", "role": "assistant"},
{"content": "hello", "role": "assistant", "name": "alice"},
{"content": "This is bob speaking.", "name": "bob", "role": "user"},
{"content": "How you doing?", "name": "sam", "role": "user"},
]
Expand All @@ -745,7 +745,7 @@ def test_clear_agents_history():
{"content": "How you doing?", "name": "sam", "role": "user"},
]
assert agent2_history == [
{"content": "This is bob speaking.", "role": "assistant"},
{"content": "This is bob speaking.", "role": "assistant", "name": "bob"},
{"content": "How you doing?", "name": "sam", "role": "user"},
]
assert groupchat.messages == [
Expand All @@ -759,12 +759,12 @@ def test_clear_agents_history():
agent1_history = list(agent1._oai_messages.values())[0]
agent2_history = list(agent2._oai_messages.values())[0]
assert agent1_history == [
{"content": "hello", "role": "assistant"},
{"content": "hello", "role": "assistant", "name": "alice"},
{"content": "This is bob speaking.", "name": "bob", "role": "user"},
{"content": "How you doing?", "name": "sam", "role": "user"},
]
assert agent2_history == [
{"content": "This is bob speaking.", "role": "assistant"},
{"content": "This is bob speaking.", "role": "assistant", "name": "bob"},
{"content": "How you doing?", "name": "sam", "role": "user"},
]
assert groupchat.messages == [
Expand Down Expand Up @@ -822,6 +822,7 @@ def test_clear_agents_history():
"content": "example tool response",
"tool_responses": [{"tool_call_id": "call_emulated", "role": "tool", "content": "example tool response"}],
"role": "tool",
"name": "alice",
},
]

Expand Down Expand Up @@ -1218,7 +1219,7 @@ def test_role_for_select_speaker_messages():
# into a message attribute called 'override_role'. This is evaluated in Conversable Agent's _append_oai_message function
# e.g.: message={'content':self.select_speaker_prompt(agents),'override_role':self.role_for_select_speaker_messages},
message = {"content": "A prompt goes here.", "override_role": groupchat.role_for_select_speaker_messages}
checking_agent._append_oai_message(message, "assistant", speaker_selection_agent)
checking_agent._append_oai_message(message, "assistant", speaker_selection_agent, is_sending=True)

# Test default is "system"
assert len(checking_agent.chat_messages) == 1
Expand All @@ -1227,15 +1228,15 @@ def test_role_for_select_speaker_messages():
# Test as "user"
groupchat.role_for_select_speaker_messages = "user"
message = {"content": "A prompt goes here.", "override_role": groupchat.role_for_select_speaker_messages}
checking_agent._append_oai_message(message, "assistant", speaker_selection_agent)
checking_agent._append_oai_message(message, "assistant", speaker_selection_agent, is_sending=True)

assert len(checking_agent.chat_messages) == 1
assert checking_agent.chat_messages[speaker_selection_agent][-1]["role"] == "user"

# Test as something unusual
groupchat.role_for_select_speaker_messages = "SockS"
message = {"content": "A prompt goes here.", "override_role": groupchat.role_for_select_speaker_messages}
checking_agent._append_oai_message(message, "assistant", speaker_selection_agent)
checking_agent._append_oai_message(message, "assistant", speaker_selection_agent, is_sending=True)

assert len(checking_agent.chat_messages) == 1
assert checking_agent.chat_messages[speaker_selection_agent][-1]["role"] == "SockS"
Expand Down Expand Up @@ -1646,6 +1647,7 @@ def test_speaker_selection_validate_speaker_name():
True,
{
"content": groupchat.select_speaker_auto_multiple_template.format(agentlist=agent_list_string),
"name": "checking_agent",
"override_role": groupchat.role_for_select_speaker_messages,
},
)
Expand Down Expand Up @@ -1692,6 +1694,7 @@ def test_speaker_selection_validate_speaker_name():
True,
{
"content": groupchat.select_speaker_auto_none_template.format(agentlist=agent_list_string),
"name": "checking_agent",
"override_role": groupchat.role_for_select_speaker_messages,
},
)
Expand Down Expand Up @@ -1761,6 +1764,7 @@ def test_select_speaker_auto_messages():
True,
{
"content": custom_multiple_names_msg.replace("{agentlist}", "['Alice', 'Bob']"),
"name": "checking_agent",
"override_role": groupchat.role_for_select_speaker_messages,
},
)
Expand All @@ -1770,6 +1774,7 @@ def test_select_speaker_auto_messages():
True,
{
"content": custom_no_names_msg.replace("{agentlist}", "['Alice', 'Bob']"),
"name": "checking_agent",
"override_role": groupchat.role_for_select_speaker_messages,
},
)
Expand Down

0 comments on commit 522ef49

Please sign in to comment.