fix(_utils/_transform): propagate __api_exclude__ in _async_transform_recursive#3324
Open
rmotgi1227 wants to merge 1 commit into
Open
fix(_utils/_transform): propagate __api_exclude__ in _async_transform_recursive#3324rmotgi1227 wants to merge 1 commit into
rmotgi1227 wants to merge 1 commit into
Conversation
…_recursive The sync _transform_recursive path correctly passes exclude=getattr(data, "__api_exclude__", None) to model_dump() when it encounters a pydantic BaseModel value. The async _async_transform_recursive path omitted this kwarg, so any field listed in __api_exclude__ was serialised into the outgoing payload when the async client was used. ParsedResponse (openai/types/responses/parsed_response.py) marks its parsed_arguments field with __api_exclude__ precisely to stop client-side computed data from being sent back to the API. With the async client that exclusion had no effect and parsed_arguments leaked into every multi-turn request body that included a ParsedResponse in the history. Fix: add the missing exclude= kwarg to the model_dump() call in _async_transform_recursive, matching the sync implementation.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b762fc6492
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| inside get_type_hints(). | ||
| """ | ||
|
|
||
| import asyncio |
There was a problem hiding this comment.
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
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
The sync
_transform_recursivepath (line 221 of_utils/_transform.py) correctly passesexclude=getattr(data, "__api_exclude__", None)tomodel_dump()when it encounters a pydanticBaseModelvalue. The async counterpart_async_transform_recursive(line 387) was missing this argument.Bug
Impact
ParsedResponse(inopenai/types/responses/parsed_response.py) declares:This is specifically to prevent the client-side computed
parsed_argumentsfield from being sent back to the API server in multi-turn conversations. With the async client that exclusion had no effect:parsed_argumentsleaked into every request body that included aParsedResponsein the message history.Fix
One-line change — add the missing
excludekwarg to match the sync path:Tests
Added
tests/test_async_transform_api_exclude.pywith three tests (no live API calls):test_async_transform_excludes_api_excluded_fields— asserts__api_exclude__fields are absent from the async output (this test fails before the fix, passes after)test_sync_transform_excludes_api_excluded_fields— sanity-check that the sync path behaves the sametest_async_transform_pydantic_model_without_api_exclude— models without__api_exclude__are unaffectedRun with
PYTHONPATH=src pytest tests/test_async_transform_api_exclude.py.