Skip to content

fix: preserve the stream property for chat/completions calls#164

Merged
SasSwart merged 3 commits intomainfrom
jjs/fix-streaming
Feb 5, 2026
Merged

fix: preserve the stream property for chat/completions calls#164
SasSwart merged 3 commits intomainfrom
jjs/fix-streaming

Conversation

@SasSwart
Copy link
Contributor

@SasSwart SasSwart commented Feb 4, 2026

In the openai-go SDK, a conflict between a recent optimization and the way in which options are set resulted in an issue whereby request bodies for chat/completions calls were being discarded.

This PR fixes the issue by taking full control of the request body handling. We now use the WithRequestBody and WithJSONSet options ourselves to ensure that the request payload is sent exactly as we need it.

Integration tests have been added to guard against future regression.

Related to internal Slack thread.

@SasSwart SasSwart requested a review from ssncferreira February 5, 2026 08:24
Copy link
Contributor

@ssncferreira ssncferreira left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM :shipit:
Can you add a simple description to the PR and link it to the slack thread for future reference?

Comment on lines +2193 to +2204
var reqBody map[string]any
if err := json.Unmarshal(body, &reqBody); err != nil {
return fmt.Errorf("request should be valid JSON: %w", err)
}

// Verify required fields for OpenAI responses
// Note: Using map here since there's no specific SDK type for responses endpoint
model, ok := reqBody["model"]
if !ok || model == "" {
return fmt.Errorf("model field is required but missing or empty")
}
return nil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can actually use responses.ResponseNewParams:

Suggested change
var reqBody map[string]any
if err := json.Unmarshal(body, &reqBody); err != nil {
return fmt.Errorf("request should be valid JSON: %w", err)
}
// Verify required fields for OpenAI responses
// Note: Using map here since there's no specific SDK type for responses endpoint
model, ok := reqBody["model"]
if !ok || model == "" {
return fmt.Errorf("model field is required but missing or empty")
}
return nil
var req responses.ResponseNewParams
if err := json.Unmarshal(body, &req); err != nil {
return fmt.Errorf("request should unmarshal into ResponseNewParams: %w", err)
}
// Collect all validation errors
var errs []string
if req.Model == "" {
errs = append(errs, "model field is required but empty")
}
if len(errs) > 0 {
return fmt.Errorf("validation failed: %s", strings.Join(errs, "; "))
}
return nil

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can add this as a follow-up 👍

@SasSwart SasSwart merged commit aed7dd9 into main Feb 5, 2026
4 checks passed
SasSwart added a commit to coder/coder that referenced this pull request Feb 5, 2026
SasSwart added a commit to coder/coder that referenced this pull request Feb 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants