Skip to content

fix: align structured output readers with Sorbet types - #385

Open
jbeckwith-oai wants to merge 4 commits into
mainfrom
codex/typed-structured-output-readers
Open

fix: align structured output readers with Sorbet types#385
jbeckwith-oai wants to merge 4 commits into
mainfrom
codex/typed-structured-output-readers

Conversation

@jbeckwith-oai

@jbeckwith-oai jbeckwith-oai commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Summary

Fix the contract mismatch between the Tapioca structured-output compiler introduced in #364 and the raw-value compatibility rollback in #375.

  • Materialize typed readers only for application-defined OpenAI::BaseModel structured-output classes.
  • Keep caller-owned hashes and arrays untouched in [] and to_h, including on directly constructed and reassigned structured-output models.
  • Preserve already-materialized parsed-response values, including nullable array identity, and retain precise nested model, array, union, enum, boolean, and nullable Sorbet signatures.
  • Reject missing/non-nullable nil, non-viable boolean, enum, and mutated-container values with OpenAI::Errors::ConversionError instead of violating generated reader types.
  • Add a cross-boundary contract matrix that checks generated RBIs against actual construction, assignment, and parsed-response behavior.

Why existing tests did not catch this

  1. fix: coerce nested BaseModel fields #295 had already enabled global nested coercion when feat: add Tapioca typing for structured outputs #364 introduced the Tapioca compiler, so concrete nested signatures matched the then-current behavior.
  2. The compiler regression generated RBI text and ran Sorbet against a synthetic parsed value, but never instantiated the models or compared runtime reader values to those signatures.
  3. revert: nested BaseModel coercion (#295) #375 reverted global coercion and added strong raw-value compatibility tests for generated/internal request models and parsed API models, but did not exercise application-defined structured-output models or the Tapioca compiler contract.
  4. Existing structured-output streaming tests used scalar-only models, so neither nested readers nor nested array elements exposed the mismatch.

The new compiler test closes the gap by checking every advertised field shape against runtime readers for direct construction, explicit assignment, and parsed responses in the same subprocess that generates and typechecks the RBI. It additionally checks raw identity through both accessors, nullable array elements and parsed array identity, enum conversion, both union branches, nested model inheritance, caller-owned mutations, and conversion errors for invalid booleans, enums, and nested container values.

Design tradeoff

Globally restoring nested coercion would regress the published generated-request model contract. Widening generated readers to model/hash unions would make signatures truthful but would also require narrowing every nested field in normal parsed-response code. Documenting the signatures as parsed-only would leave direct construction and assignment unsound.

Instead, the structured-output subclass prepends one small reader module per application-defined model. Each wrapper delegates to the original generated reader for existing validation, returns already-materialized values unchanged, and uses the existing canonical converter only when the raw value does not already match its declared target. Raw storage remains unchanged, and generated SDK request models never enter this path.

Setters are intentionally not added to generated RBIs: the documented compiler contract promises readers.

Castiron ownership

No upstream Castiron compiler or renderer change is required.

The SDK contributor guide explicitly says the generator never modifies lib/openai/helpers/. Castiron's authoritative generated-file-excludes.yaml independently excludes README.md globally, lib/openai/helpers/ for Ruby, and the handwritten test/ tree except generated resource tests and namespace scaffolding. I evaluated that policy against every changed PR path:

EXCLUDED README.md
EXCLUDED lib/openai/helpers/structured_output/base_model.rb
EXCLUDED lib/openai/helpers/structured_output/array_of.rb
EXCLUDED test/openai/helpers/structured_output_test.rb
EXCLUDED test/openai/tapioca/base_model_compiler_test.rb

Castiron's Ruby renderer generates package/client/model/resource/signature files and resource tests; it does not emit the handwritten structured-output helper or these tests. The existing Tapioca compiler file is unchanged.

Verification

  • mise exec ruby@4.0.6 -- ./scripts/test — 639 runs, 2,608 assertions.
  • mise exec ruby@3.4.10 -- ./scripts/test — 639 runs, 2,608 assertions.
  • mise exec ruby@3.3.12 -- ./scripts/test — 639 runs, 2,608 assertions.
  • mise exec ruby@4.0.6 -- bundle exec rake lint — 2,612 RuboCop files clean, Sorbet clean, and 1,212 RBS files valid.
  • Expanded Tapioca compiler/runtime contract regression passes on Ruby 3.3, 3.4, and 4.0.
  • Existing raw-value compatibility suite remains green.
  • mise exec ruby@4.0.6 -- bundle exec rake build:gem; package contains the handwritten structured-output runtime and compiler with no sorbet-runtime runtime dependency.
  • Required thermo-nuclear code-quality review repeated with no findings.

@jbeckwith-oai
jbeckwith-oai requested a review from a team as a code owner August 13, 2026 17:38
@openai-sdks

openai-sdks Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

OkTest Summary

🟡 Running for Ruby SDK PR #385.

View OkTest run #31732928350

SDK merge (pending) · head (ff1511fcda04) · base (8ca842693f44) · OkTest (a845fa206fa4)

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1290cf5e9d

ℹ️ 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".

Comment thread lib/openai/helpers/structured_output/base_model.rb
@jbeckwith-oai jbeckwith-oai added the generator Touches generated SDK files label Aug 13, 2026

@HAYDEN-OAI HAYDEN-OAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed e01f430b4572b8eeebf5d64ac3625dd5f0a0b9c0. I found two remaining reader-contract gaps in the new wrapper; details are inline. I also agree with the existing unresolved @api private documentation note.

Comment thread lib/openai/helpers/structured_output/base_model.rb Outdated
Comment thread lib/openai/helpers/structured_output/base_model.rb Outdated
@jbeckwith-oai
jbeckwith-oai force-pushed the codex/typed-structured-output-readers branch from f658366 to 2bebdfd Compare August 13, 2026 18:46

@HAYDEN-OAI HAYDEN-OAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Re-reviewed the new head 2bebdfda62cdece88e476bb831f1028bd402b264. The previous conversion-viability, nullable-array identity, and private-API findings are addressed. One remaining non-nil reader-contract gap is inline.

Comment thread lib/openai/helpers/structured_output/base_model.rb Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2bebdfda62

ℹ️ 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".


case converted
when target
return converted if state.fetch(:error).nil? && state.fetch(:exactness).fetch(:no).zero?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Clear stale union errors before rejecting valid conversions

When a union contains an incompatible alternative before a valid zero-field model (for example, OpenAI::UnionOf[String, EmptyModel] with {}), Union#coerce leaves the first alternative's state[:error] in place even though the model conversion succeeds exactly. This check therefore raises ConversionError for a value that converted to the generated union type; clear/reset stale branch errors or determine success from the selected conversion rather than the shared error slot.

Useful? React with 👍 / 👎.

value
else
state = OpenAI::Internal::Type::Converter.new_coerce_state(translate_names: false)
converted = OpenAI::Internal::Type::Converter.coerce(target, value, state: state)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Convert JSON strings for supported Symbol fields

When a structured-output field is declared as Symbol, its generated JSON schema is a string, so every value returned by the API reaches this reader as a String. The canonical converter has no String-to-Symbol conversion for the Symbol class, causing this new wrapper to raise ConversionError for every valid response instead of satisfying the generated Symbol reader type; explicitly convert these strings or stop advertising Symbol as a supported structured-output field type.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

generator Touches generated SDK files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants