-
Notifications
You must be signed in to change notification settings - Fork 2.6k
fix: handle empty responses from Docker Model Runner and similar APIs #8228
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Add fallback to yield empty text chunk when no content is received - Prevents "The language model did not provide any assistant messages" error - Fixes issue with Docker Model Runner integration - Add tests for empty response handling Fixes #8226
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed my own code. Found it suspiciously adequate. 3/10 for originality.
| type: "text", | ||
| text: "", | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider applying this same empty response handling to other providers that might face similar issues (like ollama.ts, native-ollama.ts). This would ensure consistent behavior across all OpenAI-compatible providers.
|
|
||
| for (const processedChunk of matcher.final()) { | ||
| if (processedChunk.text) { | ||
| hasContent = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This check for processedChunk.text seems redundant since text chunks would have already set hasContent = true at line 117 when delta?.content exists. Consider removing this redundant check.
| }) | ||
| }) | ||
| }) | ||
| }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding a test case for multiple empty chunks before usage information to ensure the empty text chunk is only added once, not multiple times.
| // This can happen with some Docker Model Runner configurations | ||
| if (!hasContent) { | ||
| yield { | ||
| type: "text", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding a debug log here like console.debug('[BaseOpenAiCompatibleProvider] No content received, yielding empty text chunk') to help with troubleshooting in production.
Description
This PR fixes an issue where Docker Model Runner and similar OpenAI-compatible APIs that return empty responses cause RooCode to display the error: "The language model did not provide any assistant messages".
Problem
When using Docker Model Runner as an inference engine, some model configurations may return streaming responses without any content chunks, only usage information. This causes the Task.ts error handler to trigger since no assistant message content was received.
Solution
Changes
Testing
Related Issue
Fixes #8226
Feedback
This PR attempts to address Issue #8226. The implementation ensures backward compatibility while handling the edge case of empty API responses from Docker Model Runner and similar services. Feedback and guidance are welcome!
Important
Fixes handling of empty responses from Docker Model Runner by yielding empty text chunks to prevent errors, with tests added for various scenarios.
base-openai-compatible-provider.tsandlm-studio.ts, added logic to yield an empty text chunk if no content is received during streaming to prevent errors.base-openai-compatible.spec.tsfor handling empty responses, normal responses, and responses with only usage information.This description was created by
for f487380. You can customize this summary. It will automatically update as commits are pushed.