From befcbdf4230a5906bd6d1b9bfaa2fcdfdff8fd72 Mon Sep 17 00:00:00 2001 From: Shahar Yair Date: Thu, 19 Dec 2024 15:31:08 +0200 Subject: [PATCH 1/6] fix: adding "required" to the sub properties in order to enforce structured output of the llm --- .../genai/langchain_google_genai/_function_utils.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/libs/genai/langchain_google_genai/_function_utils.py b/libs/genai/langchain_google_genai/_function_utils.py index d7227268..defad8c7 100644 --- a/libs/genai/langchain_google_genai/_function_utils.py +++ b/libs/genai/langchain_google_genai/_function_utils.py @@ -315,10 +315,15 @@ def _get_properties_from_schema(schema: Dict) -> Dict[str, Any]: if properties_item.get("type_") == glm.Type.ARRAY and v.get("items"): properties_item["items"] = _get_items_from_schema_any(v.get("items")) - if properties_item.get("type_") == glm.Type.OBJECT and v.get("properties"): - properties_item["properties"] = _get_properties_from_schema_any( - v.get("properties") - ) + if properties_item.get("type_") == glm.Type.OBJECT: + v_properties = v.get("properties") + if v_properties: + properties_item["properties"] = _get_properties_from_schema_any(v_properties) + if isinstance(v_properties, dict): + properties_item["required"] = [ + k for k, v in v_properties.items() if "default" not in v + ] + if k == "title" and "description" not in properties_item: properties_item["description"] = k + " is " + str(v) From d07a09cd38fa1a1f32f62179484072f56b9806f9 Mon Sep 17 00:00:00 2001 From: Tom Mahler Date: Thu, 19 Dec 2024 16:12:56 +0200 Subject: [PATCH 2/6] added fix to deal with anyof --- libs/genai/langchain_google_genai/_function_utils.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libs/genai/langchain_google_genai/_function_utils.py b/libs/genai/langchain_google_genai/_function_utils.py index defad8c7..865ec0b4 100644 --- a/libs/genai/langchain_google_genai/_function_utils.py +++ b/libs/genai/langchain_google_genai/_function_utils.py @@ -316,6 +316,8 @@ def _get_properties_from_schema(schema: Dict) -> Dict[str, Any]: properties_item["items"] = _get_items_from_schema_any(v.get("items")) if properties_item.get("type_") == glm.Type.OBJECT: + if v.get('anyOf') and isinstance(v['anyOf'], list) and isinstance(v['anyOf'][0], dict): + v = v['anyOf'][0] v_properties = v.get("properties") if v_properties: properties_item["properties"] = _get_properties_from_schema_any(v_properties) @@ -323,7 +325,7 @@ def _get_properties_from_schema(schema: Dict) -> Dict[str, Any]: properties_item["required"] = [ k for k, v in v_properties.items() if "default" not in v ] - + if k == "title" and "description" not in properties_item: properties_item["description"] = k + " is " + str(v) From 0adac0e9aaf76e7eb89c30ac010edcdf9602e507 Mon Sep 17 00:00:00 2001 From: Shahar Yair Date: Thu, 19 Dec 2024 17:14:44 +0200 Subject: [PATCH 3/6] Fix: Providing dummy type for object without properties --- libs/genai/langchain_google_genai/_function_utils.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libs/genai/langchain_google_genai/_function_utils.py b/libs/genai/langchain_google_genai/_function_utils.py index 865ec0b4..f3bfa273 100644 --- a/libs/genai/langchain_google_genai/_function_utils.py +++ b/libs/genai/langchain_google_genai/_function_utils.py @@ -325,6 +325,9 @@ def _get_properties_from_schema(schema: Dict) -> Dict[str, Any]: properties_item["required"] = [ k for k, v in v_properties.items() if "default" not in v ] + else: + # Providing dummy type for object without properties + properties_item["type_"] = glm.Type.STRING if k == "title" and "description" not in properties_item: properties_item["description"] = k + " is " + str(v) From c2461242b347c37f8da8f89369eaed637d1231d0 Mon Sep 17 00:00:00 2001 From: Tom Mahler Date: Thu, 19 Dec 2024 17:53:21 +0200 Subject: [PATCH 4/6] after make format --- libs/genai/langchain_google_genai/_function_utils.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/libs/genai/langchain_google_genai/_function_utils.py b/libs/genai/langchain_google_genai/_function_utils.py index f3bfa273..829f9549 100644 --- a/libs/genai/langchain_google_genai/_function_utils.py +++ b/libs/genai/langchain_google_genai/_function_utils.py @@ -316,11 +316,17 @@ def _get_properties_from_schema(schema: Dict) -> Dict[str, Any]: properties_item["items"] = _get_items_from_schema_any(v.get("items")) if properties_item.get("type_") == glm.Type.OBJECT: - if v.get('anyOf') and isinstance(v['anyOf'], list) and isinstance(v['anyOf'][0], dict): - v = v['anyOf'][0] + if ( + v.get("anyOf") + and isinstance(v["anyOf"], list) + and isinstance(v["anyOf"][0], dict) + ): + v = v["anyOf"][0] v_properties = v.get("properties") if v_properties: - properties_item["properties"] = _get_properties_from_schema_any(v_properties) + properties_item["properties"] = _get_properties_from_schema_any( + v_properties + ) if isinstance(v_properties, dict): properties_item["required"] = [ k for k, v in v_properties.items() if "default" not in v From eb1c23f3f6910a2e092cf319f89af3a02a73b788 Mon Sep 17 00:00:00 2001 From: Shahar Yair Date: Wed, 25 Dec 2024 10:00:58 +0200 Subject: [PATCH 5/6] Added tests for _get_properties_from_schema --- .../tests/unit_tests/test_function_utils.py | 165 +++++++++++++++++- 1 file changed, 164 insertions(+), 1 deletion(-) diff --git a/libs/genai/tests/unit_tests/test_function_utils.py b/libs/genai/tests/unit_tests/test_function_utils.py index 536c2a99..8bdb871e 100644 --- a/libs/genai/tests/unit_tests/test_function_utils.py +++ b/libs/genai/tests/unit_tests/test_function_utils.py @@ -1,4 +1,4 @@ -from typing import Any, Generator, Optional, Union +from typing import Any, Generator, List, Optional, Union from unittest.mock import MagicMock, patch import google.ai.generativelanguage as glm @@ -17,6 +17,169 @@ ) +def test_tool_with_anyof_nullable_param() -> None: + """ + Example test that checks a string parameter marked as Optional, + verifying it's recognized as a 'string' & 'nullable'. + """ + + @tool(parse_docstring=True) + def possibly_none( + a: Optional[str] = None, + ) -> str: + """ + A test function whose argument can be a string or None. + + Args: + a: Possibly none. + """ + return "value" + + # Convert to OpenAI, then to GenAI, then to dict + oai_tool = convert_to_openai_tool(possibly_none) + genai_tool = convert_to_genai_function_declarations([oai_tool]) + genai_tool_dict = tool_to_dict(genai_tool) + + fn_decl = genai_tool_dict["function_declarations"][0] + parameters = fn_decl["parameters"] + a_property = parameters["properties"]["a"] + + assert a_property["type_"] == glm.Type.STRING, "Expected 'a' to be STRING." + assert a_property.get("nullable") is True, "Expected 'a' to be marked as nullable." + + +def test_tool_with_array_anyof_nullable_param() -> None: + """ + Checks an array parameter marked as Optional, verifying it's recognized + as an 'array' & 'nullable', and that the items are correctly typed. + """ + + @tool(parse_docstring=True) + def possibly_none_list( + items: Optional[List[str]] = None, + ) -> str: + """ + A test function whose argument can be a list of strings or None. + + Args: + items: Possibly a list of strings or None. + """ + return "value" + + # Convert to OpenAI tool + oai_tool = convert_to_openai_tool(possibly_none_list) + + # Manually assign the 'items' type in the parameters + oai_tool["function"]["parameters"]["properties"]["items"]["items"] = { + "type": "string" + } + + # Convert to GenAI, then to dict + genai_tool = convert_to_genai_function_declarations([oai_tool]) + genai_tool_dict = tool_to_dict(genai_tool) + + fn_decl = genai_tool_dict["function_declarations"][0] + parameters = fn_decl["parameters"] + items_property = parameters["properties"]["items"] + + # Assertions + assert items_property["type_"] == glm.Type.ARRAY, "Expected 'items' to be ARRAY." + assert ( + items_property.get("nullable") is True + ), "Expected 'items' to be marked as nullable." + # Check that the array items are recognized as strings + assert ( + items_property["items"]["type_"] == glm.Type.STRING + ), "Expected array items to be STRING." + + +def test_tool_with_nested_object_anyof_nullable_param() -> None: + """ + Checks an object parameter (dict) marked as Optional, verifying it's recognized + as an 'object' but defaults to string if there are no real properties, + and that it is 'nullable'. + """ + + @tool(parse_docstring=True) + def possibly_none_dict( + data: Optional[dict] = None, + ) -> str: + """ + A test function whose argument can be an object (dict) or None. + + Args: + data: Possibly a dict or None. + """ + return "value" + + # Convert to OpenAI, then to GenAI, then to dict + oai_tool = convert_to_openai_tool(possibly_none_dict) + genai_tool = convert_to_genai_function_declarations([oai_tool]) + genai_tool_dict = tool_to_dict(genai_tool) + + fn_decl = genai_tool_dict["function_declarations"][0] + parameters = fn_decl["parameters"] + data_property = parameters["properties"]["data"] + + assert data_property["type_"] in [ + glm.Type.OBJECT, + glm.Type.STRING, + ], "Expected 'data' to be recognized as an OBJECT or fallback to STRING." + assert ( + data_property.get("nullable") is True + ), "Expected 'data' to be marked as nullable." + + +def test_tool_with_enum_anyof_nullable_param() -> None: + """ + Checks a parameter with an enum, marked as Optional, verifying it's recognized + as 'string' & 'nullable', and that the 'enum' field is captured. + """ + + @tool(parse_docstring=True) + def possibly_none_enum( + status: Optional[str] = None, + ) -> str: + """ + A test function whose argument can be an enum string or None. + + Args: + status: Possibly one of ("active", "inactive", "pending") or None. + """ + return "value" + + # Convert to OpenAI tool + oai_tool = convert_to_openai_tool(possibly_none_enum) + + # Manually override the 'enum' for the 'status' property in the parameters + oai_tool["function"]["parameters"]["properties"]["status"]["enum"] = [ + "active", + "inactive", + "pending", + ] + + # Convert to GenAI, then to dict + genai_tool = convert_to_genai_function_declarations([oai_tool]) + genai_tool_dict = tool_to_dict(genai_tool) + + fn_decl = genai_tool_dict["function_declarations"][0] + parameters = fn_decl["parameters"] + status_property = parameters["properties"]["status"] + + # Assertions + assert ( + status_property["type_"] == glm.Type.STRING + ), "Expected 'status' to be STRING." + assert ( + status_property.get("nullable") is True + ), "Expected 'status' to be marked as nullable." + assert status_property.get("enum") == [ + "active", + "inactive", + "pending", + ], "Expected 'status' to have enum values." + + def test_format_tool_to_genai_function() -> None: @tool def get_datetime() -> str: From dbaf111498f20290c3316fd9041d531918fd0e2e Mon Sep 17 00:00:00 2001 From: Tom Mahler Date: Wed, 25 Dec 2024 11:08:53 +0200 Subject: [PATCH 6/6] fixed linting errors in tests --- .../tests/unit_tests/test_function_utils.py | 95 +++++++++++++++---- 1 file changed, 74 insertions(+), 21 deletions(-) diff --git a/libs/genai/tests/unit_tests/test_function_utils.py b/libs/genai/tests/unit_tests/test_function_utils.py index 8bdb871e..482248e9 100644 --- a/libs/genai/tests/unit_tests/test_function_utils.py +++ b/libs/genai/tests/unit_tests/test_function_utils.py @@ -39,12 +39,27 @@ def possibly_none( oai_tool = convert_to_openai_tool(possibly_none) genai_tool = convert_to_genai_function_declarations([oai_tool]) genai_tool_dict = tool_to_dict(genai_tool) + assert isinstance(genai_tool_dict, dict), "Expected a dict." - fn_decl = genai_tool_dict["function_declarations"][0] - parameters = fn_decl["parameters"] - a_property = parameters["properties"]["a"] + function_declarations = genai_tool_dict.get("function_declarations") + assert isinstance( + function_declarations, + list, + ), "Expected a list." - assert a_property["type_"] == glm.Type.STRING, "Expected 'a' to be STRING." + fn_decl = function_declarations[0] + assert isinstance(fn_decl, dict), "Expected a dict." + + parameters = fn_decl.get("parameters") + assert isinstance(parameters, dict), "Expected a dict." + + properties = parameters.get("properties") + assert isinstance(properties, dict), "Expected a dict." + + a_property = properties.get("a") + assert isinstance(a_property, dict), "Expected a dict." + + assert a_property.get("type_") == glm.Type.STRING, "Expected 'a' to be STRING." assert a_property.get("nullable") is True, "Expected 'a' to be marked as nullable." @@ -77,20 +92,34 @@ def possibly_none_list( # Convert to GenAI, then to dict genai_tool = convert_to_genai_function_declarations([oai_tool]) genai_tool_dict = tool_to_dict(genai_tool) + assert isinstance(genai_tool_dict, dict), "Expected a dict." + + function_declarations = genai_tool_dict.get("function_declarations") + assert isinstance(function_declarations, list), "Expected a list." - fn_decl = genai_tool_dict["function_declarations"][0] - parameters = fn_decl["parameters"] - items_property = parameters["properties"]["items"] + fn_decl = function_declarations[0] + assert isinstance(fn_decl, dict), "Expected a dict." + + parameters = fn_decl.get("parameters") + assert isinstance(parameters, dict), "Expected a dict." + + properties = parameters.get("properties") + assert isinstance(properties, dict), "Expected a dict." + + items_property = properties.get("items") + assert isinstance(items_property, dict), "Expected a dict." # Assertions - assert items_property["type_"] == glm.Type.ARRAY, "Expected 'items' to be ARRAY." assert ( - items_property.get("nullable") is True - ), "Expected 'items' to be marked as nullable." + items_property.get("type_") == glm.Type.ARRAY + ), "Expected 'items' to be ARRAY." + assert items_property.get("nullable"), "Expected 'items' to be marked as nullable." # Check that the array items are recognized as strings - assert ( - items_property["items"]["type_"] == glm.Type.STRING - ), "Expected array items to be STRING." + + items = items_property.get("items") + assert isinstance(items, dict), "Expected 'items' to be a dict." + + assert items.get("type_") == glm.Type.STRING, "Expected array items to be STRING." def test_tool_with_nested_object_anyof_nullable_param() -> None: @@ -116,12 +145,24 @@ def possibly_none_dict( oai_tool = convert_to_openai_tool(possibly_none_dict) genai_tool = convert_to_genai_function_declarations([oai_tool]) genai_tool_dict = tool_to_dict(genai_tool) + assert isinstance(genai_tool_dict, dict), "Expected a dict." + + function_declarations = genai_tool_dict.get("function_declarations") + assert isinstance(function_declarations, list), "Expected a list." + + fn_decl = function_declarations[0] + assert isinstance(fn_decl, dict), "Expected a dict." + + parameters = fn_decl.get("parameters") + assert isinstance(parameters, dict), "Expected a dict." + + properties = parameters.get("properties") + assert isinstance(properties, dict), "Expected a dict." - fn_decl = genai_tool_dict["function_declarations"][0] - parameters = fn_decl["parameters"] - data_property = parameters["properties"]["data"] + data_property = properties.get("data") + assert isinstance(data_property, dict), "Expected a dict." - assert data_property["type_"] in [ + assert data_property.get("type_") in [ glm.Type.OBJECT, glm.Type.STRING, ], "Expected 'data' to be recognized as an OBJECT or fallback to STRING." @@ -161,14 +202,26 @@ def possibly_none_enum( # Convert to GenAI, then to dict genai_tool = convert_to_genai_function_declarations([oai_tool]) genai_tool_dict = tool_to_dict(genai_tool) + assert isinstance(genai_tool_dict, dict), "Expected a dict." + + function_declarations = genai_tool_dict.get("function_declarations") + assert isinstance(function_declarations, list), "Expected a list." + + fn_decl = function_declarations[0] + assert isinstance(fn_decl, dict), "Expected a dict." + + parameters = fn_decl.get("parameters") + assert isinstance(parameters, dict), "Expected a dict." + + properties = parameters.get("properties") + assert isinstance(properties, dict), "Expected a dict." - fn_decl = genai_tool_dict["function_declarations"][0] - parameters = fn_decl["parameters"] - status_property = parameters["properties"]["status"] + status_property = properties.get("status") + assert isinstance(status_property, dict), "Expected a dict." # Assertions assert ( - status_property["type_"] == glm.Type.STRING + status_property.get("type_") == glm.Type.STRING ), "Expected 'status' to be STRING." assert ( status_property.get("nullable") is True