diff --git a/src/openai/lib/_pydantic.py b/src/openai/lib/_pydantic.py index 3cfe224cb1..dd8cc7ff64 100644 --- a/src/openai/lib/_pydantic.py +++ b/src/openai/lib/_pydantic.py @@ -46,8 +46,36 @@ def _ensure_strict_json_schema( for definition_name, definition_schema in definitions.items(): _ensure_strict_json_schema(definition_schema, path=(*path, "definitions", definition_name), root=root) + # The API requires `additionalProperties: false` on every object, with no exceptions. + # + # A Pydantic model with at least one declared property has a fixed shape to close the + # object around, so any non-`False` `additionalProperties` there - `True` for a plain + # `extra="allow"` model, or a schema for one with typed extras (`__pydantic_extra__` + # annotated to validate them) - is corrected to `False`. That only forbids keys beyond + # the ones already declared; it doesn't change what the *declared* fields accept. + # + # But `additionalProperties` can also describe a genuine arbitrary-key mapping with no + # fixed shape at all - no declared properties for it to be closed around: a + # `Dict[str, ...]`-shaped field or mapping `RootModel` produces a schema-valued + # `additionalProperties` describing the values' type; a `Dict[str, Any]`-shaped field, an + # `Any`-valued mapping `RootModel`, or a bare `extra="allow"` model with zero declared + # fields produce `additionalProperties: True`. Overwriting any of these with `False` would + # silently turn the object into one that only accepts `{}`, rather than the mapping it was + # declared as - so the API has no way to represent it in a strict schema, and we raise + # instead of silently producing a broken one. typ = json_schema.get("type") - if typ == "object" and "additionalProperties" not in json_schema: + if typ == "object": + additional_properties = json_schema.get("additionalProperties", False) + properties = json_schema.get("properties") + has_declared_properties = is_dict(properties) and len(properties) > 0 + + if additional_properties is not False and not has_declared_properties: + raise TypeError( + "Objects that accept arbitrary keys (e.g. a `Dict[str, ...]`-shaped field, a " + 'mapping `RootModel`, or a bare `extra="allow"` model with no declared fields) ' + "are not supported in strict schemas, since the API requires " + f"`additionalProperties: false` on every object; path={path}" + ) json_schema["additionalProperties"] = False # object types diff --git a/tests/lib/test_pydantic.py b/tests/lib/test_pydantic.py index 754a15151c..d1012bb7f4 100644 --- a/tests/lib/test_pydantic.py +++ b/tests/lib/test_pydantic.py @@ -1,8 +1,10 @@ from __future__ import annotations from enum import Enum +from typing import Any, Dict -from pydantic import Field, BaseModel +import pytest +from pydantic import Field, BaseModel, ConfigDict from inline_snapshot import snapshot import openai @@ -409,3 +411,114 @@ def test_nested_inline_ref_expansion() -> None: "additionalProperties": False, } ) + + +class ModelWithExtraAllowed(BaseModel): + model_config = ConfigDict(extra="allow") + + name: str = Field(description="The name field.") + + +def test_additional_properties_is_forced_false_even_when_extra_allow() -> None: + """A Pydantic model with `extra="allow"` produces `additionalProperties: True` from + Pydantic itself, but the API requires `additionalProperties: false` on every object with + no exceptions - so `to_strict_json_schema` must override it rather than leave it alone. + """ + if PYDANTIC_V1: + pytest.skip("extra='allow' schema generation differs on Pydantic v1") + + schema = to_strict_json_schema(ModelWithExtraAllowed) + assert schema["additionalProperties"] is False + + +class ModelWithDictField(BaseModel): + data: Dict[str, str] = Field(description="A mapping field.") + + +def test_dict_field_raises_instead_of_silently_dropping_value_schema() -> None: + """A `Dict[str, ...]`-shaped field produces a schema-valued `additionalProperties` + describing the values' type (e.g. `{"type": "string"}`), not a boolean. The API can't + represent an arbitrary-key mapping in a strict schema, so `to_strict_json_schema` must + raise rather than silently overwrite that value with `False` - which would turn the field + into an object that only accepts `{}`, changing its meaning instead of reporting the + actual limitation. + """ + with pytest.raises(TypeError, match="additionalProperties"): + to_strict_json_schema(ModelWithDictField) + + +class ModelWithDictAnyField(BaseModel): + data: Dict[str, Any] = Field(description="An unconstrained mapping field.") + + +def test_dict_any_field_raises_instead_of_silently_allowing_only_empty_object() -> None: + """A `Dict[str, Any]`-shaped field produces `additionalProperties: True` with no declared + `properties` - the same boolean Pydantic uses for `extra="allow"`, but here there are no + declared fields to close the object around. Forcing `False` would silently accept only + `{}` instead of the arbitrary mapping the field was declared as, so this must raise just + like the schema-valued case above. + """ + if PYDANTIC_V1: + pytest.skip("Pydantic v1 omits `additionalProperties` entirely for `Dict[str, Any]`") + + with pytest.raises(TypeError, match="additionalProperties"): + to_strict_json_schema(ModelWithDictAnyField) + + +class ModelWithTypedExtra(BaseModel): + model_config = ConfigDict(extra="allow") + __pydantic_extra__: Dict[str, int] # type: ignore[misc] + + name: str = Field(description="A declared field.") + + +def test_typed_extra_is_forced_false_just_like_untyped_extra_allow() -> None: + """A Pydantic v2 model with `extra="allow"` and a typed `__pydantic_extra__` produces a + schema-valued `additionalProperties` (e.g. `{"type": "integer"}`) instead of `True`, but it + still has declared properties to close the object around, exactly like the untyped + `extra="allow"` case - so it must be forced to `False` too, not rejected just because the + value happens to be a schema rather than a boolean. + """ + if PYDANTIC_V1: + pytest.skip("typed `__pydantic_extra__` is not available on Pydantic v1") + + schema = to_strict_json_schema(ModelWithTypedExtra) + assert schema["additionalProperties"] is False + + +class EmptyModelWithExtraAllowed(BaseModel): + model_config = ConfigDict(extra="allow") + + +def test_empty_extra_allow_model_raises_instead_of_forcing_empty_object() -> None: + """An `extra="allow"` model with zero declared fields behaves like an unconstrained + mapping - any keys are allowed - unlike the with-declared-fields case above, where closing + the object to the declared fields is a reasonable strict-schema approximation. With no + fields to close around, this must raise instead of silently accepting only `{}`. + """ + if PYDANTIC_V1: + pytest.skip("extra='allow' schema generation differs on Pydantic v1") + + with pytest.raises(TypeError, match="additionalProperties"): + to_strict_json_schema(EmptyModelWithExtraAllowed) + + +def test_mapping_root_model_raises_instead_of_silently_dropping_value_schema() -> None: + """A mapping `RootModel` (e.g. `RootModel[Dict[str, str]]`) produces the same + schema-valued `additionalProperties` as a `Dict[str, ...]`-shaped field, just at the top + level of the schema instead of nested under a field - so it must raise for the same + reason. + """ + if PYDANTIC_V1: + pytest.skip("RootModel is not available on Pydantic v1") + + # Imported locally: `pydantic.RootModel` doesn't exist on Pydantic v1, and a module-level + # import would raise `ImportError` during test collection - before the `PYDANTIC_V1` skip + # above ever gets a chance to run - breaking collection for the entire file on that lane. + from pydantic import RootModel + + class MappingRootModel(RootModel[Dict[str, str]]): + pass + + with pytest.raises(TypeError, match="additionalProperties"): + to_strict_json_schema(MappingRootModel)