[bong-water-water-bong]

Refs #1788 (condition 7: a tool-call round trip on the served route).

The fault

forward-serve renders the container's own chat template, but handed it only chat_template_kwargs and enable_thinking. A request's tools went to the grammar and the answer parser and never to the template, so the model was never told what it could call.

Before, on the NPU with MiniCPM-V-4.7-1B-NPU2 (main 6df0386, ONEBIT_JINJA=ON, get_weather(city), temperature 0, "What is the weather in Paris right now? Use the tool."):

[prefix] slot=-1 cached=0 of 25 reason=no slot shares a prefix
first reply: finish length, content "I don't have real-time weather data…", tool_calls null

The reference (tokenizer.apply_chat_template(messages, tools=tools, add_generation_prompt=True)) renders the same request as 292 tokens.

What changes

1. npu::request_template_vars (app/npu_template.h): tools is a template variable, in the caller's key order; an empty list is left out. The route uses it for the prompt and for the --prefix-pin probe.
2. The qwen3 answer markup also reads the call form this template (and Qwen3.5's) teaches: <tool_call><function=NAME><parameter=KEY>VALUE</parameter></function></tool_call>. With the tools rendered the model wrote exactly that, and it came back as content with tool_calls null. A value is a JSON value when it is an object, array, number, true/false/null, otherwise the text itself. A block that is neither form goes back to content unchanged, as before.
3. npu::messages_for_template: an earlier call's function.arguments (a JSON string in the API) is handed to the template as the object. This template walks tool_call.arguments|items, so the second turn was a 400: Unknown (built-in) filter 'items' for type String.

After, on the NPU (this branch at d47afeb, same pack, same request, one npu-hold ticket, serve ended with the ticket)

== condition 3: known answer through the pack's template, three identical requests
  content 'Paris' finish stop | prompt 24 tok, prefill 2128 ms, 1 tok in 575 ms | wall 2.7 s
  content 'Paris' finish stop | prompt 24 tok, prefill 2184 ms, 1 tok in 578 ms | wall 2.8 s
  content 'Paris' finish stop | prompt 24 tok, prefill 2283 ms, 1 tok in 589 ms | wall 2.9 s
  identical: True
== condition 7: a tool-call round trip
  first reply: finish tool_calls, content 'I need to get the current weather for Paris, so I will call the get_weather function.\n\n', tool_calls [{"function": {"arguments": "{\"city\":\"Paris\"}", "name": "get_weather"}, "id": "call_0", "type": "function"}] (wall 32.8 s)
  after the tool result: finish stop, content 'The weather in Paris is 17 degrees Celsius with light rain.\n\nThe current weather in Paris is 17 degrees Celsius with light rain.' (wall 20.1 s)
[prefix] slot=-1 cached=0 of 292 reason=no slot shares a prefix
[prefix] slot=0 cached=291 of 376 reason=hit

The tools prompt is now 292 tokens, the reference's count.

Tests

ctest -R "npu_answer|npu_template": 2/2 pass. New arms:

ok   request tools: the template sees the request's tools
ok   request tools: in the caller's key order
ok   request tools: kwargs and enable_thinking still arrive
ok   request tools: an empty list renders no tools block
ok   call arguments: as the API sends them (a string) this template cannot walk them
ok   call arguments: the template walks the object, in the string's key order
ok   call arguments: a string that is not a JSON object is left as sent
ok   function form: one call, named
ok   function form: a bare value is a string argument
ok   function form: the markup leaves content
ok   function form: the prose before it stays
ok   function form: JSON values are values, text is a string (a date is not a number), in the model's order
ok   function form: a call-only reply has null content
ok   function form: no parameters is {}
ok   function form: an unclosed parameter goes back to content
ok   function form: text between parameters goes back to content
ok   function form: streamed in pieces, the same call
ok   function form: streamed, no markup in the deltas

The first function form arm is the reply the model wrote on the device.

Not in this PR

• Argument typing by the tool's parameter schema: without it a string argument whose text is a JSON literal (17) comes back as the literal. Noted in the code and in docs/npu-chat-template.md.
• tool_choice: "required" still constrains the answer to the JSON call form (the existing grammar); both forms parse. Not measured on this pack.
• Other packs with a tools template were not re-run on the device; for them the prompt now carries the tools block where it did not before.

Docs: docs/npu-chat-template.md.

🤖 Generated with Claude Code

[bong-water-water-bong]

ready for review at d47afeb

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 1788 - Partially compliant Compliant requirements: tools: a tool-call round trip parsed and answered Non-compliant requirements: imports with zero gaps and passes its self-check against a named reference serves on --device npu answers a known-answer prompt through the model's own chat template, 3 identical requests returning identical text a measured decode tok/s and time to first token under npu-hold the row names the kernel set and the engine commit that produced it images: one known image, one known question, an answer that is only obtainable from the picture — not "it accepted the request" prefix cache: a repeated prefix (text and a picture) hits the cache and the second TTFT is measurably lower Requires further human verification: imports with zero gaps and passes its self-check against a named reference serves on --device npu answers a known-answer prompt through the model's own chat template, 3 identical requests returning identical text a measured decode tok/s and time to first token under npu-hold the row names the kernel set and the engine commit that produced it images: one known image, one known question, an answer that is only obtainable from the picture — not "it accepted the request" prefix cache: a repeated prefix (text and a picture) hits the cache and the second TTFT is measurably lower

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Function Call Parsing Logic The new close_function_call method in AnswerStream introduces logic to parse tool calls formatted as <function=NAME><parameter=KEY>VALUE</parameter></function>. While the implementation handles basic cases, it may not robustly handle all edge cases of malformed or complex parameter structures, potentially leading to incorrect parsing or crashes. Specifically, the parsing relies on string matching and assumes a specific format that may not be fully validated, which could lead to unexpected behavior in production. // The second call form a <tool_call> block carries (Qwen3.5's template, and MiniCPM-V 4.7's, teach it): // <function=NAME> // <parameter=KEY> // VALUE // </parameter> // </function> // The template writes a string argument bare and everything else through `tojson`, so a value is read back // the same way: an object, an array, a number, true/false/null are JSON; anything else is the string itself, // less the one newline the form puts on each side. False on any other shape (the caller then tries the JSON // form and, failing that, puts the markup back into content). // shortcut: without the request's schema a string argument whose text is a JSON literal ("17", "true") comes // back as that literal; type by the tool's parameter schema here if a caller needs it kept a string. bool close_function_call() { const std::string body = answer_detail::trim(call_text_); static const std::string fopen_ = "<function=", fclose_ = "</function>", popen_ = "<parameter=", pclose_ = "</parameter>"; if (body.compare(0, fopen_.size(), fopen_) != 0) return false; if (body.size() < fclose_.size() || body.compare(body.size() - fclose_.size(), fclose_.size(), fclose_) != 0) return false; const size_t name_end = body.find('>', fopen_.size()); if (name_end == std::string::npos) return false; ToolCall c; c.id = "call_" + std::to_string(calls_.size()); c.name = answer_detail::trim(body.substr(fopen_.size(), name_end - fopen_.size())); if (c.name.empty() || c.name.find('<') != std::string::npos) return false; nlohmann::ordered_json args = nlohmann::ordered_json::object(); size_t i = name_end + 1; const size_t end = body.size() - fclose_.size(); auto space = [&] { while (i < end && (body[i] == ' ' || body[i] == '\n' || body[i] == '\r' || body[i] == '\t')) ++i; }; for (space(); i < end; space()) { if (body.compare(i, popen_.size(), popen_) != 0) return false; const size_t key_end = body.find('>', i + popen_.size()); if (key_end == std::string::npos || key_end >= end) return false; const std::string key = answer_detail::trim(body.substr(i + popen_.size(), key_end - i - popen_.size())); const size_t close = body.find(pclose_, key_end + 1); if (key.empty() || close == std::string::npos || close > end) return false; std::string value = body.substr(key_end + 1, close - key_end - 1); if (!value.empty() && value.front() == '\n') value.erase(0, 1); if (!value.empty() && value.back() == '\n') value.pop_back(); const std::string t = answer_detail::trim(value); nlohmann::ordered_json v = value; const bool literal = !t.empty() && (t[0] == '{' || t[0] == '[' || t[0] == '-' || std::isdigit((unsigned char)t[0]) || t == "true" || t == "false" || t == "null"); if (literal) { const nlohmann::ordered_json parsed = nlohmann::ordered_json::parse(t, nullptr, /*allow_exceptions=*/false); if (!parsed.is_discarded() && !parsed.is_string()) v = parsed; } args[key] = std::move(v); i = close + pclose_.size(); } c.arguments = args.dump(); calls_.push_back(std::move(c)); return true; } Template Variable Handling The request_template_vars function adds tools to the template variables, but it only includes tools if it's an array and not empty. This could lead to inconsistency if a request explicitly sends an empty tools array, which should still be passed to the template to allow for conditional rendering. This might affect templates that rely on the presence of an empty tools array to render correctly. inline nlohmann::ordered_json request_template_vars(const nlohmann::ordered_json& body, bool enable_thinking) { nlohmann::ordered_json extra = nlohmann::ordered_json::object(); if (body.contains("chat_template_kwargs") && body["chat_template_kwargs"].is_object()) extra = body["chat_template_kwargs"]; if (body.contains("tools") && body["tools"].is_array() && !body["tools"].empty()) extra["tools"] = body["tools"]; extra["enable_thinking"] = enable_thinking; return extra; }

[bong-water-water-bong]

VERDICT: READY at d47afeb — a template that never received tools cannot emit a tool call, and that is a protocol bug rather than a formatting one.

Refs #1788 (condition 7: a tool-call round trip on the served route).
## The fault
`forward-serve` renders the container's own chat template, but handed it only `chat_template_kwargs` and `enable_thinkin[g]`

Why this closes a class rather than a case

Three fixes tonight have now been the same shape: #2064 (stop set decided before the template was known), #2144 (turn end read off the template), #2151 (what enable_thinking means per pack), and this one — the template's inputs. In each, the route decided a protocol fact from something other than the render. Here the render was given chat_template_kwargs and enable_thinking but not the request's tools, so any template that spells tool definitions into the prompt (Qwen3.5, ChatML-with-tools, Gemma 4) rendered a prompt with no tools in it — and then the model either invents calls or answers as if it had none. No error anywhere: the request succeeds.

It also matters for #1779 fault 4, which is where this lane started: a tool-call parser is only testable against a route that emits calls, and this is the missing half — the parser's fixtures are unit tests, but the served route could never produce the input.

Two things the acceptance should show

1. the round trip, which is what column 7 of F6: MiniCPM-V 4.7 35B-A3B (MoE vision) on our own NPU runtime — images, tools, prefix cache, measured row #1788 asks for: a request with tools, a reply that contains a call, and the call parsed back into {name, arguments} — because "the template got the tools" is a claim about the input, while the round trip is the claim about the behaviour;
2. tools absent vs tools: [] — a template that checks tools for truthiness renders no tool block for [], and one that checks is defined renders an empty block. Those are different prompts, and a request that omits the field must not be given an empty one. One arm each, since this is exactly the kind of distinction that a "tools work now" test passes over.

On the scope line

"condition 7: a tool-call round trip on the served route" is the right way to attach this to a 15-condition row: it says which condition this PR moves, which is what a lead needs to see progress without reading the diff.

[bong-water-water-bong]

VERDICT at d47afeb: READY

A request's tools reaching the chat template is the kind of fix that changes an answer without changing a byte of kernel code. request_template_vars(ordered, thinking_from_body…) collects the per-request facts and passes them as extra into render_chat_template, so the model is shown the tool definitions it was actually called with. Without it the template renders a prompt with no tools and the model answers as if none were available — a failure that looks like a weak model rather than a plumbing gap.

Passing extra alongside add_generation_prompt, bos_text and eos_text puts the per-request variables in the same call as the fixed template facts, which is right: they are all inputs to one rendering, and a second path that injected tools afterwards would drift from the template's own ordering.

Same family as #2118/#2131/#2151 — four changes today on the demo path's visible behaviour (whitespace, reasoning-token accounting, thinking template order, and now tools), which is what a release row needs and what a kernel-only view would miss entirely.

[bong-water-water-bong]

VERDICT at d47afeb: READY

Reviewed head d47afeb (refs #1788, condition 7). This is the second verdict on this head.

Checked

• git merge-tree onto main: no conflicts. CI build (success) and CodeQL green.
• ctest npu_template and npu_answer_parser pass, built from this head. The new template arms ran in this build (no jinja renderer needed for them): "request tools: the template sees the request's tools", "… in the caller's key order", "an empty list renders no tools block", and "call arguments: the template walks the object, in the string's key order".
• The parser's existing arms still pass, including the reasoning-token ones from #2131, so the new call form does not disturb the splitter they count over.
• Mutants of app/npu_answer_parser.h, each rebuilt and run:

mutant	arm that fails
parameter values always strings	"JSON values are values, text is a string (a date is not a number)"
the <function=…> form switched off	"function form: one call, named"
text between parameters accepted	"function form: text between parameters goes back to content"

I restored the file (git status clean).

• The fallback is safe. close_function_call() returns false for anything that is not exactly <function=NAME> followed by <parameter=K>V</parameter>…</function>, so a JSON-form call or a malformed block takes the existing path, as the body says.

Note, not blocking: a parameter value that parses as JSON becomes JSON, so a numeric-looking string argument (an id 12345, a zip 90210) reaches the tool as a number even when the tool's schema says "type": "string". A date or a leading-zero value stays a string, because the JSON parse fails, which the arm pins. When tools carries a schema, consulting parameters.properties[key].type would settle it.

Not re-run by me: the NPU round trip on MiniCPM-V-4.7-1B (finish tool_calls and the second turn).