mirror of
https://github.com/crewAIInc/crewAI.git
synced 2026-09-21 18:36:47 +00:00
fix(schema): support list-form type arrays in JSON schema conversion (#7281)
* fix(schema): support list-form "type" arrays in JSON schema conversion
_json_schema_to_pydantic_type already handles anyOf/oneOf for nullable
unions -- the form Pydantic's own schema generation produces for
Optional[T] fields -- but had no handling for the other, equally valid
JSON Schema way of expressing the same thing: a list-form type array,
e.g. {"type": ["string", "null"]}. This is what .NET/System.Text.Json
-based schema generators produce instead, so any MCP tool schema from
a non-Python server using this form crashed create_model_from_schema
outright with "Unsupported JSON schema type: ['string', 'null']" --
taking down the entire MCPServerAdapter connection, not just the one
affected tool.
Confirmed against a real self-hosted MCP server (Equibles,
github.com/daniel3303/Equibles): several of its tools (e.g.
ListCompanyDocuments's startDate/endDate filters) use exactly this
pattern, and MCPServerAdapter couldn't connect to it at all as a
result -- reproduced identically on both Windows and macOS.
Fix mirrors the existing anyOf/oneOf handling: treat each entry in a
list-form type the same way an anyOf member is handled, building a
Union of the corresponding Python types. A single-element list
collapses to that one type via typing.Union's own behavior, and
"null" entries resolve to None (matching how the type == "null"
branch already behaves), producing the same Optional[T] shape as the
anyOf case would.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix(schema): preserve union members when applying FORMAT_TYPE_MAP
CodeRabbit flagged this reviewing #7058: the format override in
_json_schema_to_pydantic_field replaced the whole resolved type with
FORMAT_TYPE_MAP[format_], even when that type was a Union built from a
list-form `type` (or anyOf/oneOf) rather than a plain `str`. For a
schema like {"type": ["string", "null"], "format": "date-time"}, this
collapsed Union[str, None] down to plain datetime, silently dropping
the null option -- masked for non-required fields by the
Optional-rewrap at the end of the same function, but not for a
required-but-nullable field (a valid, if unusual, JSON Schema shape).
The same override also drops any non-string members of a multi-type
array (e.g. ["string", "integer", "null"]) regardless of required
status, since nothing rewraps those.
Narrow the override to the `str` member specifically: replace `type_`
outright when it's already plain `str`, or substitute only the `str`
element inside a Union via get_origin/get_args, leaving null and
other type-array members untouched.
Added two tests covering the previously-broken cases: a required
nullable formatted field, and a multi-type array (string/integer/null)
with a format.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Co-authored-by: Vidit Ostwal <110953813+Vidit-Ostwal@users.noreply.github.com>
This commit is contained in:
@@ -30,6 +30,8 @@ from typing import (
|
||||
TypedDict,
|
||||
Union,
|
||||
cast,
|
||||
get_args,
|
||||
get_origin,
|
||||
)
|
||||
import uuid
|
||||
|
||||
@@ -1042,7 +1044,20 @@ def _json_schema_to_pydantic_field(
|
||||
elif len(allowed_schemes) == 1 and allowed_schemes[0] == "file":
|
||||
pydantic_type = FileUrl
|
||||
|
||||
type_ = pydantic_type
|
||||
# `type_` can be a Union built from a list-form `type` (or anyOf/oneOf)
|
||||
# rather than a plain `str`, e.g. `{"type": ["string", "null"],
|
||||
# "format": "date-time"}`. Replacing the whole thing with
|
||||
# `pydantic_type` would silently drop the other members (null,
|
||||
# non-string alternatives) instead of just narrowing the string one.
|
||||
if type_ is str:
|
||||
type_ = pydantic_type
|
||||
elif get_origin(type_) is Union:
|
||||
type_ = Union[ # noqa: UP007
|
||||
tuple(
|
||||
pydantic_type if member is str else member
|
||||
for member in get_args(type_)
|
||||
)
|
||||
]
|
||||
|
||||
if isinstance(type_, type) and issubclass(type_, str):
|
||||
if "minLength" in json_schema:
|
||||
@@ -1215,6 +1230,28 @@ def _json_schema_to_pydantic_type(
|
||||
|
||||
type_ = json_schema.get("type")
|
||||
|
||||
if isinstance(type_, list):
|
||||
# JSON Schema also allows "type" to be an array, e.g.
|
||||
# {"type": ["string", "null"]} -- the .NET/System.Text.Json-style
|
||||
# way of expressing a nullable field. Pydantic's own schema
|
||||
# generation instead uses anyOf/oneOf for this (handled above), so
|
||||
# external tool schemas (e.g. from a non-Python MCP server) are the
|
||||
# main source of this form. Treat each entry the same way anyOf's
|
||||
# members are handled just above: build a Union of the
|
||||
# corresponding Python types. A single-element list collapses to
|
||||
# that one type, matching typing.Union's own behavior.
|
||||
member_types = [
|
||||
_json_schema_to_pydantic_type(
|
||||
{**json_schema, "type": member},
|
||||
root_schema,
|
||||
name_=f"{name_ or 'Union'}Option{i}",
|
||||
enrich_descriptions=enrich_descriptions,
|
||||
in_progress=in_progress,
|
||||
)
|
||||
for i, member in enumerate(type_)
|
||||
]
|
||||
return Union[tuple(member_types)] # noqa: UP007
|
||||
|
||||
if type_ == "string":
|
||||
return str
|
||||
if type_ == "integer":
|
||||
|
||||
@@ -303,6 +303,122 @@ class TestUnionTypes:
|
||||
assert Model(value="hello").value == "hello"
|
||||
assert Model(value=3.14).value == pytest.approx(3.14)
|
||||
|
||||
def test_type_array_nullable_string_with_format(self) -> None:
|
||||
"""type: ["string", "null"] -- the .NET/System.Text.Json-style way
|
||||
of expressing an optional field, as opposed to Pydantic's own
|
||||
anyOf-based form. Seen in real MCP tool schemas from non-Python
|
||||
servers (e.g. Equibles' ListCompanyDocuments startDate/endDate
|
||||
filters). The format="date-time" here (also straight from that
|
||||
real schema) is applied by the existing FORMAT_TYPE_MAP logic once
|
||||
the list-form type no longer raises, so the field lands as a real
|
||||
datetime rather than str -- that's the pre-existing, correct
|
||||
behavior for any date-time-formatted field, not something this fix
|
||||
changes."""
|
||||
schema = {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"startDate": {
|
||||
"description": "Optional start date filter in YYYY-MM-DD format",
|
||||
"type": ["string", "null"],
|
||||
"format": "date-time",
|
||||
"default": None,
|
||||
},
|
||||
},
|
||||
}
|
||||
Model = create_model_from_schema(schema)
|
||||
assert Model(startDate="2026-01-01").startDate == datetime.datetime(
|
||||
2026, 1, 1
|
||||
)
|
||||
assert Model(startDate=None).startDate is None
|
||||
assert Model().startDate is None
|
||||
|
||||
def test_type_array_nullable_string_no_format(self) -> None:
|
||||
schema = {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"note": {"type": ["string", "null"]},
|
||||
},
|
||||
}
|
||||
Model = create_model_from_schema(schema)
|
||||
assert Model(note="hello").note == "hello"
|
||||
assert Model(note=None).note is None
|
||||
assert Model().note is None
|
||||
|
||||
def test_type_array_multiple_non_null(self) -> None:
|
||||
schema = {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"value": {"type": ["string", "integer", "null"]},
|
||||
},
|
||||
}
|
||||
Model = create_model_from_schema(schema)
|
||||
assert Model(value="hello").value == "hello"
|
||||
assert Model(value=42).value == 42
|
||||
assert Model(value=None).value is None
|
||||
|
||||
def test_type_array_single_element(self) -> None:
|
||||
schema = {
|
||||
"type": "object",
|
||||
"properties": {"value": {"type": ["string"]}},
|
||||
"required": ["value"],
|
||||
}
|
||||
Model = create_model_from_schema(schema)
|
||||
assert Model(value="hello").value == "hello"
|
||||
|
||||
def test_type_array_required_nullable_string_with_format(self) -> None:
|
||||
"""A required-but-nullable formatted field, e.g. `{"type":
|
||||
["string", "null"], "format": "date-time"}` inside a "required"
|
||||
list -- a valid JSON Schema shape meaning the key must be present
|
||||
but its value may be null. Before this fix, the FORMAT_TYPE_MAP
|
||||
override in `_json_schema_to_pydantic_field` replaced the whole
|
||||
`Union[datetime, None]` with plain `datetime`, so passing `None`
|
||||
would fail validation even though the schema explicitly allows it.
|
||||
The `not is_required` Optional-rewrap at the end of that function
|
||||
doesn't fire for required fields, so this case wasn't masked the
|
||||
way the non-required version (test above) was.
|
||||
"""
|
||||
schema = {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"startDate": {
|
||||
"type": ["string", "null"],
|
||||
"format": "date-time",
|
||||
},
|
||||
},
|
||||
"required": ["startDate"],
|
||||
}
|
||||
Model = create_model_from_schema(schema)
|
||||
assert Model(startDate="2026-01-01").startDate == datetime.datetime(
|
||||
2026, 1, 1
|
||||
)
|
||||
assert Model(startDate=None).startDate is None
|
||||
with pytest.raises(Exception):
|
||||
Model()
|
||||
|
||||
def test_type_array_multiple_non_null_with_format(self) -> None:
|
||||
"""A list-form type with more than one non-null member plus a
|
||||
recognized format, e.g. `{"type": ["string", "integer", "null"],
|
||||
"format": "date-time"}`. Before this fix, the FORMAT_TYPE_MAP
|
||||
override collapsed the entire Union down to plain `datetime`,
|
||||
silently dropping the `integer` alternative regardless of whether
|
||||
the field was required. The fix narrows only the `str` member of
|
||||
the union to the formatted type, leaving `integer` and `None`
|
||||
alone.
|
||||
"""
|
||||
schema = {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"value": {
|
||||
"type": ["string", "integer", "null"],
|
||||
"format": "date-time",
|
||||
},
|
||||
},
|
||||
}
|
||||
Model = create_model_from_schema(schema)
|
||||
assert Model(value="2026-01-01").value == datetime.datetime(2026, 1, 1)
|
||||
assert Model(value=42).value == 42
|
||||
assert Model(value=None).value is None
|
||||
|
||||
|
||||
class TestAllOfMerging:
|
||||
def test_allof_merges_properties(self) -> None:
|
||||
|
||||
Reference in New Issue
Block a user