-
Notifications
You must be signed in to change notification settings - Fork 146
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
Replace print commands with structured messages #282
base: main
Are you sure you want to change the base?
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.
Looks good. Could you check whether any doc change is needed, for example, about the interface to get usage summary? cc @yiranwu0 @marklysze
MessageRole = Literal["assistant", "function", "tool"] | ||
|
||
|
||
class BaseMessage(BaseModel): |
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.
Suggestion: add docstr to explain how to use this class in a number of steps for developers who need to define a new message type. For example, 1. define a subclass with a few data fields and override the print
method, 2. define a function to create an instance of the subclass, like create_...
, 3. In agentchat
, when ..., do ....
Question: Can this class support multimodal messages, such as image, audio, video? cc @BabyCNM
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.
This is just the first part of the task, the next one is to refactor IOStream classes to use structured messages, especially with WebSockets. We'll write a blog post on how to use it to build UI easier.
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'll have to add multimodal messages in another PR. It needs to be supported by client classes in a unified way first.
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 create the followup issue #331
No docs change needed because I didn't modify signature of existing functions and methods. I just moved the print statements to single place. I am not done yet though, still there are few places to change. |
Why are these changes needed?
Related issue number
Checks