Skip to content

Commit 98ae0a0

Browse files
pwilkinpwilkin
authored
common/parser: fix handling of tool definition with missing properties key (#21128)
1 parent 3a14a54 commit 98ae0a0

3 files changed

Lines changed: 352 additions & 25 deletions

File tree

‎common/chat-auto-parser-generator.cpp‎

Lines changed: 18 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ common_chat_params peg_generator::generate_parser(const common_chat_template &
6565
data.grammar = build_grammar([&](const common_grammar_builder & builder) {
6666
foreach_function(inputs.tools, [&](const json & tool) {
6767
const auto & function = tool.at("function");
68-
auto schema = function.at("parameters");
68+
auto schema = function.contains("parameters") ? function.at("parameters") : json::object();
6969
builder.resolve_refs(schema);
7070
});
7171
parser.build_grammar(builder, data.grammar_lazy);
@@ -221,7 +221,7 @@ common_peg_parser analyze_tools::build_tool_parser_tag_json(parser_build_context
221221
foreach_function(inputs.tools, [&](const json & tool) {
222222
const auto & func = tool.at("function");
223223
std::string name = func.at("name");
224-
const auto & schema = func.at("parameters");
224+
const auto & schema = func.contains("parameters") ? func.at("parameters") : json::object();
225225

226226
// Build call_id parser based on position (if supported)
227227
common_peg_parser call_id_section = p.eps();
@@ -282,19 +282,11 @@ common_peg_parser analyze_tools::build_tool_parser_tag_tagged(parser_build_conte
282282
common_peg_parser tool_choice = p.choice();
283283

284284
foreach_function(inputs.tools, [&](const json & tool) {
285-
const auto & func = tool.at("function");
286-
std::string name = func.at("name");
287-
const auto & params = func.at("parameters");
288-
289-
if (!params.contains("properties") || !params.at("properties").is_object()) {
290-
return;
291-
}
292-
293-
const auto & properties = params.at("properties");
285+
const auto & func = tool.at("function");
286+
std::string name = func.at("name");
287+
const auto & params = func.contains("parameters") ? func.at("parameters") : json::object();
288+
const auto & properties = params.contains("properties") ? params.at("properties") : json::object();
294289
std::set<std::string> required;
295-
if (params.contains("required") && params.at("required").is_array()) {
296-
params.at("required").get_to(required);
297-
}
298290

299291
// Build parser for each argument, separating required and optional
300292
std::vector<common_peg_parser> required_parsers;
@@ -311,17 +303,18 @@ common_peg_parser analyze_tools::build_tool_parser_tag_tagged(parser_build_conte
311303
}
312304
}
313305

314-
auto arg = p.tool_arg(
315-
p.tool_arg_open(arguments.name_prefix + p.tool_arg_name(p.literal(param_name)) +
316-
arguments.name_suffix) +
317-
arguments.value_prefix +
318-
(type == "string" ? p.tool_arg_string_value(p.schema(p.until(arguments.value_suffix),
319-
"tool-" + name + "-arg-" + param_name + "-schema",
320-
param_schema, true)) :
321-
p.tool_arg_json_value(p.schema(
322-
p.json(), "tool-" + name + "-arg-" + param_name + "-schema", param_schema, false)) +
323-
p.space()) +
324-
p.tool_arg_close(p.literal(arguments.value_suffix)));
306+
auto arg =
307+
p.tool_arg(p.tool_arg_open(arguments.name_prefix + p.tool_arg_name(p.literal(param_name)) +
308+
arguments.name_suffix) +
309+
arguments.value_prefix +
310+
(type == "string" ?
311+
p.tool_arg_string_value(p.schema(p.until(arguments.value_suffix),
312+
"tool-" + name + "-arg-" + param_name + "-schema",
313+
param_schema, true)) :
314+
p.tool_arg_json_value(p.schema(
315+
p.json(), "tool-" + name + "-arg-" + param_name + "-schema", param_schema, false)) +
316+
p.space()) +
317+
p.tool_arg_close(p.literal(arguments.value_suffix)));
325318

326319
auto named_arg = p.rule("tool-" + name + "-arg-" + param_name, arg);
327320
if (is_required) {

‎models/templates/Qwen3.5-4B.jinja‎

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
{%- set image_count = namespace(value=0) %}
2+
{%- set video_count = namespace(value=0) %}
3+
{%- macro render_content(content, do_vision_count, is_system_content=false) %}
4+
{%- if content is string %}
5+
{{- content }}
6+
{%- elif content is iterable and content is not mapping %}
7+
{%- for item in content %}
8+
{%- if 'image' in item or 'image_url' in item or item.type == 'image' %}
9+
{%- if is_system_content %}
10+
{{- raise_exception('System message cannot contain images.') }}
11+
{%- endif %}
12+
{%- if do_vision_count %}
13+
{%- set image_count.value = image_count.value + 1 %}
14+
{%- endif %}
15+
{%- if add_vision_id %}
16+
{{- 'Picture ' ~ image_count.value ~ ': ' }}
17+
{%- endif %}
18+
{{- '<|vision_start|><|image_pad|><|vision_end|>' }}
19+
{%- elif 'video' in item or item.type == 'video' %}
20+
{%- if is_system_content %}
21+
{{- raise_exception('System message cannot contain videos.') }}
22+
{%- endif %}
23+
{%- if do_vision_count %}
24+
{%- set video_count.value = video_count.value + 1 %}
25+
{%- endif %}
26+
{%- if add_vision_id %}
27+
{{- 'Video ' ~ video_count.value ~ ': ' }}
28+
{%- endif %}
29+
{{- '<|vision_start|><|video_pad|><|vision_end|>' }}
30+
{%- elif 'text' in item %}
31+
{{- item.text }}
32+
{%- else %}
33+
{{- raise_exception('Unexpected item type in content.') }}
34+
{%- endif %}
35+
{%- endfor %}
36+
{%- elif content is none or content is undefined %}
37+
{{- '' }}
38+
{%- else %}
39+
{{- raise_exception('Unexpected content type.') }}
40+
{%- endif %}
41+
{%- endmacro %}
42+
{%- if not messages %}
43+
{{- raise_exception('No messages provided.') }}
44+
{%- endif %}
45+
{%- if tools and tools is iterable and tools is not mapping %}
46+
{{- '<|im_start|>system\n' }}
47+
{{- "# Tools\n\nYou have access to the following functions:\n\n<tools>" }}
48+
{%- for tool in tools %}
49+
{{- "\n" }}
50+
{{- tool | tojson }}
51+
{%- endfor %}
52+
{{- "\n</tools>" }}
53+
{{- '\n\nIf you choose to call a function ONLY reply in the following format with NO suffix:\n\n<tool_call>\n<function=example_function_name>\n<parameter=example_parameter_1>\nvalue_1\n</parameter>\n<parameter=example_parameter_2>\nThis is the value for the second parameter\nthat can span\nmultiple lines\n</parameter>\n</function>\n</tool_call>\n\n<IMPORTANT>\nReminder:\n- Function calls MUST follow the specified format: an inner <function=...></function> block must be nested within <tool_call></tool_call> XML tags\n- Required parameters MUST be specified\n- You may provide optional reasoning for your function call in natural language BEFORE the function call, but NOT after\n- If there is no function call available, answer the question like normal with your current knowledge and do not tell the user about function calls\n</IMPORTANT>' }}
54+
{%- if messages[0].role == 'system' %}
55+
{%- set content = render_content(messages[0].content, false, true)|trim %}
56+
{%- if content %}
57+
{{- '\n\n' + content }}
58+
{%- endif %}
59+
{%- endif %}
60+
{{- '<|im_end|>\n' }}
61+
{%- else %}
62+
{%- if messages[0].role == 'system' %}
63+
{%- set content = render_content(messages[0].content, false, true)|trim %}
64+
{{- '<|im_start|>system\n' + content + '<|im_end|>\n' }}
65+
{%- endif %}
66+
{%- endif %}
67+
{%- set ns = namespace(multi_step_tool=true, last_query_index=messages|length - 1) %}
68+
{%- for message in messages[::-1] %}
69+
{%- set index = (messages|length - 1) - loop.index0 %}
70+
{%- if ns.multi_step_tool and message.role == "user" %}
71+
{%- set content = render_content(message.content, false)|trim %}
72+
{%- if not(content.startswith('<tool_response>') and content.endswith('</tool_response>')) %}
73+
{%- set ns.multi_step_tool = false %}
74+
{%- set ns.last_query_index = index %}
75+
{%- endif %}
76+
{%- endif %}
77+
{%- endfor %}
78+
{%- if ns.multi_step_tool %}
79+
{{- raise_exception('No user query found in messages.') }}
80+
{%- endif %}
81+
{%- for message in messages %}
82+
{%- set content = render_content(message.content, true)|trim %}
83+
{%- if message.role == "system" %}
84+
{%- if not loop.first %}
85+
{{- raise_exception('System message must be at the beginning.') }}
86+
{%- endif %}
87+
{%- elif message.role == "user" %}
88+
{{- '<|im_start|>' + message.role + '\n' + content + '<|im_end|>' + '\n' }}
89+
{%- elif message.role == "assistant" %}
90+
{%- set reasoning_content = '' %}
91+
{%- if message.reasoning_content is string %}
92+
{%- set reasoning_content = message.reasoning_content %}
93+
{%- else %}
94+
{%- if '</think>' in content %}
95+
{%- set reasoning_content = content.split('</think>')[0].rstrip('\n').split('<think>')[-1].lstrip('\n') %}
96+
{%- set content = content.split('</think>')[-1].lstrip('\n') %}
97+
{%- endif %}
98+
{%- endif %}
99+
{%- set reasoning_content = reasoning_content|trim %}
100+
{%- if loop.index0 > ns.last_query_index %}
101+
{{- '<|im_start|>' + message.role + '\n<think>\n' + reasoning_content + '\n</think>\n\n' + content }}
102+
{%- else %}
103+
{{- '<|im_start|>' + message.role + '\n' + content }}
104+
{%- endif %}
105+
{%- if message.tool_calls and message.tool_calls is iterable and message.tool_calls is not mapping %}
106+
{%- for tool_call in message.tool_calls %}
107+
{%- if tool_call.function is defined %}
108+
{%- set tool_call = tool_call.function %}
109+
{%- endif %}
110+
{%- if loop.first %}
111+
{%- if content|trim %}
112+
{{- '\n\n<tool_call>\n<function=' + tool_call.name + '>\n' }}
113+
{%- else %}
114+
{{- '<tool_call>\n<function=' + tool_call.name + '>\n' }}
115+
{%- endif %}
116+
{%- else %}
117+
{{- '\n<tool_call>\n<function=' + tool_call.name + '>\n' }}
118+
{%- endif %}
119+
{%- if tool_call.arguments is defined %}
120+
{%- for args_name, args_value in tool_call.arguments|items %}
121+
{{- '<parameter=' + args_name + '>\n' }}
122+
{%- set args_value = args_value | tojson | safe if args_value is mapping or (args_value is sequence and args_value is not string) else args_value | string %}
123+
{{- args_value }}
124+
{{- '\n</parameter>\n' }}
125+
{%- endfor %}
126+
{%- endif %}
127+
{{- '</function>\n</tool_call>' }}
128+
{%- endfor %}
129+
{%- endif %}
130+
{{- '<|im_end|>\n' }}
131+
{%- elif message.role == "tool" %}
132+
{%- if loop.previtem and loop.previtem.role != "tool" %}
133+
{{- '<|im_start|>user' }}
134+
{%- endif %}
135+
{{- '\n<tool_response>\n' }}
136+
{{- content }}
137+
{{- '\n</tool_response>' }}
138+
{%- if not loop.last and loop.nextitem.role != "tool" %}
139+
{{- '<|im_end|>\n' }}
140+
{%- elif loop.last %}
141+
{{- '<|im_end|>\n' }}
142+
{%- endif %}
143+
{%- else %}
144+
{{- raise_exception('Unexpected message role.') }}
145+
{%- endif %}
146+
{%- endfor %}
147+
{%- if add_generation_prompt %}
148+
{{- '<|im_start|>assistant\n' }}
149+
{%- if enable_thinking is defined and enable_thinking is false %}
150+
{{- '<think>\n\n</think>\n\n' }}
151+
{%- else %}
152+
{{- '<think>\n' }}
153+
{%- endif %}
154+
{%- endif %}

0 commit comments

Comments
 (0)