-
Notifications
You must be signed in to change notification settings - Fork 381
Add a quick opt-in option to switch to gpt-5 and fix issues around gpt-5 #363
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
--- | ||
'@openai/agents-openai': patch | ||
'@openai/agents-core': patch | ||
--- | ||
|
||
Add a quick opt-in option to switch to gpt-5 |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,11 +1,32 @@ | ||
import { Agent, run, fileSearchTool, withTrace } from '@openai/agents'; | ||
import OpenAI, { toFile } from 'openai'; | ||
|
||
async function main() { | ||
const client = new OpenAI({ | ||
apiKey: process.env.OPENAI_API_KEY, | ||
}); | ||
|
||
const text = `Arrakis, the desert planet in Frank Herbert's "Dune," was inspired by the scarcity of water | ||
as a metaphor for oil and other finite resources.`; | ||
const upload = await client.files.create({ | ||
file: await toFile(Buffer.from(text, 'utf-8'), 'cafe.txt'), | ||
purpose: 'assistants', | ||
}); | ||
const vectorStore = await client.vectorStores.create({ | ||
name: 'Arrakis', | ||
}); | ||
console.log(vectorStore); | ||
const indexed = await client.vectorStores.files.createAndPoll( | ||
vectorStore.id, | ||
{ file_id: upload.id }, | ||
); | ||
console.log(indexed); | ||
|
||
const agent = new Agent({ | ||
name: 'File searcher', | ||
instructions: 'You are a helpful agent.', | ||
tools: [ | ||
fileSearchTool(['vs_67bf88953f748191be42b462090e53e7'], { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. replaced a specific vector store id |
||
fileSearchTool([vectorStore.id], { | ||
maxNumResults: 3, | ||
includeSearchResults: true, | ||
}), | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,11 @@ import type { InputGuardrail, OutputGuardrail } from './guardrail'; | |
import { AgentHooks } from './lifecycle'; | ||
import { getAllMcpTools, type MCPServer } from './mcp'; | ||
import type { Model, ModelSettings, Prompt } from './model'; | ||
import { | ||
getDefaultModelSettings, | ||
gpt5ReasoningSettingsRequired, | ||
isGpt5Default, | ||
} from './defaultModel'; | ||
import type { RunContext } from './runContext'; | ||
import { | ||
type FunctionTool, | ||
|
@@ -165,8 +170,10 @@ export interface AgentConfiguration< | |
handoffOutputTypeWarningEnabled?: boolean; | ||
|
||
/** | ||
* The model implementation to use when invoking the LLM. By default, if not set, the agent will | ||
* use the default model configured in modelSettings.defaultModel | ||
* The model implementation to use when invoking the LLM. | ||
* | ||
* By default, if not set, the agent will use the default model returned by | ||
* getDefaultModel (currently "gpt-4.1"). | ||
*/ | ||
model: string | Model; | ||
|
||
|
@@ -348,7 +355,7 @@ export class Agent< | |
this.handoffDescription = config.handoffDescription ?? ''; | ||
this.handoffs = config.handoffs ?? []; | ||
this.model = config.model ?? ''; | ||
this.modelSettings = config.modelSettings ?? {}; | ||
this.modelSettings = config.modelSettings ?? getDefaultModelSettings(); | ||
this.tools = config.tools ?? []; | ||
this.mcpServers = config.mcpServers ?? []; | ||
this.inputGuardrails = config.inputGuardrails ?? []; | ||
|
@@ -359,6 +366,23 @@ export class Agent< | |
this.toolUseBehavior = config.toolUseBehavior ?? 'run_llm_again'; | ||
this.resetToolChoice = config.resetToolChoice ?? true; | ||
|
||
if ( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I image reviewers may think "Is this necessary?" but it's necessary for smoother developer experience. See also: openai/openai-agents-python#1534 |
||
// The user sets a non-default model | ||
config.model !== undefined && | ||
// The default model is gpt-5 | ||
isGpt5Default() && | ||
// However, the specified model is not a gpt-5 model | ||
(typeof config.model !== 'string' || | ||
!gpt5ReasoningSettingsRequired(config.model)) && | ||
// The model settings are not customized for the specified model | ||
config.modelSettings === undefined | ||
) { | ||
// 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. | ||
this.modelSettings = {}; | ||
} | ||
|
||
// --- Runtime warning for handoff output type compatibility --- | ||
if ( | ||
config.handoffOutputTypeWarningEnabled === undefined || | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,56 @@ | ||
import { loadEnv } from './config'; | ||
import { ModelSettings } from './model'; | ||
|
||
export const OPENAI_DEFAULT_MODEL_ENV_VARIABLE_NAME = 'OPENAI_DEFAULT_MODEL'; | ||
|
||
/** | ||
* Returns True if the model name is a GPT-5 model and reasoning settings are required. | ||
*/ | ||
export function gpt5ReasoningSettingsRequired(modelName: string): boolean { | ||
if (modelName.startsWith('gpt-5-chat')) { | ||
// gpt-5-chat-latest does not require reasoning settings | ||
return false; | ||
} | ||
// matches any of gpt-5 models | ||
return modelName.startsWith('gpt-5'); | ||
} | ||
|
||
/** | ||
* Returns True if the default model is a GPT-5 model. | ||
* This is used to determine if the default model settings are compatible with GPT-5 models. | ||
* If the default model is not a GPT-5 model, the model settings are compatible with other models. | ||
*/ | ||
export function isGpt5Default(): boolean { | ||
return gpt5ReasoningSettingsRequired(getDefaultModel()); | ||
} | ||
|
||
/** | ||
* Returns the default model name. | ||
*/ | ||
export function getDefaultModel(): string { | ||
const env = loadEnv(); | ||
return ( | ||
env[OPENAI_DEFAULT_MODEL_ENV_VARIABLE_NAME]?.toLowerCase() ?? 'gpt-4.1' | ||
); | ||
} | ||
|
||
/** | ||
* Returns the default model settings. | ||
* If the default model is a GPT-5 model, returns the GPT-5 default model settings. | ||
* Otherwise, returns the legacy default model settings. | ||
*/ | ||
export function getDefaultModelSettings(model?: string): ModelSettings { | ||
const _model = model ?? getDefaultModel(); | ||
if (gpt5ReasoningSettingsRequired(_model)) { | ||
return { | ||
providerData: { | ||
// We chose "low" instead of "minimal" because some of the built-in tools | ||
// (e.g., file search, image generation, etc.) do not support "minimal" | ||
// If you want to use "minimal" reasoning effort, you can pass your own model settings | ||
reasoning: { effort: 'low' }, | ||
text: { verbosity: 'low' }, | ||
}, | ||
}; | ||
} | ||
return {}; | ||
} |
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 tend to be confused with this MCP server tools' rule (having the repo name in the MCP server URL), so added additional instructions. This works for both gpt-4 and gpt-5.