From a838fb4d6df8985e327f17dbfee6f169d197c0ec Mon Sep 17 00:00:00 2001 From: Alde Rojas Date: Wed, 9 Sep 2026 10:10:34 -0500 Subject: [PATCH] cont : simplify schema resolution --- common/chat-auto-parser-generator.cpp | 45 +++------- common/json-schema-to-grammar.cpp | 74 ---------------- common/json-schema-to-grammar.h | 19 ---- common/json-schema.cpp | 53 ++++++++++- common/json-schema.h | 73 ++++++++------- common/parsers/deepseek.cpp | 29 ++---- common/parsers/minicpm5.cpp | 43 ++++----- common/parsers/minimax-m3.cpp | 5 +- common/parsers/muse-glimmer.cpp | 41 ++++----- common/parsers/parsers.cpp | 18 ++-- common/parsers/parsers.h | 4 +- common/parsers/qwen3-coder.cpp | 18 ++-- tests/test-json-schema-to-grammar.cpp | 116 ------------------------ tests/test-json-schema.cpp | 122 ++++++++++++++++++++++++++ 14 files changed, 283 insertions(+), 377 deletions(-) diff --git a/common/chat-auto-parser-generator.cpp b/common/chat-auto-parser-generator.cpp index d7e117e4d9..03c98c22dd 100644 --- a/common/chat-auto-parser-generator.cpp +++ b/common/chat-auto-parser-generator.cpp @@ -5,6 +5,7 @@ #include "common.h" #include "json-schema-to-grammar.h" #include "log.h" +#include "parsers/parsers.h" #include "peg-parser.h" #include @@ -12,16 +13,6 @@ using json = common_json; -// Helper to iterate over tools/functions -static void foreach_function(const json & tools, const std::function & fn) { - for (const auto & tool : tools) { - if (!tool.contains("type") || tool.at("type") != "function" || !tool.contains("function")) { - continue; - } - fn(tool); - } -} - namespace autoparser { parser_build_context::parser_build_context(common_chat_peg_builder & p, const generation_params & inputs) : @@ -383,43 +374,27 @@ common_peg_parser analyze_tools::build_tool_parser_tag_tagged(parser_build_conte common_peg_parser tool_choice = p.choice(); foreach_function(inputs.tools, [&](const json & tool) { - const auto & func = tool.at("function"); - std::string name = func.at("name"); - auto params = func.contains("parameters") ? func.at("parameters") : json::object(); - const auto & properties = params.contains("properties") ? params.at("properties") : json::object(); - - std::set required; - if (params.contains("required")) { - required = params.at("required").get>(); - } - - auto schema_info = common_schema_info(); - schema_info.resolve_refs(params); + const auto & func = tool.at("function"); + std::string name = func.at("name"); // 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(); - + foreach_parameter(func, [&](const common_schema_property & param, const json & param_schema) { auto arg = - p.tool_arg(p.tool_arg_open(arguments.name_prefix + p.tool_arg_name(p.literal(param_name)) + + p.tool_arg(p.tool_arg_open(arguments.name_prefix + p.tool_arg_name(p.literal(param.name)) + arguments.name_suffix) + arguments.value_prefix + - (schema_info.resolves_to_string(param_schema) ? + (param.schema->resolves_to_string() ? p.ac(p.tool_arg_string_value(until_suffix) + p.tool_arg_close(p.literal(arguments.value_suffix)), arguments.value_suffix) : (p.tool_arg_json_value(p.schema( - p.json(), "tool-" + name + "-arg-" + param_name + "-schema", param_schema, false)) + + p.json(), "tool-" + name + "-arg-" + param.name + "-schema", param_schema, false)) + 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.rule("tool-" + name + "-arg-" + param.name, arg); + (param.required ? required_parsers : optional_parsers).push_back(named_arg); + }); // Build required arg sequence in definition order common_peg_parser args_seq = p.eps(); diff --git a/common/json-schema-to-grammar.cpp b/common/json-schema-to-grammar.cpp index 347b98d6b1..ad09fb71e1 100644 --- a/common/json-schema-to-grammar.cpp +++ b/common/json-schema-to-grammar.cpp @@ -1009,80 +1009,6 @@ public: } }; -bool common_schema_resolves_to_string(const common_schema & schema) { - std::unordered_set visited; - - std::function check = [&](const common_schema & s) -> bool { - switch (s.kind()) { - case COMMON_SCHEMA_KIND_STRING: - return true; - case COMMON_SCHEMA_KIND_CONST: - return static_cast(s).value.is_string(); - case COMMON_SCHEMA_KIND_ENUM: - for (const auto & v : static_cast(s).values) { - if (v.is_string()) { - return true; - } - } - return false; - case COMMON_SCHEMA_KIND_REF: { - // a cycle is taken as not a string, to be safe - const auto * target = static_cast(s).target; - return target && visited.insert(target).second && check(*target); - } - case COMMON_SCHEMA_KIND_ANY_OF: - for (const auto & child : static_cast(s).children) { - if (check(*child)) { - return true; - } - } - return false; - case COMMON_SCHEMA_KIND_ALL_OF: { - // every child must allow a string, an any child constrains nothing - bool any_string = false; - for (const auto & child : static_cast(s).children) { - if (child->kind() == COMMON_SCHEMA_KIND_ANY) { - continue; - } - if (!check(*child)) { - return false; - } - any_string = true; - } - return any_string; - } - default: - return false; - } - }; - - return check(schema); -} - -void common_schema_info::resolve_refs(const common_json & schema) { - // a schema that does not parse is reported when its grammar is built, here it only answers no - try { - common_schema_parse(schema, doc_); - } catch (const std::runtime_error &) { - } -} - -bool common_schema_info::resolves_to_string(const common_json & schema) { - try { - return common_schema_resolves_to_string(*common_schema_parse(schema, doc_)); - } catch (const std::runtime_error &) { - return false; - } -} - -common_schema_kinds common_schema_info::resolve_kinds(const common_json & schema) { - try { - return common_schema_resolve_kinds(*common_schema_parse(schema, doc_)); - } catch (const std::runtime_error &) { - return {}; - } -} - std::string json_schema_to_grammar(const common_json & schema, bool force_gbnf) { #ifdef LLAMA_USE_LLGUIDANCE if (!force_gbnf) { diff --git a/common/json-schema-to-grammar.h b/common/json-schema-to-grammar.h index f476fed3fd..c9b5a08f58 100644 --- a/common/json-schema-to-grammar.h +++ b/common/json-schema-to-grammar.h @@ -10,25 +10,6 @@ std::string json_schema_to_grammar(const common_json & schema, bool force_gbnf = false); std::string json_schema_to_grammar(const common_schema_document & schema); -// Whether a value matching the schema may be a string, through any branch of it. -// Some models emit raw string values rather than JSON-encoded strings for string parameters. -bool common_schema_resolves_to_string(const common_schema & schema); - -// Probes the sub-schemas of one JSON schema, e.g. the parameters of a tool -class common_schema_info { - common_schema_document doc_; - - public: - // Parses the schema, so that the $refs of its sub-schemas resolve - void resolve_refs(const common_json & schema); - - // common_schema_resolves_to_string() for a sub-schema of a schema given to resolve_refs(), false when it does not parse - bool resolves_to_string(const common_json & schema); - - // common_schema_resolve_kinds() for a sub-schema of a schema given to resolve_refs(), empty when it does not parse - common_schema_kinds resolve_kinds(const common_json & schema); -}; - struct common_grammar_builder { std::function add_rule; std::function add_schema; diff --git a/common/json-schema.cpp b/common/json-schema.cpp index 1aa67c9eb6..fc54ec8965 100644 --- a/common/json-schema.cpp +++ b/common/json-schema.cpp @@ -422,7 +422,56 @@ static common_schema_kinds resolve_kinds(const common_schema & s, std::unordered return {}; } -common_schema_kinds common_schema_resolve_kinds(const common_schema & schema) { +common_schema_kinds common_schema::resolve_kinds() const { std::unordered_set visited; - return resolve_kinds(schema, visited); + return ::resolve_kinds(*this, visited); +} + +static bool resolves_to_string(const common_schema & s, std::unordered_set & visited) { + switch (s.kind()) { + case COMMON_SCHEMA_KIND_STRING: + return true; + case COMMON_SCHEMA_KIND_CONST: + return static_cast(s).value.is_string(); + case COMMON_SCHEMA_KIND_ENUM: + for (const auto & v : static_cast(s).values) { + if (v.is_string()) { + return true; + } + } + return false; + case COMMON_SCHEMA_KIND_REF: { + // a cycle is taken as not a string, to be safe + const auto * target = static_cast(s).target; + return target && visited.insert(target).second && resolves_to_string(*target, visited); + } + case COMMON_SCHEMA_KIND_ANY_OF: + for (const auto & child : static_cast(s).children) { + if (resolves_to_string(*child, visited)) { + return true; + } + } + return false; + case COMMON_SCHEMA_KIND_ALL_OF: { + // every child must allow a string, an any child constrains nothing + bool any_string = false; + for (const auto & child : static_cast(s).children) { + if (child->kind() == COMMON_SCHEMA_KIND_ANY) { + continue; + } + if (!resolves_to_string(*child, visited)) { + return false; + } + any_string = true; + } + return any_string; + } + default: + return false; + } +} + +bool common_schema::resolves_to_string() const { + std::unordered_set visited; + return ::resolves_to_string(*this, visited); } diff --git a/common/json-schema.h b/common/json-schema.h index 3151e90280..bf87bf7c5e 100644 --- a/common/json-schema.h +++ b/common/json-schema.h @@ -37,10 +37,49 @@ enum common_schema_format { COMMON_SCHEMA_FORMAT_DATE_TIME, }; +// A set of the kinds of value a schema may match: only the value kinds NULL to OBJECT occur, a tuple counts as an array +class common_schema_kinds { + uint32_t mask_ = 0; + + public: + common_schema_kinds() = default; + common_schema_kinds(std::initializer_list kinds) { + for (auto kind : kinds) { + add(kind); + } + } + + static common_schema_kinds all() { + return { COMMON_SCHEMA_KIND_NULL, COMMON_SCHEMA_KIND_BOOLEAN, COMMON_SCHEMA_KIND_NUMBER, COMMON_SCHEMA_KIND_INTEGER, + COMMON_SCHEMA_KIND_STRING, COMMON_SCHEMA_KIND_ARRAY, COMMON_SCHEMA_KIND_OBJECT }; + } + + void add(common_schema_kind kind) { mask_ |= 1u << kind; } + + bool has(common_schema_kind kind) const { return (mask_ & (1u << kind)) != 0; } + bool is_only(common_schema_kind kind) const { return mask_ == (1u << kind); } + bool empty() const { return mask_ == 0; } + + common_schema_kinds & operator|=(const common_schema_kinds & other) { mask_ |= other.mask_; return *this; } + common_schema_kinds & operator&=(const common_schema_kinds & other) { mask_ &= other.mask_; return *this; } + + bool operator==(const common_schema_kinds & other) const { return mask_ == other.mask_; } + bool operator!=(const common_schema_kinds & other) const { return mask_ != other.mask_; } +}; + // Base class for all nodes, the concrete ones are the common_schema_* structs below struct common_schema { virtual ~common_schema() = default; virtual common_schema_kind kind() const = 0; + + // The kinds of value matching the schema: the union over anyOf, the intersection over allOf, every kind for an any. + // A number schema accepts integers too, so it resolves to both. + common_schema_kinds resolve_kinds() const; + + // Whether a value matching the schema may be a string, through any branch of it. + // Unlike resolve_kinds() an any does not count: some models emit raw string values rather than + // JSON-encoded strings for string parameters, and an unconstrained parameter is parsed as JSON. + bool resolves_to_string() const; }; using common_schema_ptr = std::unique_ptr; @@ -158,37 +197,3 @@ common_schema_document common_schema_parse(const common_json & schema); // A $ref it cannot resolve on its own is looked up in doc.refs, the targets it resolves itself are added there. // doc is unchanged when the schema is rejected. common_schema_ptr common_schema_parse(const common_json & schema, common_schema_document & doc); - -// A set of the kinds of value a schema may match: only the value kinds NULL to OBJECT occur, a tuple counts as an array -class common_schema_kinds { - uint32_t mask_ = 0; - - public: - common_schema_kinds() = default; - common_schema_kinds(std::initializer_list kinds) { - for (auto kind : kinds) { - add(kind); - } - } - - static common_schema_kinds all() { - return { COMMON_SCHEMA_KIND_NULL, COMMON_SCHEMA_KIND_BOOLEAN, COMMON_SCHEMA_KIND_NUMBER, COMMON_SCHEMA_KIND_INTEGER, - COMMON_SCHEMA_KIND_STRING, COMMON_SCHEMA_KIND_ARRAY, COMMON_SCHEMA_KIND_OBJECT }; - } - - void add(common_schema_kind kind) { mask_ |= 1u << kind; } - - bool has(common_schema_kind kind) const { return (mask_ & (1u << kind)) != 0; } - bool is_only(common_schema_kind kind) const { return mask_ == (1u << kind); } - bool empty() const { return mask_ == 0; } - - common_schema_kinds & operator|=(const common_schema_kinds & other) { mask_ |= other.mask_; return *this; } - common_schema_kinds & operator&=(const common_schema_kinds & other) { mask_ &= other.mask_; return *this; } - - bool operator==(const common_schema_kinds & other) const { return mask_ == other.mask_; } - bool operator!=(const common_schema_kinds & other) const { return mask_ != other.mask_; } -}; - -// The kinds of value matching the schema: the union over anyOf, the intersection over allOf, every kind for an any. -// A number schema accepts integers too, so it resolves to both. -common_schema_kinds common_schema_resolve_kinds(const common_schema & schema); diff --git a/common/parsers/deepseek.cpp b/common/parsers/deepseek.cpp index 5e25817272..c9e33fad65 100644 --- a/common/parsers/deepseek.cpp +++ b/common/parsers/deepseek.cpp @@ -149,39 +149,24 @@ common_chat_params common_chat_params_init_deepseek_v3_2(const common_chat_templ foreach_function(inputs.tools, [&](const json & tool) { const auto & function = tool.at("function"); std::string name = function.at("name"); - auto params = function.contains("parameters") ? function.at("parameters") : json::object(); - const auto & props = params.contains("properties") ? params.at("properties") : json::object(); - - std::set required; - if (params.contains("required")) { - required = params.at("required").get>(); - } - - auto schema_info = common_schema_info(); - schema_info.resolve_refs(params); std::vector required_parsers; std::vector optional_parsers; - for (const auto & [param_name, param_schema] : props.items()) { - bool is_required = required.find(param_name) != required.end(); - bool is_string = schema_info.resolves_to_string(param_schema); + foreach_parameter(function, [&](const common_schema_property & param, const json & param_schema) { + bool is_string = param.schema->resolves_to_string(); auto arg = p.tool_arg( - p.tool_arg_open(p.literal(PARAM_START + " name=\"") + p.tool_arg_name(p.literal(param_name)) + + p.tool_arg_open(p.literal(PARAM_START + " name=\"") + p.tool_arg_name(p.literal(param.name)) + p.literal("\" string=\"" + std::string(is_string ? "true" : "false") + "\">")) + (is_string ? p.tool_arg_string_value(p.until(PARAM_END)) : - p.tool_arg_json_value(p.schema(p.json(), "tool-" + name + "-arg-" + param_name + "-schema", + p.tool_arg_json_value(p.schema(p.json(), "tool-" + name + "-arg-" + param.name + "-schema", param_schema, false))) + p.tool_arg_close(p.literal(PARAM_END))); - 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.rule("tool-" + name + "-arg-" + param.name, arg); + (param.required ? required_parsers : optional_parsers).push_back(named_arg); + }); common_peg_parser args_seq = p.eps(); for (size_t i = 0; i < required_parsers.size(); i++) { diff --git a/common/parsers/minicpm5.cpp b/common/parsers/minicpm5.cpp index e6e0abf066..4de988f3ac 100644 --- a/common/parsers/minicpm5.cpp +++ b/common/parsers/minicpm5.cpp @@ -71,32 +71,27 @@ common_chat_params common_chat_params_init_minicpm5(const common_chat_template & 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(); + + std::vector arg_rules; + foreach_parameter(function, [&](const common_schema_property & prop, const json & prop_schema) { + auto value_parser = p.eps(); + if (prop.schema->resolves_to_string()) { + value_parser = string_value; + } else { + value_parser = p.tool_arg_json_value( + p.schema(p.json(), "tool-" + name + "-arg-" + prop.name + "-schema", prop_schema, false) + ) + p.tool_arg_close(p.literal("")); + } + + arg_rules.push_back(p.tool_arg( + p.tool_arg_open(p.literal("")) + + value_parser + )); + }); 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()) { - auto value_parser = p.eps(); - if (schema_info.resolves_to_string(prop_schema)) { - value_parser = string_value; - } else { - value_parser = p.tool_arg_json_value( - p.schema(p.json(), "tool-" + name + "-arg-" + prop_name + "-schema", prop_schema, false) - ) + p.tool_arg_close(p.literal("")); - } - - auto arg_rule = p.tool_arg( - p.tool_arg_open(p.literal("")) + - value_parser - ); - - arg_choice |= arg_rule; - } - args = p.zero_or_more(arg_choice + p.space()); + if (!arg_rules.empty()) { + args = p.zero_or_more(p.choice(arg_rules) + p.space()); } auto tool_parser = p.tool( diff --git a/common/parsers/minimax-m3.cpp b/common/parsers/minimax-m3.cpp index ff23ea153c..99ebb456c8 100644 --- a/common/parsers/minimax-m3.cpp +++ b/common/parsers/minimax-m3.cpp @@ -99,8 +99,7 @@ common_chat_params common_chat_params_init_minimax_m3(const common_chat_template std::string name = function.at("name"); auto params = function.contains("parameters") ? function.at("parameters") : json::object(); - auto schema_info = common_schema_info(); - schema_info.resolve_refs(params); + auto doc = common_schema_parse(params); // The template expands argument values recursively in XML (see the to_xml() macro) std::function value_of; @@ -123,7 +122,7 @@ common_chat_params common_chat_params_init_minimax_m3(const common_chat_template auto close_tag = p.tool_arg_close(p.literal(close)); // A string accepts anything, so a union with a string alternative is a string - if (schema_info.resolves_to_string(schema)) { + if (common_schema_parse(schema, doc)->resolves_to_string()) { return p.ac(p.tool_arg_string_value(p.until(close)) + close_tag, close); } diff --git a/common/parsers/muse-glimmer.cpp b/common/parsers/muse-glimmer.cpp index 7f4dfcd511..c25ebec264 100644 --- a/common/parsers/muse-glimmer.cpp +++ b/common/parsers/muse-glimmer.cpp @@ -74,31 +74,26 @@ common_chat_params common_chat_params_init_muse_glimmer(const common_chat_templa 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(); + + std::vector arg_rules; + foreach_parameter(function, [&](const common_schema_property & prop, const json & prop_schema) { + auto value_parser = p.eps(); + if (prop.schema->resolves_to_string()) { + value_parser = string_value; + } else { + value_parser = p.tool_arg_json_value( + p.schema(p.json(), "tool-" + name + "-arg-" + prop.name + "-schema", prop_schema, false)) + + p.tool_arg_close(p.literal("")); + } + + arg_rules.push_back(p.tool_arg( + p.tool_arg_open(p.literal("")) + + value_parser)); + }); 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()) { - auto value_parser = p.eps(); - if (schema_info.resolves_to_string(prop_schema)) { - value_parser = string_value; - } else { - value_parser = p.tool_arg_json_value( - p.schema(p.json(), "tool-" + name + "-arg-" + prop_name + "-schema", prop_schema, false)) - + p.tool_arg_close(p.literal("")); - } - - auto arg_rule = p.tool_arg( - p.tool_arg_open(p.literal("")) + - value_parser); - - arg_choice |= arg_rule; - } - args = p.zero_or_more(arg_choice + p.space()); + if (!arg_rules.empty()) { + args = p.zero_or_more(p.choice(arg_rules) + p.space()); } auto tool_parser = p.tool( diff --git a/common/parsers/parsers.cpp b/common/parsers/parsers.cpp index 0a4d5cfbb5..8d017a0cdd 100644 --- a/common/parsers/parsers.cpp +++ b/common/parsers/parsers.cpp @@ -2,8 +2,6 @@ #include "log.h" -#include - void foreach_function(const json & tools, const std::function & fn) { for (const auto & tool : tools) { if (!tool.contains("type") || tool.at("type") != "function" || !tool.contains("function")) { @@ -14,21 +12,17 @@ void foreach_function(const json & tools, const std::function & fn) { +void foreach_parameter(const json & function, const std::function & fn) { if (!function.contains("parameters") || !function.at("parameters").is_object()) { return; } const auto & params = function.at("parameters"); - if (!params.contains("properties") || !params.at("properties").is_object()) { + auto doc = common_schema_parse(params); + const auto * object = dynamic_cast(doc.root.get()); + if (!object) { return; } - const auto & props = params.at("properties"); - std::set required; - if (params.contains("required") && params.at("required").is_array()) { - required = params.at("required").get>(); - } - for (const auto & [name, prop] : props.items()) { - bool is_required = (required.find(name) != required.end()); - fn(name, prop, is_required); + for (const auto & prop : object->properties) { + fn(prop, params.at("properties").at(prop.name)); } } diff --git a/common/parsers/parsers.h b/common/parsers/parsers.h index 7898f00071..9dcf7d497c 100644 --- a/common/parsers/parsers.h +++ b/common/parsers/parsers.h @@ -20,8 +20,8 @@ using json = common_json; // iterate over the function tools of an OpenAI-style tools array void foreach_function(const json & tools, const std::function & fn); -// iterate over the parameters of a function tool, flagging the ones listed as required -void foreach_parameter(const json & function, const std::function & fn); +// iterate over the parameters of a function tool: the parsed schema of each one for probing, its JSON for the grammar +void foreach_parameter(const json & function, const std::function & fn); // render a template; the override arguments let a parser feed in messages, tools or context it has rewritten std::string common_chat_template_direct_apply_impl( diff --git a/common/parsers/qwen3-coder.cpp b/common/parsers/qwen3-coder.cpp index 8a1e521370..b086ec0963 100644 --- a/common/parsers/qwen3-coder.cpp +++ b/common/parsers/qwen3-coder.cpp @@ -93,28 +93,24 @@ common_chat_params common_chat_params_init_qwen3_coder(const common_chat_templat auto tool_choice = p.choice(); foreach_function(inputs.tools, [&](const json & tool) { - const auto & function = tool.at("function"); - std::string name = function.at("name"); - auto parameters = function.contains("parameters") ? function.at("parameters") : json::object(); - - auto schema_info = common_schema_info(); - schema_info.resolve_refs(parameters); + const auto & function = tool.at("function"); + std::string name = function.at("name"); std::vector required_args; std::vector optional_args; - foreach_parameter(function, [&](const std::string & param_name, const json & param_schema, bool is_required) { - auto rule_name = "tool-" + name + "-arg-" + param_name; + foreach_parameter(function, [&](const common_schema_property & param, const json & param_schema) { + auto rule_name = "tool-" + name + "-arg-" + param.name; - auto arg_open = p.tool_arg_open("\n"); + auto arg_open = p.tool_arg_open("\n"); - auto arg_value = schema_info.resolves_to_string(param_schema) ? + auto arg_value = param.schema->resolves_to_string() ? arg_string : p.tool_arg_json_value(p.schema(p.json(), rule_name + "-schema", param_schema)) + arg_close; auto arg_rule = p.rule(rule_name, p.tool_arg(arg_open + arg_value)); - (is_required ? required_args : optional_args).push_back(arg_rule); + (param.required ? required_args : optional_args).push_back(arg_rule); }); // Accept required arguments in any order, as Qwen does not always adhere to the diff --git a/tests/test-json-schema-to-grammar.cpp b/tests/test-json-schema-to-grammar.cpp index 81e1ff7edc..f207240b34 100755 --- a/tests/test-json-schema-to-grammar.cpp +++ b/tests/test-json-schema-to-grammar.cpp @@ -1510,123 +1510,7 @@ static void test_all(const std::string & title, std::functionresolves_to_string()); + }; + + t.test("types", [&](testing & t) { + check(t, R"({"type": "string"})", true); + check(t, R"({"type": "integer"})", false); + check(t, R"({"type": "number"})", false); + check(t, R"({"type": "boolean"})", false); + check(t, R"({"type": "object"})", false); + check(t, R"({"type": "array"})", false); + check(t, R"({"type": ["string", "null"]})", true); + check(t, R"({"type": ["integer", "null"]})", false); + }); + + t.test("an any is not a string", [&](testing & t) { + check(t, R"({})", false); + check(t, R"({"minLength": 1})", false); + check(t, R"({"maxLength": 10})", false); + check(t, R"({"format": "email"})", false); + }); + + t.test("string keywords", [&](testing & t) { + check(t, R"({"pattern": "^[a-z]+$"})", true); + check(t, R"({"format": "date"})", true); + check(t, R"({"format": "uuid"})", true); + }); + + t.test("const and enum", [&](testing & t) { + check(t, R"({"const": "hello"})", true); + check(t, R"({"const": 123})", false); + check(t, R"({"enum": ["a", "b", "c"]})", true); + check(t, R"({"enum": [1, 2, 3]})", false); + check(t, R"({"enum": [1, "a", null]})", true); + }); + + t.test("any_of", [&](testing & t) { + check(t, R"({"anyOf": [{"type": "string"}, {"type": "integer"}]})", true); + check(t, R"({"anyOf": [{"type": "integer"}, {"type": "boolean"}]})", false); + check(t, R"({"oneOf": [{"type": "string"}, {"type": "number"}]})", true); + check(t, R"({"oneOf": [{"type": "object"}, {"type": "array"}]})", false); + check(t, R"({"anyOf": [{"anyOf": [{"type": "integer"}, {"type": "string"}]}, {"type": "boolean"}]})", true); + }); + + t.test("all_of", [&](testing & t) { + check(t, R"({"allOf": [{"type": "string"}, {"minLength": 1}]})", true); + check(t, R"({"allOf": [{"type": "string"}, {"type": "integer"}]})", false); + check(t, R"({"allOf": [{"minLength": 1}, {"maxLength": 2}]})", false); + }); + + t.test("ref", [&](testing & t) { + check(t, R"({"$ref": "#/$defs/str", "$defs": {"str": {"type": "string"}}})", true); + check(t, R"({"$ref": "#/$defs/num", "$defs": {"num": {"type": "integer"}}})", false); + check(t, R"({"$ref": "#/$defs/n", "$defs": {"n": {"anyOf": [{"$ref": "#/$defs/n"}, {"type": "string"}]}}})", true); + check(t, R"({"$ref": "#/$defs/n", "$defs": {"n": {"$ref": "#/$defs/n"}}})", false); + }); +} + +// e.g. {number, integer}, in kind order +static std::string dump(const common_schema_kinds & kinds) { + static const char * names[] = { "null", "boolean", "number", "integer", "string", "array", "object" }; + static const common_schema_kind order[] = { COMMON_SCHEMA_KIND_NULL, COMMON_SCHEMA_KIND_BOOLEAN, COMMON_SCHEMA_KIND_NUMBER, + COMMON_SCHEMA_KIND_INTEGER, COMMON_SCHEMA_KIND_STRING, COMMON_SCHEMA_KIND_ARRAY, + COMMON_SCHEMA_KIND_OBJECT }; + std::string out; + for (size_t i = 0; i < 7; i++) { + if (kinds.has(order[i])) { + out += (out.empty() ? "" : ", ") + std::string(names[i]); + } + } + return "{" + out + "}"; +} + +static void test_resolve_kinds(testing & t) { + auto check = [](testing & t, const std::string & schema, const common_schema_kinds & expected) { + t.assert_equal(schema, dump(expected), dump(parse(schema).root->resolve_kinds())); + }; + + t.test("types", [&](testing & t) { + check(t, R"({"type": "string"})", { COMMON_SCHEMA_KIND_STRING }); + check(t, R"({"type": "integer"})", { COMMON_SCHEMA_KIND_INTEGER }); + check(t, R"({"type": "number"})", { COMMON_SCHEMA_KIND_NUMBER, COMMON_SCHEMA_KIND_INTEGER }); + check(t, R"({"type": ["string", "null"]})", { COMMON_SCHEMA_KIND_STRING, COMMON_SCHEMA_KIND_NULL }); + }); + + t.test("an any is every kind", [&](testing & t) { + check(t, R"({"description": "anything"})", common_schema_kinds::all()); + check(t, R"({"minLength": 1})", common_schema_kinds::all()); + }); + + t.test("structural keywords", [&](testing & t) { + check(t, R"({"properties": {"a": {"type": "string"}}})", { COMMON_SCHEMA_KIND_OBJECT }); + check(t, R"({"items": {"type": "string"}})", { COMMON_SCHEMA_KIND_ARRAY }); + check(t, R"({"prefixItems": [{"type": "string"}]})", { COMMON_SCHEMA_KIND_ARRAY }); + }); + + t.test("const and enum", [&](testing & t) { + check(t, R"({"const": 1.5})", { COMMON_SCHEMA_KIND_NUMBER }); + check(t, R"({"enum": [1, "a", null]})", { COMMON_SCHEMA_KIND_INTEGER, COMMON_SCHEMA_KIND_STRING, COMMON_SCHEMA_KIND_NULL }); + }); + + t.test("any_of is the union", [&](testing & t) { + check(t, R"({"anyOf": [{"type": "string"}, {"type": "integer"}]})", { COMMON_SCHEMA_KIND_STRING, COMMON_SCHEMA_KIND_INTEGER }); + }); + + t.test("all_of is the intersection", [&](testing & t) { + check(t, R"({"allOf": [{"type": ["string", "number"]}, {"type": ["number", "object"]}]})", { COMMON_SCHEMA_KIND_NUMBER, COMMON_SCHEMA_KIND_INTEGER }); + check(t, R"({"allOf": [{"type": "string"}, {"description": "x"}]})", { COMMON_SCHEMA_KIND_STRING }); + check(t, R"({"allOf": [{"type": "string"}, {"type": "integer"}]})", {}); + }); + + t.test("ref", [&](testing & t) { + check(t, R"({"$ref": "#/$defs/u", "$defs": {"u": {"anyOf": [{"type": "boolean"}, {"type": "array"}]}}})", + { COMMON_SCHEMA_KIND_BOOLEAN, COMMON_SCHEMA_KIND_ARRAY }); + check(t, R"({"$ref": "#/$defs/n", "$defs": {"n": {"anyOf": [{"$ref": "#/$defs/n"}, {"type": "string"}]}}})", + { COMMON_SCHEMA_KIND_STRING }); + }); +} + static void test_errors(testing & t) { t.test("not a schema", [](testing & t) { assert_error(t, R"([])", "#: schema must be an object"); @@ -721,6 +841,8 @@ int main(int argc, char * argv[]) { t.test("any_of", test_any_of); t.test("all_of", test_all_of); t.test("ref", test_ref); + t.test("resolves_to_string", test_resolves_to_string); + t.test("resolve_kinds", test_resolve_kinds); t.test("errors", test_errors); return t.summary();