From 2a31c9a30cf984f39a3ba71e66f3efee1bc59aa7 Mon Sep 17 00:00:00 2001 From: Xuan Son Nguyen Date: Mon, 29 Dec 2025 00:38:29 +0100 Subject: [PATCH] a lot of fixes --- common/jinja/jinja-value.cpp | 134 ++++++++++++++++++++++++++++++++++- common/jinja/jinja-value.h | 12 ++-- common/jinja/jinja-vm.cpp | 80 +++++++++++++++------ tests/test-chat-jinja.cpp | 7 +- 4 files changed, 202 insertions(+), 31 deletions(-) diff --git a/common/jinja/jinja-value.cpp b/common/jinja/jinja-value.cpp index 9461901c6d..f382a64a86 100644 --- a/common/jinja/jinja-value.cpp +++ b/common/jinja/jinja-value.cpp @@ -127,6 +127,44 @@ const func_builtins & global_builtins() { throw raised_exception("strftime_now: failed to format time"); } }}, + {"range", [](const func_args & args) -> value { + if (args.args.size() < 1 || args.args.size() > 3) { + throw raised_exception("slice() takes between 1 and 3 arguments"); + } + int64_t arg0 = is_val(args.args[0]) ? args.args[0]->as_int() : 0; + int64_t arg1 = is_val(args.args[1]) ? args.args[1]->as_int() : -1; + int64_t arg2 = is_val(args.args[2]) ? args.args[2]->as_int() : 1; + + int64_t start, stop, step; + if (args.args.size() == 1) { + start = 0; + stop = arg0; + step = 1; + } else if (args.args.size() == 2) { + start = arg0; + stop = arg1; + step = 1; + } else { + start = arg0; + stop = arg1; + step = arg2; + } + + auto out = mk_val(); + if (step == 0) { + throw raised_exception("range() step argument must not be zero"); + } + if (step > 0) { + for (int64_t i = start; i < stop; i += step) { + out->push_back(mk_val(i)); + } + } else { + for (int64_t i = start; i > stop; i += step) { + out->push_back(mk_val(i)); + } + } + return out; + }}, // tests {"test_is_boolean", test_type_fn}, @@ -416,7 +454,9 @@ const func_builtins & value_array_t::get_builtins() const { return mk_val(static_cast(arr.size())); }}, {"slice", [](const func_args & args) -> value { - args.ensure_count(4); + if (args.args.size() < 1 || args.args.size() > 4) { + throw raised_exception("slice() takes between 1 and 4 arguments"); + } int64_t start = is_val(args.args[1]) ? args.args[1]->as_int() : 0; int64_t stop = is_val(args.args[2]) ? args.args[2]->as_int() : -1; int64_t step = is_val(args.args[3]) ? args.args[3]->as_int() : 1; @@ -465,7 +505,77 @@ const func_builtins & value_array_t::get_builtins() const { } return result; }}, - // TODO: reverse, sort, join, string, unique + {"rejectattr", [](const func_args & args) -> value { + value input = args.args[0]; + if (!is_val(input)) { + throw raised_exception("rejectattr() first argument must be an array, got " + input->type()); + } + std::vector rejected; + for (size_t i = 1; i < args.args.size(); ++i) { + const auto & v = args.args[i]; + if (!is_val(v)) { + throw raised_exception("rejectattr() attributes must be strings, got " + v->type()); + } + JJ_DEBUG("rejectattr: rejecting attribute '%s'", v->as_string().str().c_str()); + rejected.push_back(v->as_string().str()); + } + auto result = mk_val(); + for (const auto & item : input->as_array()) { + if (!is_val(item)) { + result->push_back(item); + continue; + } + const auto & obj = item->as_object(); + bool match = false; + for (const auto & attr : rejected) { + auto it = obj.find(attr); + if (it != obj.end() && !it->second->is_undefined() && (!is_val(it->second) || it->second->as_bool())) { + match = true; + break; + } + } + if (!match) { + result->push_back(item); + } + } + return result; + }}, + {"join", [](const func_args & args) -> value { + if (args.args.size() < 1 || args.args.size() > 2) { + throw raised_exception("join() takes one or two arguments"); + } + if (!is_val(args.args[0])) { + throw raised_exception("join() first argument must be an array"); + } + const auto & arr = args.args[0]->as_array(); + std::string delim = (args.args.size() > 1 && is_val(args.args[1])) ? args.args[1]->as_string().str() : ""; + std::string result; + for (size_t i = 0; i < arr.size(); ++i) { + if (!is_val(arr[i])) { + throw raised_exception("join() can only join arrays of strings"); + } + result += arr[i]->as_string().str(); + if (i < arr.size() - 1) { + result += delim; + } + } + return mk_val(result); + }}, + {"string", [](const func_args & args) -> value { + args.ensure_vals(); + auto str = mk_val(); + gather_string_parts_recursive(args.args[0], str); + return str; + }}, + {"sort", [](const func_args &) -> value { + throw std::runtime_error("Array sort builtin not implemented"); + }}, + {"reverse", [](const func_args &) -> value { + throw std::runtime_error("Array reverse builtin not implemented"); + }}, + {"unique", [](const func_args &) -> value { + throw std::runtime_error("Array unique builtin not implemented"); + }}, }; return builtins; } @@ -523,6 +633,26 @@ const func_builtins & value_object_t::get_builtins() const { return builtins; } +const func_builtins & value_null_t::get_builtins() const { + static const func_builtins builtins = { + {"list", [](const func_args &) -> value { + // fix for meetkai-functionary-medium-v3.1.jinja + // TODO: hide under a flag? + return mk_val(); + }}, + {"selectattr", [](const func_args &) -> value { + // fix for meetkai-functionary-medium-v3.1.jinja + // TODO: hide under a flag? + return mk_val(); + }}, + }; + return builtins; +} + + +////////////////////////////////// + + static value from_json(const nlohmann::json & j) { if (j.is_null()) { return mk_val(); diff --git a/common/jinja/jinja-value.h b/common/jinja/jinja-value.h index 04c6c6da28..3289a0de59 100644 --- a/common/jinja/jinja-value.h +++ b/common/jinja/jinja-value.h @@ -96,13 +96,14 @@ struct func_args { std::vector args; context & ctx; func_args(context & ctx) : ctx(ctx) {} - void ensure_count(size_t count) const { - if (args.size() != count) { - throw std::runtime_error("Expected " + std::to_string(count) + " arguments, got " + std::to_string(args.size())); + void ensure_count(size_t min, size_t max = 999) const { + if (args.size() < min || args.size() > max) { + throw std::runtime_error("Expected between " + std::to_string(min) + " and " + std::to_string(max) + " arguments, got " + std::to_string(args.size())); } } value get_kwarg(const std::string & key) const; // utility functions + // TODO: allow optional arguments template void ensure_vals() const { ensure_count(1); ensure_val(args[0]); @@ -310,12 +311,15 @@ struct value_null_t : public value_t { virtual bool is_null() const override { return true; } virtual bool as_bool() const override { return false; } virtual std::string as_repr() const override { return type(); } + virtual const func_builtins & get_builtins() const override; }; using value_null = std::shared_ptr; struct value_undefined_t : public value_t { - virtual std::string type() const override { return "Undefined"; } + std::string hint; // for debugging, to indicate where undefined came from + value_undefined_t(const std::string & h = "") : hint(h) {} + virtual std::string type() const override { return hint.empty() ? "Undefined" : "Undefined (hint: '" + hint + "')"; } virtual bool is_undefined() const override { return true; } virtual bool as_bool() const override { return false; } virtual std::string as_repr() const override { return type(); } diff --git a/common/jinja/jinja-vm.cpp b/common/jinja/jinja-vm.cpp index 4c38ebde7d..0211ef9013 100644 --- a/common/jinja/jinja-vm.cpp +++ b/common/jinja/jinja-vm.cpp @@ -19,13 +19,16 @@ void enable_debug(bool enable) { g_jinja_debug = enable; } -static value_array exec_statements(const statements & stmts, context & ctx) { +static value_string exec_statements(const statements & stmts, context & ctx) { auto result = mk_val(); for (const auto & stmt : stmts) { JJ_DEBUG("Executing statement of type %s", stmt->type().c_str()); result->push_back(stmt->execute(ctx)); } - return result; + // convert to string parts + value_string str = mk_val(); + gather_string_parts_recursive(result, str); + return str; } // execute with error handling @@ -66,7 +69,7 @@ value identifier::execute_impl(context & ctx) { return mk_val(builtins.at(val), val); } else { JJ_DEBUG("Identifier '%s' not found, returning undefined", val.c_str()); - return mk_val(); + return mk_val(val); } } @@ -83,7 +86,6 @@ value object_literal::execute_impl(context & ctx) { value binary_expression::execute_impl(context & ctx) { value left_val = left->execute(ctx); - JJ_DEBUG("Executing binary expression %s '%s' %s", left_val->type().c_str(), op.value.c_str(), right->type().c_str()); // Logical operators if (op.value == "and") { @@ -94,6 +96,7 @@ value binary_expression::execute_impl(context & ctx) { // Equality operators value right_val = right->execute(ctx); + JJ_DEBUG("Executing binary expression %s '%s' %s", left_val->type().c_str(), op.value.c_str(), right_val->type().c_str()); if (op.value == "==") { return mk_val(value_compare(left_val, right_val)); } else if (op.value == "!=") { @@ -168,10 +171,18 @@ value binary_expression::execute_impl(context & ctx) { } } else if (is_val(right_val)) { auto & arr = right_val->as_array(); - bool member = std::find_if(arr.begin(), arr.end(), [&](const value& v) { return v == left_val; }) != arr.end(); + bool member = false; + for (const auto & item : arr) { + if (value_compare(left_val, item)) { + member = true; + break; + } + } if (op.value == "in") { + JJ_DEBUG("Checking membership: %s in Array is %d", left_val->type().c_str(), member); return mk_val(member); } else if (op.value == "not in") { + JJ_DEBUG("Checking non-membership: %s not in Array is %d", left_val->type().c_str(), !member); return mk_val(!member); } } @@ -220,7 +231,7 @@ static value try_builtin_func(const std::string & name, const value & input, boo return mk_val(it->second, input, name); } if (undef_on_missing) { - return mk_val(); + return mk_val(name); } throw std::runtime_error("Unknown (built-in) filter '" + name + "' for type " + input->type()); } @@ -330,7 +341,10 @@ value if_statement::execute_impl(context & ctx) { out->push_back(stmt->execute(ctx)); } } - return out; + // convert to string parts + value_string str = mk_val(); + gather_string_parts_recursive(out, str); + return str; } value for_statement::execute_impl(context & ctx) { @@ -437,8 +451,8 @@ value for_statement::execute_impl(context & ctx) { loop_obj->insert("first", mk_val(i == 0)); loop_obj->insert("last", mk_val(i == filtered_items.size() - 1)); loop_obj->insert("length", mk_val(filtered_items.size())); - loop_obj->insert("previtem", i > 0 ? filtered_items[i - 1] : mk_val()); - loop_obj->insert("nextitem", i < filtered_items.size() - 1 ? filtered_items[i + 1] : mk_val()); + loop_obj->insert("previtem", i > 0 ? filtered_items[i - 1] : mk_val("previtem")); + loop_obj->insert("nextitem", i < filtered_items.size() - 1 ? filtered_items[i + 1] : mk_val("nextitem")); ctx.var["loop"] = loop_obj; scope_update_fns[i](ctx); try { @@ -460,7 +474,10 @@ value for_statement::execute_impl(context & ctx) { } } - return result; + // convert to string parts + value_string str = mk_val(); + gather_string_parts_recursive(result, str); + return str; } value set_statement::execute_impl(context & ctx) { @@ -515,24 +532,41 @@ value set_statement::execute_impl(context & ctx) { value macro_statement::execute_impl(context & ctx) { std::string name = cast_stmt(this->name)->val; - const func_handler func = [this, &ctx, name](const func_args & args) -> value { - JJ_DEBUG("Invoking macro '%s' with %zu arguments", name.c_str(), args.args.size()); + + const func_handler func = [this, name, &ctx](const func_args & args) -> value { + size_t expected_count = this->args.size(); + size_t input_count = args.args.size(); + + JJ_DEBUG("Invoking macro '%s' with %zu input arguments (expected %zu)", name.c_str(), input_count, expected_count); context macro_ctx(ctx); // new scope for macro execution // bind parameters - size_t param_count = this->args.size(); - size_t arg_count = args.args.size(); - for (size_t i = 0; i < param_count; ++i) { - std::string param_name = cast_stmt(this->args[i])->val; - if (i < arg_count) { + for (size_t i = 0; i < expected_count; ++i) { + if (i < input_count) { + std::string param_name = cast_stmt(this->args[i])->val; + JJ_DEBUG(" Binding parameter '%s' to argument of type %s", param_name.c_str(), args.args[i]->type().c_str()); macro_ctx.var[param_name] = args.args[i]; } else { - macro_ctx.var[param_name] = mk_val(); + auto & default_arg = this->args[i]; + if (is_stmt(default_arg)) { + auto kwarg = cast_stmt(default_arg); + std::string param_name = cast_stmt(kwarg->key)->val; + JJ_DEBUG(" Binding parameter '%s' to default argument of type %s", param_name.c_str(), kwarg->val->type().c_str()); + macro_ctx.var[param_name] = kwarg->val->execute(ctx); + } else { + throw std::runtime_error("Not enough arguments provided to macro '" + name + "'"); + } + //std::string param_name = cast_stmt(default_args[i])->val; + //JJ_DEBUG(" Binding parameter '%s' to default", param_name.c_str()); + //macro_ctx.var[param_name] = default_args[i]->execute(ctx); } } // execute macro body - return exec_statements(this->body, macro_ctx); + JJ_DEBUG("Executing macro '%s' body with %zu statements", name.c_str(), this->body.size()); + auto res = exec_statements(this->body, macro_ctx); + JJ_DEBUG("Macro '%s' execution complete, result: %s", name.c_str(), res->val_str.str().c_str()); + return res; }; JJ_DEBUG("Defining macro '%s' with %zu parameters", name.c_str(), args.size()); @@ -548,9 +582,9 @@ value member_expression::execute_impl(context & ctx) { JJ_DEBUG("Member expression, computing property type %s", this->property->type().c_str()); if (is_stmt(this->property)) { auto s = cast_stmt(this->property); - value start_val = s->start_expr ? s->start_expr->execute(ctx) : mk_val(); - value stop_val = s->stop_expr ? s->stop_expr->execute(ctx) : mk_val(); - value step_val = s->step_expr ? s->step_expr->execute(ctx) : mk_val(); + value start_val = s->start_expr ? s->start_expr->execute(ctx) : mk_val("start"); + value stop_val = s->stop_expr ? s->stop_expr->execute(ctx) : mk_val("stop"); + value step_val = s->step_expr ? s->step_expr->execute(ctx) : mk_val("step"); // translate to function call: obj.slice(start, stop, step) JJ_DEBUG("Member expression is a slice: start %s, stop %s, step %s", @@ -572,7 +606,7 @@ value member_expression::execute_impl(context & ctx) { JJ_DEBUG("Member expression on object type %s, property type %s", object->type().c_str(), property->type().c_str()); - value val = mk_val(); + value val = mk_val("object_property"); if (is_val(object)) { JJ_DEBUG("%s", "Accessing property on undefined object, returning undefined"); diff --git a/tests/test-chat-jinja.cpp b/tests/test-chat-jinja.cpp index 997d463061..72f3ee9822 100644 --- a/tests/test-chat-jinja.cpp +++ b/tests/test-chat-jinja.cpp @@ -58,7 +58,7 @@ int main(void) { void run(std::string contents) { - // jinja::enable_debug(true); + jinja::enable_debug(true); jinja::lexer lexer; jinja::preprocess_options options; @@ -90,7 +90,10 @@ void run(std::string contents) { "content": {"__input__": "I am fine, thank you!"} } ], - "eos_token": "" + "bos_token": "", + "eos_token": "", + "functions": "", + "datetime": "" })"; jinja::global_from_json(ctx, nlohmann::json::parse(json_inp));