-
Notifications
You must be signed in to change notification settings - Fork 510
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
admin takeover in group chat #1209
Conversation
@@ -12,6 +12,7 @@ class GroupChat: | |||
agents: List[Agent] | |||
messages: List[Dict] | |||
max_round: int = 10 | |||
admin_name: str = "Admin" # the name of the admin agent |
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.
Maybe check if admin_name exists here? So you don't need to throw expection in run_chat
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.
how do we check it "here"?
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.
My understand is admin_name
must be the name of an agent in agents
, Otherwise the keyboardInterruption
will be raised when human user tries to interrupt group chat?
If my understand is correct it would be better to check if admin_name
is legit(a.k.a is one of an agents
) in constructor.
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.
admin_name
is optional. There doesn't have to be an admin. Only when there is, we support using KeyboardInterruption to take over.
flaml/autogen/agentchat/groupchat.py
Outdated
# speaker selection msg from an agent | ||
speaker = config.select_speaker(speaker, self) | ||
reply = speaker.generate_reply(sender=self) | ||
if i != groupchat.max_round - 1: |
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.
Please add a comment to explain what's being done here?
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.
added.
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.
Looks good to me. Although there can be a few more comments added to the notebook to explain the role of the different agents.
Why are these changes needed?
Related issue number
Checks