fix(models): allow extra_args when chat completions kwarg is omitted#3200
Closed
adityasingh2400 wants to merge 1 commit intoopenai:mainfrom
Closed
fix(models): allow extra_args when chat completions kwarg is omitted#3200adityasingh2400 wants to merge 1 commit intoopenai:mainfrom
adityasingh2400 wants to merge 1 commit intoopenai:mainfrom
Conversation
Mirror the Responses API fix from openai#3185 in the chat completions path. Previously, supplying any optional first-class field (e.g. `temperature`) through `ModelSettings.extra_args` would falsely raise a duplicate-argument TypeError whenever the corresponding model setting was unset (resolved to the OpenAI `omit` sentinel). Now we only flag a duplicate when the first-class kwarg holds a real value, while still rejecting genuine collisions.
Member
|
#3192 already suggested this |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Mirror of #3185 (Responses API fix) for the Chat Completions path.
When a user supplies a first-class request parameter (e.g.
temperature,top_p,max_tokens,metadata, ...) throughModelSettings.extra_argswithout also setting the correspondingModelSettingsfield, the chat completions path falsely raises:This happens because the field in
create_kwargsresolves to OpenAI'somitsentinel (not absent), and the duplicate-key check uses a plainsetintersection. The Responses API path had the same bug, fixed in #3185 by skipping keys whose first-class value isOmit/NotGiven. The same pattern still exists inopenai_chatcompletions.py::_fetch_response— this PR applies the same fix here.Behavior change
ModelSettings(extra_args={"temperature": 0.25})raised TypeError unlesstemperaturewas alsoNone-by-default normalization.extra_argsvalue passes through.ModelSettings(temperature=0.5, extra_args={"temperature": 0.25})) still raise TypeError as before.Test plan
uv run pytest tests/models/test_openai_chatcompletions.py tests/models/test_openai_chatcompletions_stream.py tests/models/test_openai_chatcompletions_converter.py tests/models/test_kwargs_functionality.py(59 passed)ruff check+ruff format --checkcleantest_fetch_response_allows_extra_arg_when_explicit_arg_is_omitted— fails on main, passes after fixtest_fetch_response_rejects_duplicate_extra_args_with_explicit_value— guards regression on the genuine-collision caseIssue number
N/A (parallel of #3185 for chat completions)
Checks
make lint/make formatequivalent (ruff check/ruff format --check)