From 790cf51aabd61763486050dec7451d9147cb7c61 Mon Sep 17 00:00:00 2001 From: Aldehir Rojas Date: Sat, 12 Sep 2026 19:08:52 -0500 Subject: [PATCH] chat : improve parsing of complex types in qwen3-coder (#28742) * chat : improve schema support in qwen3 parser * cont : clean up grammar a bit --- common/parsers/qwen3-coder.cpp | 31 ++++++++++++++++-- tests/test-chat.cpp | 59 ++++++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+), 3 deletions(-) diff --git a/common/parsers/qwen3-coder.cpp b/common/parsers/qwen3-coder.cpp index dfc7440847..7938a20279 100644 --- a/common/parsers/qwen3-coder.cpp +++ b/common/parsers/qwen3-coder.cpp @@ -104,9 +104,34 @@ common_chat_params common_chat_params_init_qwen3_coder(const common_chat_templat auto arg_open = p.tool_arg_open("\n"); - auto arg_value = param.schema->may_be_string() ? - arg_string : - p.tool_arg_json_value(p.schema(p.json(), rule_name + "-schema", doc, *param.schema)) + arg_close; + auto types = param.schema->value_types(); + + auto arg_value = p.eps(); + if (!types.has(common_chat_schema::TYPE_STRING)) { + arg_value = p.tool_arg_json_value(p.schema(p.json(), rule_name + "-schema", doc, *param.schema)) + arg_close; + } else if (types.is_only(common_chat_schema::TYPE_STRING)) { + arg_value = arg_string; + } else { + // The string alternative accepts any text, so the grammar only keeps the raw string + // rule. The parser still tries the JSON alternatives first to type the value. + auto json_value = p.choice(); + if (types.has(common_chat_schema::TYPE_OBJECT)) { + json_value |= p.json_object(); + } + if (types.has(common_chat_schema::TYPE_ARRAY)) { + json_value |= p.json_array(); + } + if (types.has(common_chat_schema::TYPE_NUMBER) || types.has(common_chat_schema::TYPE_INTEGER)) { + json_value |= p.json_number(); + } + if (types.has(common_chat_schema::TYPE_BOOLEAN)) { + json_value |= p.json_bool(); + } + if (types.has(common_chat_schema::TYPE_NULL)) { + json_value |= p.json_null(); + } + arg_value = p.gbnf(p.atomic(p.tool_arg_json_value(json_value) + arg_close) | arg_string, "xml-arg-string"); + } auto arg_rule = p.rule(rule_name, p.tool_arg(arg_open + arg_value)); diff --git a/tests/test-chat.cpp b/tests/test-chat.cpp index 1aef83f430..30a7237e31 100644 --- a/tests/test-chat.cpp +++ b/tests/test-chat.cpp @@ -846,6 +846,25 @@ static common_chat_tool nullable_int_tool{ })", }; +static common_chat_tool string_union_tool{ + /* .name = */ "set_union", + /* .description = */ "Set values whose types are unions with string", + /* .parameters = */ R"({ + "type": "object", + "properties": { + "value": { + "type": ["string", "object"], + "description": "A string or object value" + }, + "amount": { + "type": ["string", "integer"], + "description": "A string or integer value" + } + }, + "required": ["value", "amount"] + })", +}; + static common_chat_tool enum_no_type_tool{ /* .name = */ "set_unit", /* .description = */ "Set a temperature unit", @@ -3805,6 +3824,46 @@ static void test_template_output_peg_parsers(bool detailed_debug) { }) .run(); + // nullable string given null - parses as JSON null, not the string "null" + tst.test( + "\n" + "\n" + "\nnull\n\n" + "\n" + "") + .tools({ nullable_string_tool }) + .expect_tool_calls({ + { "set_nullable_str", R"({"name": null})", {} }, + }) + .run(); + + // unions with string - JSON values of the other types are typed, everything else is a string + tst.test( + "\n" + "\n" + "\n{\"a\": 1}\n\n" + "\n2 dollars\n\n" + "\n" + "") + .tools({ string_union_tool }) + .expect_tool_calls({ + { "set_union", R"({"value": {"a": 1}, "amount": "2 dollars"})", {} }, + }) + .run(); + + tst.test( + "\n" + "\n" + "\n{not valid json\n\n" + "\n42\n\n" + "\n" + "") + .tools({ string_union_tool }) + .expect_tool_calls({ + { "set_union", R"({"value": "{not valid json", "amount": 42})", {} }, + }) + .run(); + // enum without explicit type key - should infer string from enum values tst.test( "\n"