diff --git a/src/agents/items.py b/src/agents/items.py index 1a699077a..8e7d1cfc3 100644 --- a/src/agents/items.py +++ b/src/agents/items.py @@ -361,6 +361,9 @@ def _maybe_get_output_as_structured_function_output( if isinstance(output, (ToolOutputText, ToolOutputImage, ToolOutputFileContent)): return output elif isinstance(output, dict): + # Require explicit 'type' field in dict to be considered a structured output + if "type" not in output: + return None try: return ValidToolOutputPydanticModelsTypeAdapter.validate_python(output) except pydantic.ValidationError: diff --git a/src/agents/tool.py b/src/agents/tool.py index 66c70c29d..39db129b7 100644 --- a/src/agents/tool.py +++ b/src/agents/tool.py @@ -15,7 +15,7 @@ from openai.types.responses.tool_param import CodeInterpreter, ImageGeneration, Mcp from openai.types.responses.web_search_tool import Filters as WebSearchToolFilters from openai.types.responses.web_search_tool_param import UserLocation -from pydantic import BaseModel, TypeAdapter, ValidationError +from pydantic import BaseModel, TypeAdapter, ValidationError, model_validator from typing_extensions import Concatenate, NotRequired, ParamSpec, TypedDict from . import _debug @@ -75,6 +75,13 @@ class ToolOutputImage(BaseModel): file_id: str | None = None detail: Literal["low", "high", "auto"] | None = None + @model_validator(mode="after") + def check_at_least_one_required_field(self) -> ToolOutputImage: + """Validate that at least one of image_url or file_id is provided.""" + if self.image_url is None and self.file_id is None: + raise ValueError("At least one of image_url or file_id must be provided") + return self + class ToolOutputImageDict(TypedDict, total=False): """TypedDict variant for image tool outputs.""" @@ -98,6 +105,13 @@ class ToolOutputFileContent(BaseModel): file_id: str | None = None filename: str | None = None + @model_validator(mode="after") + def check_at_least_one_required_field(self) -> ToolOutputFileContent: + """Validate that at least one of file_data, file_url, or file_id is provided.""" + if self.file_data is None and self.file_url is None and self.file_id is None: + raise ValueError("At least one of file_data, file_url, or file_id must be provided") + return self + class ToolOutputFileContentDict(TypedDict, total=False): """TypedDict variant for file content tool outputs.""" diff --git a/tests/test_tool_output_conversion.py b/tests/test_tool_output_conversion.py index bef4745bd..cd3a2a11a 100644 --- a/tests/test_tool_output_conversion.py +++ b/tests/test_tool_output_conversion.py @@ -76,7 +76,52 @@ def test_tool_call_output_item_mixed_list() -> None: assert items[2]["type"] == "input_file" and items[2]["file_data"] == "ZmlsZS1kYXRh" +def test_tool_call_output_item_image_forwards_file_id_and_detail() -> None: + """Ensure image outputs forward provided file_id and detail fields.""" + call = _make_tool_call() + out = ToolOutputImage(file_id="file_123", detail="high") + payload = ItemHelpers.tool_call_output_item(call, out) + + assert payload["type"] == "function_call_output" + assert payload["call_id"] == call.call_id + item = payload["output"][0] + assert isinstance(item, dict) + assert item["type"] == "input_image" + assert item["file_id"] == "file_123" + assert item["detail"] == "high" + + +def test_tool_call_output_item_file_forwards_file_id_and_filename() -> None: + """Ensure file outputs forward provided file_id and filename fields.""" + call = _make_tool_call() + out = ToolOutputFileContent(file_id="file_456", filename="report.pdf") + payload = ItemHelpers.tool_call_output_item(call, out) + + assert payload["type"] == "function_call_output" + assert payload["call_id"] == call.call_id + item = payload["output"][0] + assert isinstance(item, dict) + assert item["type"] == "input_file" + assert item["file_id"] == "file_456" + assert item["filename"] == "report.pdf" + + +def test_tool_call_output_item_file_forwards_file_url() -> None: + """Ensure file outputs forward provided file_url when present.""" + call = _make_tool_call() + out = ToolOutputFileContent(file_url="https://example.com/report.pdf") + payload = ItemHelpers.tool_call_output_item(call, out) + + assert payload["type"] == "function_call_output" + assert payload["call_id"] == call.call_id + item = payload["output"][0] + assert isinstance(item, dict) + assert item["type"] == "input_file" + assert item["file_url"] == "https://example.com/report.pdf" + + def test_tool_call_output_item_text_dict_variant() -> None: + """Dict with type='text' and text field should be treated as structured output.""" call = _make_tool_call() # Dict variant using the pydantic model schema (type="text"). out = {"type": "text", "text": "hey"} @@ -91,45 +136,237 @@ def test_tool_call_output_item_text_dict_variant() -> None: assert item["text"] == "hey" -def test_tool_call_output_item_image_forwards_file_id_and_detail() -> None: - """Ensure image outputs forward provided file_id and detail fields.""" +def test_tool_call_output_item_image_dict_variant() -> None: + """Dict with type='image' and image_url field should be treated as structured output.""" call = _make_tool_call() - out = ToolOutputImage(file_id="file_123", detail="high") + out = {"type": "image", "image_url": "http://example.com/img.png", "detail": "auto"} payload = ItemHelpers.tool_call_output_item(call, out) assert payload["type"] == "function_call_output" assert payload["call_id"] == call.call_id + assert isinstance(payload["output"], list) and len(payload["output"]) == 1 + item = payload["output"][0] + assert isinstance(item, dict) + assert item["type"] == "input_image" + assert item["image_url"] == "http://example.com/img.png" + assert item["detail"] == "auto" + + +def test_tool_call_output_item_image_dict_variant_with_file_id() -> None: + """Dict with type='image' and image_url field should be treated as structured output.""" + call = _make_tool_call() + out = {"type": "image", "file_id": "file_123"} + payload = ItemHelpers.tool_call_output_item(call, out) + + assert payload["type"] == "function_call_output" + assert payload["call_id"] == call.call_id + assert isinstance(payload["output"], list) and len(payload["output"]) == 1 item = payload["output"][0] assert isinstance(item, dict) assert item["type"] == "input_image" assert item["file_id"] == "file_123" - assert item["detail"] == "high" -def test_tool_call_output_item_file_forwards_file_id_and_filename() -> None: - """Ensure file outputs forward provided file_id and filename fields.""" +def test_tool_call_output_item_file_dict_variant_with_file_data() -> None: + """Dict with type='file' and file_data field should be treated as structured output.""" call = _make_tool_call() - out = ToolOutputFileContent(file_id="file_456", filename="report.pdf") + out = {"type": "file", "file_data": "foobar", "filename": "report.pdf"} payload = ItemHelpers.tool_call_output_item(call, out) assert payload["type"] == "function_call_output" assert payload["call_id"] == call.call_id + assert isinstance(payload["output"], list) and len(payload["output"]) == 1 item = payload["output"][0] assert isinstance(item, dict) assert item["type"] == "input_file" - assert item["file_id"] == "file_456" + assert item["file_data"] == "foobar" assert item["filename"] == "report.pdf" -def test_tool_call_output_item_file_forwards_file_url() -> None: - """Ensure file outputs forward provided file_url when present.""" +def test_tool_call_output_item_file_dict_variant_with_file_url() -> None: + """Dict with type='file' and file_url field should be treated as structured output.""" call = _make_tool_call() - out = ToolOutputFileContent(file_url="https://example.com/report.pdf") + out = {"type": "file", "file_url": "https://example.com/report.pdf", "filename": "report.pdf"} payload = ItemHelpers.tool_call_output_item(call, out) assert payload["type"] == "function_call_output" assert payload["call_id"] == call.call_id + assert isinstance(payload["output"], list) and len(payload["output"]) == 1 item = payload["output"][0] assert isinstance(item, dict) assert item["type"] == "input_file" assert item["file_url"] == "https://example.com/report.pdf" + assert item["filename"] == "report.pdf" + + +def test_tool_call_output_item_file_dict_variant_with_file_id() -> None: + """Dict with type='file' and file_id field should be treated as structured output.""" + call = _make_tool_call() + out = {"type": "file", "file_id": "file_123", "filename": "report.pdf"} + payload = ItemHelpers.tool_call_output_item(call, out) + + assert payload["type"] == "function_call_output" + assert payload["call_id"] == call.call_id + assert isinstance(payload["output"], list) and len(payload["output"]) == 1 + item = payload["output"][0] + assert isinstance(item, dict) + assert item["type"] == "input_file" + assert item["file_id"] == "file_123" + assert item["filename"] == "report.pdf" + + +def test_tool_call_output_item_image_with_extra_fields() -> None: + """Dict with type='image', image_url, and extra fields should still be converted.""" + call = _make_tool_call() + out = {"type": "image", "image_url": "http://example.com/img.png", "foobar": 213} + payload = ItemHelpers.tool_call_output_item(call, out) + + assert payload["type"] == "function_call_output" + assert payload["call_id"] == call.call_id + assert isinstance(payload["output"], list) and len(payload["output"]) == 1 + item = payload["output"][0] + assert isinstance(item, dict) + assert item["type"] == "input_image" + assert item["image_url"] == "http://example.com/img.png" + # Extra field should be ignored by Pydantic + assert "foobar" not in item + + +def test_tool_call_output_item_mixed_list_with_valid_dicts() -> None: + """List with valid dict variants (with type field) should be converted.""" + call = _make_tool_call() + out = [ + {"type": "text", "text": "hello"}, + {"type": "image", "image_url": "http://example.com/img.png"}, + {"type": "file", "file_id": "file_123"}, + ] + payload = ItemHelpers.tool_call_output_item(call, out) + + assert payload["type"] == "function_call_output" + assert payload["call_id"] == call.call_id + assert isinstance(payload["output"], list) and len(payload["output"]) == 3 + + assert payload["output"][0]["type"] == "input_text" + assert payload["output"][0]["text"] == "hello" + assert payload["output"][1]["type"] == "input_image" + assert payload["output"][1]["image_url"] == "http://example.com/img.png" + assert payload["output"][2]["type"] == "input_file" + assert payload["output"][2]["file_id"] == "file_123" + + +def test_tool_call_output_item_text_type_only_not_converted() -> None: + """Dict with only type='text' should NOT be treated as structured output.""" + call = _make_tool_call() + out = {"type": "text"} + payload = ItemHelpers.tool_call_output_item(call, out) + + assert payload["type"] == "function_call_output" + assert payload["call_id"] == call.call_id + # Should be converted to string since it doesn't have required fields + assert isinstance(payload["output"], str) + assert payload["output"] == "{'type': 'text'}" + + +def test_tool_call_output_item_image_type_only_not_converted() -> None: + """Dict with only type='image' should NOT be treated as structured output.""" + call = _make_tool_call() + out = {"type": "image"} + payload = ItemHelpers.tool_call_output_item(call, out) + + assert payload["type"] == "function_call_output" + assert payload["call_id"] == call.call_id + # Should be converted to string since it doesn't have required fields + assert isinstance(payload["output"], str) + assert payload["output"] == "{'type': 'image'}" + + +def test_tool_call_output_item_file_type_only_not_converted() -> None: + """Dict with only type='file' should NOT be treated as structured output.""" + call = _make_tool_call() + out = {"type": "file"} + payload = ItemHelpers.tool_call_output_item(call, out) + + assert payload["type"] == "function_call_output" + assert payload["call_id"] == call.call_id + assert isinstance(payload["output"], str) + assert payload["output"] == "{'type': 'file'}" + + +def test_tool_call_output_item_empty_dict_not_converted() -> None: + """Empty dict should NOT be treated as structured output.""" + call = _make_tool_call() + out: dict[str, str] = {} + payload = ItemHelpers.tool_call_output_item(call, out) + + assert payload["type"] == "function_call_output" + assert payload["call_id"] == call.call_id + assert isinstance(payload["output"], str) + assert payload["output"] == "{}" + + +def test_tool_call_output_item_dict_without_type_not_converted() -> None: + """Dict without 'type' field should NOT be treated as structured output.""" + call = _make_tool_call() + out = {"msg": "1234"} + payload = ItemHelpers.tool_call_output_item(call, out) + + assert payload["type"] == "function_call_output" + assert payload["call_id"] == call.call_id + # Should be converted to string since it lacks 'type' field + assert isinstance(payload["output"], str) + assert payload["output"] == "{'msg': '1234'}" + + +def test_tool_call_output_item_image_dict_variant_with_location_not_converted() -> None: + """Dict with type='image' and location field should NOT be treated as structured output.""" + call = _make_tool_call() + out = {"type": "image", "location": "/path/to/img.png"} + payload = ItemHelpers.tool_call_output_item(call, out) + + assert payload["type"] == "function_call_output" + assert payload["call_id"] == call.call_id + # Should be converted to string since it lacks required fields (image_url or file_id) + assert isinstance(payload["output"], str) + assert payload["output"] == "{'type': 'image', 'location': '/path/to/img.png'}" + + +def test_tool_call_output_item_file_dict_variant_with_path_not_converted() -> None: + """Dict with type='file' and path field should NOT be treated as structured output.""" + call = _make_tool_call() + out = {"type": "file", "path": "/path/to/file.txt"} + payload = ItemHelpers.tool_call_output_item(call, out) + + assert payload["type"] == "function_call_output" + assert payload["call_id"] == call.call_id + # Should be converted to string since it lacks required fields (file_data, file_url, or file_id) + assert isinstance(payload["output"], str) + assert payload["output"] == "{'type': 'file', 'path': '/path/to/file.txt'}" + + +def test_tool_call_output_item_list_without_type_not_converted() -> None: + """List with dicts lacking 'type' field should NOT be treated as structured output.""" + call = _make_tool_call() + out = [{"msg": "foobar"}] + payload = ItemHelpers.tool_call_output_item(call, out) + + assert payload["type"] == "function_call_output" + assert payload["call_id"] == call.call_id + # Should be converted to string since list items lack 'type' field + assert isinstance(payload["output"], str) + assert payload["output"] == "[{'msg': 'foobar'}]" + + +def test_tool_call_output_item_mixed_list_partial_invalid_not_converted() -> None: + """List with mix of valid and invalid dicts should NOT be treated as structured output.""" + call = _make_tool_call() + out = [ + {"type": "text", "text": "hello"}, # Valid + {"msg": "foobar"}, # Invalid + ] + payload = ItemHelpers.tool_call_output_item(call, out) + + assert payload["type"] == "function_call_output" + assert payload["call_id"] == call.call_id + # All-or-nothing: if any item is invalid, convert entire list to string + assert isinstance(payload["output"], str) + assert payload["output"] == "[{'type': 'text', 'text': 'hello'}, {'msg': 'foobar'}]"