Skip to content

chore(deps): upgrade openai-go/v3 to v3.28.0, drop SasSwart fork#222

Draft
kylecarbs wants to merge 1 commit intomainfrom
cj/upgrade-openai-go
Draft

chore(deps): upgrade openai-go/v3 to v3.28.0, drop SasSwart fork#222
kylecarbs wants to merge 1 commit intomainfrom
cj/upgrade-openai-go

Conversation

@kylecarbs
Copy link
Member

@kylecarbs kylecarbs commented Mar 18, 2026

The SasSwart fork of openai-go/v3 adds performance optimizations (deferred body serialization, appendCompact skip) from openai/openai-go#602, which has not been merged upstream yet.

However, the fork's deferred body serialization causes stream: true to be dropped from request bodies, breaking streaming requests when consumed alongside newer dependencies (e.g. fantasy) that use openai-go/v3 instead of v2.

The Arguments field on ResponseOutputItemUnion changed from string (in the fork) to ResponseOutputItemUnionArguments (upstream union type). Updated the one callsite and test literals accordingly.

The SasSwart fork replace (from openai/openai-go#602) is no longer
needed — upstream v3.28.0 includes those changes.

The Arguments field on ResponseOutputItemUnion changed from string
to ResponseOutputItemUnionArguments (a union type). Updated the one
callsite and test literals accordingly.
@kylecarbs kylecarbs requested a review from dannykopping March 18, 2026 17:09
@kylecarbs kylecarbs marked this pull request as draft March 18, 2026 17:11
Copy link
Member Author

Closing — the SasSwart fork has unmerged perf changes (ShouldSerializeBody, appendCompact skip) that aren't in upstream v3.28.0 yet. PR #602 is still open. Will fix at the fantasy dep level instead.

@kylecarbs kylecarbs closed this Mar 18, 2026
@kylecarbs kylecarbs reopened this Mar 18, 2026
kylecarbs added a commit to coder/coder that referenced this pull request Mar 18, 2026
Updates the charm.land/fantasy replace to the rebased cj/go1.25
branch which includes:
- chore: downgrade to Go 1.25
- feat: anthropic computer use

Also removes the SasSwart openai-go/v3 replace directive and bumps
aibridge to a matching commit (coder/aibridge#222) that works with
upstream openai-go/v3 v3.28.0. The SasSwart fork's deferred body
serialization was causing stream:true to be dropped from requests
when used alongside the updated fantasy (which moved from
openai-go/v2 to v3).
Copy link
Member Author

Why drop the SasSwart fork

The SasSwart fork defers request body serialization until after request options are applied (to let WithRequestBody skip double-serialization). The problem is that WithJSONSet — which openai-go uses internally to inject stream: true into streaming requests — depends on cfg.Body already being a serialized *bytes.Buffer when options run.

Upstream flow (works):

  1. Body params serialized → cfg.Body = *bytes.Buffer{...}
  2. cfg.Apply(opts...) runs WithJSONSet("stream", true) → patches stream: true into the existing buffer
  3. Request sent with stream: true

Fork flow (broken):

  1. cfg.Body is nil
  2. cfg.Apply(opts...) runs WithJSONSet("stream", true)cfg.Body is nil, so it creates a new buffer containing only {"stream":true}
  3. Body serialization runs → overwrites cfg.Body with the full params, clobbering the stream: true that WithJSONSet just set
  4. Request sent without stream: true

This wasn't previously triggered because charm.land/fantasy used openai-go/v2 for its OpenAI provider, which didn't go through this code path. Now that fantasy has moved to openai-go/v3, all streaming responses requests are silently downgraded to non-streaming, causing handler returned streaming response for non-streaming request errors.

The fork's perf optimizations (appendCompact skip, ShouldSerializeBody) are from openai/openai-go#602 which is still open upstream.

Copy link
Member Author

Correction to my earlier comment

The fork isn't generically broken. The issue is specifically that NewStreaming (in responses/response.go) prepends WithJSONSet("stream", true) to the options but doesn't use WithRequestBody. In the fork's deferred-serialization flow:

  1. WithJSONSet("stream", true) runs with cfg.Body == nil → creates a buffer containing only {"stream":true}
  2. Body serialization runs afterward → overwrites cfg.Body with the full params, losing stream: true

Aibridge doesn't hit this because it uses WithRequestBody to pass a pre-serialized body (line 140 in chatcompletions/streaming.go), which both populates cfg.Body before WithJSONSet runs AND sets ShouldSerializeBody = false so the later serialization doesn't clobber it. The comment on line 136 even notes this: "the SDK's serialization applies hidden request options that result in unexpected, breaking behaviour."

So aibridge is safe with the fork because it already manually controls serialization. The issue only surfaces when code (like fantasy's openai provider) calls NewStreaming directly through the SDK's standard path.

@dannykopping dannykopping requested a review from SasSwart March 19, 2026 07:57
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.

1 participant