[WIP] Add Mistral guidance#32781
Conversation
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
Comment @cursor review or bugbot run to trigger another review on this PR
| mode = "required" | ||
| else: | ||
| selected_tools = request.tools | ||
| mode = request.tool_choice |
There was a problem hiding this comment.
None tool_choice causes invalid Lark grammar generation
High Severity
When request.tool_choice is None (the default value meaning "auto" behavior), the code sets mode = request.tool_choice, passing None to get_lark_from_jinja. The Jinja template BASE_LARK_GRAMMAR only handles "auto", "required", and "none" string values. When mode is Python None, none of the template conditions match, resulting in an incomplete Lark grammar where the body: rule is never defined but is referenced by start: body. This produces an invalid grammar that will fail during parsing.
| @@ -111,9 +291,12 @@ def __init__(self, tokenizer: TokenizerLike): | |||
| "Mistral Tool Parser could not locate the tool call token in " | |||
| "the tokenizer!" | |||
| ) | |||
| self.grammar_factory = MistralGrammarFactory(tokenizer) | |||
There was a problem hiding this comment.
MistralToolParser initialization fails with non-Mistral tokenizers
High Severity
The MistralToolParser.__init__ unconditionally creates MistralGrammarFactory(tokenizer) at line 294, but MistralGrammarFactory requires a MistralTokenizer and raises ValueError for other tokenizer types. This contradicts lines 266-267 which log an info message when a non-Mistral tokenizer is detected, implying this is a supported use case. Any attempt to create MistralToolParser with a non-Mistral tokenizer will now fail during initialization.
Additional Locations (1)
| tool | ||
| for tool in request.tools | ||
| if tool.function.name == request.tool_choice.function.name | ||
| ] |
There was a problem hiding this comment.
Missing validation when named tool doesn't exist in tools list
Medium Severity
When tool_choice specifies a ChatCompletionNamedToolChoiceParam with a function name that doesn't exist in the tools list, selected_tools becomes an empty list. The grammar generation logic treats an empty list as falsy, causing any_strict_true to be False, which generates a permissive grammar allowing any tool name instead of restricting to the specified tool. The user's explicit intent to call a specific tool is silently ignored.
b7f9f33 to
f06847e
Compare
|
This pull request has merge conflicts that must be resolved before it can be |
Signed-off-by: Julien Denize <[email protected]>
Signed-off-by: juliendenize <[email protected]>
Signed-off-by: juliendenize <[email protected]>
f06847e to
9dae2c8
Compare
Purpose
This is a work in progress branch to add better guidance for Mistral Tool Parser with the Mistral Tokenizer.
To do so a lark grammar is generated when the
--tool-call-parser mistralis used:This PR also brings support for the MistralTokenizer guidance backend.
Test Plan
Right now there are no unit tests attached to the draft PR and has only been checked by manual requests.
Test Result
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.