-
Notifications
You must be signed in to change notification settings - Fork 490
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
Make auto reply method pluggable #1177
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.
What's your thoughts on ID and Group ID for agents? See my comments earlier. Otherwise it looks good to me.
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. The pluggable auto reply is useful. The group chat example is also interesting. Shall we add examples about auto reply on the documentation page (could be done in a different PR)?
Why are these changes needed?
Right now registering a new auto reply method is not entirely modular. We can register a new auto reply method but that method may require some state initialization in the constructor. That makes the effort of adding an auto reply method not a single operation. Ideally we should be able to use a single operation to register an auto reply method.
This PR enables that. One example is that we can give any ResponsiveAgent the ability to manage a group chat now, without needing to inherit the GroupChatManager class.
The change in this PR makes it easy to plug different capabilities into an agent without inheriting. It also explicitly separates the context needed for each auto reply method.
Big thanks to @ekzhu @LittleLittleCloud @BeibinLi for their inspiration.
Related issue number
#1176 and #1158 will be affected by this PR.
Checks