diff --git a/nemo_gym/openai_utils.py b/nemo_gym/openai_utils.py index 47746223f36..f1b2dd84343 100644 --- a/nemo_gym/openai_utils.py +++ b/nemo_gym/openai_utils.py @@ -15,6 +15,7 @@ import json from asyncio import sleep from typing import ( + Annotated, Any, Dict, List, @@ -56,6 +57,11 @@ from openai.types.responses import ( FunctionToolParam, Response, + ResponseCodeInterpreterToolCall, + ResponseComputerToolCall, + ResponseCustomToolCall, + ResponseFileSearchToolCall, + ResponseFunctionWebSearch, ResponseInputTextParam, ) from openai.types.responses.response_create_params import ( @@ -72,6 +78,8 @@ ResponseInputMessageContentListParam, ) from openai.types.responses.response_output_item import ( + ImageGenerationCall, + LocalShellCall, McpApprovalRequest, McpCall, McpListTools, @@ -85,7 +93,7 @@ from openai.types.responses.response_usage import ResponseUsage from openai.types.shared.chat_model import ChatModel from openai.types.shared_params import FunctionDefinition -from pydantic import BaseModel, ConfigDict, Field +from pydantic import BaseModel, BeforeValidator, ConfigDict, Field from typing_extensions import TypedDict from nemo_gym.server_utils import ( @@ -252,6 +260,38 @@ class NeMoGymResponseMcpApprovalRequest(McpApprovalRequest): server_label: Optional[str] = None +class NeMoGymResponseFileSearchToolCall(ResponseFileSearchToolCall): + """A hosted file-search call (OpenAI Responses ``file_search_call`` output item). + + The provider executes the search and returns the call in ``response.output``. + Inherits the upstream typing unchanged. + """ + + +class NeMoGymResponseFunctionWebSearch(ResponseFunctionWebSearch): + """A hosted web-search call (OpenAI Responses ``web_search_call`` output item).""" + + +class NeMoGymResponseComputerToolCall(ResponseComputerToolCall): + """A computer-use action for the client to execute (``computer_call`` output item).""" + + +class NeMoGymImageGenerationCall(ImageGenerationCall): + """A hosted image-generation call (OpenAI Responses ``image_generation_call`` output item).""" + + +class NeMoGymResponseCodeInterpreterToolCall(ResponseCodeInterpreterToolCall): + """A hosted code-interpreter call (OpenAI Responses ``code_interpreter_call`` output item).""" + + +class NeMoGymLocalShellCall(LocalShellCall): + """A local-shell command for the client to execute (``local_shell_call`` output item).""" + + +class NeMoGymResponseCustomToolCall(ResponseCustomToolCall): + """A client-executed custom tool call (OpenAI Responses ``custom_tool_call`` output item).""" + + class NeMoGymResponseInputText(ResponseInputTextParam): pass @@ -295,6 +335,16 @@ class NeMoGymResponseReasoningItemForTraining(NeMoGymResponseReasoningItem, Toke NeMoGymResponseMcpCall, NeMoGymResponseMcpListTools, NeMoGymResponseMcpApprovalRequest, + # Responses API output-call items. The upstream SDK includes these in both + # ResponseOutputItem and ResponseInputItemParam (outputs are echoed back as + # input on subsequent turns), so they belong in this shared union: + NeMoGymResponseFileSearchToolCall, + NeMoGymResponseFunctionWebSearch, + NeMoGymResponseComputerToolCall, + NeMoGymImageGenerationCall, + NeMoGymResponseCodeInterpreterToolCall, + NeMoGymLocalShellCall, + NeMoGymResponseCustomToolCall, # For training: NeMoGymEasyInputMessageForTraining, NeMoGymMessageForTraining, @@ -345,7 +395,17 @@ class NeMoGymResponseCreateParamsNonStreaming(BaseModel): ######################################## -NeMoGymResponseOutputItem = NeMoGymResponseInputItem +def _require_response_output_item_type(value: Any) -> Any: + """Prevent an untagged output item from being coerced into the wrong union member.""" + if isinstance(value, dict) and "type" not in value: + raise ValueError("Responses API output items must include a type discriminator") + return value + + +NeMoGymResponseOutputItem = Annotated[ + NeMoGymResponseInputItem, + BeforeValidator(_require_response_output_item_type), +] class NeMoGymResponseInputTokensDetails(ResponseInputTokensDetails): diff --git a/nemo_gym/responses_converter.py b/nemo_gym/responses_converter.py index a798d36dc24..e8f1b98b90b 100644 --- a/nemo_gym/responses_converter.py +++ b/nemo_gym/responses_converter.py @@ -574,25 +574,41 @@ def chat_completion_to_response( ) +_RESPONSE_OUTPUT_BOUNDARY_TYPES = frozenset( + { + "code_interpreter_call", + "computer_call", + "custom_tool_call", + "file_search_call", + "function_call", + "image_generation_call", + "local_shell_call", + "mcp_approval_request", + "mcp_call", + "mcp_list_tools", + "reasoning", + "reasoning_item", + "web_search_call", + } +) + + def split_responses_input_output_items( items: List[NeMoGymResponseOutputItem], ) -> Tuple[List[NeMoGymResponseOutputItem], List[NeMoGymResponseOutputItem]]: if not items: return [], [] + split_at = len(items) for i, item in enumerate(items): if ( getattr(item, "role", None) == "assistant" - or getattr(item, "type", None) - in { - "reasoning", - "reasoning_item", - } - or getattr(item, "type", None) in ("function_call",) + or getattr(item, "type", None) in _RESPONSE_OUTPUT_BOUNDARY_TYPES ): + split_at = i break - return items[:i], items[i:] + return items[:split_at], items[split_at:] # Backward-compatible aliases diff --git a/responses_api_agents/conversational_tool_use/simulation/app.py b/responses_api_agents/conversational_tool_use/simulation/app.py index 04aa385ad56..53ef0cfac24 100644 --- a/responses_api_agents/conversational_tool_use/simulation/app.py +++ b/responses_api_agents/conversational_tool_use/simulation/app.py @@ -542,15 +542,7 @@ def _canonicalize_run_transcript( response_payload: Dict[str, Any], ) -> tuple[NeMoGymResponseCreateParamsNonStreaming, Dict[str, Any]]: response = NeMoGymResponse.model_validate(response_payload) - has_model_output = any( - getattr(item, "role", None) == "assistant" - or getattr(item, "type", None) in {"reasoning", "reasoning_item", "function_call"} - for item in response.output - ) - if has_model_output: - input_items, output_items = split_responses_input_output_items(response.output) - else: - input_items, output_items = list(response.output), [] + input_items, output_items = split_responses_input_output_items(response.output) if not input_items: return responses_create_params, response_payload diff --git a/responses_api_agents/conversational_tool_use/simulation/tests/test_app.py b/responses_api_agents/conversational_tool_use/simulation/tests/test_app.py index 5e31c956f40..86e024515eb 100644 --- a/responses_api_agents/conversational_tool_use/simulation/tests/test_app.py +++ b/responses_api_agents/conversational_tool_use/simulation/tests/test_app.py @@ -179,6 +179,28 @@ def test_canonicalize_run_transcript_moves_user_only_terminal_output_to_input() assert canonical_response["output"] == [] +def test_canonicalize_run_transcript_preserves_tool_only_model_output() -> None: + agent = make_agent() + responses_create_params = NeMoGymResponseCreateParamsNonStreaming( + input=[NeMoGymEasyInputMessage(role="user", content="Search for the answer.")], + ) + payload = response_payload( + [ + { + "type": "web_search_call", + "id": "ws_1", + "action": {"type": "search", "query": "answer"}, + "status": "completed", + } + ] + ) + + canonical_params, canonical_response = agent._canonicalize_run_transcript(responses_create_params, payload) + + assert canonical_params is responses_create_params + assert canonical_response == payload + + def test_normalize_response_input_items_preserves_default_message_type() -> None: agent = make_agent() diff --git a/tests/unit_tests/test_openai_utils.py b/tests/unit_tests/test_openai_utils.py index e05bb274580..29ed30e92b0 100644 --- a/tests/unit_tests/test_openai_utils.py +++ b/tests/unit_tests/test_openai_utils.py @@ -13,7 +13,16 @@ # See the License for the specific language governing permissions and # limitations under the License. import pytest +from openai.types.responses import ( + ResponseCodeInterpreterToolCall, + ResponseComputerToolCall, + ResponseCustomToolCall, + ResponseFileSearchToolCall, + ResponseFunctionWebSearch, +) from openai.types.responses.response_output_item import ( + ImageGenerationCall, + LocalShellCall, McpApprovalRequest, McpCall, McpListTools, @@ -22,8 +31,15 @@ from nemo_gym.openai_utils import ( NeMoGymAsyncOpenAI, + NeMoGymImageGenerationCall, + NeMoGymLocalShellCall, NeMoGymResponse, + NeMoGymResponseCodeInterpreterToolCall, + NeMoGymResponseComputerToolCall, NeMoGymResponseCreateParamsNonStreaming, + NeMoGymResponseCustomToolCall, + NeMoGymResponseFileSearchToolCall, + NeMoGymResponseFunctionWebSearch, NeMoGymResponseInputTokensDetails, NeMoGymResponseMcpApprovalRequest, NeMoGymResponseMcpCall, @@ -130,6 +146,155 @@ def test_hosted_mcp_items_inherit_upstream_types(self) -> None: assert issubclass(NeMoGymResponseMcpApprovalRequest, McpApprovalRequest) +class TestNeMoGymResponseToolCallItems: + """Responses API output-call items (``web_search_call`` etc.) must validate rather than 500. + + The OpenAI Responses API emits these in ``response.output`` for provider- + executed tools and client-executed actions. Before they were in the union, + ``NeMoGymResponse.model_validate`` raised and the model server returned a 500 + for an upstream response that succeeded (issue #2436). + """ + + def test_web_search_call_in_response_output_validates(self) -> None: + response = NeMoGymResponse.model_validate( + _response_with_output( + [ + { + "type": "web_search_call", + "id": "ws_1", + "action": {"type": "search", "query": "official OpenAI homepage domain"}, + "status": "completed", + }, + {"type": "reasoning", "id": "r1", "summary": []}, + { + "type": "message", + "id": "m1", + "role": "assistant", + "status": "completed", + "content": [{"type": "output_text", "text": "openai.com", "annotations": []}], + }, + ] + ) + ) + call = response.output[0] + assert isinstance(call, NeMoGymResponseFunctionWebSearch) + assert call.type == "web_search_call" + assert call.status == "completed" + + def test_remaining_output_call_items_validate(self) -> None: + response = NeMoGymResponse.model_validate( + _response_with_output( + [ + { + "type": "file_search_call", + "id": "fs_1", + "queries": ["quarterly revenue"], + "status": "completed", + }, + { + "type": "computer_call", + "id": "cu_1", + "call_id": "call_1", + "action": {"type": "screenshot"}, + "pending_safety_checks": [], + "status": "completed", + }, + { + "type": "image_generation_call", + "id": "ig_1", + "result": None, + "status": "completed", + }, + { + "type": "code_interpreter_call", + "id": "ci_1", + "code": "print(42)", + "container_id": "cntr_1", + "outputs": [{"type": "logs", "logs": "42\n"}], + "status": "completed", + }, + { + "type": "local_shell_call", + "id": "ls_1", + "call_id": "call_2", + "action": {"type": "exec", "command": ["echo", "42"], "env": {}}, + "status": "completed", + }, + { + "type": "custom_tool_call", + "id": "ct_1", + "call_id": "call_3", + "name": "my_tool", + "input": "{}", + }, + ] + ) + ) + assert isinstance(response.output[0], NeMoGymResponseFileSearchToolCall) + assert isinstance(response.output[1], NeMoGymResponseComputerToolCall) + assert isinstance(response.output[2], NeMoGymImageGenerationCall) + assert isinstance(response.output[3], NeMoGymResponseCodeInterpreterToolCall) + assert isinstance(response.output[4], NeMoGymLocalShellCall) + assert isinstance(response.output[5], NeMoGymResponseCustomToolCall) + + def test_output_call_items_accepted_as_input(self) -> None: + # The upstream SDK also allows output-call items in ResponseInputItemParam: + # a rollout echoes response.output back as input on the next turn, so + # request validation must accept them too. + params = NeMoGymResponseCreateParamsNonStreaming( + input=[ + { + "type": "web_search_call", + "id": "ws_1", + "action": {"type": "search", "query": "official OpenAI homepage domain"}, + "status": "completed", + }, + { + "type": "message", + "role": "user", + "content": [{"type": "input_text", "text": "What did you find?"}], + }, + ] + ) + assert isinstance(params.input[0], NeMoGymResponseFunctionWebSearch) + + def test_output_call_items_require_type_discriminator(self) -> None: + with pytest.raises(ValidationError): + NeMoGymResponse.model_validate( + _response_with_output( + [ + { + "id": "ws_1", + "action": {"type": "search", "query": "official OpenAI homepage domain"}, + "status": "completed", + } + ] + ) + ) + + pairs = ( + (NeMoGymResponseFileSearchToolCall, ResponseFileSearchToolCall), + (NeMoGymResponseFunctionWebSearch, ResponseFunctionWebSearch), + (NeMoGymResponseComputerToolCall, ResponseComputerToolCall), + (NeMoGymImageGenerationCall, ImageGenerationCall), + (NeMoGymResponseCodeInterpreterToolCall, ResponseCodeInterpreterToolCall), + (NeMoGymLocalShellCall, LocalShellCall), + (NeMoGymResponseCustomToolCall, ResponseCustomToolCall), + ) + assert all(gym_cls.model_fields["type"].is_required() for gym_cls, _ in pairs) + + def test_output_call_items_inherit_upstream_types(self) -> None: + # These must inherit the upstream openai typing rather than redefine it + # from scratch, so schema drift is caught when the openai pin moves. + assert issubclass(NeMoGymResponseFileSearchToolCall, ResponseFileSearchToolCall) + assert issubclass(NeMoGymResponseFunctionWebSearch, ResponseFunctionWebSearch) + assert issubclass(NeMoGymResponseComputerToolCall, ResponseComputerToolCall) + assert issubclass(NeMoGymImageGenerationCall, ImageGenerationCall) + assert issubclass(NeMoGymResponseCodeInterpreterToolCall, ResponseCodeInterpreterToolCall) + assert issubclass(NeMoGymLocalShellCall, LocalShellCall) + assert issubclass(NeMoGymResponseCustomToolCall, ResponseCustomToolCall) + + class TestRoutedExpertsWireFormats: _BASE = { "prompt_token_ids": [1, 2], diff --git a/tests/unit_tests/test_responses_converter.py b/tests/unit_tests/test_responses_converter.py index ad93c5475d8..6572b944004 100644 --- a/tests/unit_tests/test_responses_converter.py +++ b/tests/unit_tests/test_responses_converter.py @@ -14,6 +14,8 @@ # limitations under the License. """Unit tests for the shared Responses API <-> Chat Completions converter.""" +from types import SimpleNamespace + import pytest from openai.types.completion_usage import CompletionTokensDetails, CompletionUsage, PromptTokensDetails @@ -633,6 +635,40 @@ def test_split_on_reasoning(): assert outputs == [reasoning] +@pytest.mark.parametrize( + "output_type", + [ + "code_interpreter_call", + "computer_call", + "custom_tool_call", + "file_search_call", + "function_call", + "image_generation_call", + "local_shell_call", + "mcp_approval_request", + "mcp_call", + "mcp_list_tools", + "reasoning", + "reasoning_item", + "web_search_call", + ], +) +def test_split_on_model_output_item_type(output_type: str): + user = NeMoGymEasyInputMessage(role="user", content="hi", type="message") + output_item = SimpleNamespace(type=output_type) + inputs, outputs = split_responses_input_output_items([user, output_item]) + assert inputs == [user] + assert outputs == [output_item] + + +def test_split_input_only_items(): + system = NeMoGymEasyInputMessage(role="system", content="policy", type="message") + user = NeMoGymEasyInputMessage(role="user", content="hi", type="message") + inputs, outputs = split_responses_input_output_items([system, user]) + assert inputs == [system, user] + assert outputs == [] + + def test_round_trip_with_tool_calls(converter: ResponsesConverter): """A chat message with reasoning + tool calls survives a round trip back to chat params.""" choice = NeMoGymChoice(