chore: Improve UX for Orchestration chat scenario #543
Conversation
shibeshduw
changed the title
| * @returns A list of all messages. | ||
| */ | ||
| getAllMessages(choiceIndex = 0): ChatMessages { | ||
| const messages = (this.data.module_results.templating ?? []).map( |
There was a problem hiding this comment.
[q] Why the transformation? Everything you get from the templating results can be passed as it is in message history right?
There was a problem hiding this comment.
Yes that is true but if the user would use this convenience for anything else, they would have to handle complex objects when dealing with MultiChatMessage.
There was a problem hiding this comment.
Sorry, I am not quite sure I understand. What else would the user use this for?
I thought the main idea here is to pass the existing templating messages and the new response message back as history. The template should be assignable as it is, since the types are same. This will also not handle tool_calls.
| ).map(message => | ||
| this.isMultiChatMessage(message) | ||
| ? this.handleMultiChatMessage(message as MultiChatMessage) | ||
| : this.handleSingleChatMessage(message as SingleChatMessage) |
There was a problem hiding this comment.
[req] I would just return the message from here, because the mapping is not required. The message type is already how you want it to be
There was a problem hiding this comment.
Do you mean both the mapping or just handleSingleChatMessage?
There was a problem hiding this comment.
just handleSingleChatMessage.
| : this.handleSingleChatMessage(message as SingleChatMessage) | ||
| ); | ||
|
|
||
| const content = this.getChoices().find(c => c.index === choiceIndex) |
There was a problem hiding this comment.
[q] How do we handle cases when the assistant response has empty content and tool_calls? It should retain that structure.
There was a problem hiding this comment.
I updated this to take the entire message instead of just content.
Context
Closes SAP/ai-sdk-js-backlog#228.
What this PR does and why it is needed