fix(handoffs): add to_input_list_raw and include_nested_summary param… #2276
Conversation
|
I don't think this could be a right solution for the issue. |
![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: 5052df5a28
ℹ️ 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".
src/agents/result.py
Outdated
| else: | ||
| new_items = [ | ||
| item.to_input_item() | ||
| for item in self.new_items | ||
| if not self._is_nested_history_summary(item.to_input_item()) |
There was a problem hiding this comment.
In handoff runs, RunResult.input is replaced with the nested summary (nested.input_history in the handoff path), while new_items already contains the raw items from earlier steps. Here include_nested_summary=False only filters self.new_items, so the summary sitting in self.input is still returned. That means to_input_list_raw() won’t actually remove the <CONVERSATION HISTORY> wrapper for the default handoff flow, and the duplication/parsing issue it’s meant to fix persists.
Useful? React with 👍 / 👎.
Add option to exclude nested handoff history summary messages from to_input_list() output, fixing issue where handoff scenarios produce unparsable duplicated data. - Filter nested summaries from BOTH self.input AND new_items (addresses review) - Add include_nested_summary parameter to to_input_list() method (default True) - Add to_input_list_raw() convenience method for flat history without summaries - Add _is_nested_history_summary() helper to detect CONVERSATION HISTORY markers - Add comprehensive tests for summary detection logic Closes openai#2258
HemanthIITJ
force-pushed
the
fix/issue-2258-to-input-list-dedup-v2
branch
from
5052df5 to
bdc137f
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e9ce4041a6
ℹ️ 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".
| content = item.get("content") | ||
| if not isinstance(content, str): | ||
| return False | ||
| return "<CONVERSATION HISTORY>" in content and "</CONVERSATION HISTORY>" in content |
There was a problem hiding this comment.
_is_nested_history_summary hardcodes the default <CONVERSATION HISTORY> markers, but the handoff API lets callers change these via set_conversation_history_wrappers (see src/agents/handoffs/history.py). If a user customizes the wrappers, to_input_list(include_nested_summary=False)/to_input_list_raw() will no longer detect and filter the nested summary, so the duplicated nested history remains and the parsing issue this change targets persists. Consider reading the current wrappers (e.g., via get_conversation_history_wrappers) when checking the content.
Useful? React with 👍 / 👎.
|
to_input_list_raw is not used at all. I don't think this is a solution. So, as discussed in the issue, if a developer doesn't need the nested handoff data, they can turn it off, plus we'll move this nested handoffs to an opt-in feature in the next minor release. |
2 participants
…eter
Add option to exclude nested handoff history summary messages from to_input_list() output, fixing issue where handoff scenarios produce unparsable duplicated data.
Closes #2258