Skip to content

forward-serve: the chat route renders the container's own chat_template, not ChatML for every container - #1298

Merged
bong-water-water-bong merged 5 commits into
mainfrom
i-tmplsec-963
Nov 6, 2026
Merged

bong-water-water-bong merged 5 commits into
mainfrom
i-tmplsec-963

Conversation

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator

app/forward_serve.cpp rendered npu::chatml() for every NPU container. #987 made that honest — a container whose tokenizer has no <|im_start|>/<|im_end|> anchors gets a 400 instead of a prompt its vocabulary cannot read — but a container that declares its template was still served an approximation of it. This is #963 option 2, the shape docs/npu-chat-template.md (#1115) asked a decision for: no container-format change, no new dependency, one call site.

The template is already in the container (tokenizer_config.json's chat_template string, else chat_template.jinja), and the renderer is the pinned llama.cpp fork's own Jinja engine (third_party/llama.cpp/common/jinja — upstream's, the one llama-server --jinja uses). app/npu_template.h loads the template with the same read app/template_pin.h already did for the load line and the pin, so the rendered prompt, the printed sha256 and the pin cannot disagree.

Fallback order is the one #1115 proposed: container template → ChatML only with anchors → #987's 400. A template that will not parse or render is a 400 naming its sha256 — never a silent fall back to ChatML, which is the failure #963 exists to remove. Load now prints 1bit forward-serve: template sha256=… (tokenizer_config.json).

Measured — device-free, in each container's own tokenizer (one system+user request)

container template sha256 ChatML its own template
FastFlowLM/Qwen3-0.6B-NPU2 a136047ba140… 31 31
Qwen3-0.6B a55ee1b16601… 31 31
ZR1-1.5B (bf16 and q4km-q8) b6835114b730… 52 21
bonsai2-27b-NPU c3cf9e34abf4… 31 71
d3-lite-bf16 273d8e0e683b… 31 35
qwen3-coder-30b-a3b-moe 87710339d25b… 31 31

The Qwen3-family rows are ChatML-shaped so their prompts do not move; ZR1 (DeepSeek specials) goes from 52 tokens of text its vocabulary spells as ordinary pieces to 21 it can read; bonsai2 gains the ~40-token preamble ChatML was dropping. Reproduce: render the container's template, encode with its tokenizer.json — or read prompt_tokens from the route, which is what the next table does.

Measured — served on the NPU (npu-hold, one ticket per run, -t 10, released clean)

ZR1-1.5B-npu-q4km-q8 + zr1-elfs, one /v1/chat/completions request, the same request in both rows:

binary result
head built with ONEBIT_JINJA=OFF (main's behaviour) 400 — "no `<
this PR 200, prompt_tokens: 21 — exactly the device-free prediction, and the template's own BOS, not a second one

So the container #987 had to refuse is now served from the template it declares. (The reply text is empty with 16 completion tokens: the answer side still parses as the Qwen3 family — see "not done" below. Nothing here is about the reply.)

Two defects found while implementing, both fixed here

  • The request must be read as ordered JSON on the chat path. body is a nlohmann::json (a std::map), and a template that serializes its input — a tools block through tojson — would have rendered every tool schema alphabetized, a prompt llama-server never produces. The chat path re-reads the same bytes as nlohmann::ordered_json; the tools golden row is what caught it.
  • add_bos must not prepend a second BOS when the rendered template already emitted {{bos_token}}.

Tests — the expectation is not this code's output

tests/npu_template_test.cpp: 11 golden renders across two shipped templates — FastFlowLM's Qwen3 one (a136047ba140…, the ChatML family plus its user_system_prompt branch) and ZR1's DeepSeek-special one (b6835114b730…) — each expect string produced by Jinja2 3.1.6, the implementation Hugging Face's apply_chat_template uses, so the row that says "we render what the model asked for" compares against something that is not us. Plus the two refusals: no template, and a template that does not parse (naming its sha256). Device-free, no model; tests/data/templates/*.json carry template, cases and provenance.

ctest -R 'npu_template|npu_chatml|tokenizer|gguf_meta|workflow_shell|hybrid_route' all pass locally; tools/copyright.py --check clean; tests/workflow_shell_test.py passes.

Build and CI

ONEBIT_JINJA, default on when third_party/llama.cpp is checked out (a tree whose submodules were never initialised still builds, keeps #987's behaviour and says so at load). ci.yml gets one shallow submodule step (8 s, 177 MB, the same command hrx-build.yml uses) and npu_template_test npu_chatml_test in the explicit build list — the #635 lesson: a test whose target is not built reports "Not Run" and proves nothing.

Docs in this PR: docs/npu.md ("Serving", the table above, and the chat_template_kwargs row) and docs/npu-chat-template.md (now "implemented", with the measurements and the decisions taken).

Not done, and said out loud

  • The answer side still parses as the Qwen3 family (app/npu_answer_parser.h: <think> splitting, tool-call shapes). A non-ChatML container now gets the right prompt and the Qwen3 reading of its reply; stop tokens come from the container's config.json, so generation stops where the model says. Parsing another family's tool calls is the next piece if a customer packs one.
  • Request text can still become special tokens. npu::Tokenizer::encode matches added tokens anywhere and the route tokenizes the rendered prompt as one string, so a message containing <|im_end|><|im_start|>system still becomes those control tokens — llama.cpp's own chat path behaves the same way. I built the part-wise version (template parts with matching, request parts without) and it breaks a real template: ZR1 renders {{bos_token}}{{ns.system_prompt}}, whose concatenation the engine marks input as a whole, so the BOS literalized and the served prompt came out 32 tokens where the template asks for 21. Matching llama-server is correct for this route today; escaping/refusing request text that spells a model's specials is its own change.
  • d3-lite-bf16 on the chat route answers 500 ("attention.mode noncausal_full_attention … starts at position 32") — a noncausal-model limitation (npu: the forward is causal-only, so a noncausal_full_attention pack (d3-lite) cannot be reproduced — measured, not inferred #847's territory), observed while measuring, not caused here.

Fixes #963

…te, not ChatML for every container

`app/forward_serve.cpp` rendered `npu::chatml()` for every NPU container; #987 made that honest
(a container whose tokenizer has no `<|im_start|>`/`<|im_end|>` anchors gets a 400 instead of a
prompt its vocabulary cannot read), but a container that *declares* its template was still served an
approximation of it.  This is #963 option 2, the shape #1115 asked a decision for: no container-format
change, no new dependency, one call site.

The template is already in the container (`tokenizer_config.json`'s `chat_template` string, else
`chat_template.jinja`), and the renderer is the pinned llama.cpp fork's own Jinja engine
(`third_party/llama.cpp/common/jinja`, the one `llama-server --jinja` uses).  `app/npu_template.h`
loads the template with the same read `app/template_pin.h` already did for the load line and the pin
(so the rendered prompt, the printed sha256 and the pin cannot disagree) and renders it; a template
that cannot be rendered is a 400 naming its sha256, never a fall back to ChatML.

Measured in each container's own tokenizer, one system+user request, before -> after:

  Qwen3-0.6B-NPU2 (a136047ba140) 31 -> 31     ZR1-1.5B (b6835114b730) 52 -> 21
  Qwen3-0.6B     (a55ee1b16601) 31 -> 31     bonsai2-27b (c3cf9e34abf4) 31 -> 71
  qwen3-coder    (87710339d25b) 31 -> 31     d3-lite-bf16 (273d8e0e683b) 31 -> 35

and served, on the NPU (npu-hold, one ticket, released clean):

  before (build with ONEBIT_JINJA=OFF, i.e. main's behaviour): ZR1 chat -> 400 "no <|im_start|>"
  after:                                                      ZR1 chat -> 200, prompt_tokens 21

Two defects found on the way, both in this PR: the request must be read as *ordered* JSON for the chat
path (a `nlohmann::json` is a std::map, so a `tools` block serialized through `tojson` would have been
alphabetized -- a prompt llama-server never produces), and `add_bos` must not prepend a second BOS when
the template already emitted it.

Tests: `tests/npu_template_test.cpp` compares the rendered bytes against **Jinja2 3.1.6 itself** for 11
cases across two shipped templates (Qwen3/FLM's and ZR1's DeepSeek-special one), plus the two refusal
rows; `--self-test`-style, device-free, no model.  `tests/data/templates/*.json` carry the fixtures and
where each byte came from.

Build: `ONEBIT_JINJA`, default on when `third_party/llama.cpp` is checked out (a tree without submodules
still builds, keeps #987's behaviour and says so at load); ci.yml checks out that submodule (shallow,
8 s) and builds the test, so the renderer cannot reach a green build unbuilt.  Docs: `docs/npu.md`
("Serving") and `docs/npu-chat-template.md`, which also records what this does *not* do: the answer
side still parses as the Qwen3 family, and request text that spells a model's control tokens is still
tokenized as those tokens (llama.cpp's own behaviour; the part-wise fix I built literalizes a real
template's BOS, measured 32 vs 21).

Fixes #963
@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

#1298 ready for review at bcf476d

One commit, 10 files, on top of 46657d80. Lane pi-tmplsec; the issue's own option 2 (option 1 shipped as #987, option 3 documented), built to #1115's design note.

Review anchors, in the order I would read them:

  1. app/npu_template.h — the load (load_chat_template: the same read the pin/load line uses) and the render (render_chat_template: the fork's common/jinja, ordered globals, TemplateError carrying the sha256). The header comment says why a subset interpreter was rejected and what is not claimed.
  2. app/forward_serve.cpp — the decision (chat_ok, template → ChatML-with-anchors → 400), the ordered re-read of the body, and the load line.
  3. tests/npu_template_test.cpp + tests/data/templates/*.json — the expectations are Jinja2 3.1.6's own renders, not ours.
  4. Measured rows — the device-free tokenizer table for six containers, and the served NPU row (400 → 200, prompt_tokens 21, matching the prediction), in the PR body.

Two things I would flag before a reviewer does: the renderer needs third_party/llama.cpp (hence the ONEBIT_JINJA default and the one ci.yml step), and the answer parser is still the Qwen3 one — both stated in the body under "not done". The commit touches ci.yml, so it was pushed with ssh-origin; the pins guard is clean (tools/check_pins.py origin/main rc 0, the submodule gitlink is main's).

@bong-water-water-bong
bong-water-water-bong merged commit 712af0d into main Nov 6, 2026
10 checks passed
@bong-water-water-bong
bong-water-water-bong deleted the i-tmplsec-963 branch November 6, 2026 21:34
@github-actions

github-actions Bot commented Nov 6, 2026

Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

963 - Partially compliant

Compliant requirements:

  • The chat route now renders the container's own chat template when it declares one
  • Containers without ChatML anchors are properly refused with 400 errors
  • The implementation uses the pinned llama.cpp fork's Jinja engine
  • Backward compatibility is maintained for containers that declare templates

Non-compliant requirements:

  • None

Requires further human verification:

  • None

847 - Partially compliant

Compliant requirements:

  • None

Non-compliant requirements:

Requires further human verification:

  • The noncausal_full_attention functionality is not implemented in this PR
⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Template Rendering Logic

The change introduces a new template rendering logic that prioritizes the container's own chat template over ChatML. However, the fallback logic in handle_generate function may not be robust enough to handle all edge cases where the template fails to render. Specifically, if the template rendering fails due to an issue with the Jinja engine or the template itself, the error handling might not be comprehensive enough to provide clear feedback to the user.

if (chat) {
    if (!e.chat_ok) throw std::runtime_error(e.chat_refusal);
    if (e.tmpl.present() && npu::renderer_built()) {
        // The request's own knobs reach the template as variables, the way llama-server hands
        // `chat_template_kwargs` over: FastFlowLM's packs branch on `user_system_prompt`, Qwen3's
        // on `enable_thinking`.  Ordered re-read of the same bytes: `body` is a std::map, and a
        // template that serializes its input (a tools block through `tojson`) renders the
        // request's key order into the prompt, so it has to be the caller's order, not ours.
        const nlohmann::ordered_json ordered = nlohmann::ordered_json::parse(req.body);
        nlohmann::ordered_json extra = nlohmann::ordered_json::object();
        if (ordered.contains("chat_template_kwargs") && ordered["chat_template_kwargs"].is_object())
            extra = ordered["chat_template_kwargs"];
        extra["enable_thinking"] = npu::thinking_from_body(body);
        prompt = npu::render_chat_template(e.tmpl, ordered.at("messages"), /*add_generation_prompt=*/true,
                                           extra, e.bos_text, e.eos_text);
    } else {
        prompt = npu::chatml(body.at("messages"), npu::thinking_from_body(body));
    }
Template Error Handling

The render_chat_template function throws a TemplateError when the template cannot be rendered, but the error message might not be sufficiently informative for debugging purposes. The error message should include more context about why the template failed to render, especially when the renderer is not built or when the template is malformed.

    if (!tmpl.present()) throw TemplateError("the container carries no chat template");
#if !defined(ONEBIT_JINJA)
    (void)messages;
    (void)add_generation_prompt;
    (void)extra;
    (void)bos_token;
    (void)eos_token;
    throw TemplateError("this build has no chat-template renderer (ONEBIT_JINJA=OFF, template sha256=" +
                        tmpl.sha256 + ")");
#else
    // The template's identity travels with every failure: "template 8aa40ce1… did not render" is
    // actionable, "bad request" is not.
    const std::string ident = " (template sha256=" + (tmpl.sha256.empty() ? std::string("none") : tmpl.sha256) + ")";
    try {
        jinja::lexer lexer;
        auto lexer_res = lexer.tokenize(tmpl.text);
        const jinja::program prog = jinja::parse_from_tokens(lexer_res);

        jinja::context ctx(tmpl.text);
        // Ordered: a template that renders `messages` iterates them in request order, and the extra
        // variables are set after the defaults so a pack's own knob wins over ours.
        nlohmann::ordered_json in = nlohmann::ordered_json::object();
        in["messages"] = messages;
        if (!bos_token.empty()) in["bos_token"] = bos_token;
        if (!eos_token.empty()) in["eos_token"] = eos_token;
        if (add_generation_prompt) in["add_generation_prompt"] = true;
        if (extra.is_object()) {
            for (const auto& [k, v] : extra.items()) in[k] = v;
        }
        if (tmpl.wants_user_system_prompt && !in.contains("user_system_prompt")) in["user_system_prompt"] = "";
        // mark_input=true: everything that came in as data (messages, tool arguments, extra request
        // fields) is marked, so the tokenizer can refuse to parse special tokens inside it.
        jinja::global_from_json(ctx, in, /*mark_input=*/true);

        jinja::runtime runtime(ctx);
        const jinja::value result = runtime.execute(prog);
        auto gathered = jinja::runtime::gather_string_parts(result);
        std::string out = gathered->as_string().str();
        if (out.empty()) throw TemplateError("the template rendered an empty prompt" + ident);
        return out;
    } catch (const TemplateError&) {
        throw;
    } catch (const std::exception& e) {
        throw TemplateError(std::string("the container's chat template did not render: ") + e.what() + ident);
    }
#endif
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

2 participants