fix: to_input_list() produces unparsable data after handoff with nesting - #2661
fix: to_input_list() produces unparsable data after handoff with nesting#2661giulio-leone wants to merge 4 commits into
Conversation
When nest_handoff_history=True, self.input is replaced with a nested summary that embeds function_call/function_call_output items as text. However, to_input_list() was still iterating self.new_items (the unfiltered session items), producing orphaned structured function_call items without a parent assistant message — causing the API to reject the input. Fix: to_input_list() now prefers _model_input_items (populated during handoffs with correctly filtered items) over new_items. Falls back to new_items for non-handoff runs where _model_input_items is empty. Closes #2258 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
![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.
Reviewed commit: ce85435d68
ℹ️ 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".
|
|
||
| # Prefer _model_input_items when populated (after handoffs with nesting) | ||
| # to avoid orphaned function_call/function_call_output items. | ||
| source_items = getattr(self, "_model_input_items", None) or self.new_items |
There was a problem hiding this comment.
This unconditional preference for _model_input_items regresses to_input_list() in server-managed conversation runs (conversation_id/previous_response_id): in run.py, the non-streaming loop feeds run_single_turn with pending_server_items deltas when available (items_for_model at lines 971-975) and then overwrites pending_server_items with only turn_result.new_step_items (line 1096), so _model_input_items can represent only recent deltas rather than the full local transcript after multiple turns. With this change, to_input_list() can return truncated history where it previously returned the full new_items transcript, which breaks manual replay or switching back to client-managed history.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — fixed in ddad0f7. Now _model_input_items is only preferred when its content actually differs from new_items (evidence of handoff-nesting filtering). In server-managed conversation runs the full new_items transcript is used instead.
Only prefer _model_input_items over new_items when their content actually differs (evidence that handoff nesting filtered items). In server-managed conversation runs (_conversation_id set), _model_input_items may only hold recent deltas rather than the full transcript, so falling back unconditionally could truncate history. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ddad0f7caf
ℹ️ 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".
| has_nesting = model_input_items and model_input_items != self.new_items | ||
| source_items = model_input_items if has_nesting else self.new_items |
There was a problem hiding this comment.
Fresh evidence: even with this change, the has_nesting heuristic still treats server-managed conversations as nested because _model_input_items often differs from new_items for non-nesting reasons. In run.py, server-managed turns pass only pending_server_items deltas to run_single_turn (items_for_model at lines 971-975) and then reset pending_server_items to turn_result.new_step_items (line 1096), while new_items continues accumulating full session_items; after multiple turns, to_input_list() will select _model_input_items here and return truncated history instead of the full transcript.
Useful? React with 👍 / 👎.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Pushed a follow-up for the latest review thread. The fallback now only uses |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Pushed That was the sole failing CI item ( Re-validated locally after the formatting-only follow-up:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 74046951a3
ℹ️ 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".
| }: | ||
| contains_structured_items = True | ||
| break | ||
| if candidate_items and not contains_structured_items: |
There was a problem hiding this comment.
In server-managed conversation mode (conversation_id / previous_response_id), _model_input_items is not guaranteed to be the full transcript: the run loops pass only pending_server_items deltas on later turns (src/agents/run.py around items_for_model and pending_server_items, mirrored in src/agents/run_internal/run_loop.py). This branch now prefers _model_input_items whenever those deltas contain no function_call/function_call_output/reasoning items, so to_input_list() can return only the last delta instead of all accumulated new_items, breaking replay or switching back to client-managed history after a multi-turn server-managed run.
Useful? React with 👍 / 👎.
|
Thanks for sending this patch. I've reviewed this PR and found that it doesn't yet resolve the issue plus its heuristic approach inside the to_input_list method is not robust enough in terms of long-term maintenance of this SDK. I'll come up with my draft PR that aims to resolve the same issue later on. Your feedback on the PR would be appreciated. |
|
Closing this in favor of #2667 |
2 participants
Problem
When
nest_handoff_history=True(the default),to_input_list()produces unparsable data that the API rejects with orphanedfunction_call/function_call_outputitems.Root Cause
During a handoff with nesting:
self.inputis replaced with the nested summary (a single assistant message containing<CONVERSATION HISTORY>text that embeds the function_call items as text lines)self.new_itemsretains the unfiltered session items (including rawfunction_callandfunction_call_outputitems from the pre-handoff agent)_model_input_itemsis populated with the filtered items that are consistent with the nestedself.inputto_input_list()was concatenatingself.input(nested summary) + items fromself.new_items(unfiltered), producing:function_call/function_call_outputitems without a parent assistant messageThe API rejects these orphaned items.
Fix
to_input_list()now prefers_model_input_itemswhen populated (after handoffs) overself.new_items. This ensures the items are consistent with the (possibly nested)self.input. Falls back toself.new_itemsfor non-handoff runs where_model_input_itemsis empty.Tests
Added 2 tests to
test_handoff_history_duplication.py:test_to_input_list_uses_model_input_items_when_available— verifies no orphaned function_call items after handofftest_to_input_list_falls_back_to_new_items_without_handoff— verifies normal non-handoff behavior unchangedAll 40 handoff/result tests pass.
Closes #2258