Skip to content

fix: remove redundant deep copy in input preprocessing#3922

Open
AAliKKhan wants to merge 1 commit into
openai:mainfrom
AAliKKhan:fix/redundant-deep-copy
Open

fix: remove redundant deep copy in input preprocessing#3922
AAliKKhan wants to merge 1 commit into
openai:mainfrom
AAliKKhan:fix/redundant-deep-copy

Conversation

@AAliKKhan

Copy link
Copy Markdown
Contributor

ItemHelpers.input_to_new_input_list() already calls _to_dump_compatible() internally (items.py:772). The caller in OpenAIResponsesModel._create_request() was calling it a second time, creating an unnecessary deep copy of the entire input history on every model turn. For multi-turn conversations with large histories this doubles memory and CPU overhead.

@seratch seratch left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the optimization. ItemHelpers.input_to_new_input_list() already recursively materializes list inputs, so removing the second conversion is appropriate. However, test_responses_materializes_iterator_payload currently monkeypatches that helper to return an unmaterialized iterable and therefore depends on the conversion this PR removes. Please update the test to pass the nested iterable through the real ItemHelpers.input_to_new_input_list() implementation, while preserving the assertions that input and tool iterables are materialized exactly once. Once that existing regression coverage passes, this should be ready for another review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants