From f75f36af0fe1b0380a1537aaf991916691298a1b Mon Sep 17 00:00:00 2001 From: obchain Date: Wed, 27 May 2026 13:14:15 +0530 Subject: [PATCH] fix(genai): preserve nested-object required and nullable in tool schemas `_get_properties_from_schema` corrupted nested object schemas in two ways: 1. For any nested object property, the source schema's `required` list was discarded and rebuilt as "every property without a `default` key". A schema with `required: ["name", "role"]` and four nullable-optional fields ended up with all six fields marked required. 2. `_is_nullable_schema` only recognised `anyOf: [T, null]` shapes. On a second conversion pass (e.g. when `_dict_to_genai_schema` recurses into an already-converted inner object) it did not recognise an explicit `nullable: true` key, silently dropping the flag. Honour the source `required` list when present and short-circuit `_is_nullable_schema` on explicit `nullable: true`. Adds four regression tests covering both bugs. Closes #1725 --- .../langchain_google_genai/_function_utils.py | 18 ++- .../tests/unit_tests/test_function_utils.py | 113 ++++++++++++++++++ 2 files changed, 128 insertions(+), 3 deletions(-) diff --git a/libs/genai/langchain_google_genai/_function_utils.py b/libs/genai/langchain_google_genai/_function_utils.py index ee68d5916..4acae7af8 100644 --- a/libs/genai/langchain_google_genai/_function_utils.py +++ b/libs/genai/langchain_google_genai/_function_utils.py @@ -571,9 +571,16 @@ def _get_properties_from_schema(schema: dict) -> dict[str, Any]: v_properties ) if isinstance(v_properties, dict): - properties_item["required"] = [ - k for k, v in v_properties.items() if "default" not in v - ] + # Honor the source schema's explicit `required` list when + # present. JSON Schema (and Pydantic's + # `model_json_schema()`) emit `required` explicitly, so a + # missing key means "no required fields" rather than + # "everything without a default". + source_required = v.get("required") + if isinstance(source_required, list): + properties_item["required"] = list(source_required) + else: + properties_item["required"] = [] elif not v.get("additionalProperties"): # Only provide dummy type for object without properties AND without # additionalProperties @@ -657,6 +664,11 @@ def _get_nullable_type_from_schema(schema: dict[str, Any]) -> types.Type | None: def _is_nullable_schema(schema: dict[str, Any]) -> bool: + # Already-converted schemas carry an explicit `nullable: true` flag. + # Recognize it so re-entrant conversion (e.g. round-tripping through + # `tool_to_dict`) does not silently drop the flag. + if schema.get("nullable") is True: + return True if "anyOf" in schema: schema_types = [ _get_nullable_type_from_schema(sub_schema) for sub_schema in schema["anyOf"] diff --git a/libs/genai/tests/unit_tests/test_function_utils.py b/libs/genai/tests/unit_tests/test_function_utils.py index 14eb4bb02..770ea8f80 100644 --- a/libs/genai/tests/unit_tests/test_function_utils.py +++ b/libs/genai/tests/unit_tests/test_function_utils.py @@ -1462,3 +1462,116 @@ def calculator(a: int | float, b: int | float) -> float: assert b_property.get("type") is None, ( "When 'any_of' is present, 'type' field must NOT be set." ) + + +def test_nested_object_honors_source_required() -> None: + """Regression test for #1725 (Bug A). + + Nested object schemas must use the source schema's explicit ``required`` + list and not synthesise it from "every property without a default". + """ + tool = { + "name": "create_contact", + "description": "Create a contact.", + "parameters": { + "type": "object", + "properties": { + "contact": { + "type": "object", + "properties": { + "name": {"type": "string"}, + "role": {"type": "string"}, + "email": {"anyOf": [{"type": "string"}, {"type": "null"}]}, + "phone": {"anyOf": [{"type": "string"}, {"type": "null"}]}, + }, + "required": ["name", "role"], + }, + }, + "required": ["contact"], + }, + } + + fn = _format_to_genai_function_declaration(tool) + assert fn.parameters is not None + assert fn.parameters.properties is not None + contact = fn.parameters.properties["contact"] + assert contact.required == ["name", "role"] + + +def test_nested_object_without_required_defaults_to_empty() -> None: + """Regression test for #1725 (Bug A, fallback). + + A nested object schema with no ``required`` key must produce an empty + required list, not "every property". + """ + tool = { + "name": "use_address", + "description": "Use an address.", + "parameters": { + "type": "object", + "properties": { + "address": { + "type": "object", + "properties": { + "street": {"type": "string"}, + "city": {"type": "string"}, + }, + }, + }, + "required": ["address"], + }, + } + + fn = _format_to_genai_function_declaration(tool) + assert fn.parameters is not None + assert fn.parameters.properties is not None + address = fn.parameters.properties["address"] + assert address.required == [] + + +def test_is_nullable_schema_honors_explicit_flag() -> None: + """Regression test for #1725 (Bug B). + + ``_is_nullable_schema`` must recognise an already-converted schema + carrying an explicit ``nullable: true`` flag so a second conversion + pass does not silently drop it. + """ + from langchain_google_genai._function_utils import _is_nullable_schema + + assert _is_nullable_schema({"type": "string", "nullable": True}) is True + assert _is_nullable_schema({"type": "string", "nullable": False}) is False + assert _is_nullable_schema({"type": "string"}) is False + + +def test_nested_anyof_nullable_preserved_through_reconversion() -> None: + """Regression test for #1725 (Bug B, end-to-end). + + A nested ``anyOf: [T, null]`` property must remain ``nullable=True`` + after the converter has run, even when the inner object is re-entered + (e.g. via ``tool_to_dict`` round-tripping). + """ + tool = { + "name": "create_contact", + "description": "Create a contact.", + "parameters": { + "type": "object", + "properties": { + "contact": { + "type": "object", + "properties": { + "email": {"anyOf": [{"type": "string"}, {"type": "null"}]}, + }, + "required": [], + }, + }, + "required": ["contact"], + }, + } + + fn = _format_to_genai_function_declaration(tool) + assert fn.parameters is not None + assert fn.parameters.properties is not None + contact = fn.parameters.properties["contact"] + assert contact.properties is not None + email = contact.properties["email"] + assert email.nullable is True