Skip to content

Fix loading after ActiveSupport 6 subclass extensions - #336

Open
jbeckwith-oai wants to merge 1 commit into
mainfrom
codex/fix-active-support-subclasses
Open

Fix loading after ActiveSupport 6 subclass extensions#336
jbeckwith-oai wants to merge 1 commit into
mainfrom
codex/fix-active-support-subclasses

Conversation

@jbeckwith-oai

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

Copy link
Copy Markdown
Contributor

Summary

  • avoid Class#subclasses while registering generated model OrHash aliases because ActiveSupport 5 and 6 implement it through ==
  • enumerate only direct BaseModel subclasses and compare their superclass by object identity
  • add an isolated load-order regression test that reproduces ActiveSupport 6's implementation

Fixes #335.

Why this is narrow

This does not monkeypatch ActiveSupport or Ruby, and it does not change BaseModel.==. The identity-safe enumeration is local to generated model initialization.

Test plan

  • ruby -Itest over the load-order, BaseModel, Sorbet runtime, and structured-output tests (25 runs, 326 assertions)
  • load after real ActiveSupport 5.2.8.1, 6.0.6.1, 6.1.7.10, and 7.0.8.7 gems
  • RuboCop on the changed Ruby files

Companion generator change: openai/openai#1237327.

@jbeckwith-oai
jbeckwith-oai requested a review from a team as a code owner August 4, 2026 20:33
Copilot AI review requested due to automatic review settings August 4, 2026 20:33
@openai-sdks

openai-sdks Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

OkTest Summary

237/237 SDK tests passed in 9.718s for Ruby SDK PR #336.

Test results — 42 files
Test Result Time
tests/chat-completions-complex-body.test.ts ✅ Passed 210ms
tests/chat-completions-create.test.ts ✅ Passed 248ms
tests/chat-completions-stream.test.ts ✅ Passed 135ms
tests/files-content-binary.test.ts ✅ Passed 298ms
tests/files-create-multipart.test.ts ✅ Passed 253ms
tests/files-list-pagination.test.ts ✅ Passed 189ms
tests/initialize-config.test.ts ✅ Passed 173ms
tests/instance-isolation.test.ts ✅ Passed 254ms
tests/models-list.test.ts ✅ Passed 262ms
tests/responses-background-lifecycle.test.ts ✅ Passed 218ms
tests/responses-body-method-errors.test.ts ✅ Passed 490ms
tests/responses-cancel-timeout.test.ts ✅ Passed 272ms
tests/responses-cancel.test.ts ✅ Passed 306ms
tests/responses-compact-retries.test.ts ✅ Passed 290ms
tests/responses-compact.test.ts ✅ Passed 281ms
tests/responses-create-advanced-stream.test.ts ✅ Passed 219ms
tests/responses-create-advanced.test.ts ✅ Passed 189ms
tests/responses-create-disconnect.test.ts ✅ Passed 175ms
tests/responses-create-errors.test.ts ✅ Passed 403ms
tests/responses-create-malformed-api-responses.test.ts ✅ Passed 161ms
tests/responses-create-retries.test.ts ✅ Passed 300ms
tests/responses-create-stream-failures.test.ts ✅ Passed 148ms
tests/responses-create-stream-timeout.test.ts ✅ Passed 232ms
tests/responses-create-stream-wire.test.ts ✅ Passed 2.256s
tests/responses-create-stream.test.ts ✅ Passed 146ms
tests/responses-create-terminal-states.test.ts ✅ Passed 303ms
tests/responses-create-timeout.test.ts ✅ Passed 238ms
tests/responses-create.test.ts ✅ Passed 300ms
tests/responses-delete.test.ts ✅ Passed 237ms
tests/responses-input-items-errors.test.ts ✅ Passed 189ms
tests/responses-input-items-list.test.ts ✅ Passed 207ms
tests/responses-input-items-options.test.ts ✅ Passed 327ms
tests/responses-input-tokens-count-timeout.test.ts ✅ Passed 296ms
tests/responses-input-tokens-count.test.ts ✅ Passed 225ms
tests/responses-malformed-inputs.test.ts ✅ Passed 2.431s
tests/responses-not-found-errors.test.ts ✅ Passed 371ms
tests/responses-parse.test.ts ✅ Passed 211ms
tests/responses-retrieve-retries.test.ts ✅ Passed 323ms
tests/responses-retrieve.test.ts ✅ Passed 266ms
tests/responses-stored-method-errors.test.ts ✅ Passed 817ms
tests/retry-behavior.test.ts ✅ Passed 1.989s
tests/sdk-error-shape.test.ts ✅ Passed 513ms

View OkTest run #30948396049

SDK merge (f7515ce163ab) · head (5f5a53305938) · base (e3cbb6833713) · OkTest (91635c6a2723)

Copilot AI 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.

Pull request overview

This PR addresses a load-order failure when ActiveSupport 5/6 subclass/descendant extensions are loaded before openai, by avoiding code paths that invoke OpenAI::Internal::Type::BaseModel.== during model alias initialization.

Changes:

  • Reworks generated model OrHash alias registration to avoid Class#subclasses and use superclass identity checks.
  • Adds an isolated regression test that simulates ActiveSupport 6’s Class#descendants/#subclasses behavior and asserts require "openai" succeeds.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
test/openai/load_order_test.rb Adds a subprocess-based regression test covering the ActiveSupport 6-style subclass extensions load-order scenario.
lib/openai/models.rb Changes how BaseModel subclasses are enumerated for OrHash alias registration to avoid triggering structural equality during load.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/openai/models.rb
Comment on lines +7 to +10
subclasses =
ObjectSpace.each_object(base_model.singleton_class).select do |candidate|
!candidate.singleton_class? && candidate.superclass.equal?(base_model)
end
@jbeckwith-oai jbeckwith-oai added the generator Touches generated SDK files label Aug 4, 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 exact commit 5f5a533 and the complete implementation and load-order regression case. On MRI Ruby 3.3.12, verified SDK loading both without ActiveSupport and with real ActiveSupport 5.2.8.1, 6.0.6.1, 6.1.7.10, 7.0.8.7, 7.1.5, and 8.0.3. All 2,266 generated model classes receive OrHash aliases, and public structural class equality remains unchanged. Ruby singleton-class inheritance makes ObjectSpace.each_object(base_model.singleton_class) correctly enumerate descendant classes; the existing automated concern is a false positive. No substantive issues found.

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.

Library does not load when ActiveSupport 6 is included

3 participants