fix(chat): update response handling to stream and inline code block parsing VSCODE-620 #835
Conversation
|
|
||
| // This will stream all of the response content and create a string from it. | ||
| // It should only be used when the entire response is needed at one time. | ||
| async getChatResponseContent({ |
There was a problem hiding this comment.
Do we use this function anywhere? Can't find.
There was a problem hiding this comment.
It's used for the namespace prompt currently, and will also be used with the intent prompt.
| }, | ||
| onIdentifierStreamed: (content: string) => { | ||
| this._streamCodeBlockActions({ runnableContent: content, stream }); | ||
| }, |
There was a problem hiding this comment.
In which cases processStreamFragment and onIdentifierStreamed can differ from the current call? Maybe we don't need to pass them as an argument? Same with codeBlockIdentifier. Looks like it constant anyway.
There was a problem hiding this comment.
This gives it a bit of flexibility so that if we want to add additional handlers for other languages it's easy. It is a bit of an early generalization, I wasn't thinking it adds too much complexity though. I'm happy simplifying it if that's preferred. We are only using it in this way at the moment.
There was a problem hiding this comment.
I like that it's isolated and potentially reusable. Processing the content of the stream while still displaying it to the user seems like a common-enough scenario that justifies making it a bit more general-purpose.
| const contentToStream = contentSinceLastIdentifier.slice( | ||
| lastIndex, | ||
| endIndex | ||
| ); |
There was a problem hiding this comment.
Do you know if the code block can be mixed with the normal text withing one fragment? Or maybe the code block always takes the whole fragment.
There was a problem hiding this comment.
There are tests for both cases, code block all in one fragment, code block and identifiers split between fragments. I was seeing the fragments are typically 1-3 characters.
There was a problem hiding this comment.
After spending some time with streamParsing.ts, I am 98% convinced the algorithm is correct, though it seems a bit more complex than strictly necessary. I don't think we need to refactor it and the tests are solid, so we can revisit at some point in the future. Similarly with the participant chat functions - we now have a handful of those, so I'd like to move them to a separate wrapper class in the future.
| }, | ||
| onIdentifierStreamed: (content: string) => { | ||
| this._streamCodeBlockActions({ runnableContent: content, stream }); | ||
| }, |
There was a problem hiding this comment.
I like that it's isolated and potentially reusable. Processing the content of the stream while still displaying it to the user seems like a common-enough scenario that justifies making it a bit more general-purpose.
Co-authored-by: Nikola Irinchev <[email protected]>
Co-authored-by: Nikola Irinchev <[email protected]>
* VSCODE-528-mongodb-copilot: fix(chat): update response handling to stream and inline code block parsing VSCODE-620 (#835) feat(participant): route generic prompt by intent and update generic prompt VSCODE-572 (#830) # Conflicts: # src/participant/participant.ts # src/participant/prompts/query.ts
VSCODE-620
These changes make all of the model responses feel a faster. We're now streaming them as they come in instead of building one string out of the response and then streaming that.
Also adds support for multiple code blocks. We now inline the code actions as the code comes in. Hopefully it's written in a way we can change that out as we update to a different code block identifier in an upcoming ticket.