diff --git a/common/chat-auto-parser-generator.cpp b/common/chat-auto-parser-generator.cpp index 3461566677..4eed6f27b6 100644 --- a/common/chat-auto-parser-generator.cpp +++ b/common/chat-auto-parser-generator.cpp @@ -293,9 +293,7 @@ common_peg_parser analyze_tools::build_tool_parser_tag_tagged(parser_build_conte 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; + std::vector args; for (const auto & [param_name, param_schema] : properties.items()) { bool is_required = required.find(param_name) != required.end(); @@ -312,31 +310,15 @@ 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) { - required_parsers.push_back(named_arg); - } else { - optional_parsers.push_back(named_arg); + auto named_arg = p.repeat(p.rule("tool-" + name + "-arg-" + param_name, arg), is_required ? 1 : 0, 1); + + if (!args.empty()) { + args.push_back(p.space()); } + args.push_back(named_arg); } - // 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(); - } - args_seq = args_seq + required_parsers[i]; - } - - // Build optional args with flexible ordering - if (!optional_parsers.empty()) { - 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, -1); - } + common_peg_parser args_seq = p.sequence(args); // Build call_id parser based on position (if supported) common_peg_parser call_id_section = p.eps(); @@ -357,7 +339,7 @@ common_peg_parser analyze_tools::build_tool_parser_tag_tagged(parser_build_conte func_parser = p.atomic(p.tool_open(function.name_prefix + p.tool_name(p.literal(name)) + function.name_suffix) + call_id_section) + p.space() + args_seq; matched_atomic = true; - } else if (!arguments.name_prefix.empty() && !required_parsers.empty()) { + } else if (!arguments.name_prefix.empty() && !required.empty()) { // Only peek for an arg tag when there are required args that must follow. // When all args are optional, the model may emit no arg tags at all (#20650). func_parser = p.atomic(p.tool_open(function.name_prefix + p.tool_name(p.literal(name)) + function.name_suffix) + diff --git a/tests/test-chat.cpp b/tests/test-chat.cpp index 1c4da68195..563a4a6605 100644 --- a/tests/test-chat.cpp +++ b/tests/test-chat.cpp @@ -2144,57 +2144,19 @@ static void test_template_output_peg_parsers(bool detailed_debug) { .expect_reconstruction() .run(); - // Test flexible optional argument ordering (2 required + 4 optional, reversed optional order) + // Test optional argument ordering (2 required + 4 optional) tst.test( "\n" "\n" "\nhello\n\n" "\n42\n\n" - "\n100\n\n" "\n200\n\n" + "\n100\n\n" "\n" "") .tools({ tool_2req_4opt }) .expect_tool_calls({ - { "tool_2req_4opt", R"({"req1": "hello", "req2": 42, "opt4": 100, "opt2": 200})", {} }, - }) - .expect_reconstruction() - .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"})", {} }, - }) - .expect_reconstruction() - .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})", {} }, + { "tool_2req_4opt", R"({"req1": "hello", "req2": 42, "opt2": 200, "opt4": 100})", {} }, }) .expect_reconstruction() .run();