From fb77c74fa17dfb6518d92277c651ff0d7cd2f1ce Mon Sep 17 00:00:00 2001 From: xianghuijin Date: Thu, 20 Mar 2025 21:22:27 +0800 Subject: [PATCH 1/9] Fix potential infinite tool call loop by resetting tool_choice after tool execution --- src/agents/_run_impl.py | 32 ++++ tests/test_tool_choice_reset.py | 303 ++++++++++++++++++++++++++++++++ 2 files changed, 335 insertions(+) create mode 100644 tests/test_tool_choice_reset.py diff --git a/src/agents/_run_impl.py b/src/agents/_run_impl.py index 2849538d..a60ae1d6 100644 --- a/src/agents/_run_impl.py +++ b/src/agents/_run_impl.py @@ -47,6 +47,7 @@ ) from .lifecycle import RunHooks from .logger import logger +from .model_settings import ModelSettings from .models.interface import ModelTracing from .run_context import RunContextWrapper, TContext from .stream_events import RunItemStreamEvent, StreamEvent @@ -206,6 +207,37 @@ async def execute_tools_and_side_effects( new_step_items.extend([result.run_item for result in function_results]) new_step_items.extend(computer_results) + # Reset tool_choice to "auto" after tool execution to prevent infinite loops + if (processed_response.functions or processed_response.computer_actions): + # Reset agent's model_settings + if agent.model_settings.tool_choice == "required" or isinstance(agent.model_settings.tool_choice, str): + # Create a new model_settings to avoid modifying the original shared instance + agent.model_settings = ModelSettings( + temperature=agent.model_settings.temperature, + top_p=agent.model_settings.top_p, + frequency_penalty=agent.model_settings.frequency_penalty, + presence_penalty=agent.model_settings.presence_penalty, + tool_choice="auto", # Reset to auto + parallel_tool_calls=agent.model_settings.parallel_tool_calls, + truncation=agent.model_settings.truncation, + max_tokens=agent.model_settings.max_tokens, + ) + + # Also reset run_config's model_settings if it exists + if run_config.model_settings and (run_config.model_settings.tool_choice == "required" or + isinstance(run_config.model_settings.tool_choice, str)): + # Create a new model_settings for run_config + run_config.model_settings = ModelSettings( + temperature=run_config.model_settings.temperature, + top_p=run_config.model_settings.top_p, + frequency_penalty=run_config.model_settings.frequency_penalty, + presence_penalty=run_config.model_settings.presence_penalty, + tool_choice="auto", # Reset to auto + parallel_tool_calls=run_config.model_settings.parallel_tool_calls, + truncation=run_config.model_settings.truncation, + max_tokens=run_config.model_settings.max_tokens, + ) + # Second, check if there are any handoffs if run_handoffs := processed_response.handoffs: return await cls.execute_handoffs( diff --git a/tests/test_tool_choice_reset.py b/tests/test_tool_choice_reset.py new file mode 100644 index 00000000..e01a5f0d --- /dev/null +++ b/tests/test_tool_choice_reset.py @@ -0,0 +1,303 @@ +from unittest import mock +import asyncio +import json +from typing import List + +from agents import Agent, ModelSettings, RunConfig, function_tool, Runner +from agents.models.interface import ModelResponse +from agents.items import Usage +from openai.types.responses.response_function_tool_call import ResponseFunctionToolCall + + +@function_tool +def echo(text: str) -> str: + """Echo the input text""" + return text + + +# Mock model implementation that always calls tools when tool_choice is set +class MockModel: + def __init__(self, tool_call_counter): + self.tool_call_counter = tool_call_counter + + async def get_response(self, **kwargs): + tools = kwargs.get("tools", []) + model_settings = kwargs.get("model_settings") + + # Increment the counter to track how many times this model is called + self.tool_call_counter["count"] += 1 + + # If we've been called many times, we're likely in an infinite loop + if self.tool_call_counter["count"] > 5: + self.tool_call_counter["potential_infinite_loop"] = True + + # Always create a tool call if tool_choice is required/specific + tool_calls = [] + if model_settings and model_settings.tool_choice: + if model_settings.tool_choice in ["required", "echo"] and tools: + # Create a mock function call to the first tool + tool = tools[0] + tool_calls.append( + ResponseFunctionToolCall( + id="call_1", + name=tool.name, + arguments=json.dumps({"text": "This is a test"}), + call_id="call_1", + type="function_call", + ) + ) + + return ModelResponse( + output=tool_calls, + referenceable_id="123", + usage=Usage(input_tokens=10, output_tokens=10, total_tokens=20), + ) + + +class TestToolChoiceReset: + async def test_tool_choice_resets_after_call(self): + """Test that tool_choice is reset to 'auto' after tool call when set to 'required'""" + # Create an agent with tool_choice="required" + agent = Agent( + name="Test agent", + tools=[echo], + model_settings=ModelSettings(tool_choice="required"), + ) + + # Directly modify the model_settings + # Instead of trying to run the full execute_tools_and_side_effects, + # we'll just test the tool_choice reset logic directly + processed_response = mock.MagicMock() + processed_response.functions = [mock.MagicMock()] # At least one function call + processed_response.computer_actions = [] + + # Create a mock run_config + run_config = mock.MagicMock() + run_config.model_settings = None + + # Execute our code under test + if processed_response.functions: + # Reset agent's model_settings + if agent.model_settings.tool_choice == "required" or isinstance(agent.model_settings.tool_choice, str): + agent.model_settings = ModelSettings( + temperature=agent.model_settings.temperature, + top_p=agent.model_settings.top_p, + frequency_penalty=agent.model_settings.frequency_penalty, + presence_penalty=agent.model_settings.presence_penalty, + tool_choice="auto", # Reset to auto + parallel_tool_calls=agent.model_settings.parallel_tool_calls, + truncation=agent.model_settings.truncation, + max_tokens=agent.model_settings.max_tokens, + ) + + # Also reset run_config's model_settings if it exists + if run_config.model_settings and (run_config.model_settings.tool_choice == "required" or + isinstance(run_config.model_settings.tool_choice, str)): + run_config.model_settings = ModelSettings( + temperature=run_config.model_settings.temperature, + top_p=run_config.model_settings.top_p, + frequency_penalty=run_config.model_settings.frequency_penalty, + presence_penalty=run_config.model_settings.presence_penalty, + tool_choice="auto", # Reset to auto + parallel_tool_calls=run_config.model_settings.parallel_tool_calls, + truncation=run_config.model_settings.truncation, + max_tokens=run_config.model_settings.max_tokens, + ) + + # Check that tool_choice was reset to "auto" + assert agent.model_settings.tool_choice == "auto" + + async def test_tool_choice_resets_from_specific_function(self): + """Test tool_choice reset to 'auto' after call when set to specific function name""" + # Create an agent with tool_choice set to a specific function + agent = Agent( + name="Test agent", + instructions="You are a test agent", + tools=[echo], + model="gpt-4-0125-preview", + model_settings=ModelSettings(tool_choice="echo"), + ) + + # Execute our code under test + processed_response = mock.MagicMock() + processed_response.functions = [mock.MagicMock()] # At least one function call + processed_response.computer_actions = [] + + # Create a mock run_config + run_config = mock.MagicMock() + run_config.model_settings = None + + # Execute our code under test + if processed_response.functions: + # Reset agent's model_settings + if agent.model_settings.tool_choice == "required" or isinstance(agent.model_settings.tool_choice, str): + agent.model_settings = ModelSettings( + temperature=agent.model_settings.temperature, + top_p=agent.model_settings.top_p, + frequency_penalty=agent.model_settings.frequency_penalty, + presence_penalty=agent.model_settings.presence_penalty, + tool_choice="auto", # Reset to auto + parallel_tool_calls=agent.model_settings.parallel_tool_calls, + truncation=agent.model_settings.truncation, + max_tokens=agent.model_settings.max_tokens, + ) + + # Also reset run_config's model_settings if it exists + if run_config.model_settings and (run_config.model_settings.tool_choice == "required" or + isinstance(run_config.model_settings.tool_choice, str)): + run_config.model_settings = ModelSettings( + temperature=run_config.model_settings.temperature, + top_p=run_config.model_settings.top_p, + frequency_penalty=run_config.model_settings.frequency_penalty, + presence_penalty=run_config.model_settings.presence_penalty, + tool_choice="auto", # Reset to auto + parallel_tool_calls=run_config.model_settings.parallel_tool_calls, + truncation=run_config.model_settings.truncation, + max_tokens=run_config.model_settings.max_tokens, + ) + + # Check that tool_choice was reset to "auto" + assert agent.model_settings.tool_choice == "auto" + + async def test_tool_choice_no_reset_when_auto(self): + """Test that tool_choice is not changed when it's already set to 'auto'""" + # Create an agent with tool_choice="auto" + agent = Agent( + name="Test agent", + tools=[echo], + model_settings=ModelSettings(tool_choice="auto"), + ) + + # Execute our code under test + processed_response = mock.MagicMock() + processed_response.functions = [mock.MagicMock()] # At least one function call + processed_response.computer_actions = [] + + # Create a mock run_config + run_config = mock.MagicMock() + run_config.model_settings = None + + # Execute our code under test + if processed_response.functions: + # Reset agent's model_settings + if agent.model_settings.tool_choice == "required" or isinstance(agent.model_settings.tool_choice, str): + agent.model_settings = ModelSettings( + temperature=agent.model_settings.temperature, + top_p=agent.model_settings.top_p, + frequency_penalty=agent.model_settings.frequency_penalty, + presence_penalty=agent.model_settings.presence_penalty, + tool_choice="auto", # Reset to auto + parallel_tool_calls=agent.model_settings.parallel_tool_calls, + truncation=agent.model_settings.truncation, + max_tokens=agent.model_settings.max_tokens, + ) + + # Also reset run_config's model_settings if it exists + if run_config.model_settings and (run_config.model_settings.tool_choice == "required" or + isinstance(run_config.model_settings.tool_choice, str)): + run_config.model_settings = ModelSettings( + temperature=run_config.model_settings.temperature, + top_p=run_config.model_settings.top_p, + frequency_penalty=run_config.model_settings.frequency_penalty, + presence_penalty=run_config.model_settings.presence_penalty, + tool_choice="auto", # Reset to auto + parallel_tool_calls=run_config.model_settings.parallel_tool_calls, + truncation=run_config.model_settings.truncation, + max_tokens=run_config.model_settings.max_tokens, + ) + + # Check that tool_choice remains "auto" + assert agent.model_settings.tool_choice == "auto" + + async def test_run_config_tool_choice_reset(self): + """Test that run_config.model_settings.tool_choice is reset to 'auto'""" + # Create an agent with default model_settings + agent = Agent( + name="Test agent", + tools=[echo], + model_settings=ModelSettings(tool_choice=None), + ) + + # Create a run_config with tool_choice="required" + run_config = RunConfig() + run_config.model_settings = ModelSettings(tool_choice="required") + + # Execute our code under test + processed_response = mock.MagicMock() + processed_response.functions = [mock.MagicMock()] # At least one function call + processed_response.computer_actions = [] + + # Execute our code under test + if processed_response.functions: + # Reset agent's model_settings + if agent.model_settings.tool_choice == "required" or isinstance(agent.model_settings.tool_choice, str): + agent.model_settings = ModelSettings( + temperature=agent.model_settings.temperature, + top_p=agent.model_settings.top_p, + frequency_penalty=agent.model_settings.frequency_penalty, + presence_penalty=agent.model_settings.presence_penalty, + tool_choice="auto", # Reset to auto + parallel_tool_calls=agent.model_settings.parallel_tool_calls, + truncation=agent.model_settings.truncation, + max_tokens=agent.model_settings.max_tokens, + ) + + # Also reset run_config's model_settings if it exists + if run_config.model_settings and (run_config.model_settings.tool_choice == "required" or + isinstance(run_config.model_settings.tool_choice, str)): + run_config.model_settings = ModelSettings( + temperature=run_config.model_settings.temperature, + top_p=run_config.model_settings.top_p, + frequency_penalty=run_config.model_settings.frequency_penalty, + presence_penalty=run_config.model_settings.presence_penalty, + tool_choice="auto", # Reset to auto + parallel_tool_calls=run_config.model_settings.parallel_tool_calls, + truncation=run_config.model_settings.truncation, + max_tokens=run_config.model_settings.max_tokens, + ) + + # Check that run_config's tool_choice was reset to "auto" + assert run_config.model_settings.tool_choice == "auto" + + @mock.patch("agents.run.Runner._get_model") + async def test_integration_prevents_infinite_loop(self, mock_get_model): + """Integration test to verify that tool_choice reset prevents infinite loops""" + # Create a counter to track model calls and detect potential infinite loops + tool_call_counter = {"count": 0, "potential_infinite_loop": False} + + # Set up our mock model that will always use tools when tool_choice is set + mock_model_instance = MockModel(tool_call_counter) + # Return our mock model directly + mock_get_model.return_value = mock_model_instance + + # Create an agent with tool_choice="required" to force tool usage + agent = Agent( + name="Test agent", + instructions="You are a test agent", + tools=[echo], + model_settings=ModelSettings(tool_choice="required"), + # Use "run_llm_again" to allow LLM to continue after tool calls + # This would cause infinite loops without the tool_choice reset + tool_use_behavior="run_llm_again", + ) + + # Set a timeout to catch potential infinite loops that our fix doesn't address + try: + # Run the agent with a timeout + async def run_with_timeout(): + return await Runner.run(agent, input="Test input") + + result = await asyncio.wait_for(run_with_timeout(), timeout=2.0) + + # Verify the agent ran successfully + assert result is not None + + # Verify the tool was called at least once but not too many times + # (indicating no infinite loop) + assert tool_call_counter["count"] >= 1 + assert tool_call_counter["count"] < 5 + assert not tool_call_counter["potential_infinite_loop"] + + except asyncio.TimeoutError: + # If we hit a timeout, the test failed - we likely have an infinite loop + assert False, "Timeout occurred, potential infinite loop detected" \ No newline at end of file From cde67f2b7194b974fef9e4f3fd9e0e4898f14ca4 Mon Sep 17 00:00:00 2001 From: xianghuijin Date: Thu, 20 Mar 2025 21:26:59 +0800 Subject: [PATCH 2/9] Rename test file for better clarity --- ..._tool_choice_reset.py => test_prevent_infinite_tool_loop.py} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename tests/{test_tool_choice_reset.py => test_prevent_infinite_tool_loop.py} (99%) diff --git a/tests/test_tool_choice_reset.py b/tests/test_prevent_infinite_tool_loop.py similarity index 99% rename from tests/test_tool_choice_reset.py rename to tests/test_prevent_infinite_tool_loop.py index e01a5f0d..c60a9514 100644 --- a/tests/test_tool_choice_reset.py +++ b/tests/test_prevent_infinite_tool_loop.py @@ -54,7 +54,7 @@ async def get_response(self, **kwargs): ) -class TestToolChoiceReset: +class TestPreventInfiniteToolLoop: async def test_tool_choice_resets_after_call(self): """Test that tool_choice is reset to 'auto' after tool call when set to 'required'""" # Create an agent with tool_choice="required" From 9384a0fb3fd13151c010d3f45c89bfcb05172784 Mon Sep 17 00:00:00 2001 From: xianghuijin Date: Thu, 20 Mar 2025 21:28:24 +0800 Subject: [PATCH 3/9] Revert test file name to match project naming style --- ..._prevent_infinite_tool_loop.py => test_tool_choice_reset.py} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename tests/{test_prevent_infinite_tool_loop.py => test_tool_choice_reset.py} (99%) diff --git a/tests/test_prevent_infinite_tool_loop.py b/tests/test_tool_choice_reset.py similarity index 99% rename from tests/test_prevent_infinite_tool_loop.py rename to tests/test_tool_choice_reset.py index c60a9514..e01a5f0d 100644 --- a/tests/test_prevent_infinite_tool_loop.py +++ b/tests/test_tool_choice_reset.py @@ -54,7 +54,7 @@ async def get_response(self, **kwargs): ) -class TestPreventInfiniteToolLoop: +class TestToolChoiceReset: async def test_tool_choice_resets_after_call(self): """Test that tool_choice is reset to 'auto' after tool call when set to 'required'""" # Create an agent with tool_choice="required" From d169d792885d07ba674d8f2c1bcc7be7f08d556b Mon Sep 17 00:00:00 2001 From: xianghuijin Date: Thu, 20 Mar 2025 21:49:38 +0800 Subject: [PATCH 4/9] Update documentation for tool_choice auto-reset feature --- docs/agents.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/agents.md b/docs/agents.md index 1c314739..c9c39aed 100644 --- a/docs/agents.md +++ b/docs/agents.md @@ -142,4 +142,6 @@ Supplying a list of tools doesn't always mean the LLM will use a tool. You can f !!! note - If requiring tool use, you should consider setting [`Agent.tool_use_behavior`] to stop the Agent from running when a tool output is produced. Otherwise, the Agent might run in an infinite loop, where the LLM produces a tool call , and the tool result is sent to the LLM, and this infinite loops because the LLM is always forced to use a tool. + To prevent infinite loops, the framework automatically resets `tool_choice` to "auto" after a tool call when it's set to "required" or a specific function name. This allows the model to decide whether to make additional tool calls in subsequent turns. + + If you want the Agent to completely stop after a tool call (rather than continuing with auto mode), you can set [`Agent.tool_use_behavior="stop_on_first_tool"`] which will directly use the tool output as the final response without further LLM processing. From bbcda753df84b008d79b1107ca5c7c2f908da206 Mon Sep 17 00:00:00 2001 From: xianghuijin Date: Sat, 22 Mar 2025 14:10:09 +0800 Subject: [PATCH 5/9] fix: optimize tool_choice reset logic and fix lint errors - Refactor tool_choice reset to target only problematic edge cases - Replace manual ModelSettings recreation with dataclasses.replace - Fix line length and error handling lint issues in tests --- src/agents/_run_impl.py | 63 +++++---- tests/test_tool_choice_reset.py | 221 +++++++++++++++----------------- 2 files changed, 137 insertions(+), 147 deletions(-) diff --git a/src/agents/_run_impl.py b/src/agents/_run_impl.py index a60ae1d6..1f896d75 100644 --- a/src/agents/_run_impl.py +++ b/src/agents/_run_impl.py @@ -1,6 +1,7 @@ from __future__ import annotations import asyncio +import dataclasses import inspect from collections.abc import Awaitable from dataclasses import dataclass @@ -51,7 +52,7 @@ from .models.interface import ModelTracing from .run_context import RunContextWrapper, TContext from .stream_events import RunItemStreamEvent, StreamEvent -from .tool import ComputerTool, FunctionTool, FunctionToolResult +from .tool import ComputerTool, FunctionTool, FunctionToolResult, Tool from .tracing import ( SpanError, Trace, @@ -208,34 +209,22 @@ async def execute_tools_and_side_effects( new_step_items.extend(computer_results) # Reset tool_choice to "auto" after tool execution to prevent infinite loops - if (processed_response.functions or processed_response.computer_actions): - # Reset agent's model_settings - if agent.model_settings.tool_choice == "required" or isinstance(agent.model_settings.tool_choice, str): - # Create a new model_settings to avoid modifying the original shared instance - agent.model_settings = ModelSettings( - temperature=agent.model_settings.temperature, - top_p=agent.model_settings.top_p, - frequency_penalty=agent.model_settings.frequency_penalty, - presence_penalty=agent.model_settings.presence_penalty, - tool_choice="auto", # Reset to auto - parallel_tool_calls=agent.model_settings.parallel_tool_calls, - truncation=agent.model_settings.truncation, - max_tokens=agent.model_settings.max_tokens, + if processed_response.functions or processed_response.computer_actions: + tools = agent.tools + # Only reset in the problematic scenarios where loops are likely unintentional + if cls._should_reset_tool_choice(agent.model_settings, tools): + agent.model_settings = dataclasses.replace( + agent.model_settings, + tool_choice="auto" ) - - # Also reset run_config's model_settings if it exists - if run_config.model_settings and (run_config.model_settings.tool_choice == "required" or - isinstance(run_config.model_settings.tool_choice, str)): - # Create a new model_settings for run_config - run_config.model_settings = ModelSettings( - temperature=run_config.model_settings.temperature, - top_p=run_config.model_settings.top_p, - frequency_penalty=run_config.model_settings.frequency_penalty, - presence_penalty=run_config.model_settings.presence_penalty, - tool_choice="auto", # Reset to auto - parallel_tool_calls=run_config.model_settings.parallel_tool_calls, - truncation=run_config.model_settings.truncation, - max_tokens=run_config.model_settings.max_tokens, + + if ( + run_config.model_settings and + cls._should_reset_tool_choice(run_config.model_settings, tools) + ): + run_config.model_settings = dataclasses.replace( + run_config.model_settings, + tool_choice="auto" ) # Second, check if there are any handoffs @@ -328,6 +317,24 @@ async def execute_tools_and_side_effects( next_step=NextStepRunAgain(), ) + @classmethod + def _should_reset_tool_choice(cls, model_settings: ModelSettings, tools: list[Tool]) -> bool: + if model_settings is None or model_settings.tool_choice is None: + return False + + # for specific tool choices + if ( + isinstance(model_settings.tool_choice, str) and + model_settings.tool_choice not in ["auto", "required", "none"] + ): + return True + + # for one tool and required tool choice + if model_settings.tool_choice == "required": + return len(tools) == 1 + + return False + @classmethod def process_model_response( cls, diff --git a/tests/test_tool_choice_reset.py b/tests/test_tool_choice_reset.py index e01a5f0d..b47c4d97 100644 --- a/tests/test_tool_choice_reset.py +++ b/tests/test_tool_choice_reset.py @@ -1,13 +1,15 @@ -from unittest import mock import asyncio +import dataclasses import json -from typing import List +from unittest import mock -from agents import Agent, ModelSettings, RunConfig, function_tool, Runner -from agents.models.interface import ModelResponse -from agents.items import Usage from openai.types.responses.response_function_tool_call import ResponseFunctionToolCall +from agents import Agent, ModelSettings, RunConfig, Runner, function_tool +from agents.items import Usage +from agents.models.interface import ModelResponse +from agents.tool import Tool + @function_tool def echo(text: str) -> str: @@ -15,22 +17,39 @@ def echo(text: str) -> str: return text +def should_reset_tool_choice(model_settings: ModelSettings, tools: list[Tool]) -> bool: + if model_settings is None or model_settings.tool_choice is None: + return False + + # for specific tool choices + if ( + isinstance(model_settings.tool_choice, str) and + model_settings.tool_choice not in ["auto", "required", "none"] + ): + return True + + # for one tool and required tool choice + if model_settings.tool_choice == "required": + return len(tools) == 1 + + return False + # Mock model implementation that always calls tools when tool_choice is set class MockModel: def __init__(self, tool_call_counter): self.tool_call_counter = tool_call_counter - + async def get_response(self, **kwargs): tools = kwargs.get("tools", []) model_settings = kwargs.get("model_settings") - + # Increment the counter to track how many times this model is called self.tool_call_counter["count"] += 1 - + # If we've been called many times, we're likely in an infinite loop if self.tool_call_counter["count"] > 5: self.tool_call_counter["potential_infinite_loop"] = True - + # Always create a tool call if tool_choice is required/specific tool_calls = [] if model_settings and model_settings.tool_choice: @@ -46,7 +65,7 @@ async def get_response(self, **kwargs): type="function_call", ) ) - + return ModelResponse( output=tool_calls, referenceable_id="123", @@ -60,7 +79,7 @@ async def test_tool_choice_resets_after_call(self): # Create an agent with tool_choice="required" agent = Agent( name="Test agent", - tools=[echo], + tools=[echo], # Only one tool model_settings=ModelSettings(tool_choice="required"), ) @@ -77,31 +96,22 @@ async def test_tool_choice_resets_after_call(self): # Execute our code under test if processed_response.functions: - # Reset agent's model_settings - if agent.model_settings.tool_choice == "required" or isinstance(agent.model_settings.tool_choice, str): - agent.model_settings = ModelSettings( - temperature=agent.model_settings.temperature, - top_p=agent.model_settings.top_p, - frequency_penalty=agent.model_settings.frequency_penalty, - presence_penalty=agent.model_settings.presence_penalty, - tool_choice="auto", # Reset to auto - parallel_tool_calls=agent.model_settings.parallel_tool_calls, - truncation=agent.model_settings.truncation, - max_tokens=agent.model_settings.max_tokens, + # Apply the targeted reset logic + tools = agent.tools + if should_reset_tool_choice(agent.model_settings, tools): + agent.model_settings = dataclasses.replace( + agent.model_settings, + tool_choice="auto" # Reset to auto ) - + # Also reset run_config's model_settings if it exists - if run_config.model_settings and (run_config.model_settings.tool_choice == "required" or - isinstance(run_config.model_settings.tool_choice, str)): - run_config.model_settings = ModelSettings( - temperature=run_config.model_settings.temperature, - top_p=run_config.model_settings.top_p, - frequency_penalty=run_config.model_settings.frequency_penalty, - presence_penalty=run_config.model_settings.presence_penalty, - tool_choice="auto", # Reset to auto - parallel_tool_calls=run_config.model_settings.parallel_tool_calls, - truncation=run_config.model_settings.truncation, - max_tokens=run_config.model_settings.max_tokens, + if ( + run_config.model_settings and + should_reset_tool_choice(run_config.model_settings, tools) + ): + run_config.model_settings = dataclasses.replace( + run_config.model_settings, + tool_choice="auto" # Reset to auto ) # Check that tool_choice was reset to "auto" @@ -115,7 +125,7 @@ async def test_tool_choice_resets_from_specific_function(self): instructions="You are a test agent", tools=[echo], model="gpt-4-0125-preview", - model_settings=ModelSettings(tool_choice="echo"), + model_settings=ModelSettings(tool_choice="echo"), # Specific function name ) # Execute our code under test @@ -129,31 +139,22 @@ async def test_tool_choice_resets_from_specific_function(self): # Execute our code under test if processed_response.functions: - # Reset agent's model_settings - if agent.model_settings.tool_choice == "required" or isinstance(agent.model_settings.tool_choice, str): - agent.model_settings = ModelSettings( - temperature=agent.model_settings.temperature, - top_p=agent.model_settings.top_p, - frequency_penalty=agent.model_settings.frequency_penalty, - presence_penalty=agent.model_settings.presence_penalty, - tool_choice="auto", # Reset to auto - parallel_tool_calls=agent.model_settings.parallel_tool_calls, - truncation=agent.model_settings.truncation, - max_tokens=agent.model_settings.max_tokens, + # Apply the targeted reset logic + tools = agent.tools + if should_reset_tool_choice(agent.model_settings, tools): + agent.model_settings = dataclasses.replace( + agent.model_settings, + tool_choice="auto" # Reset to auto ) - + # Also reset run_config's model_settings if it exists - if run_config.model_settings and (run_config.model_settings.tool_choice == "required" or - isinstance(run_config.model_settings.tool_choice, str)): - run_config.model_settings = ModelSettings( - temperature=run_config.model_settings.temperature, - top_p=run_config.model_settings.top_p, - frequency_penalty=run_config.model_settings.frequency_penalty, - presence_penalty=run_config.model_settings.presence_penalty, - tool_choice="auto", # Reset to auto - parallel_tool_calls=run_config.model_settings.parallel_tool_calls, - truncation=run_config.model_settings.truncation, - max_tokens=run_config.model_settings.max_tokens, + if ( + run_config.model_settings and + should_reset_tool_choice(run_config.model_settings, tools) + ): + run_config.model_settings = dataclasses.replace( + run_config.model_settings, + tool_choice="auto" # Reset to auto ) # Check that tool_choice was reset to "auto" @@ -179,49 +180,40 @@ async def test_tool_choice_no_reset_when_auto(self): # Execute our code under test if processed_response.functions: - # Reset agent's model_settings - if agent.model_settings.tool_choice == "required" or isinstance(agent.model_settings.tool_choice, str): - agent.model_settings = ModelSettings( - temperature=agent.model_settings.temperature, - top_p=agent.model_settings.top_p, - frequency_penalty=agent.model_settings.frequency_penalty, - presence_penalty=agent.model_settings.presence_penalty, - tool_choice="auto", # Reset to auto - parallel_tool_calls=agent.model_settings.parallel_tool_calls, - truncation=agent.model_settings.truncation, - max_tokens=agent.model_settings.max_tokens, + # Apply the targeted reset logic + tools = agent.tools + if should_reset_tool_choice(agent.model_settings, tools): + agent.model_settings = dataclasses.replace( + agent.model_settings, + tool_choice="auto" # Reset to auto ) - + # Also reset run_config's model_settings if it exists - if run_config.model_settings and (run_config.model_settings.tool_choice == "required" or - isinstance(run_config.model_settings.tool_choice, str)): - run_config.model_settings = ModelSettings( - temperature=run_config.model_settings.temperature, - top_p=run_config.model_settings.top_p, - frequency_penalty=run_config.model_settings.frequency_penalty, - presence_penalty=run_config.model_settings.presence_penalty, - tool_choice="auto", # Reset to auto - parallel_tool_calls=run_config.model_settings.parallel_tool_calls, - truncation=run_config.model_settings.truncation, - max_tokens=run_config.model_settings.max_tokens, + if ( + run_config.model_settings and + should_reset_tool_choice(run_config.model_settings, tools) + ): + run_config.model_settings = dataclasses.replace( + run_config.model_settings, + tool_choice="auto" # Reset to auto ) # Check that tool_choice remains "auto" assert agent.model_settings.tool_choice == "auto" - + async def test_run_config_tool_choice_reset(self): """Test that run_config.model_settings.tool_choice is reset to 'auto'""" # Create an agent with default model_settings agent = Agent( name="Test agent", - tools=[echo], + tools=[echo], # Only one tool model_settings=ModelSettings(tool_choice=None), ) - + # Create a run_config with tool_choice="required" run_config = RunConfig() run_config.model_settings = ModelSettings(tool_choice="required") - + # Execute our code under test processed_response = mock.MagicMock() processed_response.functions = [mock.MagicMock()] # At least one function call @@ -229,47 +221,38 @@ async def test_run_config_tool_choice_reset(self): # Execute our code under test if processed_response.functions: - # Reset agent's model_settings - if agent.model_settings.tool_choice == "required" or isinstance(agent.model_settings.tool_choice, str): - agent.model_settings = ModelSettings( - temperature=agent.model_settings.temperature, - top_p=agent.model_settings.top_p, - frequency_penalty=agent.model_settings.frequency_penalty, - presence_penalty=agent.model_settings.presence_penalty, - tool_choice="auto", # Reset to auto - parallel_tool_calls=agent.model_settings.parallel_tool_calls, - truncation=agent.model_settings.truncation, - max_tokens=agent.model_settings.max_tokens, + # Apply the targeted reset logic + tools = agent.tools + if should_reset_tool_choice(agent.model_settings, tools): + agent.model_settings = dataclasses.replace( + agent.model_settings, + tool_choice="auto" # Reset to auto ) - + # Also reset run_config's model_settings if it exists - if run_config.model_settings and (run_config.model_settings.tool_choice == "required" or - isinstance(run_config.model_settings.tool_choice, str)): - run_config.model_settings = ModelSettings( - temperature=run_config.model_settings.temperature, - top_p=run_config.model_settings.top_p, - frequency_penalty=run_config.model_settings.frequency_penalty, - presence_penalty=run_config.model_settings.presence_penalty, - tool_choice="auto", # Reset to auto - parallel_tool_calls=run_config.model_settings.parallel_tool_calls, - truncation=run_config.model_settings.truncation, - max_tokens=run_config.model_settings.max_tokens, + if ( + run_config.model_settings and + should_reset_tool_choice(run_config.model_settings, tools) + ): + run_config.model_settings = dataclasses.replace( + run_config.model_settings, + tool_choice="auto" # Reset to auto ) - + # Check that run_config's tool_choice was reset to "auto" assert run_config.model_settings.tool_choice == "auto" - + @mock.patch("agents.run.Runner._get_model") async def test_integration_prevents_infinite_loop(self, mock_get_model): """Integration test to verify that tool_choice reset prevents infinite loops""" # Create a counter to track model calls and detect potential infinite loops tool_call_counter = {"count": 0, "potential_infinite_loop": False} - + # Set up our mock model that will always use tools when tool_choice is set mock_model_instance = MockModel(tool_call_counter) # Return our mock model directly mock_get_model.return_value = mock_model_instance - + # Create an agent with tool_choice="required" to force tool usage agent = Agent( name="Test agent", @@ -280,24 +263,24 @@ async def test_integration_prevents_infinite_loop(self, mock_get_model): # This would cause infinite loops without the tool_choice reset tool_use_behavior="run_llm_again", ) - + # Set a timeout to catch potential infinite loops that our fix doesn't address try: # Run the agent with a timeout async def run_with_timeout(): return await Runner.run(agent, input="Test input") - + result = await asyncio.wait_for(run_with_timeout(), timeout=2.0) - + # Verify the agent ran successfully assert result is not None - + # Verify the tool was called at least once but not too many times # (indicating no infinite loop) assert tool_call_counter["count"] >= 1 assert tool_call_counter["count"] < 5 assert not tool_call_counter["potential_infinite_loop"] - - except asyncio.TimeoutError: + + except asyncio.TimeoutError as err: # If we hit a timeout, the test failed - we likely have an infinite loop - assert False, "Timeout occurred, potential infinite loop detected" \ No newline at end of file + raise AssertionError("Timeout occurred, potential infinite loop detected") from err From 8f2f76cb65b340a0371021f98a3bf9276a5b8cf1 Mon Sep 17 00:00:00 2001 From: xianghuijin Date: Sat, 22 Mar 2025 14:22:47 +0800 Subject: [PATCH 6/9] docs: Update tool_choice reset documentation to match implementation --- docs/agents.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/docs/agents.md b/docs/agents.md index c9c39aed..1e04f7e9 100644 --- a/docs/agents.md +++ b/docs/agents.md @@ -142,6 +142,11 @@ Supplying a list of tools doesn't always mean the LLM will use a tool. You can f !!! note - To prevent infinite loops, the framework automatically resets `tool_choice` to "auto" after a tool call when it's set to "required" or a specific function name. This allows the model to decide whether to make additional tool calls in subsequent turns. + To prevent infinite loops, the framework automatically resets `tool_choice` to "auto" after a tool call in the following scenarios: + + 1. When `tool_choice` is set to a specific function name (any string that's not "auto", "required", or "none") + 2. When `tool_choice` is set to "required" AND there is only one tool available + + This targeted reset mechanism allows the model to decide whether to make additional tool calls in subsequent turns while avoiding infinite loops in these specific cases. If you want the Agent to completely stop after a tool call (rather than continuing with auto mode), you can set [`Agent.tool_use_behavior="stop_on_first_tool"`] which will directly use the tool output as the final response without further LLM processing. From 6ed0bee67242032f2a82c3ca133e6571e2a73866 Mon Sep 17 00:00:00 2001 From: xianghuijin Date: Sun, 23 Mar 2025 17:20:23 +0800 Subject: [PATCH 7/9] fix: prevent modifying the original agent's model_settings This fixes the issue where the original agent's model_settings was being directly modified during the tool choice reset process. The original implementation caused the agent's tool_choice to unintentionally reset to "auto" for subsequent runs, which could be unexpected behavior. The fix creates new copies of the agent and model settings objects using dataclasses.replace() instead of modifying the original objects. This ensures that the tool choice reset is limited to the current run only, maintaining the expected behavior for sequential runs with the same agent. Addresses feedback from @baderalfahad about the agent instance being modified when it should maintain its original state between runs. --- src/agents/_run_impl.py | 10 +- tests/test_tool_choice_reset.py | 405 +++++++++++--------------------- 2 files changed, 148 insertions(+), 267 deletions(-) diff --git a/src/agents/_run_impl.py b/src/agents/_run_impl.py index 1f896d75..1370462c 100644 --- a/src/agents/_run_impl.py +++ b/src/agents/_run_impl.py @@ -213,19 +213,25 @@ async def execute_tools_and_side_effects( tools = agent.tools # Only reset in the problematic scenarios where loops are likely unintentional if cls._should_reset_tool_choice(agent.model_settings, tools): - agent.model_settings = dataclasses.replace( + # Create a modified copy instead of modifying the original agent + new_model_settings = dataclasses.replace( agent.model_settings, tool_choice="auto" ) + # Create a new internal agent with updated settings + agent = dataclasses.replace(agent, model_settings=new_model_settings) if ( run_config.model_settings and cls._should_reset_tool_choice(run_config.model_settings, tools) ): - run_config.model_settings = dataclasses.replace( + # Also update the run_config model settings with a copy + new_run_config_settings = dataclasses.replace( run_config.model_settings, tool_choice="auto" ) + # Create a new run_config with the new settings + run_config = dataclasses.replace(run_config, model_settings=new_run_config_settings) # Second, check if there are any handoffs if run_handoffs := processed_response.handoffs: diff --git a/tests/test_tool_choice_reset.py b/tests/test_tool_choice_reset.py index b47c4d97..7dae6f63 100644 --- a/tests/test_tool_choice_reset.py +++ b/tests/test_tool_choice_reset.py @@ -1,286 +1,161 @@ -import asyncio -import dataclasses -import json -from unittest import mock +import pytest -from openai.types.responses.response_function_tool_call import ResponseFunctionToolCall +from agents import Agent, ModelSettings, Runner, Tool +from agents._run_impl import RunImpl -from agents import Agent, ModelSettings, RunConfig, Runner, function_tool -from agents.items import Usage -from agents.models.interface import ModelResponse -from agents.tool import Tool - - -@function_tool -def echo(text: str) -> str: - """Echo the input text""" - return text - - -def should_reset_tool_choice(model_settings: ModelSettings, tools: list[Tool]) -> bool: - if model_settings is None or model_settings.tool_choice is None: - return False - - # for specific tool choices - if ( - isinstance(model_settings.tool_choice, str) and - model_settings.tool_choice not in ["auto", "required", "none"] - ): - return True - - # for one tool and required tool choice - if model_settings.tool_choice == "required": - return len(tools) == 1 - - return False - -# Mock model implementation that always calls tools when tool_choice is set -class MockModel: - def __init__(self, tool_call_counter): - self.tool_call_counter = tool_call_counter - - async def get_response(self, **kwargs): - tools = kwargs.get("tools", []) - model_settings = kwargs.get("model_settings") - - # Increment the counter to track how many times this model is called - self.tool_call_counter["count"] += 1 - - # If we've been called many times, we're likely in an infinite loop - if self.tool_call_counter["count"] > 5: - self.tool_call_counter["potential_infinite_loop"] = True - - # Always create a tool call if tool_choice is required/specific - tool_calls = [] - if model_settings and model_settings.tool_choice: - if model_settings.tool_choice in ["required", "echo"] and tools: - # Create a mock function call to the first tool - tool = tools[0] - tool_calls.append( - ResponseFunctionToolCall( - id="call_1", - name=tool.name, - arguments=json.dumps({"text": "This is a test"}), - call_id="call_1", - type="function_call", - ) - ) - - return ModelResponse( - output=tool_calls, - referenceable_id="123", - usage=Usage(input_tokens=10, output_tokens=10, total_tokens=20), - ) +from .fake_model import FakeModel +from .test_responses import ( + get_function_tool, + get_function_tool_call, + get_text_message, +) class TestToolChoiceReset: - async def test_tool_choice_resets_after_call(self): - """Test that tool_choice is reset to 'auto' after tool call when set to 'required'""" - # Create an agent with tool_choice="required" - agent = Agent( - name="Test agent", - tools=[echo], # Only one tool - model_settings=ModelSettings(tool_choice="required"), - ) - - # Directly modify the model_settings - # Instead of trying to run the full execute_tools_and_side_effects, - # we'll just test the tool_choice reset logic directly - processed_response = mock.MagicMock() - processed_response.functions = [mock.MagicMock()] # At least one function call - processed_response.computer_actions = [] - - # Create a mock run_config - run_config = mock.MagicMock() - run_config.model_settings = None - - # Execute our code under test - if processed_response.functions: - # Apply the targeted reset logic - tools = agent.tools - if should_reset_tool_choice(agent.model_settings, tools): - agent.model_settings = dataclasses.replace( - agent.model_settings, - tool_choice="auto" # Reset to auto - ) - - # Also reset run_config's model_settings if it exists - if ( - run_config.model_settings and - should_reset_tool_choice(run_config.model_settings, tools) - ): - run_config.model_settings = dataclasses.replace( - run_config.model_settings, - tool_choice="auto" # Reset to auto - ) - - # Check that tool_choice was reset to "auto" - assert agent.model_settings.tool_choice == "auto" - async def test_tool_choice_resets_from_specific_function(self): - """Test tool_choice reset to 'auto' after call when set to specific function name""" - # Create an agent with tool_choice set to a specific function + def test_should_reset_tool_choice_direct(self): + """ + Test the _should_reset_tool_choice method directly with various inputs + to ensure it correctly identifies cases where reset is needed. + """ + # Case 1: tool_choice = None should not reset + model_settings = ModelSettings(tool_choice=None) + tools1: list[Tool] = [get_function_tool("tool1")] + # Cast to list[Tool] to fix type checking issues + assert not RunImpl._should_reset_tool_choice(model_settings, tools1) + + # Case 2: tool_choice = "auto" should not reset + model_settings = ModelSettings(tool_choice="auto") + assert not RunImpl._should_reset_tool_choice(model_settings, tools1) + + # Case 3: tool_choice = "none" should not reset + model_settings = ModelSettings(tool_choice="none") + assert not RunImpl._should_reset_tool_choice(model_settings, tools1) + + # Case 4: tool_choice = "required" with one tool should reset + model_settings = ModelSettings(tool_choice="required") + assert RunImpl._should_reset_tool_choice(model_settings, tools1) + + # Case 5: tool_choice = "required" with multiple tools should not reset + model_settings = ModelSettings(tool_choice="required") + tools2: list[Tool] = [get_function_tool("tool1"), get_function_tool("tool2")] + assert not RunImpl._should_reset_tool_choice(model_settings, tools2) + + # Case 6: Specific tool choice should reset + model_settings = ModelSettings(tool_choice="specific_tool") + assert RunImpl._should_reset_tool_choice(model_settings, tools1) + + @pytest.mark.asyncio + async def test_required_tool_choice_with_multiple_runs(self): + """ + Test scenario 1: When multiple runs are executed with tool_choice="required" + Ensure each run works correctly and doesn't get stuck in infinite loop + Also verify that tool_choice remains "required" between runs + """ + # Set up our fake model with responses for two runs + fake_model = FakeModel() + fake_model.add_multiple_turn_outputs([ + [get_text_message("First run response")], + [get_text_message("Second run response")] + ]) + + # Create agent with a custom tool and tool_choice="required" + custom_tool = get_function_tool("custom_tool") agent = Agent( - name="Test agent", - instructions="You are a test agent", - tools=[echo], - model="gpt-4-0125-preview", - model_settings=ModelSettings(tool_choice="echo"), # Specific function name + name="test_agent", + model=fake_model, + tools=[custom_tool], + model_settings=ModelSettings(tool_choice="required"), ) - # Execute our code under test - processed_response = mock.MagicMock() - processed_response.functions = [mock.MagicMock()] # At least one function call - processed_response.computer_actions = [] - - # Create a mock run_config - run_config = mock.MagicMock() - run_config.model_settings = None - - # Execute our code under test - if processed_response.functions: - # Apply the targeted reset logic - tools = agent.tools - if should_reset_tool_choice(agent.model_settings, tools): - agent.model_settings = dataclasses.replace( - agent.model_settings, - tool_choice="auto" # Reset to auto - ) + # First run should work correctly and preserve tool_choice + result1 = await Runner.run(agent, "first run") + assert result1.final_output == "First run response" + assert agent.model_settings.tool_choice == "required", "tool_choice should stay required" + + # Second run should also work correctly with tool_choice still required + result2 = await Runner.run(agent, "second run") + assert result2.final_output == "Second run response" + assert agent.model_settings.tool_choice == "required", "tool_choice should stay required" + + @pytest.mark.asyncio + async def test_required_with_stop_at_tool_name(self): + """ + Test scenario 2: When using required tool_choice with stop_at_tool_names behavior + Ensure it correctly stops at the specified tool + """ + # Set up fake model to return a tool call for second_tool + fake_model = FakeModel() + fake_model.set_next_output([ + get_function_tool_call("second_tool", "{}") + ]) + + # Create agent with two tools and tool_choice="required" and stop_at_tool behavior + first_tool = get_function_tool("first_tool", return_value="first tool result") + second_tool = get_function_tool("second_tool", return_value="second tool result") - # Also reset run_config's model_settings if it exists - if ( - run_config.model_settings and - should_reset_tool_choice(run_config.model_settings, tools) - ): - run_config.model_settings = dataclasses.replace( - run_config.model_settings, - tool_choice="auto" # Reset to auto - ) - - # Check that tool_choice was reset to "auto" - assert agent.model_settings.tool_choice == "auto" - - async def test_tool_choice_no_reset_when_auto(self): - """Test that tool_choice is not changed when it's already set to 'auto'""" - # Create an agent with tool_choice="auto" agent = Agent( - name="Test agent", - tools=[echo], - model_settings=ModelSettings(tool_choice="auto"), + name="test_agent", + model=fake_model, + tools=[first_tool, second_tool], + model_settings=ModelSettings(tool_choice="required"), + tool_use_behavior={"stop_at_tool_names": ["second_tool"]}, ) - # Execute our code under test - processed_response = mock.MagicMock() - processed_response.functions = [mock.MagicMock()] # At least one function call - processed_response.computer_actions = [] + # Run should stop after using second_tool + result = await Runner.run(agent, "run test") + assert result.final_output == "second tool result" + + @pytest.mark.asyncio + async def test_specific_tool_choice(self): + """ + Test scenario 3: When using a specific tool choice name + Ensure it doesn't cause infinite loops + """ + # Set up fake model to return a text message + fake_model = FakeModel() + fake_model.set_next_output([get_text_message("Test message")]) + + # Create agent with specific tool_choice + tool1 = get_function_tool("tool1") + tool2 = get_function_tool("tool2") + tool3 = get_function_tool("tool3") - # Create a mock run_config - run_config = mock.MagicMock() - run_config.model_settings = None - - # Execute our code under test - if processed_response.functions: - # Apply the targeted reset logic - tools = agent.tools - if should_reset_tool_choice(agent.model_settings, tools): - agent.model_settings = dataclasses.replace( - agent.model_settings, - tool_choice="auto" # Reset to auto - ) - - # Also reset run_config's model_settings if it exists - if ( - run_config.model_settings and - should_reset_tool_choice(run_config.model_settings, tools) - ): - run_config.model_settings = dataclasses.replace( - run_config.model_settings, - tool_choice="auto" # Reset to auto - ) - - # Check that tool_choice remains "auto" - assert agent.model_settings.tool_choice == "auto" - - async def test_run_config_tool_choice_reset(self): - """Test that run_config.model_settings.tool_choice is reset to 'auto'""" - # Create an agent with default model_settings agent = Agent( - name="Test agent", - tools=[echo], # Only one tool - model_settings=ModelSettings(tool_choice=None), + name="test_agent", + model=fake_model, + tools=[tool1, tool2, tool3], + model_settings=ModelSettings(tool_choice="tool1"), # Specific tool ) - # Create a run_config with tool_choice="required" - run_config = RunConfig() - run_config.model_settings = ModelSettings(tool_choice="required") - - # Execute our code under test - processed_response = mock.MagicMock() - processed_response.functions = [mock.MagicMock()] # At least one function call - processed_response.computer_actions = [] - - # Execute our code under test - if processed_response.functions: - # Apply the targeted reset logic - tools = agent.tools - if should_reset_tool_choice(agent.model_settings, tools): - agent.model_settings = dataclasses.replace( - agent.model_settings, - tool_choice="auto" # Reset to auto - ) - - # Also reset run_config's model_settings if it exists - if ( - run_config.model_settings and - should_reset_tool_choice(run_config.model_settings, tools) - ): - run_config.model_settings = dataclasses.replace( - run_config.model_settings, - tool_choice="auto" # Reset to auto - ) - - # Check that run_config's tool_choice was reset to "auto" - assert run_config.model_settings.tool_choice == "auto" - - @mock.patch("agents.run.Runner._get_model") - async def test_integration_prevents_infinite_loop(self, mock_get_model): - """Integration test to verify that tool_choice reset prevents infinite loops""" - # Create a counter to track model calls and detect potential infinite loops - tool_call_counter = {"count": 0, "potential_infinite_loop": False} - - # Set up our mock model that will always use tools when tool_choice is set - mock_model_instance = MockModel(tool_call_counter) - # Return our mock model directly - mock_get_model.return_value = mock_model_instance - - # Create an agent with tool_choice="required" to force tool usage + # Run should complete without infinite loops + result = await Runner.run(agent, "first run") + assert result.final_output == "Test message" + + @pytest.mark.asyncio + async def test_required_with_single_tool(self): + """ + Test scenario 4: When using required tool_choice with only one tool + Ensure it doesn't cause infinite loops + """ + # Set up fake model to return a tool call followed by a text message + fake_model = FakeModel() + fake_model.add_multiple_turn_outputs([ + # First call returns a tool call + [get_function_tool_call("custom_tool", "{}")], + # Second call returns a text message + [get_text_message("Final response")] + ]) + + # Create agent with a single tool and tool_choice="required" + custom_tool = get_function_tool("custom_tool", return_value="tool result") agent = Agent( - name="Test agent", - instructions="You are a test agent", - tools=[echo], + name="test_agent", + model=fake_model, + tools=[custom_tool], model_settings=ModelSettings(tool_choice="required"), - # Use "run_llm_again" to allow LLM to continue after tool calls - # This would cause infinite loops without the tool_choice reset - tool_use_behavior="run_llm_again", ) - # Set a timeout to catch potential infinite loops that our fix doesn't address - try: - # Run the agent with a timeout - async def run_with_timeout(): - return await Runner.run(agent, input="Test input") - - result = await asyncio.wait_for(run_with_timeout(), timeout=2.0) - - # Verify the agent ran successfully - assert result is not None - - # Verify the tool was called at least once but not too many times - # (indicating no infinite loop) - assert tool_call_counter["count"] >= 1 - assert tool_call_counter["count"] < 5 - assert not tool_call_counter["potential_infinite_loop"] - - except asyncio.TimeoutError as err: - # If we hit a timeout, the test failed - we likely have an infinite loop - raise AssertionError("Timeout occurred, potential infinite loop detected") from err + # Run should complete without infinite loops + result = await Runner.run(agent, "first run") + assert result.final_output == "Final response" From 0c747af743ffd7cc4128e94e84496afad7743360 Mon Sep 17 00:00:00 2001 From: xianghuijin Date: Sun, 23 Mar 2025 17:29:48 +0800 Subject: [PATCH 8/9] refactor: streamline tool_choice reset logic This update moves the tool_choice reset logic to a more appropriate location within the RunImpl class, ensuring that the original agent's model_settings remains unmodified during the reset process. The logic now checks for problematic scenarios before creating a modified copy of the agent's settings, maintaining expected behavior across sequential runs. This change enhances clarity and efficiency in handling tool choices. Addresses previous feedback regarding the modification of the agent instance and improves the overall structure of the reset logic. --- src/agents/_run_impl.py | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/agents/_run_impl.py b/src/agents/_run_impl.py index 1370462c..02725200 100644 --- a/src/agents/_run_impl.py +++ b/src/agents/_run_impl.py @@ -211,15 +211,6 @@ async def execute_tools_and_side_effects( # Reset tool_choice to "auto" after tool execution to prevent infinite loops if processed_response.functions or processed_response.computer_actions: tools = agent.tools - # Only reset in the problematic scenarios where loops are likely unintentional - if cls._should_reset_tool_choice(agent.model_settings, tools): - # Create a modified copy instead of modifying the original agent - new_model_settings = dataclasses.replace( - agent.model_settings, - tool_choice="auto" - ) - # Create a new internal agent with updated settings - agent = dataclasses.replace(agent, model_settings=new_model_settings) if ( run_config.model_settings and @@ -233,6 +224,16 @@ async def execute_tools_and_side_effects( # Create a new run_config with the new settings run_config = dataclasses.replace(run_config, model_settings=new_run_config_settings) + # Only reset in the problematic scenarios where loops are likely unintentional + if cls._should_reset_tool_choice(agent.model_settings, tools): + # Create a modified copy instead of modifying the original agent + new_model_settings = dataclasses.replace( + agent.model_settings, + tool_choice="auto" + ) + # Create a new internal agent with updated settings + agent = dataclasses.replace(agent, model_settings=new_model_settings) + # Second, check if there are any handoffs if run_handoffs := processed_response.handoffs: return await cls.execute_handoffs( From 07a4af1fe20c7068df59529b9c4f40316a9b1930 Mon Sep 17 00:00:00 2001 From: xianghuijin Date: Sun, 23 Mar 2025 20:41:18 +0800 Subject: [PATCH 9/9] refactor: improve comments for clarity in tool_choice reset logic --- src/agents/_run_impl.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/agents/_run_impl.py b/src/agents/_run_impl.py index 02725200..df3db42f 100644 --- a/src/agents/_run_impl.py +++ b/src/agents/_run_impl.py @@ -216,22 +216,19 @@ async def execute_tools_and_side_effects( run_config.model_settings and cls._should_reset_tool_choice(run_config.model_settings, tools) ): - # Also update the run_config model settings with a copy + # update the run_config model settings with a copy new_run_config_settings = dataclasses.replace( run_config.model_settings, tool_choice="auto" ) - # Create a new run_config with the new settings run_config = dataclasses.replace(run_config, model_settings=new_run_config_settings) - # Only reset in the problematic scenarios where loops are likely unintentional if cls._should_reset_tool_choice(agent.model_settings, tools): # Create a modified copy instead of modifying the original agent new_model_settings = dataclasses.replace( agent.model_settings, tool_choice="auto" ) - # Create a new internal agent with updated settings agent = dataclasses.replace(agent, model_settings=new_model_settings) # Second, check if there are any handoffs