feat: Add utility functionality to get the entire stream response #852
Conversation
tomfrenken
force-pushed
the
process-module-results
branch
from
8e6ce0c to
e3fe4c1
Compare
tomfrenken
requested review from
ZhongpinWang,
deekshas8 and
KavithaSiva
and removed request for
ZhongpinWang
tomfrenken
changed the title
There was a problem hiding this comment.
Perhaps I am lost with what we want to achieve with this PR. I thought we do stream lock. If _openStream is not meant for stream lock and this will happen in the other PR, I would remove try catch and add it when we implement stream lock and then you can ignore the relevant comments. The stream response level check for _openStream when calling utilities is indeed helpful.
| ); | ||
| }); | ||
|
|
||
| it('should handle aborted requests gracefully', async () => { |
There was a problem hiding this comment.
[q] I don't understand what you mean by gracefully? Don't we throw error? It doesn't feel graceful? I am a bit confused with the purpose of this test. Which part of the code would you like to test with this?
| sourceModule: string, | ||
| sourceChoice: number | undefined | ||
| ): void { | ||
| if (!message.role) { |
There was a problem hiding this comment.
[pp] I don't mind having validations. But some of them are testing if a mandatory field is really there. This feels unnecessary since if we implement everything correctly, user should never see these warnings, at least not without seeing some error first (like stream ended unexpectedly). Other places like choice.index, choice.message are IMO also unnecessary. Can you walk through those cases and remove them if you also feel the same?
Context
Part of SAP/ai-sdk-js-backlog#315.
What this PR does and why it is needed
With this PR we concatenate and validate the entire stream response, thus making it available like a regular completion response, once the stream ends.
This way, we can make the same utility functionality available for stream responses as we do already for "normal" completion responses.
Additionally, our validation layer is now always applied at the stream's end.
This functionality can then be used for our upcoming message history feature.