Skip to content

Fix: Only pass reasoning_effort to models that support it#264

Open
hahnicity wants to merge 2 commits into
google-deepmind:mainfrom
hahnicity:fix/openai-reasoning-effort-compat
Open

Fix: Only pass reasoning_effort to models that support it#264
hahnicity wants to merge 2 commits into
google-deepmind:mainfrom
hahnicity:fix/openai-reasoning-effort-compat

Conversation

@hahnicity

Copy link
Copy Markdown

Summary

  • reasoning_effort and verbosity are only supported by OpenAI reasoning models (o-series, GPT-5 family). Passing them to non-reasoning models like gpt-4o-mini causes a 400 BadRequestError: "Unrecognized request argument supplied: reasoning_effort"
  • This change conditionally includes these parameters only when the model name indicates a reasoning model (o1, o3, o4, gpt-5 prefixes)

References

Per OpenAI's reasoning guide, reasoning_effort controls how many reasoning tokens a model generates, and supported values are "model-dependent." The gpt-4o-mini model page confirms it is not a reasoning model and does not support this parameter.

Test plan

  • Verified gpt-4o-mini no longer returns 400 BadRequestError when used with Concordia
  • Reasoning model prefixes (o1, o3, o4, gpt-5) will still pass reasoning_effort and verbosity as before

The `reasoning_effort` and `verbosity` parameters are only supported by
reasoning models (o-series, GPT-5 family). Passing them to non-reasoning
models like gpt-4o-mini causes a 400 BadRequestError:

  "Unrecognized request argument supplied: reasoning_effort"

Per OpenAI's documentation, gpt-4o-mini is not a reasoning model and
does not support reasoning_effort. The parameter is documented as being
for reasoning models only (o1, o3, o4-mini, gpt-5, etc.).

Ref: https://developers.openai.com/docs/guides/reasoning
Ref: https://developers.openai.com/docs/models/gpt-4o-mini

This change conditionally includes reasoning_effort and verbosity only
when the model name indicates a reasoning model.
@google-cla

google-cla Bot commented Apr 12, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Pylint does not narrow `self._valid_locations` after a None guard on
the instance attribute. Assigning to a local variable after the check
lets pylint infer the correct non-None type.
@hahnicity

Copy link
Copy Markdown
Author

Added a fix for a pre-existing pylint failure in world_state.py (unrelated to the original change in this PR).

The CI Test Concordia job was failing on pylint errors in concordia/components/game_master/world_state.py at lines 295, 298, and 301:

  • E1135: Value 'self._valid_locations' doesn't support membership test
  • E1133: Non-iterable value self._valid_locations is used in an iterating context

These errors occur because pylint does not narrow the type of self._valid_locations (typed set | None) after a None guard check on the instance attribute. The fix assigns to a local variable after the guard, which lets pylint correctly infer the non-None type. No behavioral change.

@jzleibo jzleibo self-assigned this Apr 14, 2026
@jzleibo

jzleibo commented Apr 14, 2026

Copy link
Copy Markdown
Collaborator

This looks good to me, we should be able to merge it once you've signed the Google CLA.

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.

3 participants