-
Notifications
You must be signed in to change notification settings - Fork 409
fix(compact): preserve tool search pairs #1606
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
e45badb
820e198
2312062
5831738
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,6 +15,7 @@ | |
| "function_call_output": "function_call", | ||
| "custom_tool_call_output": "custom_tool_call", | ||
| "apply_patch_call_output": "apply_patch_call", | ||
| "tool_search_output": "tool_search_call", | ||
| } | ||
| _TOOL_CALL_TYPES = frozenset(_TOOL_CALL_TYPE_BY_OUTPUT_TYPE.values()) | ||
| _ACCOUNT_NEUTRAL_REPLAY_OMITTED_ITEM_TYPES = frozenset( | ||
|
|
@@ -39,6 +40,7 @@ | |
| "additional_tools", | ||
| "apply_patch_call", | ||
| "apply_patch_call_output", | ||
| "compaction", | ||
| "custom_tool_call", | ||
| "custom_tool_call_output", | ||
| "function_call", | ||
|
|
@@ -47,6 +49,8 @@ | |
| "input_image", | ||
| "input_text", | ||
| "message", | ||
| "tool_search_call", | ||
| "tool_search_output", | ||
| } | ||
| ) | ||
| _ACCOUNT_NEUTRAL_MESSAGE_CONTENT_TYPES = frozenset( | ||
|
|
@@ -65,6 +69,7 @@ | |
| } | ||
| _ACCOUNT_NEUTRAL_INPUT_ITEM_FIELDS = { | ||
| "additional_tools": frozenset({"role", "tools", "type"}), | ||
| "compaction": frozenset({"encrypted_content", "id", "status", "type"}), | ||
| "apply_patch_call": frozenset( | ||
| { | ||
| "call_id", | ||
|
|
@@ -93,6 +98,22 @@ | |
| "function_call_output": frozenset( | ||
| {"call_id", "caller", "id", _INTERNAL_CHAT_MESSAGE_METADATA_FIELD, "output", "status", "type"} | ||
| ), | ||
| "tool_search_call": frozenset( | ||
| {"arguments", "call_id", "caller", "execution", "id", _INTERNAL_CHAT_MESSAGE_METADATA_FIELD, "status", "type"} | ||
| ), | ||
| "tool_search_output": frozenset( | ||
| { | ||
| "call_id", | ||
| "caller", | ||
| "execution", | ||
| "id", | ||
| _INTERNAL_CHAT_MESSAGE_METADATA_FIELD, | ||
| "output", | ||
| "status", | ||
| "tools", | ||
|
Comment on lines
+104
to
+113
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Handle AGENTS.md reference: AGENTS.md:L24-L26 Useful? React with 👍 / 👎. |
||
| "type", | ||
| } | ||
| ), | ||
| } | ||
| _ACCOUNT_NEUTRAL_ITEM_STATUSES = frozenset({"completed", "failed"}) | ||
| _ACCOUNT_NEUTRAL_APPLY_PATCH_OPERATION_FIELDS = { | ||
|
|
@@ -269,6 +290,10 @@ def responses_input_items_are_self_contained_fresh_replay(input_items: list[Json | |
| item_type = item_type_value if isinstance(item_type_value, str) else None | ||
| if not _input_item_has_only_known_fields(item, item_type): | ||
| return False | ||
| if item_type == "compaction": | ||
| if not _compaction_item_is_self_contained(item): | ||
| return False | ||
| continue | ||
| call_id_value = item.get("call_id") | ||
| call_id = call_id_value if isinstance(call_id_value, str) and call_id_value else None | ||
| if item_type in _TOOL_CALL_TYPES: | ||
|
|
@@ -628,6 +653,9 @@ def _tool_call_is_self_contained(item_type: str, item: Mapping[str, JsonValue]) | |
| return _is_nonblank_string(item.get("name")) and isinstance(item.get("arguments"), str) | ||
| if item_type == "custom_tool_call": | ||
| return _is_nonblank_string(item.get("name")) and isinstance(item.get("input"), str) | ||
| if item_type == "tool_search_call": | ||
| arguments = item.get("arguments") | ||
| return isinstance(arguments, dict) and item.get("execution") in (None, "client") | ||
| operation = item.get("operation") | ||
| patch = item.get("patch") | ||
| input_value = item.get("input") | ||
|
|
@@ -640,6 +668,10 @@ def _tool_call_is_self_contained(item_type: str, item: Mapping[str, JsonValue]) | |
| return _is_nonblank_string(input_value) | ||
|
|
||
|
|
||
| def _compaction_item_is_self_contained(item: Mapping[str, JsonValue]) -> bool: | ||
| return item.get("status") in (None, "completed") and _is_nonblank_string(item.get("encrypted_content")) | ||
|
|
||
|
|
||
| def _caller_is_self_contained(item: Mapping[str, JsonValue]) -> bool: | ||
| caller = item.get("caller") | ||
| return caller is None or caller == {"type": "direct"} | ||
|
|
@@ -1017,6 +1049,8 @@ def _contains_account_scoped_input_state(value: JsonValue) -> bool: | |
| return True | ||
| if item_type == "additional_tools" and not _tools_are_account_neutral(current.get("tools")): | ||
| return True | ||
| if item_type == "compaction" and _compaction_item_is_self_contained(current): | ||
| continue | ||
|
Comment on lines
+1052
to
+1053
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Do not skip the account-scoped reference check for compaction ciphertext without a verified portability guarantee. For an unanchored full resend containing a completed compaction item, this AGENTS.md reference: AGENTS.md:L105-L110 Useful? React with 👍 / 👎. |
||
| if ( | ||
| isinstance(item_type, str) | ||
| and (item_type.endswith("_call") or item_type.endswith("_call_output")) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,3 +18,18 @@ The service MUST classify an upstream `invalid_request_error` with `param=input` | |
| #### Scenario: hosted web search wording stays unclassified | ||
| - **WHEN** upstream emits `invalid_request_error` with `param=input` and a message starting `No tool output found for web search call` | ||
| - **THEN** the service does not treat it as a missing-tool-output continuity error | ||
|
|
||
| ### Requirement: Previous-response replay trimming handles tool-search output pairs | ||
| When a Responses HTTP bridge or WebSocket continuation carries `previous_response_id` and replays already-stored response output items before a fresh `tool_search_output`, the service MUST trim the replayed `tool_search_call` prefix and preserve the `tool_search_output` plus the fresh turn. The service MUST NOT forward both the replayed `tool_search_call` and its `tool_search_output` on top of the `previous_response_id` anchor. | ||
|
Comment on lines
+22
to
+23
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Do not add the new tool-search trimming contract only to an already archived delta. AGENTS.md reference: AGENTS.md:L24-L28 Useful? React with 👍 / 👎. |
||
|
|
||
| #### Scenario: HTTP bridge trims replayed tool-search call prefix | ||
| - **GIVEN** an HTTP bridge session has a completed previous response | ||
| - **WHEN** the next request carries `previous_response_id` and input `[tool_search_call, tool_search_output, user_message]` | ||
| - **THEN** the upstream request keeps the same `previous_response_id` | ||
| - **AND** its input is `[tool_search_output, user_message]` | ||
|
|
||
| #### Scenario: WebSocket bridge trims replayed tool-search call prefix | ||
| - **GIVEN** a WebSocket Responses session has a completed previous response | ||
| - **WHEN** the next request carries `previous_response_id` and input `[tool_search_call, tool_search_output, user_message]` | ||
| - **THEN** the upstream request keeps the same `previous_response_id` | ||
| - **AND** its input is `[tool_search_output, user_message]` | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This branch changes an account-neutral pre-created retry from reconnecting to the same preferred account to excluding that account and selecting another one, which is a proxy-routing and failover contract change. The only OpenSpec delta touched by this commit documents tool-search replay-prefix trimming, not cross-account recovery, so this behavior needs its own active OpenSpec change and regression scenarios before landing.
AGENTS.md reference: AGENTS.md:L92-L98
Useful? React with 👍 / 👎.