Make Tool Output dict conversion stricter to improve backward compatibility (openai#1965)

Author: ihower

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:

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."""

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'}]"