-
Notifications
You must be signed in to change notification settings - Fork 14k
common : add parser for ministral/mistral large 3 #17713
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
@pwilkin don't worry, I'm still leaving Qwen3-Coder for you. I added some testing logic since you had ideas about how we should test. |
pwilkin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any chance you could make those test cases into universal test cases for a testcase class / lambda where the only parameters for the actual test case are the parser and the expected string? So that the test case invocations look like:
test_hello_world(&p, "Hello world");
...
test_tool_with_reasoning(&p, "{iamthinking}Thinking of calling tool get_weather{iamnotthinking}{iamcallingatool}get_weather{withparameter}place{ofvalue}Paris{imdonecallingatool}");?
|
I get where you're coming from. I'll play around with it. Another option is table driven tests. They work well at reducing duplication when you have many similar looking tests. One thing I don't like about some of the helpers at the top is that I have scroll up to find the actual content it compares against. |
I think a decent idea to combat that is to have a comment that keeps a template of all the tests with placeholders - then when you implement the model, you just move the comment to after the new model :) |
Parser implementation for Ministral 3 Reasoning/Instruct and Mistral Large 3 Instruct. It deviates from the current Mistral implementation by accepting tool calls in the form:
[TOOL_CALLS]tool_name[ARGS]{"arg1": ... }...Features
reasoning_contentwhenreasoning_format = auto/deepseek. Ifreasoning_format = none, the traces are left incontent.systemandassistantmessages containingreasoning_contentinto{"type": "thinking", "thinking": "..."}content blocks the chat template expects. server: thinking type rejected as invalid but used by Ministral 3 #17700tool_choice = autoandtool_choice = required(with thinking).response_formatwith thinking.Additional Changes
reasoning_formatduring chat param init to build the appropriate parser.make_peg_parserhelper intests/test-chat.cppfor use with peg parsers.