feat: Add builder for AssistantMessage #4351
Conversation
Signed-off-by: Thomas Vitale <[email protected]>
ThomasVitale
force-pushed
the
gh-add-builder-for-assistant-message
branch
from
eb364dc to
6994a56
Compare
| protected final List<Media> media; | ||
|
|
||
| public AssistantMessage(String content) { | ||
| this(content, Map.of()); |
There was a problem hiding this comment.
@ThomasVitale What do you think about deprecating this public usage as well - in favour of the builder? This way, new instance of AssistantMessage can only be created via builder only.
There was a problem hiding this comment.
That would be fine by me. I know that for UserMessage and SystemMessage we opted for keeping the 1-argument constructor for convenience, and forcing the Builder for any other use cases where more than one argument needs to be provided. What do you think?
There was a problem hiding this comment.
I see. That's a good point. I like the convenience aspect as well. We can keep the single argument constructor for AssistantMessage as well.
|
@ThomasVitale Thanks for the PR! |
|
Rebased and merged as f4a1c96 |
|
@ilayaperumalg thank you! |
3 participants
In alignment with SystemMessage and UserMessage, introduced a builder for AssistantMessage to simplify instantiation considering the many arguments currently needed by the constructors.
Introduced missing tests for AssistantMessage.