-
Notifications
You must be signed in to change notification settings - Fork 2.6k
fix: remove stream_options from xAI handler to fix Grok-4 API error #6703
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
- Remove stream_options parameter from xAI streaming requests as the xAI API does not support it - Add comment explaining why stream_options is not included - Update tests to not expect stream_options in xAI API calls - Add specific test case for Grok-4 streaming Fixes #6702
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.
Reviewing my own code is like debugging in production - technically possible but morally questionable.
| messages: [{ role: "system", content: systemPrompt }, ...convertToOpenAiMessages(messages)], | ||
| stream: true, | ||
| stream_options: { include_usage: true }, | ||
| // xAI API doesn't support stream_options parameter |
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.
I notice the OpenAI provider handles Grok models by checking and conditionally excluding . Since Grok can be used through both the xAI handler and OpenAI-compatible endpoints, would it be helpful to add a comment explaining why both providers need this separate handling?
| } | ||
| } | ||
|
|
||
| // Note: xAI API doesn't support stream_options, so usage data might not be available in streaming responses |
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.
The comment mentions "usage data might not be available in streaming responses" - should we add a test case that verifies the handler still works correctly when usage data is not provided by the xAI API?
| messages: expect.arrayContaining([{ role: "system", content: systemPrompt }]), | ||
| stream: true, | ||
| stream_options: { include_usage: true }, | ||
| // xAI API doesn't support stream_options |
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 comment appears both here and on line 279. Could we make this test comment more specific about what aspect of stream_options we're testing?
|
Looks like a problem with the model, closing |
Summary
This PR fixes the Grok-4 API error reported in #6702. The issue was caused by the xAI handler including the
stream_optionsparameter in streaming requests, which is not supported by the xAI API.Changes
stream_options: { include_usage: true }from the xAI handler's streaming requeststream_optionsis not includedstream_optionsin xAI API callsTesting
Related Issue
Fixes #6702
Important
Remove
stream_optionsfromXAIHandlerfor Grok-4 model to fix API error and update tests accordingly.stream_optionsfromXAIHandlerinxai.tsfor Grok-4 model to fix API error.stream_options.xai.spec.tsto remove expectations forstream_optionsin xAI API calls.stream_optionsis not included.This description was created by
for 9eb5925. You can customize this summary. It will automatically update as commits are pushed.