Add typed reasoning / text options to ModelSettings #513
Conversation
seratch
added
enhancement
🦋 Changeset detectedLatest commit: 7fcb874 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| model: aisdk(openai('gpt-5-mini')), | ||
| modelSettings: { | ||
| providerData: { | ||
| providerOptions: { |
There was a problem hiding this comment.
Just to clarify: I didn't make any changes to ai-sdk integration; Passing these options is the standard way for ai-sdk provider for OAI, so I added example code.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ 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
| const providerData = request.modelSettings.providerData ?? {}; | ||
| if ( | ||
| request.modelSettings.reasoning && | ||
| request.modelSettings.reasoning.effort | ||
| ) { | ||
| // merge the top-level reasoning.effort into provider data | ||
| providerData.reasoning_effort = request.modelSettings.reasoning.effort; | ||
| } | ||
| if (request.modelSettings.text && request.modelSettings.text.verbosity) { | ||
| // merge the top-level text.verbosity into provider data | ||
| providerData.verbosity = request.modelSettings.text.verbosity; | ||
| } |
There was a problem hiding this comment.
The new modelSettings.reasoning and .text fields are always translated into reasoning_effort and verbosity when building the chat-completions request. However, the existing compatibility shim that removes GPT‑5-only parameters when a run resolves to a non‑GPT‑5 model still only inspects providerData. If an agent uses the new typed fields and the runtime swaps to a non‑GPT‑5 model (e.g. the runner chooses gpt-4o), these lines will still populate reasoning_effort/verbosity and the OpenAI API rejects the request. The filter needs to include the new top-level fields or these assignments should be gated by model capabilities.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
@codex can you suggest potential improvements for it?
There was a problem hiding this comment.
Summary
- Suggested guarding the
reasoning_effortandverbosityassignments (or extending the non-GPT-5 filter) so fallback models likegpt-4odon’t receive GPT-5-only parameters and trigger OpenAI request errors. packages/agents-openai/src/openaiChatCompletionsModel.tsL301-L329 packages/agents-core/src/run.tsL1089-L1111
Testing
⚠️ pnpm test(not run; only provided review feedback)
There was a problem hiding this comment.
thanks, this is helpful. i will make the non-gpt-5 filter more robust
This pull request adds type-safe reasoning.effort/summary and text.verbosity to ModelSettings. See the updated examples for how they can be used.