Skip to content

Conversation

@geodavic
Copy link

Currently if an unknown tool name is provided by the model, a ModelRetry(f'Unknown tool name: {name!r}. {msg}') is raised and subsequently it will move to the retry step here. In this step, however, the max_retries will always be 1 because the tool name is unknown (i.e.tool is None). This causes the UnexpectedModelBehavior(f'Tool {name!r} exceeded max retries count of {max_retries}') to be raised, thus stopping the run.

It would be nice if there was retry logic on unknown tools, so that the model can correct itself if it accidentally returns a malformed tool name (I've seen this happen, for example, with gpt-oss-20b).

This PR adds an unknown_tool_retries parameter for this purpose.

@geodavic
Copy link
Author

geodavic commented Nov 29, 2025

One open question is: if the user doesn't pass unknown_tool_retries, should it default to retries? Or default to 1?

Currently it is set to default to 1 so as not to change existing behavior. But I think I prefer the UX of defaulting to retries (feels more correct).

@DouweM
Copy link
Collaborator

DouweM commented Dec 1, 2025

however, the max_retries will always be 1 because the tool name is unknown (i.e.tool is None). This causes the UnexpectedModelBehavior(f'Tool {name!r} exceeded max retries count of {max_retries}') to be raised, thus stopping the run.

@geodavic Isn't this only the case if the model calls the same non-existent tool twice? Because max_retries will start as 1 and current_retry as 0.

But I think I prefer the UX of defaulting to retries (feels more correct).

I would prefer not to have a new setting and to inherit the existing default tool retries instead.

@geodavic
Copy link
Author

geodavic commented Dec 1, 2025

@DouweM Yes you're right; it will have to hallucinate a tool name twice for this to happen.

So your suggestion is to not expose this as a configurable setting and just set it to retries? That's fair and I can update to reflect that. That will fix the problems I am trying to fix in my use-case at least.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants