[Mistral Grammar] Fix tool and reasoning parsing.#2
[Mistral Grammar] Fix tool and reasoning parsing.#2juliendenize wants to merge 16 commits intoimprove_mistral_parsingfrom
Conversation
juliendenize
left a comment
There was a problem hiding this comment.
Adding a bunch of comments for context
| @model_validator(mode="before") | ||
| @classmethod | ||
| def set_include_reasoning_for_none_effort(cls, data: Any) -> Any: | ||
| if data.get("reasoning_effort") == "none": | ||
| data["include_reasoning"] = False | ||
| return data |
There was a problem hiding this comment.
Was introduce by me when adding reasoning_effort="none", it was a bad idea as sometimes model would fail to not generate or include_reasoning can have side effects on the parsers.
| # Override to ensure both image_processor and tokenizer are used. | ||
| # The base get_attributes() introspects __init__ parameters and | ||
| # misses image_processor since it is created internally rather | ||
| # than passed as an __init__ argument. |
There was a problem hiding this comment.
This is due to a regression that was recently introduced, i'd need to check if this fix is sound on Monday and if so i'll open another pr just for that
| _from_tool_parser: bool = field(default=False, init=False) | ||
| """CAUTION: Should only be set by ToolParser.adjust_request""" |
There was a problem hiding this comment.
needed to know if the grammar is active without ambiguity
| _is_mistral_tool_parser = self.tool_parser is not None and issubclass( | ||
| self.tool_parser, MistralToolParser | ||
| ) | ||
| if _is_mistral_tool_parser and self.reasoning_parser_cls is not None: | ||
| MistralToolParser.model_can_reason = True |
There was a problem hiding this comment.
this is not the cleanest maybe but this is the best i found to have a non-intrusive way of knowing if reasoning should be expected by the grammar.
e90a7a1 to
955532c
Compare
c46ff15 to
40f5d9b
Compare
Signed-off-by: juliendenize <julien.denize@mistral.ai>
Signed-off-by: juliendenize <julien.denize@mistral.ai>
Signed-off-by: juliendenize <julien.denize@mistral.ai>
Signed-off-by: juliendenize <julien.denize@mistral.ai>
Signed-off-by: juliendenize <julien.denize@mistral.ai>
955532c to
3772f0c
Compare
Signed-off-by: juliendenize <julien.denize@mistral.ai>
Signed-off-by: juliendenize <julien.denize@mistral.ai>
Signed-off-by: juliendenize <julien.denize@mistral.ai>
Signed-off-by: juliendenize <julien.denize@mistral.ai>
Signed-off-by: juliendenize <julien.denize@mistral.ai>
Signed-off-by: juliendenize <julien.denize@mistral.ai>
Signed-off-by: juliendenize <julien.denize@mistral.ai>
40f5d9b to
5b4589a
Compare
e747e48 to
5c0f3b2
Compare
Purpose
When Mistral models are served with --tool-call-parser mistral and a mistral_common-compatible tokenizer (tekken/v11+), since this PR, tool parsers has a grammar-based approach:
adjust_requestinjects a Lark grammar frommistral-common's grammar factory into structured_outputs. This grammar constrains the model output to follow valid Mistral tool-call formatting at the decoding level.However, the PR only handled the grammar injection which broke the vLLM tool-parsing. This PR makes the serving layer actually use the Mistral tool parser and reasoning parser to parse the grammar-constrained output, rather than falling through to the generic vLLM tool-call parsing paths which don't understand Mistral's format.
Test Plan
The branch adds:
Test Result
The tests pass
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.