Fix unknown tool retry logic #3597
Conversation
Signed-off-by: George D. Torres <gdavtor@gmail.com>
|
One open question is: if the user doesn't pass 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 |
@geodavic Isn't this only the case if the model calls the same non-existent tool twice? Because
I would prefer not to have a new setting and to inherit the existing default tool |
|
@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 |
@geodavic Yep let's do that. |
Signed-off-by: George D. Torres <gdavtor@gmail.com>
|
@DouweM done! |
| """The cached tools for this run step.""" | ||
| failed_tools: set[str] = field(default_factory=set) | ||
| """Names of tools that failed in this run step.""" | ||
| max_unknown_tool_retries: int = 1 |
There was a problem hiding this comment.
What do you think of calling it default_max_retries?
| else: | ||
| msg = 'No tools available.' | ||
| raise ModelRetry(f'Unknown tool name: {name!r}. {msg}') | ||
| raise UnknownToolNameRetry(name, msg) |
There was a problem hiding this comment.
I'd prefer to keep this as it was for now.
|
|
||
| self._max_result_retries = output_retries if output_retries is not None else retries | ||
| self._max_tool_retries = retries | ||
| self._max_unknown_tool_retries = retries |
There was a problem hiding this comment.
We can simplify this a bit by dropping this variable as well
2 participants
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, themax_retrieswill always be 1 because the tool name is unknown (i.e.tool is None). This causes theUnexpectedModelBehavior(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_retriesparameter for this purpose.