fix: preserve generated chat parser during response parsing

.github/workflows/cmake-multi-platform.yml

@@ -14,9 +14,15 @@ on:
         required: false
         default: ''
 
+concurrency:
+  group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}
+  cancel-in-progress: true
+
 jobs:
   build:
     runs-on: ${{ matrix.os }}
+    # Without this a hung test holds a runner for the 6h default
+    timeout-minutes: 45
 
     strategy:
       # Set fail-fast to false to ensure that feedback is delivered for all matrix combinations. Consider changing this to true when your workflow is stable.
@@ -97,7 +103,7 @@ jobs:
       working-directory: ${{ steps.strings.outputs.build-output-dir }}
       # Execute tests defined by the CMake configuration. Note that --build-config is needed because the default Windows generator is a multi-config generator (Visual Studio generator).
       # See https://cmake.org/cmake/help/latest/manual/ctest.1.html for more detail
-      run: ctest --build-config ${{ matrix.build_type }}
+      run: ctest --build-config ${{ matrix.build_type }} --timeout 300 --output-on-failure
 
     - name: Verify examples run
       # Examples use POSIX headers (unistd.h) and are not available on Windows

.github/workflows/lint.yml

@@ -9,6 +9,7 @@ on:
 jobs:
   pre-commit:
     runs-on: ubuntu-latest
+    timeout-minutes: 15
 
     steps:
     - uses: actions/checkout@v4

.github/workflows/update-llama-cpp.yml

@@ -8,6 +8,7 @@ on:
 jobs:
   update-submodule:
     runs-on: ubuntu-latest
+    timeout-minutes: 30
 
     permissions:
       contents: write

CMakeLists.txt

@@ -166,9 +166,26 @@ if(AGENT_CPP_BUILD_TESTS)
     target_link_libraries(test_callbacks PRIVATE agent model ${LLAMA_COMMON_TARGET} llama)
     target_compile_features(test_callbacks PRIVATE cxx_std_17)
 
+    add_executable(test_chat_parser tests/test_chat_parser.cpp)
+    target_include_directories(test_chat_parser PRIVATE
+        src
+        tests
+        ${LLAMA_SOURCE_DIR}/common
+        ${LLAMA_SOURCE_DIR}/ggml/include
+        ${LLAMA_SOURCE_DIR}/include
+        ${LLAMA_SOURCE_DIR}/vendor
+    )
+    # The test parses responses against real chat templates shipped by llama.cpp
+    target_compile_definitions(test_chat_parser PRIVATE
+        LLAMA_TEMPLATES_DIR="${LLAMA_SOURCE_DIR}/models/templates"
+    )
+    target_link_libraries(test_chat_parser PRIVATE model ${LLAMA_COMMON_TARGET} llama)
+    target_compile_features(test_chat_parser PRIVATE cxx_std_17)
+
     add_test(NAME ToolTests COMMAND test_tool)
     add_test(NAME CallbacksTests COMMAND test_callbacks)
     add_test(NAME GrammarTests COMMAND test_grammar)
+    add_test(NAME ChatParserTests COMMAND test_chat_parser)
 
     if(AGENT_CPP_BUILD_MCP)
         add_executable(test_mcp_client tests/test_mcp_client.cpp)
@@ -185,9 +202,16 @@ if(AGENT_CPP_BUILD_TESTS)
 
     # On Windows, DLLs are placed in the bin/ directory by llama.cpp
     # We need to add this directory to PATH so tests can find the DLLs
+    # A hung test must fail rather than hold the runner until the job limit
+    set_tests_properties(ToolTests CallbacksTests GrammarTests ChatParserTests
+        PROPERTIES TIMEOUT 120
+    )
+
     if(WIN32)
-        set_tests_properties(ToolTests CallbacksTests GrammarTests PROPERTIES
-            ENVIRONMENT "PATH=${CMAKE_BINARY_DIR}/bin\;$ENV{PATH}"
+        # Multi-config generators (Visual Studio) place DLLs in bin/<config>,
+        # single-config ones in bin/ - both are on PATH so tests can load them
+        set_tests_properties(ToolTests CallbacksTests GrammarTests ChatParserTests PROPERTIES
+            ENVIRONMENT "PATH=${CMAKE_BINARY_DIR}/bin/${CMAKE_BUILD_TYPE}\;${CMAKE_BINARY_DIR}/bin/Release\;${CMAKE_BINARY_DIR}/bin\;$ENV{PATH}"
         )
     endif()
 

src/model.cpp

@@ -203,6 +203,54 @@ Model::tokenize(const std::string& prompt) const
     return prompt_tokens;
 }
 
+namespace {
+
+/// Deserializes the PEG parser derived from the chat template. An empty
+/// definition yields an empty arena, which common_chat_peg_parse() treats as a
+/// pure-content parser.
+common_peg_arena
+load_parser(const std::string& parser_definition)
+{
+    common_peg_arena arena;
+    if (!parser_definition.empty()) {
+        arena.load(parser_definition);
+    }
+    return arena;
+}
+
+common_chat_msg
+parse_with_parser(const common_peg_arena& parser,
+                  const common_chat_params& params,
+                  const std::string& response,
+                  std::optional<common_chat_format> format_override)
+{
+    common_chat_parser_params syntax;
+    // Use explicitly configured format, or fall back to auto-detected format.
+    // Note that the format only selects the mapper; the PEG parser below is
+    // what actually drives parsing.
+    syntax.format = format_override.value_or(params.format);
+    // The parser is derived from the chat template together with the
+    // generation prompt it was built against, and parsing needs both
+    // (ggml-org/llama.cpp#18675). Without them tool calls stay as raw text.
+    syntax.generation_prompt = params.generation_prompt;
+    syntax.parse_tool_calls = true;
+
+    try {
+        auto parsed_msg =
+          common_chat_peg_parse(parser, response, false, syntax);
+        parsed_msg.role = "assistant";
+        return parsed_msg;
+    } catch (const std::exception& e) {
+        // llama.cpp throws plain std::runtime_error when the output does not
+        // match the template's format; surface it as a library error so
+        // callers can catch it alongside the rest of agent.cpp.
+        throw ModelError(std::string("failed to parse model response: ") +
+                         e.what());
+    }
+}
+
+}
+
 common_chat_msg
 Model::generate(const std::vector<common_chat_msg>& messages,
                 const std::vector<common_chat_tool>& tools,
@@ -226,15 +274,24 @@ Model::generate(const std::vector<common_chat_msg>& messages,
 
     std::string response = generate_from_tokens(prompt_tokens, callback);
 
-    common_chat_parser_params syntax;
-    // Use explicitly configured format, or fall back to auto-detected format
-    syntax.format = config_.chat_format.value_or(params.format);
-    syntax.parse_tool_calls = true;
+    // The PEG parser only changes when the rendered template changes, so keep
+    // the deserialized arena around instead of re-parsing it every turn.
+    if (parser_source_ != params.parser) {
+        parser_arena_ = load_parser(params.parser);
+        parser_source_ = params.parser;
+    }
 
-    auto parsed_msg = common_chat_parse(response, false, syntax);
-    parsed_msg.role = "assistant";
+    return parse_with_parser(
+      parser_arena_, params, response, config_.chat_format);
+}
 
-    return parsed_msg;
+common_chat_msg
+parse_response(const common_chat_params& params,
+               const std::string& response,
+               std::optional<common_chat_format> format_override)
+{
+    return parse_with_parser(
+      load_parser(params.parser), params, response, format_override);
 }
 
 std::string

src/model.h

@@ -43,6 +43,21 @@ struct ModelConfig
 std::string
 load_grammar_file(const std::string& grammar_path);
 
+/// Parses a raw model response into a chat message, using the PEG parser and
+/// generation prompt derived from the model's chat template.
+/// @param params result of common_chat_templates_apply() for the turn that
+///        produced @p response
+/// @param response the raw text generated by the model
+/// @param format_override optional explicit chat format, overriding the
+///        auto-detected params.format
+/// @throws agent_cpp::ModelError if the response does not match the expected
+///         format
+common_chat_msg
+parse_response(
+  const common_chat_params& params,
+  const std::string& response,
+  std::optional<common_chat_format> format_override = std::nullopt);
+
 // Forward declaration
 class Model;
 
@@ -198,6 +213,10 @@ class Model
     std::vector<llama_token> processed_tokens_; // Track tokens in KV cache
     int n_past_ = 0;                            // Track position in KV cache
     ModelConfig config_;
+    // Chat parser derived from the template, cached across turns along with
+    // the definition it was built from
+    std::string parser_source_;
+    common_peg_arena parser_arena_;
 };
 
 } // namespace agent_cpp

tests/test_chat_parser.cpp

@@ -0,0 +1,142 @@
+#include "model.h"
+#include "test_utils.h"
+#include <fstream>
+#include <iostream>
+#include <sstream>
+#include <stdexcept>
+#include <string>
+
+#ifndef LLAMA_TEMPLATES_DIR
+#error "LLAMA_TEMPLATES_DIR must be defined"
+#endif
+
+namespace {
+
+// Progress markers on stderr, unbuffered: if a test hangs (as this one did on
+// Windows) the ctest timeout output shows exactly which phase it stalled in.
+void
+trace(const std::string& phase)
+{
+    std::cerr << "[trace] " << phase << std::endl;
+}
+
+std::string
+read_file(const std::string& path)
+{
+    std::ifstream file(path);
+    if (!file) {
+        throw std::runtime_error("failed to open " + path);
+    }
+    std::ostringstream buffer;
+    buffer << file.rdbuf();
+    return buffer.str();
+}
+
+// Mirrors how Model::generate() renders a turn, so the parser under test is
+// fed the same common_chat_params the real code path produces.
+common_chat_params
+apply_template(const std::string& template_name, bool with_tools)
+{
+    const std::string template_path =
+      std::string(LLAMA_TEMPLATES_DIR) + "/" + template_name;
+
+    trace("reading " + template_path);
+    const std::string template_source = read_file(template_path);
+
+    trace("initializing templates");
+    auto tmpls = common_chat_templates_init(nullptr, template_source);
+
+    common_chat_msg user_msg;
+    user_msg.role = "user";
+    user_msg.content = "Calculate 3 + 4";
+
+    common_chat_templates_inputs inputs;
+    inputs.messages = { user_msg };
+    if (with_tools) {
+        common_chat_tool calculator;
+        calculator.name = "calculator";
+        calculator.description = "Performs arithmetic";
+        calculator.parameters =
+          R"({"type":"object","properties":{"a":{"type":"number"},)"
+          R"("b":{"type":"number"},"operation":{"type":"string"}},)"
+          R"("required":["a","b","operation"]})";
+        inputs.tools = { calculator };
+    }
+    inputs.tool_choice = COMMON_CHAT_TOOL_CHOICE_AUTO;
+    inputs.add_generation_prompt = true;
+    inputs.enable_thinking = false;
+
+    trace("applying template");
+    auto params = common_chat_templates_apply(tmpls.get(), inputs);
+    trace("template applied");
+
+    return params;
+}
+
+}
+
+// Regression test for tool calls being returned as raw text instead of being
+// parsed. llama.cpp ggml-org/llama.cpp#18675 moved parsing to a PEG parser
+// derived from the chat template; parse_response() must forward both the
+// derived parser and the generation prompt it was built against, otherwise
+// common_chat_parse() falls back to a pure-content parser.
+TEST(test_tool_call_is_parsed_from_response)
+{
+    auto params = apply_template("ibm-granite-granite-4.0.jinja", true);
+    ASSERT_TRUE(!params.parser.empty());
+
+    const std::string response =
+      "<tool_call>\n"
+      R"({"name": "calculator", "arguments": {"a": 3, "b": 4, "operation": "add"}})"
+      "\n</tool_call>";
+
+    auto parsed = agent_cpp::parse_response(params, response);
+
+    ASSERT_EQ(parsed.role, std::string("assistant"));
+    ASSERT_EQ(parsed.tool_calls.size(), static_cast<size_t>(1));
+    ASSERT_EQ(parsed.tool_calls[0].name, std::string("calculator"));
+    ASSERT_TRUE(parsed.tool_calls[0].arguments.find("\"operation\"") !=
+                std::string::npos);
+    ASSERT_TRUE(parsed.content.empty());
+}
+
+// A plain answer must still come back as content, with no spurious tool calls.
+TEST(test_plain_response_is_parsed_as_content)
+{
+    auto params = apply_template("ibm-granite-granite-4.0.jinja", true);
+
+    auto parsed =
+      agent_cpp::parse_response(params, "The result of 3 + 4 is 7.");
+
+    ASSERT_TRUE(parsed.tool_calls.empty());
+    ASSERT_EQ(parsed.content, std::string("The result of 3 + 4 is 7."));
+}
+
+// Templates rendered without tools still parse ordinary content.
+TEST(test_response_without_tools_is_parsed_as_content)
+{
+    auto params = apply_template("ibm-granite-granite-4.0.jinja", false);
+
+    auto parsed = agent_cpp::parse_response(params, "Hello!");
+
+    ASSERT_TRUE(parsed.tool_calls.empty());
+    ASSERT_EQ(parsed.content, std::string("Hello!"));
+}
+
+int
+main()
+{
+    std::cout << "\n=== Running Chat Parser Unit Tests ===\n" << std::endl;
+
+    try {
+        RUN_TEST(test_tool_call_is_parsed_from_response);
+        RUN_TEST(test_plain_response_is_parsed_as_content);
+        RUN_TEST(test_response_without_tools_is_parsed_as_content);
+
+        std::cout << "\n=== All tests passed! ✓ ===\n" << std::endl;
+        return 0;
+    } catch (const std::exception& e) {
+        std::cerr << "\n✗ TEST FAILED: " << e.what() << std::endl;
+        return 1;
+    }
+}