From 097d59e15e994fc1ed2084169f80e53febd28a16 Mon Sep 17 00:00:00 2001 From: Priyanshu Yashwant Deshmukh Date: Fri, 21 Nov 2025 15:09:49 +0530 Subject: [PATCH 1/3] fix: keyerror in bedrock client on format tool --- autogen/oai/bedrock.py | 32 ++++++++++++-- test/oai/test_bedrock.py | 90 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 117 insertions(+), 5 deletions(-) diff --git a/autogen/oai/bedrock.py b/autogen/oai/bedrock.py index 1d7f60558c7..ae6227b4fa5 100644 --- a/autogen/oai/bedrock.py +++ b/autogen/oai/bedrock.py @@ -534,10 +534,34 @@ def format_tools(tools: list[dict[str, Any]]) -> dict[Literal["tools"], list[dic } for prop_name, prop_details in function["parameters"]["properties"].items(): - converted_tool["toolSpec"]["inputSchema"]["json"]["properties"][prop_name] = { - "type": prop_details["type"], - "description": prop_details.get("description", ""), - } + if not isinstance(prop_details, dict): + raise TypeError(f"Property '{prop_name}' schema must be a dict, got {type(prop_details)!r}") + + prop_schema: dict[str, Any] = {"description": prop_details.get("description", "")} + + for key in ( + "type", + "enum", + "default", + "anyOf", + "oneOf", + "allOf", + "items", + "const", + "format", + "minimum", + "maximum", + "minItems", + "maxItems", + "minLength", + "maxLength", + "pattern", + "additionalProperties", + ): + if key in prop_details: + prop_schema[key] = prop_details[key] + + converted_tool["toolSpec"]["inputSchema"]["json"]["properties"][prop_name] = prop_schema if "enum" in prop_details: converted_tool["toolSpec"]["inputSchema"]["json"]["properties"][prop_name]["enum"] = prop_details[ "enum" diff --git a/test/oai/test_bedrock.py b/test/oai/test_bedrock.py index 29b701f2239..6cdab558593 100644 --- a/test/oai/test_bedrock.py +++ b/test/oai/test_bedrock.py @@ -11,7 +11,7 @@ from autogen.import_utils import run_for_optional_imports from autogen.llm_config import LLMConfig -from autogen.oai.bedrock import BedrockClient, BedrockLLMConfigEntry, oai_messages_to_bedrock_messages +from autogen.oai.bedrock import BedrockClient, BedrockLLMConfigEntry, format_tools, oai_messages_to_bedrock_messages # Fixtures for mock data @@ -341,3 +341,91 @@ def test_oai_messages_to_bedrock_messages(bedrock_client: BedrockClient): ] assert messages == expected_messages, "'Please continue' message was not appended." + + +def test_format_tools_handles_various_property_shapes(): + """format_tools should faithfully copy every supported JSON Schema shape (scalars, enums, unions, arrays, nested objects).""" + cases = [ + ( + "simple_type", + {"type": "string", "description": "plain type"}, + {"type": "string", "description": "plain type"}, + ), + ( + "enum_default", + {"type": "integer", "enum": [1, 2], "default": 1}, + {"type": "integer", "enum": [1, 2], "default": 1, "description": ""}, + ), + ( + "union_anyof", + { + "anyOf": [{"type": "string"}, {"type": "null"}], + "default": None, + "description": "optional text", + }, + { + "anyOf": [{"type": "string"}, {"type": "null"}], + "default": None, + "description": "optional text", + }, + ), + ( + "array_items", + {"type": "array", "items": {"type": "number"}, "minItems": 1}, + {"type": "array", "items": {"type": "number"}, "minItems": 1, "description": ""}, + ), + ( + "object_additional", + { + "type": "object", + "additionalProperties": {"type": "boolean"}, + "required": [], + }, + { + "type": "object", + "additionalProperties": {"type": "boolean"}, + "description": "", + }, + ), + ] + + tools = [ + { + "type": "function", + "function": { + "name": "schema_tester", + "description": "verifies schema copying", + "parameters": { + "type": "object", + "properties": {name: prop for name, prop, _ in cases}, + }, + }, + } + ] + + converted_props = format_tools(tools)["tools"][0]["toolSpec"]["inputSchema"]["json"]["properties"] + + for name, _, expected in cases: + assert converted_props[name] == expected, f"schema mismatch for {name}" + + +def test_format_tools_rejects_non_dict_properties(): + """format_tools should raise TypeError when a property schema is not a dict, mirroring runtime validation.""" + tools = [ + { + "type": "function", + "function": { + "name": "bad_prop", + "description": "schema with malformed property", + "parameters": { + "type": "object", + "properties": { + "oops": "not a dict", + }, + }, + }, + } + ] + + with pytest.raises(TypeError, match="Property 'oops' schema must be a dict"): + format_tools(tools) From 981f6e8221cb3e36ecbefc9f83cc44990e5e54cd Mon Sep 17 00:00:00 2001 From: Priyanshu Yashwant Deshmukh Date: Fri, 21 Nov 2025 16:06:49 +0530 Subject: [PATCH 2/3] fix: missing await for async call --- autogen/agentchat/conversable_agent.py | 26 ++++++------ test/agentchat/test_conversable_agent.py | 50 ++++++++++++++++++++++++ test/agentchat/test_function_call.py | 4 +- 3 files changed, 66 insertions(+), 14 deletions(-) diff --git a/autogen/agentchat/conversable_agent.py b/autogen/agentchat/conversable_agent.py index 75d1e03b7cd..21e2df8267a 100644 --- a/autogen/agentchat/conversable_agent.py +++ b/autogen/agentchat/conversable_agent.py @@ -597,7 +597,7 @@ def reply_func( "config": copy.copy(config), "init_config": config, "reset_config": reset_config, - "ignore_async_in_sync_chat": ignore_async_in_sync_chat and inspect.iscoroutinefunction(reply_func), + "ignore_async_in_sync_chat": ignore_async_in_sync_chat and is_coroutine_callable(reply_func), }, ) @@ -908,9 +908,7 @@ def reply_func_from_nested_chats( if use_async: if reply_func_from_nested_chats == "summary_from_nested_chats": reply_func_from_nested_chats = self._a_summary_from_nested_chats - if not callable(reply_func_from_nested_chats) or not inspect.iscoroutinefunction( - reply_func_from_nested_chats - ): + if not callable(reply_func_from_nested_chats) or not is_coroutine_callable(reply_func_from_nested_chats): raise ValueError("reply_func_from_nested_chats must be a callable and a coroutine") async def wrapped_reply_func(recipient, messages=None, sender=None, config=None): @@ -1299,7 +1297,7 @@ def _raise_exception_on_async_reply_functions(self) -> None: f["reply_func"] for f in self._reply_func_list if not f.get("ignore_async_in_sync_chat", False) } - async_reply_functions = [f for f in reply_functions if inspect.iscoroutinefunction(f)] + async_reply_functions = [f for f in reply_functions if is_coroutine_callable(f)] if async_reply_functions: msg = ( "Async reply functions can only be used with ConversableAgent.a_initiate_chat(). The following async reply functions are found: " @@ -2391,7 +2389,7 @@ def generate_function_call_reply( call_id = message.get("id", None) func_call = message["function_call"] func = self._function_map.get(func_call.get("name", None), None) - if inspect.iscoroutinefunction(func): + if is_coroutine_callable(func): coro = self.a_execute_function(func_call, call_id=call_id) _, func_return = self._run_async_in_thread(coro) else: @@ -2420,7 +2418,7 @@ async def a_generate_function_call_reply( func_call = message["function_call"] func_name = func_call.get("name", "") func = self._function_map.get(func_name, None) - if func and inspect.iscoroutinefunction(func): + if func and is_coroutine_callable(func): _, func_return = await self.a_execute_function(func_call, call_id=call_id) else: _, func_return = self.execute_function(func_call, call_id=call_id) @@ -2875,7 +2873,7 @@ def generate_reply( reply_func = reply_func_tuple["reply_func"] if reply_func in exclude: continue - if inspect.iscoroutinefunction(reply_func): + if is_coroutine_callable(reply_func): continue if self._match_trigger(reply_func_tuple["trigger"], sender): final, reply = reply_func(self, messages=messages, sender=sender, config=reply_func_tuple["config"]) @@ -2950,7 +2948,7 @@ async def a_generate_reply( continue if self._match_trigger(reply_func_tuple["trigger"], sender): - if inspect.iscoroutinefunction(reply_func): + if is_coroutine_callable(reply_func): final, reply = await reply_func( self, messages=messages, @@ -3171,6 +3169,12 @@ def execute_function( ) try: content = func(**arguments) + if inspect.isawaitable(content): + + async def _await_result(awaitable): + return await awaitable + + content = self._run_async_in_thread(_await_result(content)) is_exec_success = True except Exception as e: content = f"Error: {e}" @@ -3238,7 +3242,7 @@ async def a_execute_function( ExecuteFunctionEvent(func_name=func_name, call_id=call_id, arguments=arguments, recipient=self) ) try: - if inspect.iscoroutinefunction(func): + if is_coroutine_callable(func): content = await func(**arguments) else: # Fallback to sync function if the function is not async @@ -3555,7 +3559,7 @@ async def _a_wrapped_func(*args, **kwargs): log_function_use(self, func, kwargs, retval) return serialize_to_str(retval) if serialize else retval - wrapped_func = _a_wrapped_func if inspect.iscoroutinefunction(func) else _wrapped_func + wrapped_func = _a_wrapped_func if is_coroutine_callable(func) else _wrapped_func # needed for testing wrapped_func._origin = func diff --git a/test/agentchat/test_conversable_agent.py b/test/agentchat/test_conversable_agent.py index 0740723b715..14681150cac 100755 --- a/test/agentchat/test_conversable_agent.py +++ b/test/agentchat/test_conversable_agent.py @@ -8,6 +8,7 @@ import asyncio import copy +import json import os import threading import time @@ -1968,6 +1969,55 @@ def sample_tool_func(my_prop: str) -> str: assert "tool2" in tool_schemas +def test_execute_function_resolves_async_tool(mock_credentials: Credentials): + """execute_function should await async tools instead of returning coroutine reprs.""" + agent = ConversableAgent(name="agent", llm_config=mock_credentials.llm_config) + observed_inputs: list[str] = [] + + @agent.register_for_execution() + @agent.register_for_llm(description="Uppercase text asynchronously") + async def uppercase_tool(text: str) -> str: + observed_inputs.append(text) + await asyncio.sleep(0) + return text.upper() + + success, payload = agent.execute_function( + {"name": "uppercase_tool", "arguments": json.dumps({"text": "nyc"})}, + call_id="tool-call-1", + ) + + assert success is True + assert payload["content"] == "NYC" + assert observed_inputs == ["nyc"] + + +def test_generate_tool_calls_reply_handles_async_tool(mock_credentials: Credentials): + """generate_tool_calls_reply should await async tools registered for execution.""" + agent = ConversableAgent(name="agent", llm_config=mock_credentials.llm_config) + + @agent.register_for_execution() + @agent.register_for_llm(description="Title case text asynchronously") + async def title_tool(text: str) -> str: + await asyncio.sleep(0) + return text.title() + + message = { + "role": "assistant", + "tool_calls": [ + { + "id": "call-xyz", + "function": {"name": "title_tool", "arguments": json.dumps({"text": "new york"})}, + } + ], + } + + handled, response = agent.generate_tool_calls_reply(messages=[message]) + assert handled is True + tool_response = response["tool_responses"][0] + assert tool_response["tool_call_id"] == "call-xyz" + assert tool_response["content"] == "New York" + + def test_create_or_get_executor(mock_credentials: Credentials): agent = ConversableAgent(name="agent", llm_config=mock_credentials.llm_config) executor_agent = None diff --git a/test/agentchat/test_function_call.py b/test/agentchat/test_function_call.py index ab1930f1e99..84b78b9b658 100755 --- a/test/agentchat/test_function_call.py +++ b/test/agentchat/test_function_call.py @@ -165,9 +165,7 @@ async def add_num(num_to_be_added): user = UserProxyAgent(name="test", function_map={"add_num": add_num}) correct_args = {"name": "add_num", "arguments": '{ "num_to_be_added": 5 }'} - # Asset coroutine doesn't match. - assert user.execute_function(func_call=correct_args)[1]["content"] != "15" - # Asset awaited coroutine does match. + assert user.execute_function(func_call=correct_args)[1]["content"] == "15" assert (await user.a_execute_function(func_call=correct_args))[1]["content"] == "15" # function name called is wrong or doesn't exist From 6bea535b9ea685801a55d8eae429c747321b907a Mon Sep 17 00:00:00 2001 From: Priyanshu Yashwant Deshmukh Date: Thu, 4 Dec 2025 17:35:07 +0530 Subject: [PATCH 3/3] test: integration test --- test/oai/test_bedrock.py | 86 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/test/oai/test_bedrock.py b/test/oai/test_bedrock.py index 6cdab558593..f6f49de865b 100644 --- a/test/oai/test_bedrock.py +++ b/test/oai/test_bedrock.py @@ -4,14 +4,19 @@ # # Portions derived from https://github.com/microsoft/autogen are under the MIT License. # SPDX-License-Identifier: MIT +import asyncio +import json +import os from unittest.mock import MagicMock, patch import pytest from pydantic import ValidationError +from autogen.agentchat import ConversableAgent from autogen.import_utils import run_for_optional_imports from autogen.llm_config import LLMConfig from autogen.oai.bedrock import BedrockClient, BedrockLLMConfigEntry, format_tools, oai_messages_to_bedrock_messages +from test.credentials import Credentials, get_credentials_from_env_vars # Fixtures for mock data @@ -429,3 +434,84 @@ def test_format_tools_rejects_non_dict_properties(): with pytest.raises(TypeError, match="Property 'oops' schema must be a dict"): format_tools(tools) + + +# Integration tests with real Bedrock credentials +@pytest.fixture +def bedrock_credentials() -> Credentials: + """Fixture to get Bedrock credentials from environment variables.""" + try: + return get_credentials_from_env_vars(filter_dict={"api_type": "bedrock"}) + except Exception: + pytest.skip("Bedrock credentials not available (AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY required)") + + +@run_for_optional_imports(["boto3", "botocore"], "bedrock") +@pytest.mark.integration +def test_execute_function_resolves_async_tool_with_bedrock(bedrock_credentials: Credentials): + """Integration test: execute_function should await async tools instead of returning coroutine reprs with real Bedrock client.""" + agent = ConversableAgent(name="agent", llm_config=bedrock_credentials.llm_config) + observed_inputs: list[str] = [] + + @agent.register_for_execution() + @agent.register_for_llm(description="Uppercase text asynchronously") + async def uppercase_tool(text: str) -> str: + observed_inputs.append(text) + await asyncio.sleep(0) + return text.upper() + + success, payload = agent.execute_function( + {"name": "uppercase_tool", "arguments": json.dumps({"text": "nyc"})}, + call_id="tool-call-1", + ) + + assert success is True + assert payload["content"] == "NYC" + assert observed_inputs == ["nyc"] + + +@run_for_optional_imports(["boto3", "botocore"], "bedrock") +@pytest.mark.asyncio +@pytest.mark.integration +@pytest.mark.skipif( + not (os.getenv("AWS_ACCESS_KEY_ID") and os.getenv("AWS_SECRET_ACCESS_KEY")), + reason="Bedrock credentials not available (AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY required)", +) +async def test_async_tool_execution_with_bedrock_integration(bedrock_credentials: Credentials): + """Integration test: async tool execution with real Bedrock client, checking summary and chat history.""" + agent = ConversableAgent( + name="agent", + llm_config=bedrock_credentials.llm_config, + system_message="You are a helpful assistant. Use tools when needed.", + ) + + observed_inputs: list[str] = [] + + @agent.register_for_execution() + @agent.register_for_llm(description="Uppercase text asynchronously. Useful for converting text to uppercase.") + async def uppercase_tool(text: str) -> str: + """Uppercase the given text.""" + observed_inputs.append(text) + await asyncio.sleep(0.1) # Simulate async operation + return text.upper() + + # Run the agent with a message that should trigger tool usage + result = await agent.a_run( + message="say 'hello bedrock'", + max_turns=3, + user_input=False, + summary_method="reflection_with_llm", + ) + + # Wait for the result to process + await result.process() + + # Assert summary is generated and contains relevant information + summary = await result.summary + assert "hello" in summary.lower() + all_content = " ".join([ + msg.get("content", "") for msg in result.chat_history if isinstance(msg.get("content"), str) + ]) + assert "HELLO BEDROCK" in all_content or "hello bedrock".upper() in all_content, ( + f"Chat history should contain the uppercase result. Content: {all_content[:200]}" + )