-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Add a quick opt-in option to switch to gpt-5 model #1534
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
- Introduce OPENAI_DEFAULT_MODEL env variable - When the default model is any of gpt-5 ones, this SDK automatically sets relevant settings - Change the default one from gpt-4o to gpt-4.1 (requires minor version bump) - Improve some of the existing examples
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comments for reviewers
@@ -12,6 +12,7 @@ | |||
|
|||
search_agent = Agent( | |||
name="FinancialSearchAgent", | |||
model="gpt-4.1", | |||
instructions=INSTRUCTIONS, | |||
tools=[WebSearchTool()], | |||
model_settings=ModelSettings(tool_choice="required"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this option is not compatible with gpt-5, so explicitly set gpt-4.1 for this example
res = await Runner.run(agent, "Which language is this repo written in?") | ||
res = await Runner.run( | ||
agent, | ||
"Which language is this repo written in? Your MCP server should know what the repo is.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gpt-5 models could be confused with this MCP server's rule (= repo information is available in the MCP server URL), so I updated the instructions to be clearer
async for event in model.stream_response( | ||
system_instructions="You are a helpful assistant that writes creative content.", | ||
input="Write a haiku about recursion in programming", | ||
model_settings=ModelSettings(), | ||
model_settings=ModelSettings(reasoning=Reasoning(effort="medium", summary="detailed")), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without these settings, reasoning summary items won't show up with OpenAI models; so I've updated this example.
reasoning_content += event.delta | ||
elif event.type == "response.output_text.delta": | ||
print(f"\033[32m{event.delta}\033[0m", end="", flush=True) # Green for regular content | ||
if not output_text_already_started: | ||
print("\n") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Improved the readability of the output
# Extract reasoning content from the result items | ||
reasoning_content = None | ||
# RunResult has 'response' attribute which has 'output' attribute |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does not work with OpenAI models, so I revised this example
print("\nCollected Final Answer:") | ||
print(content_buffer) | ||
output_text_already_started = False | ||
async for event in stream.stream_events(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above
|
||
from agents import Agent, FileSearchTool, Runner, trace | ||
|
||
|
||
async def main(): | ||
vector_store_id: str | None = None | ||
|
||
if vector_store_id is None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Enabled running this example without any preparation
@@ -168,10 +173,10 @@ class Agent(AgentBase, Generic[TContext]): | |||
"""The model implementation to use when invoking the LLM. | |||
|
|||
By default, if not set, the agent will use the default model configured in | |||
`openai_provider.DEFAULT_MODEL` (currently "gpt-4o"). | |||
`agents.models.get_default_model()` (currently "gpt-4.1"). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can safely change the default one from gpt-4o to gpt-4.1 in the next minor version (0.3.0); TS SDK has been using gpt-4.1 and there is no performance issue with it, plus I believe gpt-4.1 works even better for agentic workflows.
@@ -0,0 +1,57 @@ | |||
import copy |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file is the essential change in this PR. I've added as many comments as possible, but if there are still unclear points, I am happy to add further.
There was a problem hiding this comment.
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 suggestions.
Reply with @codex fix comments
to fix any unresolved comments.
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 that is ready for review, or mark a draft as ready for review. You can also ask for a review by commenting "@codex review".
Codex can also answer questions or update the PR. Try commenting "@codex fix this CI failure" or "@codex address that feedback".
src/agents/agent.py
Outdated
if ( | ||
# The user sets a non-default model | ||
self.model is not None | ||
and ( | ||
# The default model is gpt-5 | ||
is_gpt_5_default() is True | ||
# However, the specified model is not a gpt-5 model | ||
and ( | ||
isinstance(self.model, Model) | ||
or gpt_5_reasoning_settings_required(self.model) is False | ||
) | ||
# The model settings are not customized for the specified model | ||
and self.model_settings == get_default_model_settings() | ||
) | ||
): | ||
# In this scenario, we should use a generic model settings | ||
# because non-gpt-5 models are not compatible with the default gpt-5 model settings. | ||
# This is a best-effort attempt to make the agent work with non-gpt-5 models. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P1] Don't strip GPT-5 reasoning when a Model instance is supplied
In Agent.__post_init__
the guard at lines 294-307 treats any Model
instance as a non–GPT‑5 model (isinstance(self.model, Model)
short‑circuits). If OPENAI_DEFAULT_MODEL
is GPT‑5 and the caller passes a GPT‑5 Model
object, the code resets self.model_settings
to ModelSettings()
, removing the required reasoning config. GPT‑5 calls then fail or behave incorrectly.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since it's not possible to check the model name when a Model instance is passed, that's fine to remove the pre-sett model settings in the scenario.
This pull request enables users to quickly switch between gpt-4 and gpt-5 without lots of code changes. GPT-5 is still an opt-in option, so there is no breaking change to existing apps.
For more details, this pull request brings the following changes:
Using this recommended model settings (reasoning.effort = low) may not resolve all the concerns shared at #1397 but I believe this configuration is the most suitable for running AI agents at this moment.
I will come up with document changes in a separate pull request.