Fix grammar and partial parsing

Author: pwilkin

common/chat.cpp

@@ -2096,7 +2096,7 @@ static void common_chat_parse_seed_oss(common_chat_msg_parser & builder) {
     static const common_regex tool_call_begin_regex("<seed:tool_call>");
     static const common_regex tool_call_end_regex("</seed:tool_call>");
     static const common_regex function_regex("<function=([^>]+)>");
-    static const common_regex close_function_regex("</function>");
+    static const common_regex param_regex("<parameter=([^>]+)>");
 
     while (auto tool_res = builder.try_find_regex(tool_call_begin_regex)) {
         builder.consume_spaces();  // Consume whitespace after <seed:tool_call>
@@ -2107,7 +2107,6 @@ static void common_chat_parse_seed_oss(common_chat_msg_parser & builder) {
 
             // Parse Seed-OSS parameters <parameter=name>value</parameter>
             json args = json::object();
-            static const common_regex param_regex("<parameter=([^>]+)>");
             // Parse all parameters
             while (auto param_res = builder.try_find_regex(param_regex, std::string::npos, false)) {
                 // again, ignore noise around parameters
@@ -2128,40 +2127,28 @@ static void common_chat_parse_seed_oss(common_chat_msg_parser & builder) {
                         args[param_name] = param;
                     }
                 } else {
-                    try {
-                        if (auto param_res = builder.try_consume_json()) {
-                            auto json_obj = param_res->json;
-                            purge_healing_marker(json_obj, builder.healing_marker());
-                            if (!json_obj.is_null()) {
-                                args[param_name] = json_obj;
-                            }
-                        } else {
-                            auto rest = builder.consume_rest();
-                            args[param_name] = rest;
-                        }
-                    } catch (json::exception &) {
-                        auto rest = builder.consume_rest();
-                        args[param_name] = rest;
-                    }
-                    builder.add_tool_call(function_name, "", args.dump());
                     throw common_chat_msg_partial_exception("Incomplete tool parameter");
                 }
-            }            
+            }
             // Look for closing function tag
             auto end_func = builder.try_find_literal("</function>");
             if (end_func) {
                 builder.move_to(end_func->groups[0].end);
                 builder.consume_spaces();  // Consume whitespace after </function>
 
-                // Add the tool call with parsed arguments
-                if (!builder.add_tool_call(function_name, "", args.dump())) {
+                // Add the tool call with parsed arguments, but only if we REALLY got the literal
+                auto eaten_fragment = builder.input().substr(end_func->groups[0].begin, end_func->groups[0].end);
+                auto funlen = std::string("</function>").length();
+                if (eaten_fragment.length() >= funlen && eaten_fragment.substr(0, funlen) == std::string("</function>")) {
+                    if (!builder.add_tool_call(function_name, "", args.dump())) {
+                        throw common_chat_msg_partial_exception("Incomplete tool call");
+                    }
+                } else {
                     throw common_chat_msg_partial_exception("Incomplete tool call");
                 }
             } else {
-                builder.add_tool_call(function_name, "", args.dump()); // add partial tool parse
                 throw common_chat_msg_partial_exception("Incomplete tool call");
             }
-
             // Look for closing tool call tag
             if (auto end_tool = builder.try_find_regex(tool_call_end_regex, std::string::npos, false)) {
                 builder.move_to(end_tool->groups[0].end);
@@ -2228,8 +2215,8 @@ static common_chat_params common_chat_params_init_seed_oss(  const common_chat_t
                 std::string param_rules;
                 if (parameters.contains("properties")) {
                     for (const auto & [key, value] : parameters.at("properties").items()) {
-                        param_rules += "<parameter=" + key + ">" + builder.add_schema(name + "-arg-" + key, value) +
-                                       "</parameter>";
+                        param_rules += "\"<parameter=" + key + ">\"" + builder.add_schema(name + "-arg-" + key, value) +
+                                       "\"</parameter>\"";
                     }
                 }
 

tests/test-chat.cpp

@@ -1623,7 +1623,13 @@ static void test_template_output_parsers() {
     }
     {
         // Seed-OSS format tests
-        printf("[%s] Seed-OSS format tests\n", __func__);
+        auto tmpls = read_templates("models/templates/ByteDance-Seed-OSS.jinja");
+        std::vector<std::string> end_tokens{ "<seed:eos>" };
+
+        assert_equals(COMMON_CHAT_FORMAT_SEED_OSS, common_chat_templates_apply(tmpls.get(), inputs_no_tools).format);
+        assert_equals(COMMON_CHAT_FORMAT_SEED_OSS, common_chat_templates_apply(tmpls.get(), inputs_tools).format);
+
+        test_templates(tmpls.get(), end_tokens, message_assist, tools, "Hello, world!\nWhat's up?", /* expect_grammar_triggered= */ false);
 
         // Test simple reasoning content
         assert_msg_equals(
@@ -1689,6 +1695,21 @@ static void test_template_output_parsers() {
                     /* .reasoning_format = */ COMMON_REASONING_FORMAT_DEEPSEEK,
                 }));
 
+        // Test deltas: the number of tool calls in partial parses should never decrease
+        std::string tool_msg = "<seed:tool_call>\n"
+            "<function=fun>\n"
+            "<parameter=smth>[1, 2, 3]</parameter>\n"
+            "</function>";
+        std::size_t previousToolCalls = 0;
+        for (std::size_t i = std::string("<seed:tool_call>").length(); i < tool_msg.length() - 1; i++) {
+            auto partial = tool_msg.substr(0, i);
+            auto partial_res = common_chat_parse(partial, true, { COMMON_CHAT_FORMAT_SEED_OSS, COMMON_REASONING_FORMAT_DEEPSEEK });
+            if (partial_res.tool_calls.size() < previousToolCalls) {
+                throw std::runtime_error("Tool call size decreased on partial: " + partial + " from " + std::to_string(previousToolCalls) + " to " + std::to_string(partial_res.tool_calls.size()));
+            }
+            previousToolCalls = partial_res.tool_calls.size();
+        }
+
         // Test multiple parameters in tool call
         common_chat_msg msg_multi_param;
         msg_multi_param.role = "assistant";
@@ -1705,9 +1726,9 @@ static void test_template_output_parsers() {
                 /* is_partial= */ false,
                 {COMMON_CHAT_FORMAT_SEED_OSS}));
 
-        // Test partial parsing for incomplete tool call
+        // Test partial parsing for incomplete tool call - don't actually add the call until parsing parameters is done
         assert_msg_equals(
-            simple_assist_msg("", "", "calculate_sum", "{\"numbers\": [1]}"),
+            simple_assist_msg("", ""),
             common_chat_parse(
                 "<seed:tool_call>\n"
                 "<function=calculate_sum>\n"