-
Notifications
You must be signed in to change notification settings - Fork 746
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix potential infinite tool call loop by resetting tool_choice after … #263
base: main
Are you sure you want to change the base?
Conversation
This is a good idea! What do you think about making it a configurable param, default to |
@rm-openai Thanks for the feedback! I considered adding a config parameter, but wonder if it might add complexity without clear use cases. Most users would want to prevent infinite loops by default, and those with specific needs could already implement custom behaviors through the existing API. Unless you have specific scenarios in mind where maintaining forced tool calls is beneficial, perhaps the simpler approach is better? |
@mini-peanut, yeah one use case I had in mind was this: Setup:
If we reset Thoughts? |
@rm-openai Thanks for sharing that use case. I'd like to refine my approach to focus on the specific problem we're solving. The Problem: Setting Core Hypothesis: When a user forces a single specific function call, they rarely intend for that same function to be repeatedly called in an infinite loop. This differs from intentional sequential calling of different functions. Problem Scenario: This issue typically manifests in two specific cases:
Concerns with Adding a Configuration Parameter: Targeted Solution: We can address these specific scenarios without disrupting legitimate use cases: # Only reset in the problematic scenarios where loops are likely unintentional
if (isinstance(tool_choice, str) and tool_choice not in ["auto", "required", "none"]) or
(tool_choice == "required" and len(tools) == 1):
# Reset to "auto" This approach precisely targets the infinite loop problem without affecting the multi-tool sequential calling pattern you described, and without requiring additional configuration. |
lgtm - but would you mind fixing lint/typechecking please? can't merge without that |
- 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
@rm-openai Fixed, and the code should pass the checks. Thanks for your patience |
Fix potential infinite tool call loop by resetting tool_choice after tool execution
Summary
This PR fixes an issue where setting
tool_choice
to "required" or a specific function name could cause models to get stuck in an infinite tool call loop.When
tool_choice
is set to force tool usage, this setting persists across model invocations. This PR automatically resetstool_choice
to "auto" after tool execution, allowing the model to decide whether to make additional tool calls in subsequent turns.Unlike using
tool_use_behavior="stop_on_first_tool"
, this approach lets the model continue processing tool results while preventing forced repeated tool calls.Test plan
Checks
make lint
andmake format