From 119aad556387809c46b61ce7b3f102daed2ce499 Mon Sep 17 00:00:00 2001 From: zhangtao Date: Thu, 28 May 2026 07:18:10 +0000 Subject: [PATCH 1/9] Add minicpm5 tool call parser --- common/chat-peg-parser.cpp | 90 ++++++++++++- common/chat-peg-parser.h | 10 +- common/chat.cpp | 164 +++++++++++++++++++++++- common/chat.h | 1 + common/jinja/value.cpp | 28 ++++ tests/CMakeLists.txt | 1 + tests/test-chat-peg-parser-minicpm5.cpp | 128 ++++++++++++++++++ tests/test-chat-peg-parser.cpp | 1 + tests/test-jinja.cpp | 12 ++ 9 files changed, 428 insertions(+), 7 deletions(-) create mode 100644 tests/test-chat-peg-parser-minicpm5.cpp diff --git a/common/chat-peg-parser.cpp b/common/chat-peg-parser.cpp index a309f02765b7..675d253b6350 100644 --- a/common/chat-peg-parser.cpp +++ b/common/chat-peg-parser.cpp @@ -236,7 +236,18 @@ common_peg_parser common_chat_peg_builder::tag_with_safe_content(const std::stri } std::string & common_chat_peg_mapper::args_target() { - return (current_tool && !current_tool->name.empty()) ? current_tool->arguments : args_buffer; + common_chat_tool_call * tool = active_tool(); + return (tool && !tool->name.empty()) ? tool->arguments : args_buffer; +} + +common_chat_tool_call * common_chat_peg_mapper::active_tool() { + if (committed_tool_idx.has_value()) { + return &result.tool_calls.at(committed_tool_idx.value()); + } + if (pending_tool_call.has_value()) { + return &pending_tool_call.value(); + } + return nullptr; } std::string common_chat_peg_mapper::normalize_container_value(const std::string & input) { @@ -303,12 +314,14 @@ void common_chat_peg_mapper::map(const common_peg_ast_node & node) { if (is_tool_open) { pending_tool_call = common_chat_tool_call(); - current_tool = &pending_tool_call.value(); + committed_tool_idx.reset(); arg_count = 0; args_buffer.clear(); closing_quote_pending = false; } + common_chat_tool_call * current_tool = active_tool(); + if (is_tool_id && current_tool) { auto text = trim_trailing_space(node.text); if (text.size() >= 2 && text.front() == '"' && text.back() == '"') { @@ -329,11 +342,13 @@ void common_chat_peg_mapper::map(const common_peg_ast_node & node) { // Add the tool call to results so streaming can see it if (pending_tool_call.has_value()) { result.tool_calls.push_back(pending_tool_call.value()); + committed_tool_idx = result.tool_calls.size() - 1; pending_tool_call.reset(); - current_tool = &result.tool_calls.back(); } } + current_tool = active_tool(); + if (is_tool_args && current_tool) { // For JSON format: arguments come as a complete JSON object // For tagged format: built up from individual arg_name/arg_value nodes @@ -347,6 +362,8 @@ void common_chat_peg_mapper::map(const common_peg_ast_node & node) { closing_quote_pending = false; } + current_tool = active_tool(); + if (is_arg_name && current_tool) { std::string arg_entry; if (arg_count > 0) { @@ -414,6 +431,7 @@ void common_chat_peg_mapper::map(const common_peg_ast_node & node) { } pending_tool_call.reset(); } + committed_tool_idx.reset(); } } @@ -1056,3 +1074,69 @@ void common_chat_peg_gemma4_mapper::visit(const common_peg_ast_arena & arena, co visit(arena, child_id); } } + +common_peg_parser common_chat_peg_builder::minicpm5_xml_tool_calls(const ordered_json & tools, + bool parallel_tool_calls) { + if (!tools.is_array() || tools.empty()) { + return eps(); + } + + static const std::vector PARAM_VALUE_STOP = { + "", + " name=\"", + "", + "") + literal("]]>"), + until_one_of(PARAM_VALUE_STOP), + }); + + auto tool_choices = choice(); + + for (const auto & tool_def : tools) { + if (!tool_def.contains("function")) { + continue; + } + const auto & function = tool_def.at("function"); + const std::string name = function.at("name"); + ordered_json params = function.contains("parameters") ? function.at("parameters") : ordered_json::object(); + + auto args = eps(); + if (params.contains("properties") && params.at("properties").is_object() && + !params.at("properties").empty()) { + auto arg_choice = choice(); + for (const auto & el : params.at("properties").items()) { + const std::string & prop_name = el.key(); + const std::string & prop_type = el.value().value("type", "string"); + + auto value_parser = prop_type == "string" ? tool_arg_string_value(arg_value) : tool_arg_value(arg_value); + + auto arg_rule = tool_arg( + tool_arg_open(param_name_prefix + tool_arg_name(literal(prop_name)) + literal("\">")) + + value_parser + + tool_arg_close(optional(literal("")) + space())); + + arg_choice |= arg_rule; + } + args = zero_or_more(arg_choice + space()); + } + + auto tool_parser = tool( + tool_open(func_name_prefix + tool_name(literal(name)) + literal("\">")) + space() + tool_args(args) + + space() + tool_close(optional(literal("")))); + + tool_choices |= rule("tool-" + name, tool_parser); + } + + if (parallel_tool_calls) { + return trigger_rule("tool-calls", one_or_more(tool_choices + space())); + } + + return trigger_rule("tool-call", tool_choices); +} diff --git a/common/chat-peg-parser.h b/common/chat-peg-parser.h index b3ffd7de2dd8..ba16e10d83ef 100644 --- a/common/chat-peg-parser.h +++ b/common/chat-peg-parser.h @@ -22,13 +22,14 @@ class common_chat_peg_mapper { private: // Tool call handling state std::optional pending_tool_call; // Tool call waiting for name - common_chat_tool_call * current_tool = nullptr; + std::optional committed_tool_idx; // index in result.tool_calls after streaming push int arg_count = 0; bool closing_quote_pending = false; std::string args_buffer; // Buffer to delay arguments until tool name is known + common_chat_tool_call * active_tool(); // Returns a reference to the active argument destination string. - // Before tool_name is known, writes go to args_buffer; after, to current_tool->arguments. + // Before tool_name is known, writes go to args_buffer; after, to active_tool()->arguments. std::string & args_target(); }; @@ -136,6 +137,11 @@ class common_chat_peg_builder : public common_peg_parser_builder { bool parallel_tool_calls, bool allow_json_literals); + // MiniCPM5 XML tool calls: + // baz + common_peg_parser minicpm5_xml_tool_calls(const nlohmann::ordered_json & tools, + bool parallel_tool_calls); + private: // Python values plus JSON true/false/null. common_peg_parser python_or_json_value(); diff --git a/common/chat.cpp b/common/chat.cpp index ded8440e6688..86ad5349c4be 100644 --- a/common/chat.cpp +++ b/common/chat.cpp @@ -22,6 +22,7 @@ #include #include +#include #include #include #include @@ -762,6 +763,8 @@ const char * common_chat_format_name(common_chat_format format) { return "peg-native"; case COMMON_CHAT_FORMAT_PEG_GEMMA4: return "peg-gemma4"; + case COMMON_CHAT_FORMAT_PEG_MINICPM5: + return "peg-minicpm5"; default: throw std::runtime_error("Unknown chat format"); } @@ -2324,6 +2327,153 @@ static void func_args_not_string(json & messages) { } +static bool is_minicpm5_template(const std::string & src) { + return src.find("Tool usage guidelines:") != std::string::npos && + src.find(" name="code">...` instead of `...`. + static const std::regex LEADING_FUNC_ATTR(R"((?:^|[\n\r])\s*name=\"([^\"]+)\">)"); + out = std::regex_replace(out, LEADING_FUNC_ATTR, "\n"); + static const std::regex PARAM_ATTR(R"(>\s*name=\"([^\"]+)\">)"); + out = std::regex_replace(out, PARAM_ATTR, ">"); + + static const std::string IM_END = "<|im_end|>"; + if (out.size() >= IM_END.size() && + out.compare(out.size() - IM_END.size(), IM_END.size(), IM_END) == 0) { + out.erase(out.size() - IM_END.size()); + while (!out.empty() && (out.back() == '\n' || out.back() == ' ')) { + out.pop_back(); + } + } + + return out; +} + +// MiniCPM5 format: +// - Reasoning: {reasoning} (optional) +// - Tool calls: value +static common_chat_params common_chat_params_init_minicpm5(const common_chat_template & tmpl, + const autoparser::generation_params & inputs) { + common_chat_params data; + + data.prompt = common_chat_template_direct_apply_impl(tmpl, inputs); + data.generation_prompt = common_chat_template_generation_prompt_impl(tmpl, inputs); + data.format = COMMON_CHAT_FORMAT_PEG_MINICPM5; + data.supports_thinking = true; + data.preserved_tokens = { + "", + "", + "", + "", + "<|im_start|>", + "<|im_end|>", + "", + "", + }; + + const std::string GEN_PROMPT = "<|im_start|>assistant\n"; + const std::string THINK_START = ""; + const std::string THINK_END = ""; + const std::vector TOOL_START_MARKERS = { + THINK_END, + "\n") : + p.literal("\n\n\n\n"); + + auto reasoning = p.eps(); + if (extract_reasoning && inputs.enable_thinking) { + reasoning = p.reasoning(p.until_one_of(TOOL_START_MARKERS)) + p.optional(p.literal("\n")) + + p.optional(p.literal(THINK_END) + p.optional(p.literal("\n\n"))); + } + + auto suffix = p.optional(p.literal("<|im_end|>") + p.optional(p.literal("\n"))); + + if (!has_tools || inputs.tool_choice == COMMON_CHAT_TOOL_CHOICE_NONE) { + return assistant_prefix + thinking_prefix + reasoning + p.content(p.rest()) + suffix + end; + } + + // Do not wrap minicpm5_xml_tool_calls in another rule("tool-calls", ...): + // parallel mode already defines trigger_rule("tool-calls", ...) and a second + // rule with the same name resolves to itself -> peg stack overflow. + auto tool_calls = p.minicpm5_xml_tool_calls(inputs.tools, inputs.parallel_tool_calls); + auto content = p.content(p.until_one_of({ " common_chat_try_specialized_template( return common_chat_params_init_gemma4(tmpl, params); } + // MiniCPM5 - XML tool calls with ... + if (is_minicpm5_template(src)) { + LOG_DBG("Using specialized template: MiniCPM5\n"); + return common_chat_params_init_minicpm5(tmpl, params); + } + return std::nullopt; } @@ -2643,9 +2799,13 @@ common_chat_msg common_chat_peg_parse(const common_peg_arena & src_pars LOG_DBG("No parser definition detected, assuming pure content parser."); } + const std::string normalized_input = params.format == COMMON_CHAT_FORMAT_PEG_MINICPM5 ? + common_chat_normalize_minicpm5_output(input) : + input; + const std::string effective_input = params.generation_prompt.empty() - ? input - : params.generation_prompt + input; + ? normalized_input + : params.generation_prompt + normalized_input; //LOG_DBG("Parsing PEG input with format %s: %s\n", common_chat_format_name(params.format), effective_input.c_str()); diff --git a/common/chat.h b/common/chat.h index 5659cd42a07c..9b2c838e6643 100644 --- a/common/chat.h +++ b/common/chat.h @@ -173,6 +173,7 @@ enum common_chat_format { COMMON_CHAT_FORMAT_PEG_SIMPLE, COMMON_CHAT_FORMAT_PEG_NATIVE, COMMON_CHAT_FORMAT_PEG_GEMMA4, + COMMON_CHAT_FORMAT_PEG_MINICPM5, COMMON_CHAT_FORMAT_COUNT, // Not a format, just the # formats }; diff --git a/common/jinja/value.cpp b/common/jinja/value.cpp index cd6a36956cea..2ecfbb37133d 100644 --- a/common/jinja/value.cpp +++ b/common/jinja/value.cpp @@ -1108,6 +1108,34 @@ const func_builtins & value_array_t::get_builtins() const { std::reverse(arr.begin(), arr.end()); return is_val(val) ? mk_val(std::move(arr)) : mk_val(std::move(arr)); }}, + {"min", [](const func_args & args) -> value { + args.ensure_vals(); + const auto & arr = args.get_pos(0)->as_array(); + if (arr.empty()) { + throw raised_exception("min() arg is an empty sequence"); + } + value result = arr[0]; + for (size_t i = 1; i < arr.size(); ++i) { + if (value_compare(arr[i], result, value_compare_op::lt)) { + result = arr[i]; + } + } + return result; + }}, + {"max", [](const func_args & args) -> value { + args.ensure_vals(); + const auto & arr = args.get_pos(0)->as_array(); + if (arr.empty()) { + throw raised_exception("max() arg is an empty sequence"); + } + value result = arr[0]; + for (size_t i = 1; i < arr.size(); ++i) { + if (value_compare(arr[i], result, value_compare_op::gt)) { + result = arr[i]; + } + } + return result; + }}, {"unique", array_unique_not_implemented}, }; return builtins; diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index e284a58d1c67..1c54880f6d10 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -195,6 +195,7 @@ if (NOT WIN32 OR NOT BUILD_SHARED_LIBS) endif() llama_build_and_test(test-chat-peg-parser.cpp peg-parser/simple-tokenize.cpp) +llama_build_and_test(test-chat-peg-parser-minicpm5.cpp) llama_build_and_test(test-jinja.cpp) llama_test(test-jinja NAME test-jinja-py ARGS -py LABEL python) llama_build_and_test(test-chat-auto-parser.cpp WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}) diff --git a/tests/test-chat-peg-parser-minicpm5.cpp b/tests/test-chat-peg-parser-minicpm5.cpp new file mode 100644 index 000000000000..3148557d05f9 --- /dev/null +++ b/tests/test-chat-peg-parser-minicpm5.cpp @@ -0,0 +1,128 @@ +// Offline MiniCPM5 tool-call parse + OpenAI JSON serialization tests. +// Isolates peg-minicpm5 from llama-server to narrow segfault root cause. + +#include "chat-peg-parser.h" +#include "chat.h" +#include "common.h" +#include "testing.h" + +#include "nlohmann/json.hpp" + +#include +#include + +using json = nlohmann::ordered_json; + +static json minicpm5_tools() { + return json::parse( + R"([{"type":"function","function":{"name":"python","parameters":{"type":"object","properties":{"code":{"type":"string"}},"required":["code"]}}},{"type":"function","function":{"name":"get_current_timestamp","parameters":{"type":"object","properties":{}}}}])"); +} + +static common_chat_parser_params make_minicpm5_parser_params(const json & tools, bool parallel_tool_calls) { + auto parser = build_chat_peg_parser([&](common_chat_peg_builder & p) { + const std::string GEN_PROMPT = "<|im_start|>assistant\n"; + auto tool_calls = p.minicpm5_xml_tool_calls(tools, parallel_tool_calls); + return p.literal(GEN_PROMPT) + p.content(p.until_one_of({ "assistant\nprint('Hello, World!')", + "python", + false }, + { "stripped tags", + "<|im_start|>assistant\n name=\"python\"> name=\"code\">print('Hello, World!')", + "python", + false }, + { "empty args timestamp", + "<|im_start|>assistant\n", + "get_current_timestamp", + false }, + { "parallel tool calls", + "<|im_start|>assistant\n" + "print('x')", + "python", + true }, + }; + + for (const auto & c : cases) { + t.test(c.label, [&](testing & t) { + const auto pp = make_minicpm5_parser_params(tools, c.parallel_tool_calls); + common_chat_msg msg = common_chat_parse(c.input, false, pp); + assert_tool_call(t, msg, c.tool); + }); + } +} + +static void test_streaming_diffs(testing & t) { + t.test("streaming partial -> final diffs", [&](testing & t) { + const auto tools = minicpm5_tools(); + const auto pp = make_minicpm5_parser_params(tools, false); + + const std::string full = + "<|im_start|>assistant\nprint('Hello')"; + + common_chat_msg prv; + std::string generated; + for (size_t i = 1; i <= full.size(); ++i) { + generated = full.substr(0, i); + common_chat_msg cur = common_chat_parse(generated, i < full.size(), pp); + auto diffs = common_chat_msg_diff::compute_diffs(prv, cur); + (void) diffs; + prv = cur; + } + + assert_tool_call(t, prv, "python"); + }); +} + +static void test_set_tool_call_ids(testing & t) { + t.test("set_tool_call_ids + to_json", [&](testing & t) { + const auto pp = make_minicpm5_parser_params(minicpm5_tools(), false); + const std::string input = + "<|im_start|>assistant\nprint('x')"; + + common_chat_msg msg = common_chat_parse(input, false, pp); + std::vector ids_cache; + msg.set_tool_call_ids(ids_cache, []() { return std::string("call_test123"); }); + assert_tool_call(t, msg, "python"); + }); +} + +int main() { + testing t(std::cout); + + test_full_parse_cases(t); + test_streaming_diffs(t); + test_set_tool_call_ids(t); + + return t.summary(); +} diff --git a/tests/test-chat-peg-parser.cpp b/tests/test-chat-peg-parser.cpp index 908b13fd0ca7..5dee8aeaf00a 100644 --- a/tests/test-chat-peg-parser.cpp +++ b/tests/test-chat-peg-parser.cpp @@ -981,3 +981,4 @@ static void test_tagged_peg_parser(testing & t) { t.assert_equal("fun_post should be '>'", ">", result.tags["fun_post"]); }); } + diff --git a/tests/test-jinja.cpp b/tests/test-jinja.cpp index 8039956246c3..4e4f57f2e219 100644 --- a/tests/test-jinja.cpp +++ b/tests/test-jinja.cpp @@ -1558,6 +1558,18 @@ static void test_array_methods(testing & t) { "6" ); + test_template(t, "array|min", + "{{ [tool_calls_count, tool_sep_count]|min }}", + {{"tool_calls_count", 2}, {"tool_sep_count", 1}}, + "1" + ); + + test_template(t, "array|max", + "{{ [tool_calls_count, tool_sep_count]|max }}", + {{"tool_calls_count", 2}, {"tool_sep_count", 1}}, + "2" + ); + // not used by any chat templates // test_template(t, "array.insert()", // "{% set _ = arr.insert(1, 'x') %}{{ arr|join(',') }}", From ce364a36031e1e3095132a9ca5de5d042755a0a3 Mon Sep 17 00:00:00 2001 From: zhangtao Date: Thu, 28 May 2026 10:04:55 +0000 Subject: [PATCH 2/9] Refactor MiniCPM5 PEG parser per review feedback --- common/chat-peg-parser.cpp | 125 +++++--------- common/chat-peg-parser.h | 20 ++- common/chat.cpp | 207 ++++++++++++++---------- common/jinja/value.cpp | 16 ++ models/templates/MiniCPM5-1B.jinja | 179 ++++++++++++++++++++ tests/CMakeLists.txt | 1 - tests/test-chat-peg-parser-minicpm5.cpp | 128 --------------- tests/test-chat.cpp | 159 ++++++++++++++---- tests/test-jinja.cpp | 31 ++++ 9 files changed, 532 insertions(+), 334 deletions(-) create mode 100644 models/templates/MiniCPM5-1B.jinja delete mode 100644 tests/test-chat-peg-parser-minicpm5.cpp diff --git a/common/chat-peg-parser.cpp b/common/chat-peg-parser.cpp index 675d253b6350..ab81facbc20a 100644 --- a/common/chat-peg-parser.cpp +++ b/common/chat-peg-parser.cpp @@ -235,19 +235,47 @@ common_peg_parser common_chat_peg_builder::tag_with_safe_content(const std::stri return zero_or_more(choice({ p, content_chunk })); } -std::string & common_chat_peg_mapper::args_target() { - common_chat_tool_call * tool = active_tool(); - return (tool && !tool->name.empty()) ? tool->arguments : args_buffer; +void common_chat_peg_minicpm5_mapper::finalize_tool_call_arguments(std::string & args) { + if (args.empty() || args.front() != '{') { + return; + } + bool in_string = false; + bool escaped = false; + for (char c : args) { + if (escaped) { + escaped = false; + continue; + } + if (c == '\\' && in_string) { + escaped = true; + continue; + } + if (c == '"') { + in_string = !in_string; + } + } + if (in_string) { + args += '"'; + } + for (int d = json_brace_depth(args); d > 0; d--) { + args += '}'; + } } -common_chat_tool_call * common_chat_peg_mapper::active_tool() { - if (committed_tool_idx.has_value()) { - return &result.tool_calls.at(committed_tool_idx.value()); - } - if (pending_tool_call.has_value()) { - return &pending_tool_call.value(); +void common_chat_peg_minicpm5_mapper::from_ast(const common_peg_ast_arena & arena, + const common_peg_parse_result & parse_result) { + common_chat_peg_mapper::from_ast(arena, parse_result); + // MiniCPM5 often omits . Lenient parsing stops before tool_close, + // so finalize argument JSON on the completed response (same as tool_close handler). + if (!is_partial_) { + for (auto & tool_call : result.tool_calls) { + finalize_tool_call_arguments(tool_call.arguments); + } } - return nullptr; +} + +std::string & common_chat_peg_mapper::args_target() { + return (current_tool && !current_tool->name.empty()) ? current_tool->arguments : args_buffer; } std::string common_chat_peg_mapper::normalize_container_value(const std::string & input) { @@ -314,14 +342,12 @@ void common_chat_peg_mapper::map(const common_peg_ast_node & node) { if (is_tool_open) { pending_tool_call = common_chat_tool_call(); - committed_tool_idx.reset(); + current_tool = &pending_tool_call.value(); arg_count = 0; args_buffer.clear(); closing_quote_pending = false; } - common_chat_tool_call * current_tool = active_tool(); - if (is_tool_id && current_tool) { auto text = trim_trailing_space(node.text); if (text.size() >= 2 && text.front() == '"' && text.back() == '"') { @@ -342,13 +368,11 @@ void common_chat_peg_mapper::map(const common_peg_ast_node & node) { // Add the tool call to results so streaming can see it if (pending_tool_call.has_value()) { result.tool_calls.push_back(pending_tool_call.value()); - committed_tool_idx = result.tool_calls.size() - 1; pending_tool_call.reset(); + current_tool = &result.tool_calls.back(); } } - current_tool = active_tool(); - if (is_tool_args && current_tool) { // For JSON format: arguments come as a complete JSON object // For tagged format: built up from individual arg_name/arg_value nodes @@ -362,8 +386,6 @@ void common_chat_peg_mapper::map(const common_peg_ast_node & node) { closing_quote_pending = false; } - current_tool = active_tool(); - if (is_arg_name && current_tool) { std::string arg_entry; if (arg_count > 0) { @@ -431,7 +453,6 @@ void common_chat_peg_mapper::map(const common_peg_ast_node & node) { } pending_tool_call.reset(); } - committed_tool_idx.reset(); } } @@ -1074,69 +1095,3 @@ void common_chat_peg_gemma4_mapper::visit(const common_peg_ast_arena & arena, co visit(arena, child_id); } } - -common_peg_parser common_chat_peg_builder::minicpm5_xml_tool_calls(const ordered_json & tools, - bool parallel_tool_calls) { - if (!tools.is_array() || tools.empty()) { - return eps(); - } - - static const std::vector PARAM_VALUE_STOP = { - "", - " name=\"", - "", - "") + literal("]]>"), - until_one_of(PARAM_VALUE_STOP), - }); - - auto tool_choices = choice(); - - for (const auto & tool_def : tools) { - if (!tool_def.contains("function")) { - continue; - } - const auto & function = tool_def.at("function"); - const std::string name = function.at("name"); - ordered_json params = function.contains("parameters") ? function.at("parameters") : ordered_json::object(); - - auto args = eps(); - if (params.contains("properties") && params.at("properties").is_object() && - !params.at("properties").empty()) { - auto arg_choice = choice(); - for (const auto & el : params.at("properties").items()) { - const std::string & prop_name = el.key(); - const std::string & prop_type = el.value().value("type", "string"); - - auto value_parser = prop_type == "string" ? tool_arg_string_value(arg_value) : tool_arg_value(arg_value); - - auto arg_rule = tool_arg( - tool_arg_open(param_name_prefix + tool_arg_name(literal(prop_name)) + literal("\">")) + - value_parser + - tool_arg_close(optional(literal("")) + space())); - - arg_choice |= arg_rule; - } - args = zero_or_more(arg_choice + space()); - } - - auto tool_parser = tool( - tool_open(func_name_prefix + tool_name(literal(name)) + literal("\">")) + space() + tool_args(args) + - space() + tool_close(optional(literal("")))); - - tool_choices |= rule("tool-" + name, tool_parser); - } - - if (parallel_tool_calls) { - return trigger_rule("tool-calls", one_or_more(tool_choices + space())); - } - - return trigger_rule("tool-call", tool_choices); -} diff --git a/common/chat-peg-parser.h b/common/chat-peg-parser.h index ba16e10d83ef..be769c144300 100644 --- a/common/chat-peg-parser.h +++ b/common/chat-peg-parser.h @@ -22,14 +22,13 @@ class common_chat_peg_mapper { private: // Tool call handling state std::optional pending_tool_call; // Tool call waiting for name - std::optional committed_tool_idx; // index in result.tool_calls after streaming push + common_chat_tool_call * current_tool = nullptr; int arg_count = 0; bool closing_quote_pending = false; std::string args_buffer; // Buffer to delay arguments until tool name is known - common_chat_tool_call * active_tool(); // Returns a reference to the active argument destination string. - // Before tool_name is known, writes go to args_buffer; after, to active_tool()->arguments. + // Before tool_name is known, writes go to args_buffer; after, to current_tool->arguments. std::string & args_target(); }; @@ -41,6 +40,16 @@ class common_chat_peg_gemma4_mapper : public common_chat_peg_mapper { void visit(const common_peg_ast_arena & arena, common_peg_ast_id id); }; +class common_chat_peg_minicpm5_mapper : public common_chat_peg_mapper { + public: + common_chat_peg_minicpm5_mapper(common_chat_msg & msg, bool is_partial) : + common_chat_peg_mapper(msg), is_partial_(is_partial) {} + void from_ast(const common_peg_ast_arena & arena, const common_peg_parse_result & result) override; + private: + bool is_partial_; + static void finalize_tool_call_arguments(std::string & args); +}; + struct content_structure; struct tool_call_structure; @@ -137,11 +146,6 @@ class common_chat_peg_builder : public common_peg_parser_builder { bool parallel_tool_calls, bool allow_json_literals); - // MiniCPM5 XML tool calls: - // baz - common_peg_parser minicpm5_xml_tool_calls(const nlohmann::ordered_json & tools, - bool parallel_tool_calls); - private: // Python values plus JSON true/false/null. common_peg_parser python_or_json_value(); diff --git a/common/chat.cpp b/common/chat.cpp index 86ad5349c4be..8a8e2a407115 100644 --- a/common/chat.cpp +++ b/common/chat.cpp @@ -22,7 +22,6 @@ #include #include -#include #include #include #include @@ -2333,53 +2332,6 @@ static bool is_minicpm5_template(const std::string & src) { src.find(" name="code">...` instead of `...`. - static const std::regex LEADING_FUNC_ATTR(R"((?:^|[\n\r])\s*name=\"([^\"]+)\">)"); - out = std::regex_replace(out, LEADING_FUNC_ATTR, "\n"); - static const std::regex PARAM_ATTR(R"(>\s*name=\"([^\"]+)\">)"); - out = std::regex_replace(out, PARAM_ATTR, ">"); - - static const std::string IM_END = "<|im_end|>"; - if (out.size() >= IM_END.size() && - out.compare(out.size() - IM_END.size(), IM_END.size(), IM_END) == 0) { - out.erase(out.size() - IM_END.size()); - while (!out.empty() && (out.back() == '\n' || out.back() == ' ')) { - out.pop_back(); - } - } - - return out; -} - // MiniCPM5 format: // - Reasoning: {reasoning} (optional) // - Tool calls: value @@ -2399,7 +2351,6 @@ static common_chat_params common_chat_params_init_minicpm5(const common_chat_tem "", "", "<|im_start|>", - "<|im_end|>", "", "", }; @@ -2407,13 +2358,6 @@ static common_chat_params common_chat_params_init_minicpm5(const common_chat_tem const std::string GEN_PROMPT = "<|im_start|>assistant\n"; const std::string THINK_START = ""; const std::string THINK_END = ""; - const std::vector TOOL_START_MARKERS = { - THINK_END, - "\n") : - p.literal("\n\n\n\n"); - auto reasoning = p.eps(); - if (extract_reasoning && inputs.enable_thinking) { - reasoning = p.reasoning(p.until_one_of(TOOL_START_MARKERS)) + p.optional(p.literal("\n")) + - p.optional(p.literal(THINK_END) + p.optional(p.literal("\n\n"))); + auto thinking = p.eps(); + if (!inputs.enable_thinking) { + thinking = p.literal("\n\n\n\n"); + } else if (extract_reasoning) { + thinking = p.literal(THINK_START) + p.optional(p.literal("\n")) + p.choice({ + p.reasoning(p.until(THINK_END)) + p.literal(THINK_END) + ws(), + p.reasoning(p.until_one_of({ "") + p.optional(p.literal("\n"))); - if (!has_tools || inputs.tool_choice == COMMON_CHAT_TOOL_CHOICE_NONE) { - return assistant_prefix + thinking_prefix + reasoning + p.content(p.rest()) + suffix + end; + return assistant_prefix + thinking + p.content(p.rest()) + im_end_suffix + end; } - // Do not wrap minicpm5_xml_tool_calls in another rule("tool-calls", ...): + static const std::vector PARAM_VALUE_STOP = { + "", + "\"> name=\"", + "", + "") + p.literal("]]>"), + p.until_one_of(PARAM_VALUE_STOP), + }); + + auto tool_choice = p.choice(); + foreach_function(inputs.tools, [&](const json & tool) { + const auto & function = tool.at("function"); + const std::string name = function.at("name"); + auto params = function.contains("parameters") ? function.at("parameters") : json::object(); + + auto args = p.eps(); + if (params.contains("properties") && params.at("properties").is_object() && + !params.at("properties").empty()) { + auto schema_info = common_schema_info(); + schema_info.resolve_refs(params); + + auto arg_choice = p.choice(); + for (const auto & [prop_name, prop_schema] : params.at("properties").items()) { + const bool is_string = schema_info.resolves_to_string(prop_schema); + + auto value_parser = is_string ? + p.tool_arg_string_value(arg_value) : + p.tool_arg_json_value(p.schema(p.json(), + "tool-" + name + "-arg-" + prop_name + "-schema", + prop_schema, + false)); + + auto arg_rule = p.tool_arg( + p.tool_arg_open(p.choice({ + param_name_prefix, + param_name_prefix_compact, + }) + p.tool_arg_name(p.literal(prop_name)) + p.literal("\">")) + + value_parser + + p.tool_arg_close(p.optional(p.literal("")) + ws())); + + arg_choice |= arg_rule; + } + args = p.zero_or_more(arg_choice + ws()); + } + + auto tool_parser = p.tool( + p.tool_open(p.choice({ + func_name_prefix, + func_name_prefix_compact, + }) + p.tool_name(p.literal(name)) + p.literal("\">")) + + p.tool_args(args) + ws() + + p.tool_close(p.optional(p.literal("")))); + + tool_choice |= p.rule("tool-" + name, tool_parser); + }); + + // Do not wrap trigger_rule("tool-calls", ...) in another rule("tool-calls", ...): // parallel mode already defines trigger_rule("tool-calls", ...) and a second // rule with the same name resolves to itself -> peg stack overflow. - auto tool_calls = p.minicpm5_xml_tool_calls(inputs.tools, inputs.parallel_tool_calls); - auto content = p.content(p.until_one_of({ " mapper; if (params.format == COMMON_CHAT_FORMAT_PEG_GEMMA4) { mapper = std::make_unique(msg); + } else if (params.format == COMMON_CHAT_FORMAT_PEG_MINICPM5) { + mapper = std::make_unique(msg, is_partial); } else { mapper = std::make_unique(msg); } @@ -2849,6 +2888,8 @@ common_chat_msg common_chat_peg_parse(const common_peg_arena & src_pars std::unique_ptr mapper; if (params.format == COMMON_CHAT_FORMAT_PEG_GEMMA4) { mapper = std::make_unique(msg); + } else if (params.format == COMMON_CHAT_FORMAT_PEG_MINICPM5) { + mapper = std::make_unique(msg, is_partial); } else { mapper = std::make_unique(msg); } diff --git a/common/jinja/value.cpp b/common/jinja/value.cpp index 2ecfbb37133d..63695f699e4f 100644 --- a/common/jinja/value.cpp +++ b/common/jinja/value.cpp @@ -1109,7 +1109,15 @@ const func_builtins & value_array_t::get_builtins() const { return is_val(val) ? mk_val(std::move(arr)) : mk_val(std::move(arr)); }}, {"min", [](const func_args & args) -> value { + args.ensure_count(1, 4); args.ensure_vals(); + value attribute = args.get_kwarg_or_pos("attribute", 1); + value val_case = args.get_kwarg_or_pos("case_sensitive", 2); + if (!attribute->is_undefined()) { + throw not_implemented_exception("min: attribute not implemented"); + } + // FIXME: min is currently always case sensitive + (void) val_case; const auto & arr = args.get_pos(0)->as_array(); if (arr.empty()) { throw raised_exception("min() arg is an empty sequence"); @@ -1123,7 +1131,15 @@ const func_builtins & value_array_t::get_builtins() const { return result; }}, {"max", [](const func_args & args) -> value { + args.ensure_count(1, 4); args.ensure_vals(); + value attribute = args.get_kwarg_or_pos("attribute", 1); + value val_case = args.get_kwarg_or_pos("case_sensitive", 2); + if (!attribute->is_undefined()) { + throw not_implemented_exception("max: attribute not implemented"); + } + // FIXME: max is currently always case sensitive + (void) val_case; const auto & arr = args.get_pos(0)->as_array(); if (arr.empty()) { throw raised_exception("max() arg is an empty sequence"); diff --git a/models/templates/MiniCPM5-1B.jinja b/models/templates/MiniCPM5-1B.jinja new file mode 100644 index 000000000000..cb2934c459c6 --- /dev/null +++ b/models/templates/MiniCPM5-1B.jinja @@ -0,0 +1,179 @@ +{{- bos_token }}{%- if tools %} + {%- set tool_definitions %} + {{- "# Tools\n\nYou are provided with function signatures within XML tags:\n" }} + {%- for tool in tools %} + {{- "\n" }} + {{- tool | tojson(ensure_ascii=False) }} + {%- endfor %} + {{- '\n\n\nTool usage guidelines:\n- You may call zero or more functions. If no function calls are needed, just answer normally and do not include any .\n- When calling a function, return an XML object within using:\nparam-value\n- param-value may be multi-line. If it contains <, & or newline characters, wrap it in a CDATA block: ' }} + {%- endset %} + + {{- '<|im_start|>system\n' }} + {%- if messages[0].role == 'system' %} + {%- if '' in messages[0].content %} + {{- messages[0].content.replace('', tool_definitions) }} + {%- else %} + {{- messages[0].content + '\n\n' + tool_definitions }} + {%- endif %} + {%- else %} + {{- tool_definitions.lstrip() }} + {%- endif %} + {{- '<|im_end|>\n' }} +{%- else %} + {%- if messages[0].role == 'system' %} + {{- '<|im_start|>system\n' + messages[0].content + '<|im_end|>\n' }} + {%- endif %} +{%- endif %} +{%- set ns = namespace(multi_step_tool=true, last_query_index=messages|length - 1) %} +{%- for message in messages[::-1] %} + {%- set index = (messages|length - 1) - loop.index0 %} + {%- if ns.multi_step_tool and message.role == "user" and message.content is string and not(message.content.startswith('') and message.content.endswith('')) %} + {%- set ns.multi_step_tool = false %} + {%- set ns.last_query_index = index %} + {%- endif %} +{%- endfor %} +{%- for message in messages %} + {%- if message.content is string %} + {%- set content = message.content %} + {%- else %} + {%- set content = '' %} + {%- endif %} + {%- if (message.role == "user") or (message.role == "system" and not loop.first) %} + {{- '<|im_start|>' + message.role + '\n' + content + '<|im_end|>' + '\n' }} + {%- elif message.role == "assistant" %} + {%- set reasoning_content = '' %} + {%- if message.reasoning_content is string %} + {%- set reasoning_content = message.reasoning_content %} + {%- else %} + {%- if '' in content %} + {%- set reasoning_content = content.split('')[0].rstrip('\n').split('')[-1].lstrip('\n') %} + {%- set content = content.split('')[-1].lstrip('\n') %} + {%- endif %} + {%- endif %} + + {%- if message.tool_calls %} + {%- set content_parts = content.split('') %} + {%- set processed_content = content_parts[0] %} + {%- set tool_calls_count = message.tool_calls|length %} + {%- set tool_sep_count = content_parts|length - 1 %} + {%- set min_count = [tool_calls_count, tool_sep_count]|min %} + + {%- for i in range(1, content_parts|length) %} + {%- set tool_index = i - 1 %} + {%- if tool_index < tool_calls_count %} + {%- set tool_call = message.tool_calls[tool_index] %} + {%- if tool_call.function %} + {%- set tool_call = tool_call.function %} + {%- endif %} + {%- set single_tool_xml %} + {{- '' }} + {%- if tool_call.arguments %} + {%- set args_dict = tool_call.arguments %} + {%- for param_name, param_value in args_dict.items() %} + {{- '' }} + {%- if param_value is string and ('<' in param_value or '&' in param_value or '\n' in param_value) %} + {{- '' }} + {%- else %} + {{- param_value }} + {%- endif %} + {{- '' }} + {%- endfor %} + {%- endif %} + {{- '' }} + {%- endset %} + {%- set processed_content = processed_content + single_tool_xml + content_parts[i] %} + {%- else %} + {%- set processed_content = processed_content + content_parts[i] %} + {%- endif %} + {%- endfor %} + + {%- if tool_calls_count > tool_sep_count %} + {%- for remaining_index in range(tool_sep_count, tool_calls_count) %} + {%- set tool_call = message.tool_calls[remaining_index] %} + {%- if tool_call.function %} + {%- set tool_call = tool_call.function %} + {%- endif %} + {%- set remaining_tool_xml %} + {{- '' }} + {%- if tool_call.arguments %} + {%- set args_dict = tool_call.arguments %} + {%- for param_name, param_value in args_dict.items() %} + {{- '' }} + {%- if param_value is string and ('<' in param_value or '&' in param_value or '\n' in param_value) %} + {{- '' }} + {%- else %} + {{- param_value }} + {%- endif %} + {{- '' }} + {%- endfor %} + {%- endif %} + {{- '' }} + {%- endset %} + {%- set processed_content = processed_content + remaining_tool_xml %} + {%- endfor %} + {%- endif %} + + {%- set content = processed_content %} + {%- endif %} + + {%- if loop.index0 > ns.last_query_index %} + {%- if reasoning_content %} + {{- '<|im_start|>' + message.role + '\n\n' + reasoning_content.strip('\n') + '\n\n\n' + content.lstrip('\n') }} + {%- else %} + {{- '<|im_start|>' + message.role + '\n' + content }} + {%- endif %} + {%- else %} + {{- '<|im_start|>' + message.role + '\n' + content }} + {%- endif %} + + {%- if message.tool_calls and not has_tool_sep %} + {%- for tool_call in message.tool_calls %} + {%- if (loop.first and content) or (not loop.first) %} + {{- '\n' }} + {%- endif %} + {%- if tool_call.function %} + {%- set tool_call = tool_call.function %} + {%- endif %} + {{- '' }} + {%- if tool_call.arguments %} + {%- set args_dict = tool_call.arguments %} + {%- for param_name, param_value in args_dict.items() %} + {{- '' }} + {%- if param_value is string and ('<' in param_value or '&' in param_value or '\n' in param_value) %} + {{- '' }} + {%- else %} + {{- param_value }} + {%- endif %} + {{- '' }} + {%- endfor %} + {%- endif %} + {{- '' }} + {%- endfor %} + {%- endif %} + {{- '<|im_end|>\n' }} + {%- elif message.role == "tool" %} + {%- if loop.first or (messages[loop.index0 - 1].role != "tool") %} + {{- '<|im_start|>user' }} + {%- endif %} + {{- '\n\n' }} + {%- if message.content is string %} + {{- content }} + {%- else %} + {{- message.content | tojson(ensure_ascii=False) }} + {%- endif %} + {{- '\n' }} + {%- if loop.last or (messages[loop.index0 + 1].role != "tool") %} + {{- '<|im_end|>\n' }} + {%- endif %} + {%- endif %} +{%- endfor %} +{%- if add_generation_prompt %} + {{- '<|im_start|>assistant\n' }} + {%- if enable_thinking is defined %} + {%- if enable_thinking is false %} + {{- '\n\n\n\n' }} + {%- elif enable_thinking is true %} + {{- '\n' }} + {%- endif %} + {%- endif %} +{%- endif %} diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 1c54880f6d10..e284a58d1c67 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -195,7 +195,6 @@ if (NOT WIN32 OR NOT BUILD_SHARED_LIBS) endif() llama_build_and_test(test-chat-peg-parser.cpp peg-parser/simple-tokenize.cpp) -llama_build_and_test(test-chat-peg-parser-minicpm5.cpp) llama_build_and_test(test-jinja.cpp) llama_test(test-jinja NAME test-jinja-py ARGS -py LABEL python) llama_build_and_test(test-chat-auto-parser.cpp WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}) diff --git a/tests/test-chat-peg-parser-minicpm5.cpp b/tests/test-chat-peg-parser-minicpm5.cpp deleted file mode 100644 index 3148557d05f9..000000000000 --- a/tests/test-chat-peg-parser-minicpm5.cpp +++ /dev/null @@ -1,128 +0,0 @@ -// Offline MiniCPM5 tool-call parse + OpenAI JSON serialization tests. -// Isolates peg-minicpm5 from llama-server to narrow segfault root cause. - -#include "chat-peg-parser.h" -#include "chat.h" -#include "common.h" -#include "testing.h" - -#include "nlohmann/json.hpp" - -#include -#include - -using json = nlohmann::ordered_json; - -static json minicpm5_tools() { - return json::parse( - R"([{"type":"function","function":{"name":"python","parameters":{"type":"object","properties":{"code":{"type":"string"}},"required":["code"]}}},{"type":"function","function":{"name":"get_current_timestamp","parameters":{"type":"object","properties":{}}}}])"); -} - -static common_chat_parser_params make_minicpm5_parser_params(const json & tools, bool parallel_tool_calls) { - auto parser = build_chat_peg_parser([&](common_chat_peg_builder & p) { - const std::string GEN_PROMPT = "<|im_start|>assistant\n"; - auto tool_calls = p.minicpm5_xml_tool_calls(tools, parallel_tool_calls); - return p.literal(GEN_PROMPT) + p.content(p.until_one_of({ "assistant\nprint('Hello, World!')", - "python", - false }, - { "stripped tags", - "<|im_start|>assistant\n name=\"python\"> name=\"code\">print('Hello, World!')", - "python", - false }, - { "empty args timestamp", - "<|im_start|>assistant\n", - "get_current_timestamp", - false }, - { "parallel tool calls", - "<|im_start|>assistant\n" - "print('x')", - "python", - true }, - }; - - for (const auto & c : cases) { - t.test(c.label, [&](testing & t) { - const auto pp = make_minicpm5_parser_params(tools, c.parallel_tool_calls); - common_chat_msg msg = common_chat_parse(c.input, false, pp); - assert_tool_call(t, msg, c.tool); - }); - } -} - -static void test_streaming_diffs(testing & t) { - t.test("streaming partial -> final diffs", [&](testing & t) { - const auto tools = minicpm5_tools(); - const auto pp = make_minicpm5_parser_params(tools, false); - - const std::string full = - "<|im_start|>assistant\nprint('Hello')"; - - common_chat_msg prv; - std::string generated; - for (size_t i = 1; i <= full.size(); ++i) { - generated = full.substr(0, i); - common_chat_msg cur = common_chat_parse(generated, i < full.size(), pp); - auto diffs = common_chat_msg_diff::compute_diffs(prv, cur); - (void) diffs; - prv = cur; - } - - assert_tool_call(t, prv, "python"); - }); -} - -static void test_set_tool_call_ids(testing & t) { - t.test("set_tool_call_ids + to_json", [&](testing & t) { - const auto pp = make_minicpm5_parser_params(minicpm5_tools(), false); - const std::string input = - "<|im_start|>assistant\nprint('x')"; - - common_chat_msg msg = common_chat_parse(input, false, pp); - std::vector ids_cache; - msg.set_tool_call_ids(ids_cache, []() { return std::string("call_test123"); }); - assert_tool_call(t, msg, "python"); - }); -} - -int main() { - testing t(std::cout); - - test_full_parse_cases(t); - test_streaming_diffs(t); - test_set_tool_call_ids(t); - - return t.summary(); -} diff --git a/tests/test-chat.cpp b/tests/test-chat.cpp index 902a4c135abe..16c30a437bd2 100644 --- a/tests/test-chat.cpp +++ b/tests/test-chat.cpp @@ -986,8 +986,9 @@ struct peg_test_case { common_chat_templates_inputs params; std::string input; common_chat_msg expect; - bool is_partial = false; - bool expect_reconstruction = false; + bool is_partial = false; + bool expect_reconstruction = false; + bool expect_streaming_consistency = true; }; struct make_peg_parser { @@ -1078,36 +1079,38 @@ static void test_peg_parser(common_chat_templates * tmpls, std::string prefix = tc.input.substr(0, safe_len); common_chat_msg msg_current = parser.parse(prefix, is_partial); - for (const auto & diff : common_chat_msg_diff::compute_diffs(msg_prev, msg_current)) { - if (!diff.reasoning_content_delta.empty()) { - msg_accum.reasoning_content += diff.reasoning_content_delta; - } - if (!diff.content_delta.empty()) { - msg_accum.content += diff.content_delta; - } - if (diff.tool_call_index != std::string::npos) { - // During partial parsing, a new tool call may appear with empty name initially - // The name gets filled in as more input is parsed - while (msg_accum.tool_calls.size() <= diff.tool_call_index) { - msg_accum.tool_calls.push_back({ "", "", "" }); - } - // Always update name and id from diff (may change during incremental parsing), but only if the delta - // actually contains them - if (!diff.tool_call_delta.name.empty()) { - msg_accum.tool_calls[diff.tool_call_index].name = diff.tool_call_delta.name; + if (tc.expect_streaming_consistency) { + for (const auto & diff : common_chat_msg_diff::compute_diffs(msg_prev, msg_current)) { + if (!diff.reasoning_content_delta.empty()) { + msg_accum.reasoning_content += diff.reasoning_content_delta; } - if (!diff.tool_call_delta.id.empty()) { - msg_accum.tool_calls[diff.tool_call_index].id = diff.tool_call_delta.id; + if (!diff.content_delta.empty()) { + msg_accum.content += diff.content_delta; } - if (!diff.tool_call_delta.arguments.empty()) { - msg_accum.tool_calls[diff.tool_call_index].arguments += diff.tool_call_delta.arguments; + if (diff.tool_call_index != std::string::npos) { + // During partial parsing, a new tool call may appear with empty name initially + // The name gets filled in as more input is parsed + while (msg_accum.tool_calls.size() <= diff.tool_call_index) { + msg_accum.tool_calls.push_back({ "", "", "" }); + } + // Always update name and id from diff (may change during incremental parsing), but only if the delta + // actually contains them + if (!diff.tool_call_delta.name.empty()) { + msg_accum.tool_calls[diff.tool_call_index].name = diff.tool_call_delta.name; + } + if (!diff.tool_call_delta.id.empty()) { + msg_accum.tool_calls[diff.tool_call_index].id = diff.tool_call_delta.id; + } + if (!diff.tool_call_delta.arguments.empty()) { + msg_accum.tool_calls[diff.tool_call_index].arguments += diff.tool_call_delta.arguments; + } } } - } - try { - assert_msg_equals(msg_current, msg_accum, true); - } catch (std::exception & e) { - throw std::runtime_error((std::string("Error comparing accumulated message to current: ") + e.what()).c_str()); + try { + assert_msg_equals(msg_current, msg_accum, true); + } catch (std::exception & e) { + throw std::runtime_error((std::string("Error comparing accumulated message to current: ") + e.what()).c_str()); + } } msg_prev = msg_current; @@ -1116,7 +1119,11 @@ static void test_peg_parser(common_chat_templates * tmpls, if (!tc.is_partial) { assert_msg_equals(tc.expect, parser.parse(tc.input, false), true); } - assert_msg_equals(tc.expect, msg_accum, true); + if (tc.expect_streaming_consistency) { + assert_msg_equals(tc.expect, msg_accum, true); + } else if (tc.is_partial) { + assert_msg_equals(tc.expect, parser.parse(tc.input, true), true); + } // Test grammar if present in params if (!parser.params_.grammar.empty()) { @@ -1436,6 +1443,11 @@ class peg_test_builder { return *this; } + peg_test_builder & expect_streaming_consistency(bool val = true) { + tc_.expect_streaming_consistency = val; + return *this; + } + // Expect setters peg_test_builder & expect(const common_chat_msg & msg) { tc_.expect = msg; @@ -5518,6 +5530,88 @@ static void test_template_output_peg_parsers(bool detailed_debug) { .expect_content("Hello, world!\nWhat's up?") .run(); } + + // MiniCPM5 - XML tool calls with ... + { + auto tst = peg_tester("models/templates/MiniCPM5-1B.jinja", detailed_debug); + + tst.test("Hello, world!\nWhat's up?").expect(message_assist).run(); + + tst.test(R"(print('Hello, World!'))") + .enable_thinking(false) + .expect_streaming_consistency(false) + .tools({ python_tool }) + .expect_tool_calls({ { "python", R"#({"code": "print('Hello, World!')"})#", {} } }) + .run(); + + // Model sometimes drops opening tag names but keeps attributes. + tst.test(R"( name="python"> name="code">print('Hello, World!'))") + .enable_thinking(false) + .expect_streaming_consistency(false) + .tools({ python_tool }) + .expect_tool_calls({ { "python", R"#({"code": "print('Hello, World!')"})#", {} } }) + .run(); + + tst.test(R"(print('Hello, World!'))") + .enable_thinking(false) + .expect_streaming_consistency(false) + .tools({ python_tool }) + .expect_tool_calls({ { "python", R"#({"code": "print('Hello, World!')"})#", {} } }) + .run(); + + tst.test(R"()") + .enable_thinking(false) + .expect_streaming_consistency(false) + .tools({ empty_args_tool }) + .expect(simple_assist_msg("", "", "empty_args", "{}")) + .run(); + + tst.test(R"(print('x'))") + .enable_thinking(false) + .expect_streaming_consistency(false) + .parallel_tool_calls(true) + .tools({ python_tool }) + .expect_tool_calls({ { "python", R"#({"code": "print('x')"})#", {} } }) + .run(); + + tst.test(R"(I'm thinkingprint('hey'))") + .enable_thinking(true) + .expect_streaming_consistency(false) + .reasoning_format(COMMON_REASONING_FORMAT_AUTO) + .tools({ python_tool }) + .expect_reasoning("I'm thinking") + .expect_tool_calls({ { "python", R"#({"code": "print('hey')"})#", {} } }) + .run(); + + tst.test(R"(print('x')print('y'))") + .enable_thinking(false) + .expect_streaming_consistency(false) + .parallel_tool_calls(true) + .tools({ python_tool }) + .expect_tool_calls({ + { "python", R"#({"code": "print('x')"})#", {} }, + { "python", R"#({"code": "print('y')"})#", {} }, + }) + .run(); + + tst.test(R"(print('hey')") + .enable_thinking(false) + .expect_streaming_consistency(false) + .tools({ python_tool }) + .is_partial(true) + .expect(simple_assist_msg("", "", "python", R"#({"code": "print('hey')")#")) + .run(); + + tst.test(" thinkingHello, world!\nWhat's up?") + .reasoning_format(COMMON_REASONING_FORMAT_AUTO) + .enable_thinking(true) + .messages({ message_user, message_assist_prefill_reasoning }) + .add_generation_prompt(false) + .continue_final_message(COMMON_CHAT_CONTINUATION_REASONING) + .expect_reasoning("I'm thinking") + .expect_content("Hello, world!\nWhat's up?") + .run(); + } } static void test_template_generation_prompt() { @@ -5665,6 +5759,13 @@ static void test_template_generation_prompt() { check(tmpls, continuation_content(), "<｜Assistant｜>I'm thinkingHello, "); check(tmpls, continuation_reasoning(), "<｜Assistant｜>I'm"); } + + { + auto tmpls = read_templates("models/templates/MiniCPM5-1B.jinja"); + check(tmpls, basic(), "<|im_start|>assistant\n\n"); + check(tmpls, continuation_content(), "<|im_start|>assistant\nI'm thinkingHello, "); + check(tmpls, continuation_reasoning(), "<|im_start|>assistant\nI'm"); + } } // Test the developer role to system workaround with a simple mock template diff --git a/tests/test-jinja.cpp b/tests/test-jinja.cpp index 4e4f57f2e219..510424da8c6a 100644 --- a/tests/test-jinja.cpp +++ b/tests/test-jinja.cpp @@ -1570,6 +1570,37 @@ static void test_array_methods(testing & t) { "2" ); + test_template(t, "array|min attribute not implemented", + "{{ items|min(attribute='x') }}", + {{"items", json::array({ + json({{"x", 2}}), + json({{"x", 1}}), + })}}, + "" + ); + + test_template(t, "array|max attribute not implemented", + "{{ items|max(attribute='x') }}", + {{"items", json::array({ + json({{"x", 2}}), + json({{"x", 1}}), + })}}, + "" + ); + + // FIXME: min/max ignore case_sensitive and compare case-sensitively for now + test_template(t, "array|min case_sensitive", + "{{ items|min(case_sensitive=false) }}", + {{"items", json::array({"B", "a", "A"})}}, + "A" + ); + + test_template(t, "array|max case_sensitive", + "{{ items|max(case_sensitive=false) }}", + {{"items", json::array({"B", "a", "A"})}}, + "a" + ); + // not used by any chat templates // test_template(t, "array.insert()", // "{% set _ = arr.insert(1, 'x') %}{{ arr|join(',') }}", From e5a9c2ee7a9e43857e56fce2cfe45b81b9bc7715 Mon Sep 17 00:00:00 2001 From: zhangtao Date: Thu, 28 May 2026 11:19:16 +0000 Subject: [PATCH 3/9] Fix jinja min/max API to match Jinja2 --- common/jinja/value.cpp | 12 ++++++------ tests/test-jinja.cpp | 17 ++--------------- 2 files changed, 8 insertions(+), 21 deletions(-) diff --git a/common/jinja/value.cpp b/common/jinja/value.cpp index 63695f699e4f..5055ae9ac122 100644 --- a/common/jinja/value.cpp +++ b/common/jinja/value.cpp @@ -1111,8 +1111,8 @@ const func_builtins & value_array_t::get_builtins() const { {"min", [](const func_args & args) -> value { args.ensure_count(1, 4); args.ensure_vals(); - value attribute = args.get_kwarg_or_pos("attribute", 1); - value val_case = args.get_kwarg_or_pos("case_sensitive", 2); + value val_case = args.get_kwarg_or_pos("case_sensitive", 1); + value attribute = args.get_kwarg_or_pos("attribute", 2); if (!attribute->is_undefined()) { throw not_implemented_exception("min: attribute not implemented"); } @@ -1120,7 +1120,7 @@ const func_builtins & value_array_t::get_builtins() const { (void) val_case; const auto & arr = args.get_pos(0)->as_array(); if (arr.empty()) { - throw raised_exception("min() arg is an empty sequence"); + return mk_val(); } value result = arr[0]; for (size_t i = 1; i < arr.size(); ++i) { @@ -1133,8 +1133,8 @@ const func_builtins & value_array_t::get_builtins() const { {"max", [](const func_args & args) -> value { args.ensure_count(1, 4); args.ensure_vals(); - value attribute = args.get_kwarg_or_pos("attribute", 1); - value val_case = args.get_kwarg_or_pos("case_sensitive", 2); + value val_case = args.get_kwarg_or_pos("case_sensitive", 1); + value attribute = args.get_kwarg_or_pos("attribute", 2); if (!attribute->is_undefined()) { throw not_implemented_exception("max: attribute not implemented"); } @@ -1142,7 +1142,7 @@ const func_builtins & value_array_t::get_builtins() const { (void) val_case; const auto & arr = args.get_pos(0)->as_array(); if (arr.empty()) { - throw raised_exception("max() arg is an empty sequence"); + return mk_val(); } value result = arr[0]; for (size_t i = 1; i < arr.size(); ++i) { diff --git a/tests/test-jinja.cpp b/tests/test-jinja.cpp index 510424da8c6a..1b03eaff666b 100644 --- a/tests/test-jinja.cpp +++ b/tests/test-jinja.cpp @@ -1570,7 +1570,7 @@ static void test_array_methods(testing & t) { "2" ); - test_template(t, "array|min attribute not implemented", + test_template(t, "array|min attribute", "{{ items|min(attribute='x') }}", {{"items", json::array({ json({{"x", 2}}), @@ -1579,7 +1579,7 @@ static void test_array_methods(testing & t) { "" ); - test_template(t, "array|max attribute not implemented", + test_template(t, "array|max attribute", "{{ items|max(attribute='x') }}", {{"items", json::array({ json({{"x", 2}}), @@ -1588,19 +1588,6 @@ static void test_array_methods(testing & t) { "" ); - // FIXME: min/max ignore case_sensitive and compare case-sensitively for now - test_template(t, "array|min case_sensitive", - "{{ items|min(case_sensitive=false) }}", - {{"items", json::array({"B", "a", "A"})}}, - "A" - ); - - test_template(t, "array|max case_sensitive", - "{{ items|max(case_sensitive=false) }}", - {{"items", json::array({"B", "a", "A"})}}, - "a" - ); - // not used by any chat templates // test_template(t, "array.insert()", // "{% set _ = arr.insert(1, 'x') %}{{ arr|join(',') }}", From c5adbb203aeccdae282e91cb29d0331499e69289 Mon Sep 17 00:00:00 2001 From: zhangtao Date: Wed, 3 Jun 2026 06:11:48 +0000 Subject: [PATCH 4/9] modify by review --- common/chat.cpp | 11 +++++-- tests/test-chat.cpp | 71 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 2 deletions(-) diff --git a/common/chat.cpp b/common/chat.cpp index 8a8e2a407115..d9569536fadf 100644 --- a/common/chat.cpp +++ b/common/chat.cpp @@ -2312,10 +2312,17 @@ static void func_args_not_string(json & messages) { if (tool_call.contains("function") && tool_call["function"].contains("arguments")) { auto & args = tool_call["function"]["arguments"]; if (args.is_string()) { + const std::string args_str = args.get(); try { - args = json::parse(args.get()); + args = json::parse(args_str); } catch (const std::exception & e) { - throw std::runtime_error("Failed to parse tool call arguments as JSON: " + std::string(e.what())); + // Agent clients may replay history with truncated or malformed + // tool-call JSON (e.g. partial model output). Keep the turn + // alive by falling back to an empty object for Jinja templates + // that expect object arguments. + LOG_WRN("%s: failed to parse tool call arguments as JSON, using empty object: %s (args=%s)\n", + __func__, e.what(), args_str.c_str()); + args = json::object(); } } } diff --git a/tests/test-chat.cpp b/tests/test-chat.cpp index 16c30a437bd2..b6e6631b7578 100644 --- a/tests/test-chat.cpp +++ b/tests/test-chat.cpp @@ -5614,6 +5614,76 @@ static void test_template_output_peg_parsers(bool detailed_debug) { } } +static void test_minicpm5_malformed_tool_args_multiturn() { + LOG_DBG("%s\n", __func__); + + static const common_chat_tool web_fetch_tool{ + /* .name = */ "web_fetch", + /* .description = */ "Fetch a URL", + /* .parameters = */ R"({ + "type": "object", + "properties": { + "url": { + "type": "string", + "description": "URL to fetch" + } + }, + "required": ["url"] + })", + }; + + auto tmpls = read_templates("models/templates/MiniCPM5-1B.jinja"); + + auto apply_multiturn = [&](const std::string & malformed_args) { + common_chat_msg assist_msg = simple_assist_msg("", "", "web_fetch", malformed_args, "call0"); + + common_chat_msg tool_msg; + tool_msg.role = "tool"; + tool_msg.tool_name = "web_fetch"; + tool_msg.tool_call_id = "call0"; + tool_msg.content = "fetch failed"; + + common_chat_msg follow_up; + follow_up.role = "user"; + follow_up.content = "ls dir"; + + common_chat_templates_inputs inputs; + inputs.messages = { message_user, assist_msg, tool_msg, follow_up }; + inputs.tools = { web_fetch_tool }; + inputs.add_generation_prompt = true; + + return common_chat_templates_apply(tmpls.get(), inputs); + }; + + // Truncated JSON (matches server 500: parse error at column 7 in lexasub report). + { + auto params = apply_multiturn(R"({"url")"); + if (params.format != COMMON_CHAT_FORMAT_PEG_MINICPM5) { + throw std::runtime_error("Expected peg-minicpm5 format for malformed-args multi-turn apply"); + } + if (params.prompt.empty()) { + throw std::runtime_error("MiniCPM5 multi-turn apply returned empty prompt"); + } + if (params.prompt.find("") == std::string::npos) { + throw std::runtime_error("Expected prior tool response in prompt"); + } + if (params.prompt.find("ls dir") == std::string::npos) { + throw std::runtime_error("Expected follow-up user message in prompt"); + } + } + + // Degenerate partial JSON from agent-style tool output. + { + auto params = apply_multiturn(R"({"id":"-Updx2F7A3C7D3D9K2Z7H6H5)"); + if (params.format != COMMON_CHAT_FORMAT_PEG_MINICPM5) { + throw std::runtime_error("Expected peg-minicpm5 format for degenerate-args multi-turn apply"); + } + if (params.generation_prompt.find("<|im_start|>assistant") == std::string::npos) { + throw std::runtime_error("Expected generation prompt after malformed tool-call history"); + } + } +} + static void test_template_generation_prompt() { common_chat_msg system_msg; system_msg.role = "system"; @@ -5962,6 +6032,7 @@ int main(int argc, char ** argv) { test_tools_oaicompat_json_conversion(); test_convert_responses_to_chatcmpl(); test_developer_role_to_system_workaround(); + test_minicpm5_malformed_tool_args_multiturn(); test_template_generation_prompt(); test_template_output_peg_parsers(detailed_debug); std::cout << "\n[chat] All tests passed!" << '\n'; From 954f71736ac0e7b3dccc61a1dd78be8fad571956 Mon Sep 17 00:00:00 2001 From: zhangtao Date: Wed, 3 Jun 2026 13:47:40 +0000 Subject: [PATCH 5/9] MiniCPM5: use autoparser for XML tool calls and fix grammar preserved-token triggers --- common/chat-auto-parser-generator.cpp | 201 ++++++++++++++++++++++--- common/chat-auto-parser.h | 14 ++ common/chat-diff-analyzer.cpp | 111 +++++++++++++- common/chat-peg-parser.cpp | 20 +-- common/chat-peg-parser.h | 15 +- common/chat.cpp | 205 +------------------------- common/chat.h | 1 - tests/test-chat-peg-parser.cpp | 17 ++- tests/test-chat.cpp | 40 +++-- 9 files changed, 339 insertions(+), 285 deletions(-) diff --git a/common/chat-auto-parser-generator.cpp b/common/chat-auto-parser-generator.cpp index 37ca55c8dfa3..fd2d98248f5b 100644 --- a/common/chat-auto-parser-generator.cpp +++ b/common/chat-auto-parser-generator.cpp @@ -25,6 +25,72 @@ static void foreach_function(const json & tools, const std::function") + p.literal("]]>"); + if (!arguments.value_stop_sequences.empty()) { + return p.choice({ cdata, p.until_one_of(arguments.value_stop_sequences) }); + } + if (!arguments.value_suffix.empty()) { + return p.choice({ cdata, p.until(arguments.value_suffix) }); + } + return cdata; +} + parser_build_context::parser_build_context(common_chat_peg_builder & p, const generation_params & inputs) : p(p), inputs(inputs), @@ -75,8 +141,7 @@ common_chat_params peg_generator::generate_parser(const common_chat_template & // Build grammar if tools are present bool has_tools = autoparser.tools.format.mode != tool_format::NONE && inputs.tools.is_array() && !inputs.tools.empty(); - std::string trigger_marker = !autoparser.tools.format.section_start.empty() ? autoparser.tools.format.section_start : - autoparser.tools.format.per_call_start; + std::string trigger_marker = tool_trigger_marker(autoparser.tools); bool has_response_format = !inputs.json_schema.empty() && inputs.json_schema.is_object(); bool include_grammar = has_response_format || (has_tools && @@ -100,12 +165,26 @@ common_chat_params peg_generator::generate_parser(const common_chat_template & // Set grammar triggers based on tool section markers (fall back to per-call markers) if (data.grammar_lazy) { - data.grammar_triggers = { - { COMMON_GRAMMAR_TRIGGER_TYPE_WORD, trigger_marker } - }; - 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\"," }); + if (!autoparser.tools.format.tool_start_triggers.empty()) { + for (const auto & t : autoparser.tools.format.tool_start_triggers) { + data.grammar_triggers.push_back({ COMMON_GRAMMAR_TRIGGER_TYPE_WORD, t }); + } + if (!autoparser.tools.function.compact_name_prefix.empty()) { + const auto & cp = autoparser.tools.function.compact_name_prefix; + const auto pos = cp.find('='); + if (pos != std::string::npos) { + data.grammar_triggers.push_back( + { COMMON_GRAMMAR_TRIGGER_TYPE_WORD, cp.substr(0, pos + 1) }); + } + } + } else { + data.grammar_triggers = { + { COMMON_GRAMMAR_TRIGGER_TYPE_WORD, trigger_marker } + }; + 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\"," }); + } } } } @@ -147,7 +226,18 @@ common_peg_arena autoparser::build_parser(const generation_params & inputs, cons } else { parser = content.build_parser(ctx); } - return pure_content ? p.prefix(generation_prompt, reasoning.start) + parser : p.prefix(generation_prompt, reasoning.start) << parser; + + // When reasoning extraction is disabled but the generation prompt already contains a + // closed thinking block (e.g. MiniCPM5 with enable_thinking=false), consume the full + // generation prompt prefix so it is not emitted as assistant content. + const bool consume_full_generation_prompt = + !extract_reasoning && !reasoning.end.empty() && generation_prompt.find(reasoning.end) != std::string::npos; + + common_peg_parser generation_prefix = consume_full_generation_prompt ? + p.literal(generation_prompt) : + p.prefix(generation_prompt, reasoning.start); + + return pure_content ? generation_prefix + parser : generation_prefix << parser; }); } @@ -257,7 +347,12 @@ common_peg_parser analyze_tools::build_func_parser(common_chat_peg_builder & p, const common_peg_parser & call_id_section, bool have_call_id, const common_peg_parser & args, std::optional atomic_peek) const { - auto open = p.tool_open(function.name_prefix + p.tool_name(p.literal(name)) + function.name_suffix); + common_peg_parser name_prefix = + (!function.alt_name_prefix.empty() || !function.compact_name_prefix.empty()) ? + build_attr_name_prefix_choice(p, function.name_prefix, function.alt_name_prefix, + function.compact_name_prefix) : + p.literal(function.name_prefix); + auto open = p.tool_open(name_prefix + p.tool_name(p.literal(name)) + function.name_suffix); bool matched_atomic = false; common_peg_parser func_parser = p.eps(); @@ -275,7 +370,10 @@ common_peg_parser analyze_tools::build_func_parser(common_chat_peg_builder & p, } if (!function.close.empty()) { - func_parser = func_parser + p.space() + p.tool_close(p.literal(function.close)); + auto close_parser = format.tolerate_incomplete_xml ? + p.optional(p.literal(function.close)) : + p.literal(function.close); + func_parser = func_parser + p.space() + p.tool_close(close_parser); } else if (!format.per_call_end.empty()) { // When there's no func_close but there is a per_call_end marker, use peek() to ensure // we only emit tool_close when we can actually see the closing marker. This prevents @@ -343,22 +441,24 @@ common_peg_parser analyze_tools::build_tool_parser_tag_json(parser_build_context p.literal(format.section_start) + p.space() + tool_calls + p.space() + (format.section_end.empty() ? p.end() : p.literal(format.section_end))); } - } else { - std::string separator = ", "; // Default + } else if (!format.section_start.empty()) { + std::string separator = format.call_separator.empty() ? ", " : format.call_separator; if (inputs.parallel_tool_calls) { tool_calls = p.trigger_rule("tool-call", format.section_start + tool_choice + p.zero_or_more(separator + tool_choice) + format.section_end); } else { tool_calls = p.trigger_rule("tool-call", format.section_start + tool_choice + format.section_end); } + } else { + tool_calls = build_direct_tool_calls(p, *this, tool_choice, inputs); } if (!require_calls) { 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); + const std::string trigger_marker = tool_trigger_marker(*this); + auto content_before_tools = trigger_marker.empty() ? p.eps() : p.until(trigger_marker); return ctx.reasoning_parser + p.optional(p.content(content_before_tools)) + tool_calls + p.end(); } @@ -366,7 +466,15 @@ common_peg_parser analyze_tools::build_tool_parser_tag_tagged(parser_build_conte auto & p = ctx.p; const auto & inputs = ctx.inputs; - auto until_suffix = p.rule("until-suffix", p.until(arguments.value_suffix)); + const bool tolerant_xml = format.tolerate_incomplete_xml; + + auto until_suffix = tolerant_xml ? p.eps() : + p.rule("until-suffix", p.until(arguments.value_suffix)); + auto tolerant_arg_value = tolerant_xml ? build_tolerant_arg_value_parser(p, arguments) : p.eps(); + auto arg_name_prefix = tolerant_xml ? + build_attr_name_prefix_choice(p, arguments.name_prefix, arguments.alt_name_prefix, + arguments.compact_name_prefix) : + p.eps(); common_peg_parser tool_choice = p.choice(); @@ -376,14 +484,56 @@ common_peg_parser analyze_tools::build_tool_parser_tag_tagged(parser_build_conte auto params = func.contains("parameters") ? func.at("parameters") : json::object(); const auto & properties = params.contains("properties") ? params.at("properties") : json::object(); + auto schema_info = common_schema_info(); + schema_info.resolve_refs(params); + + common_peg_parser args_seq = p.eps(); + + if (tolerant_xml) { + common_peg_parser ws = p.space(); + + if (!properties.empty()) { + auto arg_choice = p.choice(); + for (const auto & [param_name, param_schema] : properties.items()) { + const bool is_string = schema_info.resolves_to_string(param_schema); + + auto value_parser = is_string ? + p.tool_arg_string_value(tolerant_arg_value) : + p.tool_arg_json_value(p.schema( + p.json(), "tool-" + name + "-arg-" + param_name + "-schema", param_schema, false)); + + auto arg_close = p.tool_arg_close( + (arguments.value_suffix.empty() ? p.eps() : + p.optional(p.literal(arguments.value_suffix))) + + ws); + + arg_choice |= p.tool_arg( + p.tool_arg_open(arg_name_prefix + p.tool_arg_name(p.literal(param_name)) + + arguments.name_suffix) + + arguments.value_prefix + value_parser + arg_close); + } + args_seq = p.zero_or_more(arg_choice + ws); + } + + auto func_close = function.close.empty() ? + ws : + p.tool_close(p.optional(p.literal(function.close)) + ws); + + auto tool_parser = p.tool( + p.tool_open(build_attr_name_prefix_choice(p, function.name_prefix, function.alt_name_prefix, + function.compact_name_prefix) + + p.tool_name(p.literal(name)) + function.name_suffix) + + p.tool_args(args_seq) + ws + func_close); + + tool_choice |= p.rule("tool-" + name, tool_parser); + return; + } + std::set required; if (params.contains("required")) { params.at("required").get_to(required); } - auto schema_info = common_schema_info(); - schema_info.resolve_refs(params); - // Build parser for each argument, separating required and optional std::vector required_parsers; std::vector optional_parsers; @@ -409,7 +559,6 @@ common_peg_parser analyze_tools::build_tool_parser_tag_tagged(parser_build_conte } // Build required arg sequence in definition order - common_peg_parser args_seq = p.eps(); for (size_t i = 0; i < required_parsers.size(); i++) { if (i > 0) { args_seq = args_seq + p.space(); @@ -470,8 +619,8 @@ common_peg_parser analyze_tools::build_tool_parser_tag_tagged(parser_build_conte p.literal(format.section_start) + p.space() + tool_calls + p.space() + (format.section_end.empty() ? p.end() : p.literal(format.section_end) + p.space())); } - } else { - std::string separator = ", "; // Default + } else if (!format.section_start.empty()) { + std::string separator = format.call_separator.empty() ? ", " : format.call_separator; if (inputs.parallel_tool_calls) { tool_calls = p.trigger_rule("tool-call", format.section_start + p.space() + tool_choice + @@ -481,14 +630,18 @@ common_peg_parser analyze_tools::build_tool_parser_tag_tagged(parser_build_conte tool_calls = p.trigger_rule( "tool-call", format.section_start + p.space() + tool_choice + p.space() + format.section_end); } + } else { + tool_calls = build_direct_tool_calls(p, *this, tool_choice, inputs); } if (!require_tools) { 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); + const std::string trigger_marker = tool_trigger_marker(*this); + auto content_before_tools = !format.tool_start_triggers.empty() ? + p.until_one_of(format.tool_start_triggers) : + (trigger_marker.empty() ? p.eps() : p.until(trigger_marker)); return ctx.reasoning_parser + p.optional(p.content(content_before_tools)) + tool_calls + p.end(); } diff --git a/common/chat-auto-parser.h b/common/chat-auto-parser.h index 9e8113f24421..edb2c2297d33 100644 --- a/common/chat-auto-parser.h +++ b/common/chat-auto-parser.h @@ -178,6 +178,7 @@ struct tool_format_analysis { std::string section_end; // e.g., "", "" std::string per_call_start; // e.g., "<|tool_call_begin|>", "" (for multi-call templates) std::string per_call_end; // e.g., "<|tool_call_end|>", "" + std::string call_separator; // e.g., "\n", "" — between consecutive tool calls when there is no section wrapper bool fun_name_is_key = false; // In JSON format function name is JSON key, i.e. { "": { ... arguments ... } } bool tools_array_wrapped = false; // Tool calls wrapped in JSON array [...] @@ -189,12 +190,19 @@ struct tool_format_analysis { std::string id_field; std::string gen_id_field; std::vector parameter_order; + + // XML attribute-style tools () may omit outer + // tags or closing markers; use alternate prefixes and content/tool triggers below. + bool tolerate_incomplete_xml = false; + std::vector tool_start_triggers; }; struct tool_function_analysis { std::string name_prefix; // e.g., "", "\"", ":0" std::string close; // e.g., "", "" (for tag-based) + std::string alt_name_prefix; // e.g. " name=\"" when the opening tag is truncated + std::string compact_name_prefix; // e.g. "", "" std::string value_suffix; // e.g., "", "", "" std::string separator; // e.g., "", "\n", "," + std::string alt_name_prefix; + std::string compact_name_prefix; + std::vector value_stop_sequences; }; struct tool_id_analysis { @@ -338,6 +349,9 @@ struct analyze_tools : analyze_base { // Check for and extract specific per-call markers for non-native-JSON templates with parallel call support void check_per_call_markers(); + // Extract separator between consecutive tool calls (when calls are not wrapped in section/per-call markers) + void extract_call_separator(); + // Extract function name markers void extract_function_markers(); diff --git a/common/chat-diff-analyzer.cpp b/common/chat-diff-analyzer.cpp index b166ee5a18f3..b3f80492bd91 100644 --- a/common/chat-diff-analyzer.cpp +++ b/common/chat-diff-analyzer.cpp @@ -42,8 +42,9 @@ static std::vector") == std::string::npos && analysis.reasoning.mode == reasoning_mode::NONE) { analysis.reasoning.mode = reasoning_mode::TAG_BASED; - analysis.reasoning.start = ""; - analysis.reasoning.end = ""; + // Match jinja: \n{reasoning}\n\n\n{content} + analysis.reasoning.start = "\n"; + analysis.reasoning.end = "\n\n\n"; analysis.preserved_tokens.push_back(""); analysis.preserved_tokens.push_back(""); LOG_DBG(ANSI_ORANGE "[Patch: old Qwen/Deepseek thinking template]\n" ANSI_RESET); @@ -173,8 +174,40 @@ static std::vector; calls separated by newline/ + [](const common_chat_template & tmpl, autoparser & analysis) -> void { + if (tmpl.src.find("Tool usage guidelines:") != std::string::npos && + tmpl.src.find(""; + analysis.tools.function.close = ""; + analysis.tools.arguments.name_prefix = ""; + analysis.tools.arguments.value_prefix.clear(); + analysis.tools.arguments.value_suffix = ""; + analysis.tools.arguments.value_stop_sequences = { + "", "", "diff.right; + if (both.find(FUN_SECOND) == std::string::npos) { + return; + } + + const std::string before_second = both.substr(0, both.find(FUN_SECOND)); + std::string closer = !function.close.empty() ? function.close : ""; + const size_t close_pos = before_second.rfind(closer); + if (close_pos != std::string::npos) { + format.call_separator = before_second.substr(close_pos + closer.size()); + } +} + void analyze_tools::extract_function_markers() { json assistant_nocall = json{ { "role", "assistant" }, diff --git a/common/chat-peg-parser.cpp b/common/chat-peg-parser.cpp index ab81facbc20a..8c60dd1b153f 100644 --- a/common/chat-peg-parser.cpp +++ b/common/chat-peg-parser.cpp @@ -235,7 +235,7 @@ common_peg_parser common_chat_peg_builder::tag_with_safe_content(const std::stri return zero_or_more(choice({ p, content_chunk })); } -void common_chat_peg_minicpm5_mapper::finalize_tool_call_arguments(std::string & args) { +static void finalize_incomplete_json_tool_args(std::string & args) { if (args.empty() || args.front() != '{') { return; } @@ -262,18 +262,6 @@ void common_chat_peg_minicpm5_mapper::finalize_tool_call_arguments(std::string & } } -void common_chat_peg_minicpm5_mapper::from_ast(const common_peg_ast_arena & arena, - const common_peg_parse_result & parse_result) { - common_chat_peg_mapper::from_ast(arena, parse_result); - // MiniCPM5 often omits . Lenient parsing stops before tool_close, - // so finalize argument JSON on the completed response (same as tool_close handler). - if (!is_partial_) { - for (auto & tool_call : result.tool_calls) { - finalize_tool_call_arguments(tool_call.arguments); - } - } -} - std::string & common_chat_peg_mapper::args_target() { return (current_tool && !current_tool->name.empty()) ? current_tool->arguments : args_buffer; } @@ -311,6 +299,12 @@ void common_chat_peg_mapper::from_ast(const common_peg_ast_arena & arena, result.reasoning_content.clear(); } } + + if (!is_partial_) { + for (auto & tool_call : result.tool_calls) { + finalize_incomplete_json_tool_args(tool_call.arguments); + } + } } void common_chat_peg_mapper::map(const common_peg_ast_node & node) { diff --git a/common/chat-peg-parser.h b/common/chat-peg-parser.h index be769c144300..31adccb6b8cc 100644 --- a/common/chat-peg-parser.h +++ b/common/chat-peg-parser.h @@ -11,7 +11,9 @@ class common_chat_peg_mapper { public: common_chat_msg & result; - common_chat_peg_mapper(common_chat_msg & msg) : result(msg) {} + common_chat_peg_mapper(common_chat_msg & msg, bool is_partial = false) : + result(msg), + is_partial_(is_partial) {} virtual ~common_chat_peg_mapper() = default; @@ -26,6 +28,7 @@ class common_chat_peg_mapper { int arg_count = 0; bool closing_quote_pending = false; std::string args_buffer; // Buffer to delay arguments until tool name is known + bool is_partial_ = false; // Returns a reference to the active argument destination string. // Before tool_name is known, writes go to args_buffer; after, to current_tool->arguments. @@ -40,16 +43,6 @@ class common_chat_peg_gemma4_mapper : public common_chat_peg_mapper { void visit(const common_peg_ast_arena & arena, common_peg_ast_id id); }; -class common_chat_peg_minicpm5_mapper : public common_chat_peg_mapper { - public: - common_chat_peg_minicpm5_mapper(common_chat_msg & msg, bool is_partial) : - common_chat_peg_mapper(msg), is_partial_(is_partial) {} - void from_ast(const common_peg_ast_arena & arena, const common_peg_parse_result & result) override; - private: - bool is_partial_; - static void finalize_tool_call_arguments(std::string & args); -}; - struct content_structure; struct tool_call_structure; diff --git a/common/chat.cpp b/common/chat.cpp index d9569536fadf..a20458074b78 100644 --- a/common/chat.cpp +++ b/common/chat.cpp @@ -762,8 +762,6 @@ const char * common_chat_format_name(common_chat_format format) { return "peg-native"; case COMMON_CHAT_FORMAT_PEG_GEMMA4: return "peg-gemma4"; - case COMMON_CHAT_FORMAT_PEG_MINICPM5: - return "peg-minicpm5"; default: throw std::runtime_error("Unknown chat format"); } @@ -2333,195 +2331,6 @@ static void func_args_not_string(json & messages) { } -static bool is_minicpm5_template(const std::string & src) { - return src.find("Tool usage guidelines:") != std::string::npos && - src.find("{reasoning} (optional) -// - Tool calls: value -static common_chat_params common_chat_params_init_minicpm5(const common_chat_template & tmpl, - const autoparser::generation_params & inputs) { - common_chat_params data; - - data.prompt = common_chat_template_direct_apply_impl(tmpl, inputs); - data.generation_prompt = common_chat_template_generation_prompt_impl(tmpl, inputs); - data.format = COMMON_CHAT_FORMAT_PEG_MINICPM5; - data.supports_thinking = true; - data.preserved_tokens = { - "", - "", - "", - "", - "<|im_start|>", - "", - "", - }; - - const std::string GEN_PROMPT = "<|im_start|>assistant\n"; - const std::string THINK_START = ""; - const std::string THINK_END = ""; - data.thinking_start_tag = THINK_START; - data.thinking_end_tag = THINK_END; - - auto has_tools = inputs.tools.is_array() && !inputs.tools.empty(); - auto extract_reasoning = inputs.reasoning_format != COMMON_REASONING_FORMAT_NONE; - auto include_grammar = has_tools && inputs.tool_choice != COMMON_CHAT_TOOL_CHOICE_NONE; - - if (inputs.has_continuation()) { - const auto & msg = inputs.continue_msg; - - data.generation_prompt = GEN_PROMPT + THINK_START + msg.reasoning_content; - if (inputs.continue_final_message == COMMON_CHAT_CONTINUATION_CONTENT) { - data.generation_prompt += THINK_END + msg.render_content(); - } - - data.prompt += data.generation_prompt; - } - - auto parser = build_chat_peg_parser([&](common_chat_peg_builder & p) { - auto end = p.end(); - - static const std::string SP_SPACE = "\xC4\xA0"; // U+0120 (valid UTF-8) - static const std::string SP_NL = "\xC4\x8A"; // U+010A (valid UTF-8) - - auto ws = [&]() { - return p.choice({ p.space(), p.literal(SP_SPACE), p.literal(SP_NL) }); - }; - - static const std::string IM_END = "<|im_end|>"; - auto im_end_suffix = p.optional(p.literal(IM_END) + ws()); - - auto assistant_prefix = p.literal(GEN_PROMPT); - - auto thinking = p.eps(); - if (!inputs.enable_thinking) { - thinking = p.literal("\n\n\n\n"); - } else if (extract_reasoning) { - thinking = p.literal(THINK_START) + p.optional(p.literal("\n")) + p.choice({ - p.reasoning(p.until(THINK_END)) + p.literal(THINK_END) + ws(), - p.reasoning(p.until_one_of({ " PARAM_VALUE_STOP = { - "", - "\"> name=\"", - "", - "") + p.literal("]]>"), - p.until_one_of(PARAM_VALUE_STOP), - }); - - auto tool_choice = p.choice(); - foreach_function(inputs.tools, [&](const json & tool) { - const auto & function = tool.at("function"); - const std::string name = function.at("name"); - auto params = function.contains("parameters") ? function.at("parameters") : json::object(); - - auto args = p.eps(); - if (params.contains("properties") && params.at("properties").is_object() && - !params.at("properties").empty()) { - auto schema_info = common_schema_info(); - schema_info.resolve_refs(params); - - auto arg_choice = p.choice(); - for (const auto & [prop_name, prop_schema] : params.at("properties").items()) { - const bool is_string = schema_info.resolves_to_string(prop_schema); - - auto value_parser = is_string ? - p.tool_arg_string_value(arg_value) : - p.tool_arg_json_value(p.schema(p.json(), - "tool-" + name + "-arg-" + prop_name + "-schema", - prop_schema, - false)); - - auto arg_rule = p.tool_arg( - p.tool_arg_open(p.choice({ - param_name_prefix, - param_name_prefix_compact, - }) + p.tool_arg_name(p.literal(prop_name)) + p.literal("\">")) + - value_parser + - p.tool_arg_close(p.optional(p.literal("")) + ws())); - - arg_choice |= arg_rule; - } - args = p.zero_or_more(arg_choice + ws()); - } - - auto tool_parser = p.tool( - p.tool_open(p.choice({ - func_name_prefix, - func_name_prefix_compact, - }) + p.tool_name(p.literal(name)) + p.literal("\">")) + - p.tool_args(args) + ws() + - p.tool_close(p.optional(p.literal("")))); - - tool_choice |= p.rule("tool-" + name, tool_parser); - }); - - // Do not wrap trigger_rule("tool-calls", ...) in another rule("tool-calls", ...): - // parallel mode already defines trigger_rule("tool-calls", ...) and a second - // rule with the same name resolves to itself -> peg stack overflow. - common_peg_parser tool_calls = p.eps(); - if (inputs.parallel_tool_calls) { - tool_calls = p.trigger_rule("tool-calls", p.one_or_more(tool_choice + ws())); - } else { - tool_calls = p.trigger_rule("tool-call", tool_choice); - } - - auto content = p.content(p.until_one_of({ "... - if (is_minicpm5_template(src)) { - LOG_DBG("Using specialized template: MiniCPM5\n"); - return common_chat_params_init_minicpm5(tmpl, params); - } - return std::nullopt; } @@ -2871,10 +2674,8 @@ common_chat_msg common_chat_peg_parse(const common_peg_arena & src_pars std::unique_ptr mapper; if (params.format == COMMON_CHAT_FORMAT_PEG_GEMMA4) { mapper = std::make_unique(msg); - } else if (params.format == COMMON_CHAT_FORMAT_PEG_MINICPM5) { - mapper = std::make_unique(msg, is_partial); } else { - mapper = std::make_unique(msg); + mapper = std::make_unique(msg, is_partial); } mapper->from_ast(ctx.ast, result); @@ -2895,10 +2696,8 @@ common_chat_msg common_chat_peg_parse(const common_peg_arena & src_pars std::unique_ptr mapper; if (params.format == COMMON_CHAT_FORMAT_PEG_GEMMA4) { mapper = std::make_unique(msg); - } else if (params.format == COMMON_CHAT_FORMAT_PEG_MINICPM5) { - mapper = std::make_unique(msg, is_partial); } else { - mapper = std::make_unique(msg); + mapper = std::make_unique(msg, is_partial); } mapper->from_ast(ctx.ast, result); diff --git a/common/chat.h b/common/chat.h index 9b2c838e6643..5659cd42a07c 100644 --- a/common/chat.h +++ b/common/chat.h @@ -173,7 +173,6 @@ enum common_chat_format { COMMON_CHAT_FORMAT_PEG_SIMPLE, COMMON_CHAT_FORMAT_PEG_NATIVE, COMMON_CHAT_FORMAT_PEG_GEMMA4, - COMMON_CHAT_FORMAT_PEG_MINICPM5, COMMON_CHAT_FORMAT_COUNT, // Not a format, just the # formats }; diff --git a/tests/test-chat-peg-parser.cpp b/tests/test-chat-peg-parser.cpp index 5dee8aeaf00a..ecdaa4ecf8c7 100644 --- a/tests/test-chat-peg-parser.cpp +++ b/tests/test-chat-peg-parser.cpp @@ -466,7 +466,8 @@ static void test_example_qwen3_coder(testing & t) { for (auto it = tokens.begin(); it != tokens.end(); it++) { std::string in = std::accumulate(tokens.begin(), it + 1, std::string()); - common_peg_parse_context ctx(in, (it + 1 < tokens.end()) ? COMMON_PEG_PARSE_FLAG_LENIENT : COMMON_PEG_PARSE_FLAG_NONE); + const bool is_partial = (it + 1 < tokens.end()); + common_peg_parse_context ctx(in, is_partial ? COMMON_PEG_PARSE_FLAG_LENIENT : COMMON_PEG_PARSE_FLAG_NONE); auto result = parser.parse(ctx); if (!t.assert_equal("not fail", false, result.fail())) { @@ -474,7 +475,7 @@ static void test_example_qwen3_coder(testing & t) { } common_chat_msg msg; - auto mapper = common_chat_peg_mapper(msg); + auto mapper = common_chat_peg_mapper(msg, is_partial); mapper.from_ast(ctx.ast, result); //t.log("Input: " + input); @@ -564,7 +565,8 @@ static void test_example_qwen3_non_coder(testing & t) { for (auto it = tokens.begin(); it != tokens.end(); it++) { std::string in = std::accumulate(tokens.begin(), it + 1, std::string()); - common_peg_parse_context ctx(in, (it + 1 < tokens.end()) ? COMMON_PEG_PARSE_FLAG_LENIENT : COMMON_PEG_PARSE_FLAG_NONE); + const bool is_partial = (it + 1 < tokens.end()); + common_peg_parse_context ctx(in, is_partial ? COMMON_PEG_PARSE_FLAG_LENIENT : COMMON_PEG_PARSE_FLAG_NONE); auto result = parser.parse(ctx); if (!t.assert_equal("not fail", false, result.fail())) { @@ -572,7 +574,7 @@ static void test_example_qwen3_non_coder(testing & t) { } common_chat_msg msg; - auto mapper = common_chat_peg_mapper(msg); + auto mapper = common_chat_peg_mapper(msg, is_partial); mapper.from_ast(ctx.ast, result); //t.log("Input: " + input); @@ -629,7 +631,7 @@ void test_command7_parser_compare(testing & t) { auto result = p.parse(ctx); common_chat_msg msg; - auto mapper = common_chat_peg_mapper(msg); + auto mapper = common_chat_peg_mapper(msg, is_partial); mapper.from_ast(ctx.ast, result); if (print_results) { @@ -822,7 +824,8 @@ static void test_prefix_tool_names(testing & t) { for (auto it = tokens.begin(); it != tokens.end(); it++) { std::string in = std::accumulate(tokens.begin(), it + 1, std::string()); - common_peg_parse_context ctx(in, (it + 1 < tokens.end()) ? COMMON_PEG_PARSE_FLAG_LENIENT : COMMON_PEG_PARSE_FLAG_NONE); + const bool is_partial = (it + 1 < tokens.end()); + common_peg_parse_context ctx(in, is_partial ? COMMON_PEG_PARSE_FLAG_LENIENT : COMMON_PEG_PARSE_FLAG_NONE); auto result = parser.parse(ctx); if (!t.assert_equal("not fail", false, result.fail())) { @@ -831,7 +834,7 @@ static void test_prefix_tool_names(testing & t) { } common_chat_msg msg; - auto mapper = common_chat_peg_mapper(msg); + auto mapper = common_chat_peg_mapper(msg, is_partial); mapper.from_ast(ctx.ast, result); // The critical check: during incremental parsing, we should never diff --git a/tests/test-chat.cpp b/tests/test-chat.cpp index b6e6631b7578..e971546e45b5 100644 --- a/tests/test-chat.cpp +++ b/tests/test-chat.cpp @@ -5535,25 +5535,15 @@ static void test_template_output_peg_parsers(bool detailed_debug) { { auto tst = peg_tester("models/templates/MiniCPM5-1B.jinja", detailed_debug); - tst.test("Hello, world!\nWhat's up?").expect(message_assist).run(); - - tst.test(R"(print('Hello, World!'))") - .enable_thinking(false) - .expect_streaming_consistency(false) - .tools({ python_tool }) - .expect_tool_calls({ { "python", R"#({"code": "print('Hello, World!')"})#", {} } }) - .run(); - - // Model sometimes drops opening tag names but keeps attributes. - tst.test(R"( name="python"> name="code">print('Hello, World!'))") + tst.test("Hello, world!\nWhat's up?") .enable_thinking(false) - .expect_streaming_consistency(false) - .tools({ python_tool }) - .expect_tool_calls({ { "python", R"#({"code": "print('Hello, World!')"})#", {} } }) + .reasoning_format(COMMON_REASONING_FORMAT_NONE) + .expect(message_assist) .run(); - tst.test(R"(print('Hello, World!'))") + tst.test(R"(print('Hello, World!'))") .enable_thinking(false) + .reasoning_format(COMMON_REASONING_FORMAT_NONE) .expect_streaming_consistency(false) .tools({ python_tool }) .expect_tool_calls({ { "python", R"#({"code": "print('Hello, World!')"})#", {} } }) @@ -5561,6 +5551,7 @@ static void test_template_output_peg_parsers(bool detailed_debug) { tst.test(R"()") .enable_thinking(false) + .reasoning_format(COMMON_REASONING_FORMAT_NONE) .expect_streaming_consistency(false) .tools({ empty_args_tool }) .expect(simple_assist_msg("", "", "empty_args", "{}")) @@ -5568,6 +5559,7 @@ static void test_template_output_peg_parsers(bool detailed_debug) { tst.test(R"(print('x'))") .enable_thinking(false) + .reasoning_format(COMMON_REASONING_FORMAT_NONE) .expect_streaming_consistency(false) .parallel_tool_calls(true) .tools({ python_tool }) @@ -5583,8 +5575,10 @@ static void test_template_output_peg_parsers(bool detailed_debug) { .expect_tool_calls({ { "python", R"#({"code": "print('hey')"})#", {} } }) .run(); - tst.test(R"(print('x')print('y'))") + tst.test(R"(print('x') +print('y'))") .enable_thinking(false) + .reasoning_format(COMMON_REASONING_FORMAT_NONE) .expect_streaming_consistency(false) .parallel_tool_calls(true) .tools({ python_tool }) @@ -5596,6 +5590,7 @@ static void test_template_output_peg_parsers(bool detailed_debug) { tst.test(R"(print('hey')") .enable_thinking(false) + .reasoning_format(COMMON_REASONING_FORMAT_NONE) .expect_streaming_consistency(false) .tools({ python_tool }) .is_partial(true) @@ -5658,8 +5653,8 @@ static void test_minicpm5_malformed_tool_args_multiturn() { // Truncated JSON (matches server 500: parse error at column 7 in lexasub report). { auto params = apply_multiturn(R"({"url")"); - if (params.format != COMMON_CHAT_FORMAT_PEG_MINICPM5) { - throw std::runtime_error("Expected peg-minicpm5 format for malformed-args multi-turn apply"); + if (params.format != COMMON_CHAT_FORMAT_PEG_NATIVE) { + throw std::runtime_error("Expected peg-native format for malformed-args multi-turn apply"); } if (params.prompt.empty()) { throw std::runtime_error("MiniCPM5 multi-turn apply returned empty prompt"); @@ -5675,8 +5670,8 @@ static void test_minicpm5_malformed_tool_args_multiturn() { // Degenerate partial JSON from agent-style tool output. { auto params = apply_multiturn(R"({"id":"-Updx2F7A3C7D3D9K2Z7H6H5)"); - if (params.format != COMMON_CHAT_FORMAT_PEG_MINICPM5) { - throw std::runtime_error("Expected peg-minicpm5 format for degenerate-args multi-turn apply"); + if (params.format != COMMON_CHAT_FORMAT_PEG_NATIVE) { + throw std::runtime_error("Expected peg-native format for degenerate-args multi-turn apply"); } if (params.generation_prompt.find("<|im_start|>assistant") == std::string::npos) { throw std::runtime_error("Expected generation prompt after malformed tool-call history"); @@ -5833,8 +5828,9 @@ static void test_template_generation_prompt() { { auto tmpls = read_templates("models/templates/MiniCPM5-1B.jinja"); check(tmpls, basic(), "<|im_start|>assistant\n\n"); - check(tmpls, continuation_content(), "<|im_start|>assistant\nI'm thinkingHello, "); - check(tmpls, continuation_reasoning(), "<|im_start|>assistant\nI'm"); + check(tmpls, continuation_content(), + "<|im_start|>assistant\n\nI'm thinking\n\n\nHello, "); + check(tmpls, continuation_reasoning(), "<|im_start|>assistant\n\nI'm"); } } From aceeab08ac94228ac05db73fcb78477719b26a88 Mon Sep 17 00:00:00 2001 From: zhangtao Date: Mon, 8 Jun 2026 03:47:58 +0000 Subject: [PATCH 6/9] MiniCPM5: fix streaming tool-arg placeholder and remove alt XML markers --- common/chat-peg-parser.cpp | 12 ++++++++---- tests/test-chat.cpp | 31 +++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/common/chat-peg-parser.cpp b/common/chat-peg-parser.cpp index 8c60dd1b153f..72e979aacb89 100644 --- a/common/chat-peg-parser.cpp +++ b/common/chat-peg-parser.cpp @@ -389,7 +389,8 @@ void common_chat_peg_mapper::map(const common_peg_ast_node & node) { ++arg_count; auto & target = args_target(); - if (target.empty()) { + if (target.empty() || target == "{}") { + // "{}" can appear when partial parsing closes an empty arg placeholder too early. target = "{"; } target += arg_entry; @@ -436,9 +437,12 @@ void common_chat_peg_mapper::map(const common_peg_ast_node & node) { current_tool->arguments += "\""; closing_quote_pending = false; } - // Close any unclosed braces (accounts for nested objects) - for (int d = json_brace_depth(current_tool->arguments); d > 0; d--) { - current_tool->arguments += "}"; + // Close any unclosed braces (accounts for nested objects). + // During partial parsing, keep a lone "{" open until the first arg arrives. + if (!(is_partial_ && arg_count == 0 && current_tool->arguments == "{")) { + for (int d = json_brace_depth(current_tool->arguments); d > 0; d--) { + current_tool->arguments += "}"; + } } // Add tool call to results if named; otherwise discard if (pending_tool_call.has_value()) { diff --git a/tests/test-chat.cpp b/tests/test-chat.cpp index e971546e45b5..5acc621aa483 100644 --- a/tests/test-chat.cpp +++ b/tests/test-chat.cpp @@ -478,6 +478,21 @@ static common_chat_tool python_tool{ })", }; +static common_chat_tool category_menu_tool{ + /* .name = */ "category_menu", + /* .description = */ "Show menu items for a category", + /* .parameters = */ R"({ + "type": "object", + "properties": { + "category": { + "type": "string", + "description": "Menu category name" + } + }, + "required": ["category"] + })", +}; + static common_chat_tool html_tool{ /* .name = */ "html", /* .description = */ "an html validator", @@ -5549,6 +5564,22 @@ static void test_template_output_peg_parsers(bool detailed_debug) { .expect_tool_calls({ { "python", R"#({"code": "print('Hello, World!')"})#", {} } }) .run(); + tst.test(R"(Dessert)") + .enable_thinking(false) + .reasoning_format(COMMON_REASONING_FORMAT_NONE) + .expect_streaming_consistency(false) + .tools({ category_menu_tool }) + .expect_tool_calls({ { "category_menu", R"#({"category": "Dessert"})#", {} } }) + .run(); + + tst.test(R"()") + .enable_thinking(false) + .reasoning_format(COMMON_REASONING_FORMAT_NONE) + .tools({ category_menu_tool }) + .is_partial(true) + .expect(simple_assist_msg("", "", "category_menu", "{")) + .run(); + tst.test(R"()") .enable_thinking(false) .reasoning_format(COMMON_REASONING_FORMAT_NONE) From cab364b7426e9027b76b6f42b8ba8f3948d18939 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BC=A0=E6=B6=9B?= <> Date: Wed, 10 Jun 2026 09:51:50 +0800 Subject: [PATCH 7/9] skip min/max attribute tests in -py mode --- tests/test-jinja.cpp | 35 +++++++++++++++++++---------------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/tests/test-jinja.cpp b/tests/test-jinja.cpp index 1b03eaff666b..1e41e49bb50c 100644 --- a/tests/test-jinja.cpp +++ b/tests/test-jinja.cpp @@ -1570,23 +1570,26 @@ static void test_array_methods(testing & t) { "2" ); - test_template(t, "array|min attribute", - "{{ items|min(attribute='x') }}", - {{"items", json::array({ - json({{"x", 2}}), - json({{"x", 1}}), - })}}, - "" - ); + // attribute= is not implemented in C++ yet; skip in -py mode (Python Jinja2 renders output) + if (!g_python_mode) { + test_template(t, "array|min attribute", + "{{ items|min(attribute='x') }}", + {{"items", json::array({ + json({{"x", 2}}), + json({{"x", 1}}), + })}}, + "" + ); - test_template(t, "array|max attribute", - "{{ items|max(attribute='x') }}", - {{"items", json::array({ - json({{"x", 2}}), - json({{"x", 1}}), - })}}, - "" - ); + test_template(t, "array|max attribute", + "{{ items|max(attribute='x') }}", + {{"items", json::array({ + json({{"x", 2}}), + json({{"x", 1}}), + })}}, + "" + ); + } // not used by any chat templates // test_template(t, "array.insert()", From dea9302832e2ab6bbfcf0077233edac52025bf90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BC=A0=E6=B6=9B?= <> Date: Wed, 10 Jun 2026 14:19:53 +0800 Subject: [PATCH 8/9] test-jinja: use real expected output for min/max attribute tests --- tests/test-jinja.cpp | 35 ++++++++++++++++------------------- 1 file changed, 16 insertions(+), 19 deletions(-) diff --git a/tests/test-jinja.cpp b/tests/test-jinja.cpp index 1e41e49bb50c..fe040efa9dc4 100644 --- a/tests/test-jinja.cpp +++ b/tests/test-jinja.cpp @@ -1570,26 +1570,23 @@ static void test_array_methods(testing & t) { "2" ); - // attribute= is not implemented in C++ yet; skip in -py mode (Python Jinja2 renders output) - if (!g_python_mode) { - test_template(t, "array|min attribute", - "{{ items|min(attribute='x') }}", - {{"items", json::array({ - json({{"x", 2}}), - json({{"x", 1}}), - })}}, - "" - ); + test_template(t, "array|min attribute", + "{{ items|min(attribute='x') }}", + {{"items", json::array({ + json({{"x", 2}}), + json({{"x", 1}}), + })}}, + "{'x': 1}" + ); - test_template(t, "array|max attribute", - "{{ items|max(attribute='x') }}", - {{"items", json::array({ - json({{"x", 2}}), - json({{"x", 1}}), - })}}, - "" - ); - } + test_template(t, "array|max attribute", + "{{ items|max(attribute='x') }}", + {{"items", json::array({ + json({{"x", 2}}), + json({{"x", 1}}), + })}}, + "{'x': 2}" + ); // not used by any chat templates // test_template(t, "array.insert()", From ee93483cbad5d39d302b474ca993be8eeba3a172 Mon Sep 17 00:00:00 2001 From: zhangtao Date: Mon, 15 Jun 2026 20:14:35 +0800 Subject: [PATCH 9/9] MiniCPM5: revert shared mapper and history fallbacks per review Drop streaming tool-arg placeholder workarounds from the generic PEG mapper and restore strict tool-call argument JSON parsing so MiniCPM5 support stays limited to autoparser/diff-analyzer changes. --- common/chat-peg-parser.cpp | 45 ++----------------- common/chat-peg-parser.h | 5 +-- common/chat.cpp | 15 ++----- tests/test-chat-peg-parser.cpp | 8 ++-- tests/test-chat.cpp | 79 ---------------------------------- 5 files changed, 13 insertions(+), 139 deletions(-) diff --git a/common/chat-peg-parser.cpp b/common/chat-peg-parser.cpp index 72e979aacb89..a309f02765b7 100644 --- a/common/chat-peg-parser.cpp +++ b/common/chat-peg-parser.cpp @@ -235,33 +235,6 @@ common_peg_parser common_chat_peg_builder::tag_with_safe_content(const std::stri return zero_or_more(choice({ p, content_chunk })); } -static void finalize_incomplete_json_tool_args(std::string & args) { - if (args.empty() || args.front() != '{') { - return; - } - bool in_string = false; - bool escaped = false; - for (char c : args) { - if (escaped) { - escaped = false; - continue; - } - if (c == '\\' && in_string) { - escaped = true; - continue; - } - if (c == '"') { - in_string = !in_string; - } - } - if (in_string) { - args += '"'; - } - for (int d = json_brace_depth(args); d > 0; d--) { - args += '}'; - } -} - std::string & common_chat_peg_mapper::args_target() { return (current_tool && !current_tool->name.empty()) ? current_tool->arguments : args_buffer; } @@ -299,12 +272,6 @@ void common_chat_peg_mapper::from_ast(const common_peg_ast_arena & arena, result.reasoning_content.clear(); } } - - if (!is_partial_) { - for (auto & tool_call : result.tool_calls) { - finalize_incomplete_json_tool_args(tool_call.arguments); - } - } } void common_chat_peg_mapper::map(const common_peg_ast_node & node) { @@ -389,8 +356,7 @@ void common_chat_peg_mapper::map(const common_peg_ast_node & node) { ++arg_count; auto & target = args_target(); - if (target.empty() || target == "{}") { - // "{}" can appear when partial parsing closes an empty arg placeholder too early. + if (target.empty()) { target = "{"; } target += arg_entry; @@ -437,12 +403,9 @@ void common_chat_peg_mapper::map(const common_peg_ast_node & node) { current_tool->arguments += "\""; closing_quote_pending = false; } - // Close any unclosed braces (accounts for nested objects). - // During partial parsing, keep a lone "{" open until the first arg arrives. - if (!(is_partial_ && arg_count == 0 && current_tool->arguments == "{")) { - for (int d = json_brace_depth(current_tool->arguments); d > 0; d--) { - current_tool->arguments += "}"; - } + // Close any unclosed braces (accounts for nested objects) + for (int d = json_brace_depth(current_tool->arguments); d > 0; d--) { + current_tool->arguments += "}"; } // Add tool call to results if named; otherwise discard if (pending_tool_call.has_value()) { diff --git a/common/chat-peg-parser.h b/common/chat-peg-parser.h index 31adccb6b8cc..b3ffd7de2dd8 100644 --- a/common/chat-peg-parser.h +++ b/common/chat-peg-parser.h @@ -11,9 +11,7 @@ class common_chat_peg_mapper { public: common_chat_msg & result; - common_chat_peg_mapper(common_chat_msg & msg, bool is_partial = false) : - result(msg), - is_partial_(is_partial) {} + common_chat_peg_mapper(common_chat_msg & msg) : result(msg) {} virtual ~common_chat_peg_mapper() = default; @@ -28,7 +26,6 @@ class common_chat_peg_mapper { int arg_count = 0; bool closing_quote_pending = false; std::string args_buffer; // Buffer to delay arguments until tool name is known - bool is_partial_ = false; // Returns a reference to the active argument destination string. // Before tool_name is known, writes go to args_buffer; after, to current_tool->arguments. diff --git a/common/chat.cpp b/common/chat.cpp index a20458074b78..ded8440e6688 100644 --- a/common/chat.cpp +++ b/common/chat.cpp @@ -2310,17 +2310,10 @@ static void func_args_not_string(json & messages) { if (tool_call.contains("function") && tool_call["function"].contains("arguments")) { auto & args = tool_call["function"]["arguments"]; if (args.is_string()) { - const std::string args_str = args.get(); try { - args = json::parse(args_str); + args = json::parse(args.get()); } catch (const std::exception & e) { - // Agent clients may replay history with truncated or malformed - // tool-call JSON (e.g. partial model output). Keep the turn - // alive by falling back to an empty object for Jinja templates - // that expect object arguments. - LOG_WRN("%s: failed to parse tool call arguments as JSON, using empty object: %s (args=%s)\n", - __func__, e.what(), args_str.c_str()); - args = json::object(); + throw std::runtime_error("Failed to parse tool call arguments as JSON: " + std::string(e.what())); } } } @@ -2675,7 +2668,7 @@ common_chat_msg common_chat_peg_parse(const common_peg_arena & src_pars if (params.format == COMMON_CHAT_FORMAT_PEG_GEMMA4) { mapper = std::make_unique(msg); } else { - mapper = std::make_unique(msg, is_partial); + mapper = std::make_unique(msg); } mapper->from_ast(ctx.ast, result); @@ -2697,7 +2690,7 @@ common_chat_msg common_chat_peg_parse(const common_peg_arena & src_pars if (params.format == COMMON_CHAT_FORMAT_PEG_GEMMA4) { mapper = std::make_unique(msg); } else { - mapper = std::make_unique(msg, is_partial); + mapper = std::make_unique(msg); } mapper->from_ast(ctx.ast, result); diff --git a/tests/test-chat-peg-parser.cpp b/tests/test-chat-peg-parser.cpp index ecdaa4ecf8c7..7b0a86008e7e 100644 --- a/tests/test-chat-peg-parser.cpp +++ b/tests/test-chat-peg-parser.cpp @@ -475,7 +475,7 @@ static void test_example_qwen3_coder(testing & t) { } common_chat_msg msg; - auto mapper = common_chat_peg_mapper(msg, is_partial); + auto mapper = common_chat_peg_mapper(msg); mapper.from_ast(ctx.ast, result); //t.log("Input: " + input); @@ -574,7 +574,7 @@ static void test_example_qwen3_non_coder(testing & t) { } common_chat_msg msg; - auto mapper = common_chat_peg_mapper(msg, is_partial); + auto mapper = common_chat_peg_mapper(msg); mapper.from_ast(ctx.ast, result); //t.log("Input: " + input); @@ -631,7 +631,7 @@ void test_command7_parser_compare(testing & t) { auto result = p.parse(ctx); common_chat_msg msg; - auto mapper = common_chat_peg_mapper(msg, is_partial); + auto mapper = common_chat_peg_mapper(msg); mapper.from_ast(ctx.ast, result); if (print_results) { @@ -834,7 +834,7 @@ static void test_prefix_tool_names(testing & t) { } common_chat_msg msg; - auto mapper = common_chat_peg_mapper(msg, is_partial); + auto mapper = common_chat_peg_mapper(msg); mapper.from_ast(ctx.ast, result); // The critical check: during incremental parsing, we should never diff --git a/tests/test-chat.cpp b/tests/test-chat.cpp index 5acc621aa483..624d2dc879f2 100644 --- a/tests/test-chat.cpp +++ b/tests/test-chat.cpp @@ -5572,14 +5572,6 @@ static void test_template_output_peg_parsers(bool detailed_debug) { .expect_tool_calls({ { "category_menu", R"#({"category": "Dessert"})#", {} } }) .run(); - tst.test(R"()") - .enable_thinking(false) - .reasoning_format(COMMON_REASONING_FORMAT_NONE) - .tools({ category_menu_tool }) - .is_partial(true) - .expect(simple_assist_msg("", "", "category_menu", "{")) - .run(); - tst.test(R"()") .enable_thinking(false) .reasoning_format(COMMON_REASONING_FORMAT_NONE) @@ -5640,76 +5632,6 @@ static void test_template_output_peg_parsers(bool detailed_debug) { } } -static void test_minicpm5_malformed_tool_args_multiturn() { - LOG_DBG("%s\n", __func__); - - static const common_chat_tool web_fetch_tool{ - /* .name = */ "web_fetch", - /* .description = */ "Fetch a URL", - /* .parameters = */ R"({ - "type": "object", - "properties": { - "url": { - "type": "string", - "description": "URL to fetch" - } - }, - "required": ["url"] - })", - }; - - auto tmpls = read_templates("models/templates/MiniCPM5-1B.jinja"); - - auto apply_multiturn = [&](const std::string & malformed_args) { - common_chat_msg assist_msg = simple_assist_msg("", "", "web_fetch", malformed_args, "call0"); - - common_chat_msg tool_msg; - tool_msg.role = "tool"; - tool_msg.tool_name = "web_fetch"; - tool_msg.tool_call_id = "call0"; - tool_msg.content = "fetch failed"; - - common_chat_msg follow_up; - follow_up.role = "user"; - follow_up.content = "ls dir"; - - common_chat_templates_inputs inputs; - inputs.messages = { message_user, assist_msg, tool_msg, follow_up }; - inputs.tools = { web_fetch_tool }; - inputs.add_generation_prompt = true; - - return common_chat_templates_apply(tmpls.get(), inputs); - }; - - // Truncated JSON (matches server 500: parse error at column 7 in lexasub report). - { - auto params = apply_multiturn(R"({"url")"); - if (params.format != COMMON_CHAT_FORMAT_PEG_NATIVE) { - throw std::runtime_error("Expected peg-native format for malformed-args multi-turn apply"); - } - if (params.prompt.empty()) { - throw std::runtime_error("MiniCPM5 multi-turn apply returned empty prompt"); - } - if (params.prompt.find("") == std::string::npos) { - throw std::runtime_error("Expected prior tool response in prompt"); - } - if (params.prompt.find("ls dir") == std::string::npos) { - throw std::runtime_error("Expected follow-up user message in prompt"); - } - } - - // Degenerate partial JSON from agent-style tool output. - { - auto params = apply_multiturn(R"({"id":"-Updx2F7A3C7D3D9K2Z7H6H5)"); - if (params.format != COMMON_CHAT_FORMAT_PEG_NATIVE) { - throw std::runtime_error("Expected peg-native format for degenerate-args multi-turn apply"); - } - if (params.generation_prompt.find("<|im_start|>assistant") == std::string::npos) { - throw std::runtime_error("Expected generation prompt after malformed tool-call history"); - } - } -} - static void test_template_generation_prompt() { common_chat_msg system_msg; system_msg.role = "system"; @@ -6059,7 +5981,6 @@ int main(int argc, char ** argv) { test_tools_oaicompat_json_conversion(); test_convert_responses_to_chatcmpl(); test_developer_role_to_system_workaround(); - test_minicpm5_malformed_tool_args_multiturn(); test_template_generation_prompt(); test_template_output_peg_parsers(detailed_debug); std::cout << "\n[chat] All tests passed!" << '\n';