fix: preserve discriminated union variants

[jbeckwith-oai]

Status

Draft — blocked by #373. The raw BaseModel contract fix in #373 must land first. After it merges, this branch will be refreshed so #370 contains only the union/discriminator follow-up.

Summary

• preserve genuinely unknown discriminated-union values as raw hashes without recording a conversion error
• retain structural inference for missing and nil discriminators used by optional/nullable generated schemas
• resolve generated unkeyed enum variants without allowing them to claim genuinely unknown tags
• structurally rank variants that share the same discriminator instead of selecting by registration order
• preserve unknown streaming events and make downstream helpers tolerate future variants

Coverage

The dedicated discriminator contract suite exercises real Responses, Beta Responses, and Realtime schemas, including duplicate message tags, optional and nullable discriminators, unkeyed enum variants, nested unknown variants, symbol/string key and value combinations, and truly unknown future tags.

Validation

The current stacked head is green across Ruby 3.3, 3.4, and 4.0, plus RuboCop, Sorbet, RBS validation, gem packaging, required CI, and CodeQL.

Fixes #287

[openai-sdks]

OkTest Summary

✅ 237/237 SDK tests passed in 8.921s for Ruby SDK PR #370.

Test results — 42 files

Test	Result	Time
tests/chat-completions-complex-body.test.ts	✅ Passed	165ms
tests/chat-completions-create.test.ts	✅ Passed	254ms
tests/chat-completions-stream.test.ts	✅ Passed	119ms
tests/files-content-binary.test.ts	✅ Passed	228ms
tests/files-create-multipart.test.ts	✅ Passed	142ms
tests/files-list-pagination.test.ts	✅ Passed	170ms
tests/initialize-config.test.ts	✅ Passed	179ms
tests/instance-isolation.test.ts	✅ Passed	280ms
tests/models-list.test.ts	✅ Passed	159ms
tests/responses-background-lifecycle.test.ts	✅ Passed	180ms
tests/responses-body-method-errors.test.ts	✅ Passed	587ms
tests/responses-cancel-timeout.test.ts	✅ Passed	272ms
tests/responses-cancel.test.ts	✅ Passed	243ms
tests/responses-compact-retries.test.ts	✅ Passed	228ms
tests/responses-compact.test.ts	✅ Passed	204ms
tests/responses-create-advanced-stream.test.ts	✅ Passed	237ms
tests/responses-create-advanced.test.ts	✅ Passed	177ms
tests/responses-create-disconnect.test.ts	✅ Passed	157ms
tests/responses-create-errors.test.ts	✅ Passed	350ms
tests/responses-create-malformed-api-responses.test.ts	✅ Passed	126ms
tests/responses-create-retries.test.ts	✅ Passed	268ms
tests/responses-create-stream-failures.test.ts	✅ Passed	124ms
tests/responses-create-stream-timeout.test.ts	✅ Passed	227ms
tests/responses-create-stream-wire.test.ts	✅ Passed	1.922s
tests/responses-create-stream.test.ts	✅ Passed	115ms
tests/responses-create-terminal-states.test.ts	✅ Passed	326ms
tests/responses-create-timeout.test.ts	✅ Passed	262ms
tests/responses-create.test.ts	✅ Passed	279ms
tests/responses-delete.test.ts	✅ Passed	271ms
tests/responses-input-items-errors.test.ts	✅ Passed	179ms
tests/responses-input-items-list.test.ts	✅ Passed	187ms
tests/responses-input-items-options.test.ts	✅ Passed	309ms
tests/responses-input-tokens-count-timeout.test.ts	✅ Passed	310ms
tests/responses-input-tokens-count.test.ts	✅ Passed	216ms
tests/responses-malformed-inputs.test.ts	✅ Passed	2.079s
tests/responses-not-found-errors.test.ts	✅ Passed	462ms
tests/responses-parse.test.ts	✅ Passed	186ms
tests/responses-retrieve-retries.test.ts	✅ Passed	237ms
tests/responses-retrieve.test.ts	✅ Passed	237ms
tests/responses-stored-method-errors.test.ts	✅ Passed	727ms
tests/retry-behavior.test.ts	✅ Passed	1.998s
tests/sdk-error-shape.test.ts	✅ Passed	502ms

View OkTest run #31633237279

_{SDK merge (d606488399eb) · head (f3121428ec02) · base (d516874ebca3) · OkTest (a845fa206fa4)}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a03969729a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[apcha-oai]

The keepalive fix is needed, but filtering out every keyed variant after discriminator lookup fails is not safe for existing generated SDK models. Several real unions register keyed variants whose discriminator is intentionally optional or nullable.

Repro 1: valid Responses message without type

require "openai"

state = OpenAI::Internal::Type::Converter.new_coerce_state
value = OpenAI::Internal::Type::Converter.coerce(
  OpenAI::Responses::ResponseInputItem,
  {role: "user", content: "hello"},
  state: state
)

p value.class
p state[:error]

Current behavior: OpenAI::Models::Responses::EasyInputMessage, with no error.

With this change: the original Hash plus ArgumentError: no matching variant.

EasyInputMessage#type is optional, and the SDK's own README and examples/structured_outputs_responses.rb use message hashes without type.

The corresponding model accessor also becomes a real exception:

OpenAI::Responses::ResponseCreateParams.new(
  input: [{role: "user", content: "hello"}]
).input
# => OpenAI::Errors::ConversionError

Repro 2: nullable item-reference discriminator

state = OpenAI::Internal::Type::Converter.new_coerce_state
value = OpenAI::Internal::Type::Converter.coerce(
  OpenAI::Responses::ResponseInputItem,
  {id: "item_123", type: nil},
  state: state
)

p value.class
p state[:error]

Current behavior: OpenAI::Models::Responses::ResponseInputItem::ItemReference, with no error.

With this change: Hash plus ArgumentError: no matching variant. The generated ItemReference#type is explicitly optional and nullable.

Repro 3: existing Realtime unions also have optional discriminators

[
  [OpenAI::Realtime::RealtimeAudioFormats, {rate: 24_000}],
  [OpenAI::Realtime::RealtimeAudioFormats, {}],
  [OpenAI::Realtime::RealtimeToolsConfigUnion, {name: "weather"}]
].each do |union, input|
  state = OpenAI::Internal::Type::Converter.new_coerce_state
  value = OpenAI::Internal::Type::Converter.coerce(union, input, state: state)
  p [value.class, state[:error]]
end

These currently resolve to RealtimeAudioFormats::AudioPCM, RealtimeAudioFormats::AudioPCM, and RealtimeFunctionTool, respectively. With this change, all three return a hash with ArgumentError.

Repro 4: unknown nested variants break forward compatibility

OpenAI::Responses::Response.new(
  output: [{id: "item_future", type: "future_item"}]
).output
# => OpenAI::Errors::ConversionError

Returning the hash is not enough if the union also sets state[:error]: array/model accessors propagate that error. The new SSE test only covers a top-level union, where the error state is ignored.

Please preserve structural fallback when the discriminator is missing or nullable for variants that allow that, and preserve genuinely unknown discriminator values without recording a conversion error. Alternatively, scope the unknown-event handling to SSE parsing. Regression coverage should use the actual generated Responses and Realtime unions, including an unknown variant nested in Response#output.

[jbeckwith-oai]

@apcha-oai Addressed the requested changes in 97d28491:

• Missing and nil discriminator values again use structural inference, preserving existing optional/nullable generated models.
• Present-but-unknown values skip keyed variants and fall through successfully via OpenAI::Internal::Type::Unknown, without recording a conversion error.
• Added regressions for ResponseInputItem easy messages and nullable item references, RealtimeAudioFormats, RealtimeToolsConfigUnion, ResponseCreateParams#input, and a future item nested in Response#output.
• Retained the original SSE keepalive regression and explicit unkeyed fallback behavior.

Validation is clean on Ruby 3.3.12, 3.4.10, and 4.0.6 (628 tests / 2,381 assertions each), plus Sorbet, RBS validation, RuboCop, and gem packaging.

[HAYDEN-OAI]

I found one forward-compatibility regression in the new unknown-discriminator fallback; leaving an inline comment only.

[HAYDEN-OAI]

Re-reviewed the current head; the follow-up fixes the prior fallback regression and I found no remaining substantive issues.

[jbeckwith-oai]

Pushed the forward fix in d996666f (fast-forward; no revert or force-push).

• Restored the documented raw-value contract: BaseModel#[], #to_h, and #deep_to_h retain the caller/response values, including nested hashes and discriminator strings. Typed accessors now use a separate converted-value cache, including YAML round-trips. This restores patterns such as params.to_h.fetch(:input).fetch(0).fetch(:type).
• Made discriminated unions schema-aware: unique keyed variants still take the fast path; duplicate keyed variants are structurally compared; missing or nil discriminators retain structural inference; known unkeyed enum discriminators resolve to their intended model; genuinely unknown discriminator values pass through raw without a conversion error.
• Kept streaming forward-compatible with raw unknown events and preserved private streaming snapshot mutations without changing the public raw-value contract.
• Added regressions against the actual Responses, Beta, and Realtime generated unions, nested unknown response items, SSE keepalive events, raw nested params, streaming structured tool parsing, and YAML serialization.

Validation: 635 tests / 2,427 assertions on Ruby 4.0 after the merge; the pre-merge code-equivalent suite also passed on Ruby 3.3 and 3.4. RuboCop (2,611 files), Sorbet, RBS validation (1,212 files), and gem packaging are clean.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d996666fd0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[jbeckwith-oai]

Addressed the coercion-contract feedback with dedicated regression coverage and a forward fix.

Raw model contract:

• Added a focused BaseModel raw-value contract suite covering constructor, assignment, Converter.coerce, arrays, maps, unions, deep_to_h, YAML, failed conversions, and already-typed models.
• Codified the exact nested Ruby access pattern: params.to_h.fetch(:input).fetch(0).fetch(:type), for both Responses and Beta Responses.
• Verifies that #[] and #to_h retain caller/API values while typed accessors materialize models.

Union/discriminator contract:

• Added real-schema coverage for Responses, Beta Responses, and Realtime, including duplicate message discriminators, missing and nil discriminators, unkeyed enum variants, unknown variants, nested unknown variants, and symbol/string key/value combinations.
• The duplicate-discriminator tests exposed a real ordering bug: broad EasyInputMessage variants captured more specific input/output message shapes. Duplicate keyed candidates are now structurally ranked, while unique and fallback paths keep their existing behavior.
• Added streaming coverage proving unknown events stay raw and still work with starting_after filtering.

Validation is green on Ruby 3.3.12, 3.4.10, and 4.0.6 (650 tests / 2,493 assertions each), plus RuboCop, Sorbet, RBS validation, and gem build.

[jbeckwith-oai]

Moved this PR back to draft. #373 is the prerequisite raw BaseModel contract fix and must merge first; after that, I’ll refresh #370 so its review surface is limited to union/discriminator behavior.

[jbeckwith-oai]

Generator compatibility audit complete for this draft as well: no Castiron PR is required.

• The union implementation, its RBI/RBS signatures, streaming helpers, and contract tests are repository-owned runtime/test paths excluded from generated ownership.
• A full current Ruby generation plus bootstrap overlay left union.rb, union.rbi, union.rbs, and discriminated_union_contract_test.rb byte-for-byte unchanged.
• The generated Response#output_text safety change was retained byte-for-byte as an SDK-side overlay (keep_sdk_side_for_bootstrap) with no relevant merge conflict.

After #373 lands, this branch still needs the planned refresh so its review surface is limited to discriminator behavior, but it does not need a companion generator change.

[apcha-oai]

Thanks for iterating on this. I’ve spent a considerable amount of time investigating this change with Codex, and I honestly can’t vouch for its correctness across the full range of existing schemas and discriminator configurations.
Union coercion and discriminator handling have been a known pain point across the SDKs for a long time. There isn’t really a universally correct way to decode an unknown discriminator, and getting the behavior right in the general case is genuinely difficult.*

Given that history, I don’t think this particular streaming bug warrants rewriting global union coercion at this time. Can we instead special-case keepalive, handle unknown events in the streaming decoder, or scope the fallback behavior specifically to the streaming event union?

(*) For example, given {type: "unknown", value: "hello"}, should we reject it, preserve it, infer a structurally compatible variant, or use an explicit fallback? The answer depends on whether this is a request or response, whether the union is open or closed, whether parsing is strict, and how missing, nullable, or invalid discriminators are expected to behave. Our different SDKs make different decision points about this as well. The input space is combinatorially large and difficult to validate.

These also (unfortunately) are not very theoretical. E.g. our core responses / input items rely on this behavior.

[jbeckwith-oai]

Closing this draft as part of the decision to roll back #295 and re-evaluate the model system from first principles. The global union/discriminator rewrite here is too broad for schemas with optional discriminators and multiple variants sharing a discriminator value.

#373 now performs the literal rollback. #287 remains open and should receive a narrower, streaming-scoped fix after the underlying design is specified and characterized. No Castiron PR is being closed: the generated Ruby surface is unchanged, and this runtime is intentionally SDK-owned.