[Mistral Tokenizer] allow more leniency in apply_chat_template#41658
Conversation
Signed-off-by: juliendenize <julien.denize@mistral.ai>
juliendenize
requested review from
aarnphm,
bbrowning,
chaunceyjiang,
patrickvonplaten and
sfeng33
as code owners
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request upgrades mistral_common to version 1.11.2 and refactors the Mistral tokenizer and tool parser to utilize native library methods for tool adaptation. The internal helper for preparing chat templates has been converted into a validation function, removing manual message and tool modification logic. Feedback identifies a high-severity issue where the add_generation_prompt parameter was omitted in the call to the underlying transformers tokenizer, which could lead to incorrect prompt formatting.
| messages, continue_final_message, add_generation_prompt | ||
| ) | ||
|
|
||
| return self.transformers_tokenizer.apply_chat_template( |
There was a problem hiding this comment.
The add_generation_prompt argument is missing from the call to self.transformers_tokenizer.apply_chat_template. While mistral-common often infers the generation prompt from the conversation state, the transformers tokenizer wrapper (MistralCommonBackend) explicitly supports this parameter. Omitting it causes it to default to False, which may conflict with the user's intent and vLLM's default behavior (where add_generation_prompt defaults to True).
| return self.transformers_tokenizer.apply_chat_template( | |
| return self.transformers_tokenizer.apply_chat_template( | |
| conversation=messages, | |
| tools=tools, | |
| add_generation_prompt=add_generation_prompt, | |
| continue_final_message=continue_final_message, | |
| tokenize=tokenize, | |
| padding=padding, | |
| truncation=truncation, | |
| max_length=max_length, | |
| return_tensors=None, | |
| return_dict=False, | |
| **version_kwargs, | |
| ) |
There was a problem hiding this comment.
it was already not the case. While I agree ultimately that we'd want to do that, we need to wait a bit that last Transformers version is released as I recently updated MistralCommonBackend.
Signed-off-by: juliendenize <julien.denize@mistral.ai>
|
@DarkLight1337 i don't think the CI errors are related ! If you agree could you consider merging 🙏 ? |
…project#41658) Signed-off-by: juliendenize <julien.denize@mistral.ai> Co-authored-by: Roger Wang <hey@rogerw.io>
…project#41658) Signed-off-by: juliendenize <julien.denize@mistral.ai> Co-authored-by: Roger Wang <hey@rogerw.io> Signed-off-by: Mehdi Ghanimifard <mehdi.ghanimifard@amd.com>
…project#41658) Signed-off-by: juliendenize <julien.denize@mistral.ai> Co-authored-by: Roger Wang <hey@rogerw.io> Co-authored-by: hongbolv <33214277+hongbolv@users.noreply.github.com>
…project#41658) Signed-off-by: juliendenize <julien.denize@mistral.ai> Co-authored-by: Roger Wang <hey@rogerw.io> Signed-off-by: Ifta Khairul Alam Adil <ikaadil007@gmail.com>
…project#41658) Signed-off-by: juliendenize <julien.denize@mistral.ai> Co-authored-by: Roger Wang <hey@rogerw.io> Signed-off-by: Libin Tang <libin.tang@intel.com>
6 participants
Purpose
This PR upgrade mistral-common to 1.11.2. This allows to:
Test Plan
Tested on manually made requests + updated some unit tests in the lib
Test Result
All pass
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.