diff --git a/src/common/chat/chat_args.cpp b/src/common/chat/chat_args.cpp index 06f60ef2b..bd47e08a4 100644 --- a/src/common/chat/chat_args.cpp +++ b/src/common/chat/chat_args.cpp @@ -197,18 +197,96 @@ class relaxed_writer { } } + enum class quote_verdict { CLOSES, TEXT, UNKNOWN }; + + static bool is_key_char(char c) { + return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9') || c == '_' || c == '-' || + c == '.'; + } + + /// \brief Whether the text at i, after a plain " or `, goes on like a call + /// \note Code in a string is full of "x", and "x"} so a plain quote only + /// closes when a closer, the end, or a `,key:` / `,<|"|>` follows. + /// UNKNOWN: the text ran out before the answer. + quote_verdict plain_quote_closes(size_t i) const { + const quote_verdict out_of_text = done_ ? quote_verdict::CLOSES : quote_verdict::UNKNOWN; + auto ws = [&](size_t j) { + while (j < s_.size() && is_ws(s_[j])) j++; + return j; + }; + i = ws(i); + bool closers = false; + while (i < s_.size() && is_closer(s_[i])) { + closers = true; + i = ws(i + 1); + } + if (i >= s_.size()) { + return out_of_text; + } + if (closers) { + // after the call's own closer: another call, or a member of an outer level + static const std::string CALL = "call:"; + if (s_.compare(i, CALL.size(), CALL) == 0) { + return quote_verdict::CLOSES; + } + if (s_.size() - i < CALL.size() && CALL.compare(0, s_.size() - i, s_, i, s_.size() - i) == 0) { + return out_of_text; + } + } + if (s_[i] != ',') { + return quote_verdict::TEXT; + } + i = ws(i + 1); + if (i >= s_.size()) { + return out_of_text; + } + if (is_closer(s_[i])) { + return quote_verdict::CLOSES; + } + if (s_.compare(i, GEMMA4_QUOTE.size(), GEMMA4_QUOTE) == 0) { + return quote_verdict::CLOSES; + } + size_t key_end = i; + while (key_end < s_.size() && is_key_char(s_[key_end])) key_end++; + if (key_end == i) { + const size_t rem = s_.size() - i; + if (rem < GEMMA4_QUOTE.size() && GEMMA4_QUOTE.compare(0, rem, s_, i, rem) == 0) { + return out_of_text; + } + return quote_verdict::TEXT; + } + key_end = ws(key_end); + if (key_end >= s_.size()) { + return out_of_text; + } + return s_[key_end] == ':' ? quote_verdict::CLOSES : quote_verdict::TEXT; + } + + /// \brief How many real <|"|> markers the text holds from i on + size_t real_markers_from(size_t i) const { + size_t n = 0; + for (i = s_.find(GEMMA4_QUOTE, i); i != std::string::npos; i = s_.find(GEMMA4_QUOTE, i + GEMMA4_QUOTE.size())) { + n++; + } + return n; + } + /// \brief Read a Gemma4 <|"|> string after its opener into dst, JSON escaped /// \note Gemma4 strings are raw text, so a <|"|> (or the plain " or ` the /// model sometimes writes instead) only closes the string when the /// value ends right after it; otherwise it is part of the text. Until /// the next non-space character shows which, the candidate is held. + /// The real <|"|> ends the value when a `,` or a closer follows; a + /// plain quote needs more proof (see plain_quote_closes) and waits + /// for the end of the call, so a quote in code never ends the value. bool scan_gemma4_string(std::string& dst) { while (true) { if (starved()) { return false; } size_t len = 0; - if (s_.compare(pos_, GEMMA4_QUOTE.size(), GEMMA4_QUOTE) == 0) { + const bool real = s_.compare(pos_, GEMMA4_QUOTE.size(), GEMMA4_QUOTE) == 0; + if (real) { len = GEMMA4_QUOTE.size(); } else if (s_[pos_] == '"' || s_[pos_] == '`') { len = 1; @@ -217,11 +295,27 @@ class relaxed_writer { } if (len > 0) { size_t next = pos_ + len; - while (next < s_.size() && is_ws(s_[next])) next++; - if (next >= s_.size() && !done_) { + quote_verdict verdict; + if (real) { + while (next < s_.size() && is_ws(s_[next])) next++; + if (next >= s_.size()) { + verdict = done_ ? quote_verdict::CLOSES : quote_verdict::UNKNOWN; + } else { + verdict = (s_[next] == ',' || is_closer(s_[next])) ? quote_verdict::CLOSES : quote_verdict::TEXT; + } + } else { + verdict = plain_quote_closes(next); + if (verdict == quote_verdict::CLOSES) { + // the string was opened by a real <|"|>: the quote only closes it when the + // markers after it pair up among themselves, an odd one is this string's closer + verdict = !done_ ? quote_verdict::UNKNOWN + : (real_markers_from(next) % 2 == 0 ? quote_verdict::CLOSES : quote_verdict::TEXT); + } + } + if (verdict == quote_verdict::UNKNOWN) { return false; } - if (next >= s_.size() || s_[next] == ',' || is_closer(s_[next])) { + if (verdict == quote_verdict::CLOSES) { pos_ += len; return true; } diff --git a/src/test/chat_gemma4_args/CMakeLists.txt b/src/test/chat_gemma4_args/CMakeLists.txt new file mode 100644 index 000000000..20e8743fb --- /dev/null +++ b/src/test/chat_gemma4_args/CMakeLists.txt @@ -0,0 +1,25 @@ +# Gemma4 tool call arguments: quotes inside <|"|> strings (FastFlowLM#900), the plain-quote closers the reader +# still accepts, and streaming. No NPU, no XRT, so this builds and runs on Linux CI as well as Windows. +# cmake -S src/test/chat_gemma4_args -B build-g4a && cmake --build build-g4a && ctest --test-dir build-g4a +cmake_minimum_required(VERSION 3.22) +project(chat_gemma4_args_tests LANGUAGES CXX) + +set(CMAKE_CXX_STANDARD 20) +set(CMAKE_CXX_STANDARD_REQUIRED ON) + +set(FLM_SOURCE_DIR "${CMAKE_CURRENT_LIST_DIR}/../..") + +add_executable(test_chat_gemma4_args + test_chat_gemma4_args.cpp + "${FLM_SOURCE_DIR}/common/chat/chat_args.cpp" + "${FLM_SOURCE_DIR}/common/chat/chat_format.cpp" + "${FLM_SOURCE_DIR}/common/chat/chat_msg.cpp" + "${FLM_SOURCE_DIR}/common/chat/chat_parser.cpp" + "${FLM_SOURCE_DIR}/common/chat/chat_stream.cpp") +target_include_directories(test_chat_gemma4_args PRIVATE "${FLM_SOURCE_DIR}/include") +if(MSVC) + target_compile_options(test_chat_gemma4_args PRIVATE /utf-8 /bigobj) +endif() + +enable_testing() +add_test(NAME test_chat_gemma4_args COMMAND test_chat_gemma4_args) diff --git a/src/test/chat_gemma4_args/test_chat_gemma4_args.cpp b/src/test/chat_gemma4_args/test_chat_gemma4_args.cpp new file mode 100644 index 000000000..d03edc3a5 --- /dev/null +++ b/src/test/chat_gemma4_args/test_chat_gemma4_args.cpp @@ -0,0 +1,236 @@ +/// \file test_chat_gemma4_args.cpp +/// \brief Gemma4 tool call arguments (also Nemotron-3.5-Bolt, same template): a string is raw text between <|"|> +/// markers, so code with quotes in it (np.genfromtxt("a.csv", delimiter=",")) must come through whole +/// (FastFlowLM#900). Also the plain-quote closers the reader still accepts, and streaming. No NPU needed. +#include "chat/chat_parser.hpp" + +#include +#include +#include +#include + +#include + +namespace { +int failures = 0; +#define CHECK(cond) \ + do { \ + if (!(cond)) { \ + std::printf(" FAIL %s:%d: %s\n", __FILE__, __LINE__, #cond); \ + failures++; \ + } \ + } while (0) + +using nlohmann::json; + +const std::string Q = "<|\"|>"; + +chat_msg parse(const std::string& text, bool partial = false) { + static const chat_format_spec spec = chat_format_make(chat_format_kind::GEMMA4, false); + return chat_parse(spec, text, partial, chat_parse_options{}); +} + +std::string call(const std::string& args) { + return "<|tool_call>call:file_write{" + args + "}"; +} + +/// \brief the arguments of the only call in text, discarded when the parse is not exactly one call +json one_call_args(const std::string& text) { + chat_msg m = parse(text); + if (m.tool_calls.size() != 1) { + return json::value_t::discarded; + } + return json::parse(m.tool_calls[0].arguments, nullptr, false); +} + +void test_quotes_in_code() { + std::printf("quotes in code\n"); + // the report: a quote followed by a comma or a brace inside the value + const std::string np = "import numpy as np\ndata = np.genfromtxt(\"train.csv\", delimiter=\",\")\nprint(data)"; + CHECK(one_call_args(call("content:" + Q + np + Q + ",path:" + Q + "a.py" + Q)) == + (json{{"content", np}, {"path", "a.py"}})); + const std::string d = "d = {\"a\": 1}\nx = f(\"q\",\"r\")"; + CHECK(one_call_args(call("content:" + Q + d + Q + ",path:" + Q + "a.py" + Q)) == + (json{{"content", d}, {"path", "a.py"}})); + // a quote before a closer, a quote before what looks like the next key + const std::string e = "d = {\"a\": \"b\"}"; + CHECK(one_call_args(call("content:" + Q + e + Q + ",path:" + Q + "a.py" + Q)) == + (json{{"content", e}, {"path", "a.py"}})); + const std::string js = "const o = {name: \"x\", age: 3};"; + CHECK(one_call_args(call("content:" + Q + js + Q + ",path:" + Q + "a.js" + Q)) == + (json{{"content", js}, {"path", "a.js"}})); + const std::string ticks = "echo `date`, now: `x`"; + CHECK(one_call_args(call("content:" + Q + ticks + Q)) == (json{{"content", ticks}})); + // a closer right after the quote, last argument + const std::string last = "print(\"a\", \"b\")"; + CHECK(one_call_args(call("path:" + Q + "a.py" + Q + ",content:" + Q + last + Q)) == + (json{{"content", last}, {"path", "a.py"}})); + // quotes in nested values and arrays + CHECK(one_call_args(call("o:{s:" + Q + "f(\"a\",\"b\")" + Q + ",n:5},l:[" + Q + "x\",\"y" + Q + "," + Q + "z" + Q + "]")) == + (json{{"o", {{"s", "f(\"a\",\"b\")"}, {"n", 5}}}, {"l", {"x\",\"y", "z"}}})); + // the single quote case from the report still works + CHECK(one_call_args(call("content:" + Q + "d = {'a': 1}" + Q)) == (json{{"content", "d = {'a': 1}"}})); + // two calls in one turn, the first with quotes + chat_msg two = parse(call("content:" + Q + "f(\"a\",\"b\")" + Q) + call("path:" + Q + "p" + Q)); + CHECK(two.tool_calls.size() == 2); + if (two.tool_calls.size() == 2) { + CHECK(json::parse(two.tool_calls[0].arguments) == (json{{"content", "f(\"a\",\"b\")"}})); + CHECK(json::parse(two.tool_calls[1].arguments) == (json{{"path", "p"}})); + } +} + +void test_plain_quote_closers() { + std::printf("plain quote closers\n"); + // the model sometimes closes with a plain quote or a backtick instead of <|"|> + CHECK(one_call_args(call("content:" + Q + "hello\",path:" + Q + "a.py" + Q)) == + (json{{"content", "hello"}, {"path", "a.py"}})); + CHECK(one_call_args(call("content:" + Q + "hello`,path:" + Q + "a.py" + Q)) == + (json{{"content", "hello"}, {"path", "a.py"}})); + CHECK(one_call_args(call("path:" + Q + "a.py" + Q + ",content:" + Q + "hello\"")) == + (json{{"content", "hello"}, {"path", "a.py"}})); + CHECK(one_call_args(call("l:[" + Q + "a\"," + Q + "b" + Q + "]")) == (json{{"l", {"a", "b"}}})); + // an opener the model left out + CHECK(one_call_args(call("content:hello" + Q + ",n:3")) == (json{{"content", "hello"}, {"n", 3}})); + // a call cut off inside a string still gives valid JSON + chat_msg cut = parse("<|tool_call>call:f{a:" + Q + "x\"y", false); + CHECK(cut.tool_calls.size() == 1 && !json::parse(cut.tool_calls[0].arguments, nullptr, false).is_discarded()); +} + +void test_streaming() { + std::printf("streaming\n"); + const std::string text = call("content:" + Q + "f(\"a\",\"b\"), c: \"d\"" + Q + ",path:" + Q + "a.py" + Q); + chat_msg full = parse(text); + CHECK(full.tool_calls.size() == 1); + std::string prev; + for (size_t cut = 0; cut <= text.size(); cut++) { + chat_msg p = parse(text.substr(0, cut), cut < text.size()); + if (p.tool_calls.empty()) { + continue; + } + const std::string& a = p.tool_calls[0].arguments; + // a partial parse only ever yields a prefix of the full one + CHECK(a.size() <= full.tool_calls[0].arguments.size() && + full.tool_calls[0].arguments.compare(0, a.size(), a) == 0); + CHECK(a.size() >= prev.size() && a.compare(0, prev.size(), prev) == 0); + prev = a; + } +} + +// ——————————————————————————————————————————————— +// random calls written the way the Gemma4 template writes them, with values full of quotes, commas and braces +// ——————————————————————————————————————————————— +std::mt19937_64 rng(900); +int rnd(int n) { + return static_cast(rng() % n); +} + +const char* const FRAGS[] = { + "\"", "\",", "\",\"", "\"}", "\"]", "\"},", "\"}}", "\"})", ", key:", "\", key:", "\",k:", "\", name: \"x\"", + "{a:\"b\", c:1}", "{\"a\": \"b\", \"c\": 1}", "f(\"q\",\"r\")", "np.genfromtxt(\"train.csv\", delimiter=\",\")", + "print(\"hi\")", "x = [\"a\", \"b\"]", "`", "``", "`x`, y:", "'", "'a', 'b'", "\\", "\\n", "\\\"", "\n", "\t", " ", + " ", "{", "}", "[", "]", "(", ")", ",", ":", ", ", " ,", "<", "<|", "|>", " 1 ? 6 : 8)) { + case 0: case 1: case 2: case 5: return rand_string(); + case 3: return rnd(1000) - 300; + case 4: return rnd(2) == 0; + case 6: { + json o = json::object(); + for (int i = 0, n = rnd(3); i < n; i++) o["k" + std::to_string(i)] = rand_value(depth + 1); + return o; + } + default: { + json a = json::array(); + for (int i = 0, n = rnd(4); i < n; i++) a.push_back(rand_value(depth + 1)); + return a; + } + } +} + +std::string encode(const json& v) { + if (v.is_string()) return Q + v.get() + Q; + if (v.is_boolean()) return v.get() ? "true" : "false"; + if (v.is_number_integer()) return std::to_string(v.get()); + std::string o; + bool first = true; + if (v.is_object()) { + for (auto& [k, x] : v.items()) { + o += (first ? "" : ",") + k + ":" + encode(x); + first = false; + } + return "{" + o + "}"; + } + for (auto& x : v) { + o += (first ? "" : ",") + encode(x); + first = false; + } + return "[" + o + "]"; +} + +void test_random_calls() { + std::printf("random calls\n"); + int bad = 0; + for (int it = 0; it < 3000 && bad < 3; it++) { + std::string text; + std::vector expect; + for (int c = 0, nc = 1 + rnd(2); c < nc; c++) { + json args = json::object(); + for (int i = 0, n = rnd(4); i < n; i++) args["a" + std::to_string(i)] = rand_value(0); + text += "<|tool_call>call:f" + encode(args) + ""; + expect.push_back(args); + } + chat_msg m = parse(text); + bool ok = m.tool_calls.size() == expect.size(); + for (size_t i = 0; ok && i < expect.size(); i++) { + ok = json::parse(m.tool_calls[i].arguments, nullptr, false) == expect[i]; + } + // streaming: every prefix parses to a prefix of the full result; a cut-off call is still valid JSON + for (size_t cut = 0; ok && cut <= text.size(); cut++) { + if (cut < text.size() && (text[cut] & 0xC0) == 0x80) { + continue; // not inside a UTF-8 character + } + chat_msg p = parse(text.substr(0, cut), true); + ok = p.tool_calls.size() <= m.tool_calls.size(); + for (size_t i = 0; ok && i < p.tool_calls.size(); i++) { + const std::string& a = p.tool_calls[i].arguments; + ok = m.tool_calls[i].arguments.compare(0, a.size(), a) == 0; + } + for (auto& tc : parse(text.substr(0, cut), false).tool_calls) { + ok = ok && !json::parse(tc.arguments, nullptr, false).is_discarded(); + } + } + if (!ok) { + bad++; + std::printf(" mismatch on: %s\n", text.c_str()); + } + } + CHECK(bad == 0); +} +} // namespace + +int main() { + test_quotes_in_code(); + test_plain_quote_closers(); + test_streaming(); + test_random_calls(); + if (failures > 0) { + std::printf("%d check(s) failed\n", failures); + return 1; + } + std::printf("all passed\n"); + return 0; +}