Chat: refactor Ollama chat client #3881
Conversation
| try { | ||
| const { done, value } = await reader.read() | ||
| if (typeof value === 'string') { | ||
| const parsedData = JSON.parse(value) as OllamaGenerateResponse |
There was a problem hiding this comment.
This could be fragile. We shouldn't assume that the network response is chunked on JSON boundaries, either. We should split on newlines, and then decode. The protocol is a bit like Server-Sent Events, "packets" are terminated by newlines.
There was a problem hiding this comment.
@dominiccooney hey Dom sorry i haven't got a chance to work on this since I''m still working on my sprint works, but I will try to pick this back up tomorrow. Will mark it as a draft until this is ready again, appreciate your feedback
abeatrix
changed the title
|
@dominiccooney i've applied your feedback to the PR:
|
| // Splits the decoded chunk by the new lines and filters out empty strings. | ||
| if (value) { | ||
| for (const chunk of value.split(RESPONSE_SEPARATOR).filter(Boolean)) { | ||
| const line = JSON.parse(chunk) as OllamaGenerateResponse |
There was a problem hiding this comment.
Similar to the completions client, we should buffer the chunks in case they are not cut on JSON boundaries.
What do you think about reusing the response streaming/reading part from the completions client to keep this logic in one place?
There was a problem hiding this comment.
@valerybugakov yea that'd be great! I will update this PR to reuse the one you created!
This PR fixes the issue of using PromptString as prompt text in the messages we send to the LLM, causing regression in chat response from some Ollama models.
Also updated the chat client for Ollama and Groq to have better error handling:
Log usage on complete for easier debugging purpose
Ollama:
Groq:
Test plan
Before
After