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.
| : 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. So the response will also include tool_calls.
| * @param choiceIndex - The index of the choice to parse. | ||
| * @returns A list of all messages. | ||
| */ | ||
| getAllMessages(choiceIndex = 0): ChatMessages { |
There was a problem hiding this comment.
[req] Could you please add an additional test in orchestration-client.test.ts with the a sample response json file with a multiChatMessage response?
I wasn't able to understand the code without looking at a sample response from the Java repo, so I think this would be beneficial for us too.
| role: multiChatMessage.role, | ||
| content: multiChatMessage.content | ||
| .map(content => | ||
| content.type === 'text' | ||
| ? content.text | ||
| : JSON.stringify({ | ||
| url: content.image_url.url, | ||
| detail: content.image_url.detail | ||
| }) |
There was a problem hiding this comment.
[q] Are other content types possible? Is this documented somewhere that only text and image_url are allowed for now? I feel it is better to also check if the type is image_url before stringifying.
Context
Closes SAP/ai-sdk-js-backlog#228.
What this PR does and why it is needed