Skip to content

Conversation

@ZhiRuYan
Copy link

Summary

Fix OpenAI provider to only apply the reasoningEffort parameter to models that support it (o1 and o3 series). Previously, this parameter was applied to all models when OPENAI_REASONING_EFFORT was set, which could cause unnecessary overhead or potential errors with non-o1/o3 models like gpt-5.

Changes

  • Added supportsReasoningEffort() helper function to check if a model supports the reasoning effort parameter
  • Modified logic to only apply reasoning effort to o1/o3 models
  • Updated default behavior: reasoning effort defaults to "low" only for o1/o3 models, not for all models
  • Improved code comments to clarify the behavior
  • Fixed variable reuse by introducing selectedModel variable for better code clarity

Motivation

The reasoning effort parameter is specific to OpenAI's o1 and o3 model series. Applying it to other models (like gpt-5, gpt-4, etc.) is unnecessary and could potentially cause issues.

Testing

  • ✅ All existing tests pass
  • ✅ TypeScript compilation successful
  • ✅ Linting passes
  • ✅ Code follows repository patterns

The reasoning effort parameter should only be applied to models that support
it (o1 and o3 series). Previously, the parameter was applied to all models
when OPENAI_REASONING_EFFORT was set, causing unnecessary overhead or
potential errors with non-o1/o3 models like gpt-5.

Changes:
- Added supportsReasoningEffort() helper to check if model supports reasoning effort
- Only apply reasoning effort parameter to o1/o3 models
- Updated logic to use default reasoning effort only for o1/o3 models
- Improved comments to clarify behavior
- Fixed variable reuse by introducing selectedModel variable

This ensures compatibility with all OpenAI models while maintaining the
reasoning effort feature for o1/o3 models.
@dcramer
Copy link
Member

dcramer commented Nov 15, 2025

This will break with gpt-5 which supports reasoning (and is the main reason we added this fwiw)

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.

2 participants