From b3f41f7e37df802d9f41da5ab53159aa04bcd083 Mon Sep 17 00:00:00 2001 From: Piotr Wilkin Date: Fri, 6 Mar 2026 17:45:16 +0100 Subject: [PATCH 1/3] Allow reshuffled arguments in tagged argument parser format tool calls. --- common/chat-auto-parser-generator.cpp | 59 +++++++++++++++--- tests/test-chat.cpp | 87 +++++++++++++++++++++++++++ 2 files changed, 139 insertions(+), 7 deletions(-) diff --git a/common/chat-auto-parser-generator.cpp b/common/chat-auto-parser-generator.cpp index 080bce50023..1f3f83344eb 100644 --- a/common/chat-auto-parser-generator.cpp +++ b/common/chat-auto-parser-generator.cpp @@ -4,6 +4,7 @@ #include "json-schema-to-grammar.h" #include "nlohmann/json.hpp" +#include #include #include @@ -302,8 +303,9 @@ common_peg_parser analyze_tools::build_tool_parser_tag_tagged(parser_build_conte params.at("required").get_to(required); } - // Build parser for each argument - std::vector arg_parsers; + // Build parser for each argument, separating required and optional + std::vector required_parsers; + std::vector optional_parsers; for (const auto & [param_name, param_schema] : properties.items()) { bool is_required = required.find(param_name) != required.end(); std::string type = "object"; @@ -328,20 +330,63 @@ common_peg_parser analyze_tools::build_tool_parser_tag_tagged(parser_build_conte p.space()) + p.tool_arg_close(p.literal(arguments.value_suffix))); + auto named_arg = p.rule("tool-" + name + "-arg-" + param_name, arg); if (is_required) { - arg_parsers.push_back(p.rule("tool-" + name + "-arg-" + param_name, arg)); + required_parsers.push_back(named_arg); } else { - arg_parsers.push_back(p.optional(p.rule("tool-" + name + "-arg-" + param_name, arg))); + optional_parsers.push_back(named_arg); } } - // Build arg sequence with space() between consecutive args + // Build required arg sequence in definition order common_peg_parser args_seq = p.eps(); - for (size_t i = 0; i < arg_parsers.size(); i++) { + for (size_t i = 0; i < required_parsers.size(); i++) { if (i > 0) { args_seq = args_seq + p.space(); } - args_seq = args_seq + arg_parsers[i]; + args_seq = args_seq + required_parsers[i]; + } + + // Build optional args with flexible ordering + if (!optional_parsers.empty()) { + if (optional_parsers.size() <= 4) { + // For up to 4 optional params, generate a recursive choice tree + // that allows any permutation without duplicates. + // Each level: choice(space+opt[i]+recurse(remaining-i) for each i, eps) + std::function)> build_opt_choices; + build_opt_choices = [&](std::vector remaining) -> common_peg_parser { + if (remaining.empty()) { + return p.eps(); + } + common_peg_parser choices = p.choice(); + for (size_t i = 0; i < remaining.size(); i++) { + auto idx = remaining[i]; + std::vector next; + for (size_t j = 0; j < remaining.size(); j++) { + if (j != i) { + next.push_back(remaining[j]); + } + } + choices |= p.space() + optional_parsers[idx] + build_opt_choices(next); + } + choices |= p.eps(); + return choices; + }; + std::vector all_indices(optional_parsers.size()); + for (size_t i = 0; i < optional_parsers.size(); i++) { + all_indices[i] = i; + } + args_seq = args_seq + build_opt_choices(all_indices); + } else { + // For 5+ optional params, use choice-of-any repeated up to N times. + // This may allow duplicate params as a trade-off for avoiding + // combinatorial explosion of permutations. + common_peg_parser any_opt = p.choice(); + for (const auto & opt : optional_parsers) { + any_opt |= opt; + } + args_seq = args_seq + p.repeat(p.space() + any_opt, 0, (int) optional_parsers.size()); + } } // Build call_id parser based on position (if supported) diff --git a/tests/test-chat.cpp b/tests/test-chat.cpp index fab7a3780c8..0bd95af5d43 100644 --- a/tests/test-chat.cpp +++ b/tests/test-chat.cpp @@ -637,6 +637,41 @@ static common_chat_tool quoted_unquoted_tool{ }; +static common_chat_tool tool_2req_4opt{ + /* .name = */ "tool_2req_4opt", + /* .description = */ "Tool with 2 required and 4 optional params", + /* .parameters = */ R"({ + "type": "object", + "properties": { + "req1": { "type": "string", "description": "Required string" }, + "req2": { "type": "integer", "description": "Required int" }, + "opt1": { "type": "string", "description": "Optional string 1" }, + "opt2": { "type": "integer", "description": "Optional int 1" }, + "opt3": { "type": "string", "description": "Optional string 2" }, + "opt4": { "type": "integer", "description": "Optional int 2" } + }, + "required": ["req1", "req2"] + })", +}; + +static common_chat_tool tool_2req_5opt{ + /* .name = */ "tool_2req_5opt", + /* .description = */ "Tool with 2 required and 5 optional params", + /* .parameters = */ R"({ + "type": "object", + "properties": { + "req1": { "type": "string", "description": "Required string" }, + "req2": { "type": "integer", "description": "Required int" }, + "opt1": { "type": "string", "description": "Optional string 1" }, + "opt2": { "type": "integer", "description": "Optional int 1" }, + "opt3": { "type": "string", "description": "Optional string 2" }, + "opt4": { "type": "integer", "description": "Optional int 2" }, + "opt5": { "type": "string", "description": "Optional string 3" } + }, + "required": ["req1", "req2"] + })", +}; + static std::vector tools{ special_function_tool, special_function_tool_with_optional_param, python_tool, html_tool, todo_list }; @@ -1958,6 +1993,58 @@ static void test_template_output_peg_parsers(bool detailed_debug) { { "todo_list", "{\"todos\": [{\"item\": \"Check stuff\", \"selected\": false}, {\"item\": \"Prepare stuff\", \"selected\": true}]}", {} }, }) .run(); + + // Test flexible optional argument ordering (2 required + 4 optional, reversed optional order) + tst.test( + "\n" + "\n" + "\nhello\n\n" + "\n42\n\n" + "\n100\n\n" + "\n200\n\n" + "\n" + "") + .tools({ tool_2req_4opt }) + .expect_tool_calls({ + { "tool_2req_4opt", R"({"req1": "hello", "req2": 42, "opt4": 100, "opt2": 200})", {} }, + }) + .run(); + + // Test flexible optional argument ordering (2 required + 5 optional, reversed optional order) + tst.test( + "\n" + "\n" + "\nworld\n\n" + "\n7\n\n" + "\nlast\n\n" + "\nmiddle\n\n" + "\nfirst\n\n" + "\n" + "") + .tools({ tool_2req_5opt }) + .expect_tool_calls({ + { "tool_2req_5opt", R"({"req1": "world", "req2": 7, "opt5": "last", "opt3": "middle", "opt1": "first"})", {} }, + }) + .run(); + + // Test flexible optional argument ordering (2 required + 5 optional, all 5 in shuffled order) + tst.test( + "\n" + "\n" + "\ntest\n\n" + "\n99\n\n" + "\nc\n\n" + "\na\n\n" + "\ne\n\n" + "\n4\n\n" + "\n2\n\n" + "\n" + "") + .tools({ tool_2req_5opt }) + .expect_tool_calls({ + { "tool_2req_5opt", R"({"req1": "test", "req2": 99, "opt3": "c", "opt1": "a", "opt5": "e", "opt4": 4, "opt2": 2})", {} }, + }) + .run(); } { auto tst = peg_tester("models/templates/deepseek-ai-DeepSeek-V3.1.jinja", detailed_debug); From 3d9298c03a0358265d9106d40a74691178fad43d Mon Sep 17 00:00:00 2001 From: Piotr Wilkin Date: Fri, 6 Mar 2026 22:09:37 +0100 Subject: [PATCH 2/3] Remove shuffle just keep the optional parsers in any order --- common/chat-auto-parser-generator.cpp | 41 +++------------------------ 1 file changed, 4 insertions(+), 37 deletions(-) diff --git a/common/chat-auto-parser-generator.cpp b/common/chat-auto-parser-generator.cpp index 1f3f83344eb..e42007c8248 100644 --- a/common/chat-auto-parser-generator.cpp +++ b/common/chat-auto-parser-generator.cpp @@ -349,44 +349,11 @@ common_peg_parser analyze_tools::build_tool_parser_tag_tagged(parser_build_conte // Build optional args with flexible ordering if (!optional_parsers.empty()) { - if (optional_parsers.size() <= 4) { - // For up to 4 optional params, generate a recursive choice tree - // that allows any permutation without duplicates. - // Each level: choice(space+opt[i]+recurse(remaining-i) for each i, eps) - std::function)> build_opt_choices; - build_opt_choices = [&](std::vector remaining) -> common_peg_parser { - if (remaining.empty()) { - return p.eps(); - } - common_peg_parser choices = p.choice(); - for (size_t i = 0; i < remaining.size(); i++) { - auto idx = remaining[i]; - std::vector next; - for (size_t j = 0; j < remaining.size(); j++) { - if (j != i) { - next.push_back(remaining[j]); - } - } - choices |= p.space() + optional_parsers[idx] + build_opt_choices(next); - } - choices |= p.eps(); - return choices; - }; - std::vector all_indices(optional_parsers.size()); - for (size_t i = 0; i < optional_parsers.size(); i++) { - all_indices[i] = i; - } - args_seq = args_seq + build_opt_choices(all_indices); - } else { - // For 5+ optional params, use choice-of-any repeated up to N times. - // This may allow duplicate params as a trade-off for avoiding - // combinatorial explosion of permutations. - common_peg_parser any_opt = p.choice(); - for (const auto & opt : optional_parsers) { - any_opt |= opt; - } - args_seq = args_seq + p.repeat(p.space() + any_opt, 0, (int) optional_parsers.size()); + common_peg_parser any_opt = p.choice(); + for (const auto & opt : optional_parsers) { + any_opt |= opt; } + args_seq = args_seq + p.repeat(p.space() + any_opt, 0, (int) optional_parsers.size()); } // Build call_id parser based on position (if supported) From 69018cd92a8ad9985df6e1a3a58def5b98ab93c4 Mon Sep 17 00:00:00 2001 From: Piotr Wilkin Date: Fri, 6 Mar 2026 22:33:46 +0100 Subject: [PATCH 3/3] Remove unnecessary import --- common/chat-auto-parser-generator.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/common/chat-auto-parser-generator.cpp b/common/chat-auto-parser-generator.cpp index e42007c8248..9a936f892b0 100644 --- a/common/chat-auto-parser-generator.cpp +++ b/common/chat-auto-parser-generator.cpp @@ -4,7 +4,6 @@ #include "json-schema-to-grammar.h" #include "nlohmann/json.hpp" -#include #include #include