feat: Implement assistant API streaming #272
Conversation
|
Please review @mme / @ataibarkai |
|
@Bhavyajain21 the merge checks are failing- To fix those, please follow the testing guide in the contributing quickstart. Thanks! |
| forwardMessages.length > 0 && | ||
| forwardMessages[forwardMessages.length - 1].role === "user" | ||
| ) { | ||
| const run = await this.openai.beta.threads.runs.createAndStream(threadId, { |
There was a problem hiding this comment.
you want to make the createAndStream change inside the existing submitUserMessage function.
it encapsulates additional context injections and business logic
| forwardMessages.length > 0 && | ||
| forwardMessages[forwardMessages.length - 1].role === "user" | ||
| ) { | ||
| run = await this.submitUserMessage(threadId, forwardedProps); |
There was a problem hiding this comment.
per the other comment: we want to keep this code here.
The changes should be made inside the submitUserMessage function definition
| @@ -177,6 +184,77 @@ export class OpenAIAssistantAdapter implements CopilotKitServiceAdapter { | |||
| }; | |||
| } | |||
| } | |||
|
|
|||
| async buildInstructions(forwardMessages: Message[]): Promise<string> { | |||
There was a problem hiding this comment.
the existing submitUserMessage function already performs the instruction-building and tool-building steps clearly enough, there is no need for defining these as separate functions
| } | ||
|
|
||
| async handleStreamingRun(run: OpenAI.Beta.Threads.Runs.RunStream) { | ||
| return new Promise((resolve) => { |
There was a problem hiding this comment.
this implementation is on the right direction, but I believe we want to use our existing abstractions here,
see writeChatCompletionChunk, writeChatCompletionEnd and AssistantSingleChunkReadableStream for an example.
@mme can you chime in here please?
|
@Bhavyajain21 just wanted to make sure you saw this |
|
Hey @ataibarkai, will work on the reviews today! |
/claim #266
/fixes #266