From 9c0fa6f81001e14b1d2224da7f9b6094c8520845 Mon Sep 17 00:00:00 2001 From: Xuan Son Nguyen Date: Tue, 30 Dec 2025 16:07:23 +0100 Subject: [PATCH] rm workarounds --- common/jinja/jinja-value.cpp | 32 ++++++-------------------------- common/jinja/jinja-value.h | 6 ------ common/jinja/jinja-vm.cpp | 18 +++++++++--------- common/jinja/jinja-vm.h | 3 --- common/jinja/jinja-workaround.h | 24 ------------------------ tests/test-chat-jinja.cpp | 27 +++++++++++++++------------ 6 files changed, 30 insertions(+), 80 deletions(-) delete mode 100644 common/jinja/jinja-workaround.h diff --git a/common/jinja/jinja-value.cpp b/common/jinja/jinja-value.cpp index 688f6cdb0f..2c9ce6c76c 100644 --- a/common/jinja/jinja-value.cpp +++ b/common/jinja/jinja-value.cpp @@ -404,16 +404,11 @@ const func_builtins & value_string_t::get_builtins() const { res->val_str.mark_input_based_on(input->as_string()); return res; }}, - {"selectattr", [](const func_args & args) -> value { - if (args.ctx.wrk_around.string_has_selectattr) { - // no-op, return an array containing the original string - args.ensure_vals(); - auto result = mk_val(); - result->push_back(args.args[0]); - return result; - } else { - throw raised_exception("String selectattr builtin not supported"); - } + {"selectattr", [](const func_args &) -> value { + throw std::runtime_error("String selectattr builtin not supported"); + }}, + {"rejectattr", [](const func_args &) -> value { + throw std::runtime_error("String rejectattr builtin not supported"); }}, {"indent", [](const func_args &) -> value { throw std::runtime_error("String indent builtin not implemented"); @@ -662,22 +657,7 @@ const func_builtins & value_object_t::get_builtins() const { const func_builtins & value_null_t::get_builtins() const { static const func_builtins builtins = { - {"list", [](const func_args & args) -> value { - // fix for meetkai-functionary-medium-v3.1.jinja - if (args.ctx.wrk_around.none_has_builtins) { - return mk_val(); - } else { - throw raised_exception("'list' builtin not supported for none type"); - } - }}, - {"selectattr", [](const func_args & args) -> value { - // fix for meetkai-functionary-medium-v3.1.jinja - if (args.ctx.wrk_around.none_has_builtins) { - return mk_val(); - } else { - throw raised_exception("'selectattr' builtin not supported for none type"); - } - }}, + // TODO: may need to implement this, idk }; return builtins; } diff --git a/common/jinja/jinja-value.h b/common/jinja/jinja-value.h index 3289a0de59..7c7d98d932 100644 --- a/common/jinja/jinja-value.h +++ b/common/jinja/jinja-value.h @@ -132,12 +132,6 @@ struct value_t { string val_str; bool val_bool; - // array and object are stored as shared_ptr to allow reference access - // example: - // my_obj = {"a": 1, "b": 2} - // my_arr = [my_obj] - // my_obj["a"] = 3 - // print(my_arr[0]["a"]) # should print 3 std::vector val_arr; std::map val_obj; diff --git a/common/jinja/jinja-vm.cpp b/common/jinja/jinja-vm.cpp index 8797b866f4..b99fc605f0 100644 --- a/common/jinja/jinja-vm.cpp +++ b/common/jinja/jinja-vm.cpp @@ -113,15 +113,15 @@ value binary_expression::execute_impl(context & ctx) { // Special case: `anything in undefined` is `false` and `anything not in undefined` is `true` return mk_val(op.value == "not in"); } - if (ctx.wrk_around.string_plus_undefined_is_string && (op.value == "+" || op.value == "~")) { - JJ_DEBUG("%s", "Workaround: treating undefined as empty string for string concatenation"); - auto left_str = left_val->is_undefined() ? string() : left_val->as_string(); - auto right_str = right_val->is_undefined() ? string() : right_val->as_string(); - auto output = left_str.append(right_str); - auto res = mk_val(); - res->val_str = std::move(output); - return res; - } + // if (ctx.wrk_around.string_plus_undefined_is_string && (op.value == "+" || op.value == "~")) { + // JJ_DEBUG("%s", "Workaround: treating undefined as empty string for string concatenation"); + // auto left_str = left_val->is_undefined() ? string() : left_val->as_string(); + // auto right_str = right_val->is_undefined() ? string() : right_val->as_string(); + // auto output = left_str.append(right_str); + // auto res = mk_val(); + // res->val_str = std::move(output); + // return res; + // } throw std::runtime_error("Cannot perform operation " + op.value + " on undefined values"); } else if (is_val(left_val) || is_val(right_val)) { throw std::runtime_error("Cannot perform operation on null values"); diff --git a/common/jinja/jinja-vm.h b/common/jinja/jinja-vm.h index 1526a365a1..1095d71870 100644 --- a/common/jinja/jinja-vm.h +++ b/common/jinja/jinja-vm.h @@ -2,7 +2,6 @@ #include "jinja-lexer.h" #include "jinja-value.h" -#include "jinja-workaround.h" #include #include @@ -53,8 +52,6 @@ struct context { std::time_t current_time; // for functions that need current time - workarounds wrk_around; // workarounds for non-standard jinja behavior - context() { var["true"] = mk_val(true); var["false"] = mk_val(false); diff --git a/common/jinja/jinja-workaround.h b/common/jinja/jinja-workaround.h deleted file mode 100644 index ed7e92df45..0000000000 --- a/common/jinja/jinja-workaround.h +++ /dev/null @@ -1,24 +0,0 @@ -#pragma once - -#include "jinja-value.h" - -#include -#include - -namespace jinja { - -// containing workarounds for Jinja templates that rely on non-standard behavior -// NOTE: this is kept as a dedicated file for better documentation - -struct workarounds { - // meetkai-functionary-medium-v3.1.jinja call filter on None type - bool none_has_builtins = true; - - // Olmo calls operation + between string and undefined - bool string_plus_undefined_is_string = true; - - // sheldonrobinson-Llama-Guard call selectattr on string - bool string_has_selectattr = true; -}; - -} // namespace jinja diff --git a/tests/test-chat-jinja.cpp b/tests/test-chat-jinja.cpp index 0e2f5e4faa..0ab18c0f4f 100644 --- a/tests/test-chat-jinja.cpp +++ b/tests/test-chat-jinja.cpp @@ -14,7 +14,8 @@ #include "jinja/jinja-parser.h" #include "jinja/jinja-lexer.h" -void run(std::string contents); +void run_multiple(); +void run_single(std::string contents); int main(void) { //std::string contents = "{% if messages[0]['role'] == 'system' %}{{ raise_exception('System role not supported') }}{% endif %}{% for message in messages %}{% if (message['role'] == 'user') != (loop.index0 % 2 == 0) %}{{ raise_exception('Conversation roles must alternate user/assistant/user/assistant/...') }}{% endif %}{% if (message['role'] == 'assistant') %}{% set role = 'model' %}{% else %}{% set role = message['role'] %}{% endif %}{{ '' + role + '\\n' + message['content'] | trim + '\\n' }}{% endfor %}{% if add_generation_prompt %}{{'model\\n'}}{% endif %}"; @@ -24,8 +25,16 @@ int main(void) { //std::string contents = " {{ messages[a]['content'] }} "; //std::string contents = "{% if a is not defined %}hello{% endif %}"; - //std::ifstream infile("models/templates/mistralai-Ministral-3-14B-Reasoning-2512.jinja"); std::string contents((std::istreambuf_iterator(infile)), std::istreambuf_iterator()); + std::ifstream infile("models/templates/Qwen-Qwen3-0.6B.jinja"); std::string contents((std::istreambuf_iterator(infile)), std::istreambuf_iterator()); + run_single(contents); + + //run_multiple(); + + return 0; +} + +void run_multiple(void) { std::vector failed_tests; bool stop_on_first_failure = false; @@ -65,7 +74,7 @@ int main(void) { std::ifstream infile(entry.path()); std::string contents((std::istreambuf_iterator(infile)), std::istreambuf_iterator()); try { - run(contents); + run_single(contents); } catch (const std::exception & e) { std::cout << "Exception: " << e.what() << "\n"; std::cout << "=== ERROR WITH TEMPLATE FILE: " << entry.path().string() << " ===\n"; @@ -84,27 +93,21 @@ int main(void) { for (const auto & test : failed_tests) { std::cout << "FAILED TEST: " << test << "\n"; } - return 0; } -void run(std::string contents) { +void run_single(std::string contents) { jinja::enable_debug(true); + // lexing jinja::lexer lexer; jinja::preprocess_options options; options.trim_blocks = false; options.lstrip_blocks = false; auto lexer_res = lexer.tokenize(contents, options); - for (const auto & tok : lexer_res.tokens) { - //std::cout << "token: type=" << static_cast(tok.t) << " text='" << tok.value << "' pos=" << tok.pos << "\n"; - } - std::cout << "\n=== AST ===\n"; + // compile to AST jinja::program ast = jinja::parse_from_tokens(lexer_res); - for (const auto & stmt : ast.body) { - //std::cout << "stmt type: " << stmt->type() << "\n"; - } std::cout << "\n=== RUN ===\n"; jinja::context ctx;