Skip to content

chat : tolerate dropped tool-call wrapper in tag-based formats #26244

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.

Sign up for GitHub

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

Closed
jamincollins wants to merge 1 commit into from
+150 −10
Closed

chat : tolerate dropped tool-call wrapper in tag-based formats #26244

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 59 additions & 10 deletions common/chat-auto-parser-generator.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,20 @@ common_chat_params peg_generator::generate_parser(const common_chat_template &
data.grammar_triggers = {
{ COMMON_GRAMMAR_TRIGGER_TYPE_WORD, trigger_marker }
};
// A dropped wrapper can still leave a well-formed call starting at per_call_start
// or function.name_prefix (see build_tool_parser_tag_tagged). Trigger on both too,
// when distinct, so sampling still gets grammar-constrained in that case.
if (!autoparser.tools.format.per_call_start.empty() &&
autoparser.tools.format.per_call_start != trigger_marker) {
data.grammar_triggers.push_back(
{ COMMON_GRAMMAR_TRIGGER_TYPE_WORD, autoparser.tools.format.per_call_start });
}
if (!autoparser.tools.function.name_prefix.empty() &&
autoparser.tools.function.name_prefix != trigger_marker &&
autoparser.tools.function.name_prefix != autoparser.tools.format.per_call_start) {
data.grammar_triggers.push_back(
{ COMMON_GRAMMAR_TRIGGER_TYPE_WORD, autoparser.tools.function.name_prefix });
}
if (autoparser.tools.format.openai_wrapper_trigger) {
// model emits the OpenAI function wrapper, trigger on it
data.grammar_triggers.push_back({ COMMON_GRAMMAR_TRIGGER_TYPE_WORD, "{\"type\": \"function\"," });
Expand Down Expand Up @@ -345,16 +359,22 @@ common_peg_parser analyze_tools::build_tool_parser_tag_json(parser_build_context
common_peg_parser tool_calls = p.eps();

if (!format.per_call_start.empty()) {
auto wrapped_call = format.per_call_start + tool_choice + format.per_call_end;
// See the equivalent handling in build_tool_parser_tag_tagged below for the full
// reasoning: both the per-call marker and the outer section wrapper (further down)
// can be dropped by the model independently, so make each optional rather than
// mandatory - the function tag itself (inside tool_choice) stays mandatory.
auto call_open = p.optional(p.literal(format.per_call_start));
auto call_close = format.per_call_end.empty() ? p.eps() : p.optional(p.literal(format.per_call_end));
auto wrapped_call = call_open + tool_choice + call_close;
if (inputs.parallel_tool_calls) {
tool_calls = p.trigger_rule("tool-call", wrapped_call + p.zero_or_more(p.space() + wrapped_call));
} else {
tool_calls = p.trigger_rule("tool-call", wrapped_call);
}
if (!format.section_start.empty()) {
tool_calls = p.trigger_rule("tool-calls",
p.literal(format.section_start) + p.space() + tool_calls + p.space() +
(format.section_end.empty() ? p.end() : p.literal(format.section_end)));
p.optional(p.literal(format.section_start)) + p.space() + tool_calls + p.space() +
(format.section_end.empty() ? p.end() : p.optional(p.literal(format.section_end))));
}
} else {
std::string separator = ", "; // Default
Expand All @@ -370,8 +390,18 @@ common_peg_parser analyze_tools::build_tool_parser_tag_json(parser_build_context
tool_calls = p.optional(tool_calls);
}

std::string trigger_marker = !format.section_start.empty() ? format.section_start : format.per_call_start;
auto content_before_tools = trigger_marker.empty() ? p.eps() : p.until(trigger_marker);
// See the equivalent content_before_tools handling in build_tool_parser_tag_tagged below:
// only widen the scan past section_start when section_start itself is empty.
std::vector<std::string> content_stop_markers;
if (!format.section_start.empty()) {
content_stop_markers.push_back(format.section_start);
} else if (!format.per_call_start.empty()) {
content_stop_markers.push_back(format.per_call_start);
if (!function.name_prefix.empty() && function.name_prefix != format.per_call_start) {
content_stop_markers.push_back(function.name_prefix);
}
}
auto content_before_tools = content_stop_markers.empty() ? p.eps() : p.until_one_of(content_stop_markers);
return ctx.reasoning_parser + p.optional(p.content(content_before_tools)) + tool_calls + p.end();
}

Expand Down Expand Up @@ -473,16 +503,23 @@ common_peg_parser analyze_tools::build_tool_parser_tag_tagged(parser_build_conte
common_peg_parser tool_calls = p.eps();

if (!format.per_call_start.empty()) {
auto wrapped_call = format.per_call_start + p.space() + tool_choice + p.space() + format.per_call_end;
// Models occasionally skip the per-call wrapper and jump straight to the function tag
// (a separate literal, function.name_prefix). Make per_call_start/per_call_end
// independently optional; the function tag itself (inside tool_choice) stays mandatory.
auto call_open = p.optional(p.literal(format.per_call_start) + p.space());
auto call_close = format.per_call_end.empty() ? p.eps() : p.optional(p.literal(format.per_call_end));
auto wrapped_call = call_open + tool_choice + p.space() + call_close;
if (inputs.parallel_tool_calls) {
tool_calls = p.trigger_rule("tool-call", wrapped_call + p.zero_or_more(p.space() + wrapped_call) + p.space());
} else {
tool_calls = p.trigger_rule("tool-call", wrapped_call + p.space());
}
if (!format.section_start.empty()) {
// Same reasoning as per_call_start/per_call_end above, one level up: the outer
// section wrapper can be dropped too, independently of the per-call one.
tool_calls = p.trigger_rule("tool-calls",
p.literal(format.section_start) + p.space() + tool_calls + p.space() +
(format.section_end.empty() ? p.end() : p.literal(format.section_end) + p.space()));
p.optional(p.literal(format.section_start) + p.space()) + tool_calls + p.space() +
(format.section_end.empty() ? p.end() : p.optional(p.literal(format.section_end)) + p.space()));
}
} else {
std::string separator = ", "; // Default
Expand All @@ -501,8 +538,20 @@ common_peg_parser analyze_tools::build_tool_parser_tag_tagged(parser_build_conte
tool_calls = p.optional(tool_calls);
}

std::string trigger_marker = !format.section_start.empty() ? format.section_start : format.per_call_start;
auto content_before_tools = trigger_marker.empty() ? p.eps() : p.until(trigger_marker);
// Stop the content-before-tools scan at the marker that actually begins a tool call. Only
// widen past section_start when it's empty (per_call_start IS the wrapper there, see
// above) - otherwise a chatty model quoting per_call_start/function.name_prefix-shaped
// text before the real section_start would truncate legitimate content early.
std::vector<std::string> content_stop_markers;
if (!format.section_start.empty()) {
content_stop_markers.push_back(format.section_start);
} else if (!format.per_call_start.empty()) {
content_stop_markers.push_back(format.per_call_start);
if (!function.name_prefix.empty() && function.name_prefix != format.per_call_start) {
content_stop_markers.push_back(function.name_prefix);
}
}
auto content_before_tools = content_stop_markers.empty() ? p.eps() : p.until_one_of(content_stop_markers);
return ctx.reasoning_parser + p.optional(p.content(content_before_tools)) + tool_calls + p.end();
}

Expand Down
91 changes: 91 additions & 0 deletions tests/test-chat.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3422,6 +3422,19 @@ static void test_template_output_peg_parsers(bool detailed_debug) {
})
.run();

// The model sometimes drops the <seed:tool_call> wrapper while still emitting a
// well-formed <function=...></function> block (build_tool_parser_tag_json must
// tolerate this the same way build_tool_parser_tag_tagged does for Qwen3-Coder).
tst.test(
"<function=special_function>\n"
"<parameter=arg1>1</parameter>\n"
"</function>\n")
.tools({ special_function_tool })
.expect_tool_calls({
{ "special_function", R"({"arg1": 1})", {} },
})
.run();

tst.test(
"<seed:tool_call>\n"
"<function=todo_list>\n"
Expand Down Expand Up @@ -3720,6 +3733,84 @@ static void test_template_output_peg_parsers(bool detailed_debug) {
})
.run();

// The model sometimes drops the outer <tool_call> wrapper (per_call_start/
// per_call_end) while still emitting a well-formed <function=...></function>
// block. Since Qwen3-Coder's format.section_start is empty (per_call_start is
// itself the wrapper here, not a separate outer section), this must still parse
// as a proper tool call rather than falling through to plain content.
tst.test(
"<function=special_function>\n"
"<parameter=arg1>\n"
"1\n"
"</parameter>\n"
"</function>\n")
.tools({ special_function_tool })
.expect_tool_calls({
{ "special_function", R"({"arg1": 1})", {} },
})
.run();

// Same as above, but with the leading chatter a model commonly emits before
// deciding to call a tool ("I'll help you ...").
tst.test(
"I'll help with that.\n\n"
"<function=special_function>\n"
"<parameter=arg1>\n"
"1\n"
"</parameter>\n"
"</function>\n")
.tools({ special_function_tool })
.expect_content("I'll help with that.\n\n")
.expect_tool_calls({
{ "special_function", R"({"arg1": 1})", {} },
})
.run();

// Asymmetric case: the opening <tool_call> wrapper is dropped but a trailing
// </tool_call> is still emitted. Both sides of the wrapper are independently
// optional, so this must parse too.
tst.test(
"<function=special_function>\n"
"<parameter=arg1>\n"
"1\n"
"</parameter>\n"
"</function>\n"
"</tool_call>")
.tools({ special_function_tool })
.expect_tool_calls({
{ "special_function", R"({"arg1": 1})", {} },
})
.run();

// Parallel calls where only the first wrapper is dropped and the second is
// well-formed - the optional wrapper must be evaluated independently per call,
// not just once for the whole sequence.
tst.test(
"<function=special_function>\n"
"<parameter=arg1>\n"
"1\n"
"</parameter>\n"
"</function>\n"
"<tool_call>\n"
"<function=special_function_with_opt>\n"
"<parameter=arg1>\n"
"1\n"
"</parameter>\n"
"<parameter=arg2>\n"
"2\n"
"</parameter>\n"
"</function>\n"
"</tool_call>")
.parallel_tool_calls(true)
.tools({
special_function_tool, special_function_tool_with_optional_param
})
.expect_tool_calls({
{ "special_function", R"({"arg1": 1})", {} },
{ "special_function_with_opt", R"({"arg1": 1, "arg2": 2})", {} },
})
.run();

// Continuation tests
tst.test("world!\nWhat's up?")
.messages({ message_user, message_assist_prefill_content })
Expand Down