-
Notifications
You must be signed in to change notification settings - Fork 190
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
Swarm: User-defined AfterWork string for agent selection using LLM #369
base: swarmagenttoconversable
Are you sure you want to change the base?
Conversation
Signed-off-by: Mark Sze <[email protected]>
…armafterworkmgrmsg
Signed-off-by: Mark Sze <[email protected]>
Signed-off-by: Mark Sze <[email protected]>
Signed-off-by: Mark Sze <[email protected]>
Signed-off-by: Mark Sze <[email protected]>
Signed-off-by: Mark Sze <[email protected]>
Signed-off-by: Mark Sze <[email protected]>
Signed-off-by: Mark Sze <[email protected]>
Signed-off-by: Mark Sze <[email protected]>
Signed-off-by: Mark Sze <[email protected]>
Signed-off-by: Mark Sze <[email protected]>
Thanks for the initial review @sonichi, if you could have another look over now that would be appreciated. |
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.
Look good to me overall. My only question is about the context_str class. I think this class might not be necessary at all, and also consider existing packages like dedent
.
agent_list = [ | ||
agent | ||
for agent in groupchat.agents | ||
if agent.name != __TOOL_EXECUTOR_NAME__ and not agent.name.startswith("nested_chat_") | ||
] | ||
|
||
groupchat.select_speaker_prompt_template = template | ||
return groupchat.select_speaker_prompt(agent_list) | ||
|
||
if after_work_next_agent_selection_msg is None: | ||
# If there's no selection message, restore the default and filter out the tool executor and nested chat agents | ||
groupchat.select_speaker_prompt_template = substitute_agentlist(SELECT_SPEAKER_PROMPT_TEMPLATE) | ||
elif isinstance(after_work_next_agent_selection_msg, str): | ||
# No context variable substitution for string, but agentlist will be substituted | ||
groupchat.select_speaker_prompt_template = substitute_agentlist(after_work_next_agent_selection_msg) | ||
elif isinstance(after_work_next_agent_selection_msg, ContextStr): | ||
# Replace the agentlist in the string first, putting it into a new ContextStr | ||
agent_list_replaced_string = ContextStr(substitute_agentlist(after_work_next_agent_selection_msg.template)) |
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.
Can we let the user to only pass in a str or a callable?
We can always convert the str to a ContextStr class, replace agent_list and context variables.
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.
Thanks for the review. You raise a good point and Chi and I had a chat about this as well prior. The thought behind making it an explicit choice to use ContextStr or a normal, non-substituted, string is so that the developer has complete control over the choice and they're not unsure of what will happen to the value.
@sonichi if you have anything further to add to this consideration?
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.
I guess the ambiguity happens when the str contains "{}", and whether it means a format string or not is ambiguous. That's my understanding of why ContextStr
is introduced. Is that right? Shall we add the consideration into docstr?
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.
Yes, that's right, by using "{}" the user doesn't have to be concerned about unexpected replacements (except {agentlist], which is default Group Chat behaviour)
In terms of adding to docstr, do you mean the ContextStr docstring?
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.
@yiranwu0, did you have any further thoughts on this? Are you okay with making it an explicit choice for the developer to choose ContextStr when they want the substitutions and without when they don't?
Why are these changes needed?
In line with #26, this provides the developer with an option to specify a string prompt for an LLM to use to select the next agent in an agent's AfterWork hand off.
It extends the AfterWorkOption.SWARM_MANAGER (which, by default, uses the default
auto
speaker selection options) with an additional AfterWork parameter,next_agent_selection_msg
, which becomes the selection prompt used by the LLM.next_agent_selection_msg
can be a string or a Callable that returns a string.The signature for the Callable is:
def my_selection_message(agent: ConversableAgent, messages: List[Dict[str, Any]]) -> str
The string provided, or returned from the
Callable
, will have the following string substitutions:{agentlist}
will be replaced by a comma-delimited list of agentsThe underlying groupchat's GroupChatManager will be used for the speaker selection and must, therefore, have an LLM Config. This can be set through the new
swarm_manager_args
parameter ona/initiate_swarm_chat
.Example:
Related issue number
#26
Checks