From 14ae5ffc7b3f4ede61dcd8ac7d72e9d9756dad28 Mon Sep 17 00:00:00 2001 From: Xuan Son Nguyen Date: Sat, 15 Aug 2026 14:47:40 +0200 Subject: [PATCH] messages_inp_normalizer --- common/chat.cpp | 99 ++++++++++++++++++++++++++++++++------------ tests/test-jinja.cpp | 5 +-- tests/testing.h | 4 +- 3 files changed, 76 insertions(+), 32 deletions(-) diff --git a/common/chat.cpp b/common/chat.cpp index a45c51fa0c..33d0ff8ec4 100644 --- a/common/chat.cpp +++ b/common/chat.cpp @@ -470,36 +470,80 @@ std::vector common_chat_msgs_parse_oaicompat(const json & messa return msgs; } +struct messages_inp_normalizer { + const jinja::caps & caps; + + messages_inp_normalizer(const jinja::caps & c) : caps(c) {} + + // handle supports_string_content / supports_typed_content + // if string=true and array=false, convert array to string + // if string=false and array=true, convert string to array + // if both are true, do nothing + json normalize(const json & messages) { + bool only_string = caps.supports_string_content && !caps.supports_typed_content; + bool only_typed = !caps.supports_string_content && caps.supports_typed_content; + if ((!only_string && !only_typed) || !messages.is_array()) { + return messages; + } + json normalized = json::array(); + for (const auto & msg : messages) { + json copy = msg; + auto it = copy.find("content"); + if (it != copy.end()) { + if (only_typed && it->is_string()) { + *it = json::array({ + json{ + {"type", "text"}, + {"text", it->get()}, + } + }); + } else if (only_string && it->is_array()) { + *it = concat_content_parts(*it); + } + } + normalized.push_back(std::move(copy)); + } + return normalized; + } + + // join parts with newline, do not add newline before or after media markers + static std::string concat_content_parts(const json & parts) { + std::string text; + bool last_was_media_marker = false; + for (const auto & part : parts) { + std::string type = part.value("type", ""); + bool add_new_line = true; + if (type == "text") { + add_new_line = !last_was_media_marker && !text.empty(); + last_was_media_marker = false; + } else if (type == "media_marker") { + add_new_line = false; + last_was_media_marker = true; + } else { + LOG_WRN("Ignoring content part type: %s\n", type.c_str()); + continue; + } + + if (add_new_line) { + text += '\n'; + } + + text += part.value("text", ""); + } + return text; + } +}; + static json render_message_to_json(const std::vector & msgs, const jinja::caps & c) { if (!c.supports_string_content && !c.supports_typed_content) { LOG_WRN("%s: Neither string content nor typed content is supported by the template. This is unexpected and may lead to issues.\n", __func__); } - bool only_string_accepted = c.supports_string_content && !c.supports_typed_content; - bool only_typed_accepted = !c.supports_string_content && c.supports_typed_content; - json messages = json::array(); for (const auto & msg : msgs) { - if (only_string_accepted) { - json jmsg = msg.to_json_oaicompat(/* concat_typed_text= */ true); - messages.push_back(jmsg); - } else if (only_typed_accepted) { - json jmsg = msg.to_json_oaicompat(/* concat_typed_text= */ false); - if (jmsg.at("content").is_string()) { - jmsg["content"] = json::array({ - json{ - {"type", "text"}, - {"text", jmsg.at("content").get()}, - } - }); - } - messages.push_back(jmsg); - } else { - json jmsg = msg.to_json_oaicompat(/* concat_typed_text= */ false); - messages.push_back(jmsg); - } + messages.push_back(msg.to_json_oaicompat(/* concat_typed_text= */ false)); } - return messages; + return messages_inp_normalizer(c).normalize(messages); } // DEPRECATED: only used in tests @@ -892,8 +936,11 @@ static std::string common_chat_template_direct_apply_impl( const std::optional & additional_context = std::nullopt) { jinja::context ctx(tmpl.source()); + // messages_override is already built for this template, do not touch its content parts nlohmann::ordered_json inp = nlohmann::ordered_json{ - {"messages", messages_override.has_value() ? *messages_override : inputs.messages}, + {"messages", messages_override.has_value() + ? *messages_override + : messages_inp_normalizer(tmpl.original_caps()).normalize(inputs.messages)}, {"bos_token", tmpl.bos_token()}, {"eos_token", tmpl.eos_token()}, {"enable_thinking", inputs.enable_thinking}, @@ -957,14 +1004,12 @@ static std::string common_chat_template_generation_prompt_impl( const std::optional & tools_override = std::nullopt, const std::optional & additional_context = std::nullopt) { - auto adjusted_messages = messages_override ? *messages_override : inputs.messages; - autoparser::generation_params params = inputs; params.add_generation_prompt = false; params.continue_final_message = COMMON_CHAT_CONTINUATION_NONE; - std::string no_gen_prompt = common_chat_template_direct_apply_impl(tmpl, params, adjusted_messages, tools_override, additional_context); + std::string no_gen_prompt = common_chat_template_direct_apply_impl(tmpl, params, messages_override, tools_override, additional_context); params.add_generation_prompt = true; - std::string gen_prompt = common_chat_template_direct_apply_impl(tmpl, params, adjusted_messages, tools_override, additional_context); + std::string gen_prompt = common_chat_template_direct_apply_impl(tmpl, params, messages_override, tools_override, additional_context); size_t prefix_len = 0; size_t min_size = std::min(no_gen_prompt.size(), gen_prompt.size()); diff --git a/tests/test-jinja.cpp b/tests/test-jinja.cpp index 316c67697f..1eb2a062b7 100644 --- a/tests/test-jinja.cpp +++ b/tests/test-jinja.cpp @@ -2083,7 +2083,7 @@ static void test_caps(testing & t) { }); t.test("typed content, raises on string", [](testing & t) { - // rendering with string content throws: 'selectattr' is not a String filter + // 'selectattr' is not a String filter, so it throws auto caps = get_caps( "{% for message in messages %}" "{% for content in message['content'] | selectattr('type', 'equalto', 'text') %}" @@ -2096,8 +2096,7 @@ static void test_caps(testing & t) { }); t.test("typed content, silently drops string", [](testing & t) { - // rendering with string content does not throw, but content[0]['text'] yields - // undefined for a string, so the message is silently dropped (MiniMax-M1 shape) + // no throw here, but content[0]['text'] is undefined for a string (MiniMax-M1 case) auto caps = get_caps( "{% for message in messages %}" "{{ message['content'][0]['text'] }}" diff --git a/tests/testing.h b/tests/testing.h index 5fb43a1948..891d78530a 100644 --- a/tests/testing.h +++ b/tests/testing.h @@ -23,7 +23,7 @@ struct testing { int exceptions = 0; int skipped = 0; - // set by skip(), consumed by the innermost enclosing test() + // set by skip(), read by the innermost test() bool skip_current = false; std::string skip_reason; @@ -136,7 +136,7 @@ struct testing { int before_failures = failures; int before_assertions = assertions; - // save/restore, so that a skipped subtest does not mark its parent as skipped + // do not let a skipped subtest also mark its parent as skipped bool outer_skip = skip_current; std::string outer_skip_reason = skip_reason; skip_current = false;