[Mistral Grammar] Support Grammar Factory#38150
Conversation
There was a problem hiding this comment.
Code Review
This pull request integrates llguidance and mistral-common's GrammarFactory into vLLM's Mistral tokenizer and tool parsing logic. The MistralTokenizer now exposes properties to check grammar support and access GrammarFactory and llguidance.LLTokenizer instances. The MistralToolParser's adjust_request method is enhanced to dynamically generate Lark grammars for tool calling based on request parameters for supported Mistral tokenizers. Additionally, structured output validation in sampling_params.py is refined to allow Tekken Mistral tokenizers with the guidance backend, while explicitly disallowing non-Tekken Mistral tokenizers. New tests have been added to cover this new functionality. There is no feedback to provide on the review comments as none were given.
|
This pull request has merge conflicts that must be resolved before it can be |
bbrowning
left a comment
There was a problem hiding this comment.
I know this is a draft, but I took a first pass at this anyway and have a few inline comments. All of these items may already be planned to handle as work progresses, so feel free to discard if they're already on your radar.
| or request.structured_outputs is not None | ||
| or ( | ||
| request.response_format is not None | ||
| and request.response_format.type != "text" |
There was a problem hiding this comment.
This may be a bit broad. For example, if the response_format was set to json_object would we still want to enable the grammar-based guiding? Or does the model not output JSON for its response in that case?
There was a problem hiding this comment.
So in this case we just ignore and leave the grammar to vLLM but we're indeed considering injecting our lark grammar there as well as we can handle json_schema with or without tool choice set (the model choses between calling a tool or creating a json)
There was a problem hiding this comment.
Sounds good - if it's scope creep, feel free to go with what you were originally going to do and we can iterate on this in future PRs.
a9eaa24 to
1324904
Compare
|
I reviewed the latest updates, and they look good to me. Once this is ready to come out of draft state with the updated mistral_common dependency I'm happy to take another look but we'll need an approver in this area ready to review as well to get this merged quickly. |
e90a7a1 to
955532c
Compare
|
This pull request has merge conflicts that must be resolved before it can be |
955532c to
3772f0c
Compare
|
Hi @juliendenize, the pre-commit checks have failed. Please run: uv pip install pre-commit>=4.5.1
pre-commit install
pre-commit run --all-filesThen, commit the changes and push to your branch. For future commits, Tip Is
|
e747e48 to
5c0f3b2
Compare
fef56c1
into
vllm-project:main
Signed-off-by: juliendenize <julien.denize@mistral.ai>
Signed-off-by: juliendenize <julien.denize@mistral.ai> Signed-off-by: Rishi Puri <riship@nvidia.com>
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: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
Purpose
This PR adds support to the Mistral grammar factory that creates lark grammar based on
tools,tool_choice,structured_outputsandreasoning.To do that it adds the following:
For this PR to work, mistral-common must be installed from last release that was bumped.
The Grammar factory details can be found in this mistral-common PR description regarding how tool choices and reasoning influence the grammar.
This PR will break the tool call parsing for some requests that expect a json grammar (such as
tool_choice="required"). A follow-up PR will be submitted to ensure this is fixed.This is the first PR that aims to replicate some features from the #37081 while ensuring better separation of concerns and that mistral-common hosts the grammar factory logic.
Test Plan
Added tests to:
Test Result
They all pass.
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.BEFORE SUBMITTING, PLEASE READ https://docs.vllm.ai/en/latest/contributing (anything written below this line will be removed by GitHub Actions)