Repository navigation
docs: what rendering the NPU container's own chat template takes (engine #963 option 2) - #1115
Conversation
…ine #963 option 2) `app/forward_serve.cpp:176` renders ChatML for every container, and #987 refuses the chat route for a container whose tokenizer has no ChatML anchors — so a non-ChatML container cannot be chatted with at all today. #963's option 2 (render the container's own template) was left as an owner decision because it looked like a container-format change plus a new Jinja dependency. Measured on this box, it is neither: - the containers already carry their template (`chat_template.jinja` in an imported OGA pack, `tokenizer_config.json`'s `chat_template` in a FastFlowLM pack), and #1095 already reads it to log and pin its sha256; - the pinned llama.cpp fork already vendors a C++ Jinja engine (`common/jinja/`, ~6.2 kLOC, the one `llama-server` renders with), so the route can reuse it instead of growing a second implementation; - the templates we ship use a small part of Jinja: `for`/`if`/`set`+`namespace`/filters/attribute access/string methods/macros in 24/24, 23/24, 18/24 and 10/24 templates; `{% generation %}` and content-gated branches in **none**. Sizes 209 B - 16.7 KB. The note records that, the four shipped NPU catalogue rows with what changes for each (all four are ChatML-anchored today, so nothing is blocked — this is about serving each container the template it declares, including FastFlowLM's `user_system_prompt` branch, and about the next non-ChatML container), the security rules the renderer must carry, and the three decisions it asks for: reuse `common/jinja`, container-template -> ChatML-only-with-anchors -> 400 fallback order, and a template that fails to render being a 400 naming its sha rather than a silent fall back.
|
#1115 ready for review at 0a3d328 — doc-only (1 file, +125), no code and no behaviour change. It is the design note #963's option 2 says it needs: the three costs the issue assumed (container-format change, new Jinja dependency, a big subset of Jinja) measured against this box and against the containers we actually ship. Commented on #963 with the same summary so the two are linked. |
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY at 0a3d3280 — docs only, and the one finding I could check independently is true.
Finding 1 — "the containers already carry the template" — is the one I have evidence for, from verifying #1095 on this box: for the OGA packs we import, tokenizer_config.json carries no chat_template key at all and the template lives only in chat_template.jinja beside it (I ran the tool over the imported Qwen2.5-0.5B container: JSON branch dead, .jinja 2 561 B, sha 8aa40ce1…). So the substrate for #963 option 2 does exist, and the note's premise that this is "neither a container-format change nor a new Jinja dependency" starts from a fact rather than from an assumption.
A design note that exists to shrink an owner decision — "it looked like a format change plus a dependency; measured, it is neither" — is worth having even when it ships no code, because the cost of the decision is the thing that was blocking it.
No code, no behaviour change, one new file (docs/npu-chat-template.md), so there is nothing here to regress. I did not verify findings 2 and 3 (the renderer already being in the tree) beyond reading them.
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
…pen PR would add The reviewer's blocker, and it is the same class as the text being fixed: the paragraph said the digests for the four rows "are already measured — models/pins.json records the template of every file we serve and publish", and `models/pins.json` does not exist in this tree (`ls` fails, `git cat-file -e origin/main:models/pins.json` fails). It is engine #1148, open, so a reader on `main` was sent to a path that is not there, and the "already measured" claim rested on an unmerged working tree — including the one digest that exists nowhere else yet (`short`'s 57f1fd00f0…). Now the paragraph says what a reader can check today: - three of the four rows are in `config/template-pins.json` **now** — zaya1-8b-q4_k_m.gguf b55ecfbbbc…, qwen3-coder-30b-a3b-instruct-q4_k_m.gguf 87710339d2…, qwen3.8-27b-ud-q4_k_xl.gguf 12827f24b7… — because they are the same files the load-time check already pins (verified against the tree at this head); - the fourth, `short`'s Qwen3-0.6B-Q8_0.gguf 57f1fd00f0…, is **not in any table in this tree yet**; it is measured, it is added by #1158, and it is recorded with every other served/published file in #1148's models/pins.json — said as such rather than implied to be here. The Q4NX sentence now links docs/npu-chat-template.md, which is in this tree (#1115). Nothing else in the paragraph changed: what runs today, what it catches, and what step 3 adds all verified in the review.
…one that does not (#1157) * docs/laya-first: say which template check runs today, and that the per-row digest is step 3 The section ended with a present-tense claim the tree does not meet: that `~/.local/share/1bit/laya-first.json` "ships with the digests measured on 2026-11-05" and that "the table's validator (§8 step 3) fails" a row that skips one. Measured on main: `config/laya-first.json` — the file compiled in as `laya_first_default.h` — has no `template_sha256` in any of its four rows, nothing in `app/`, `src/`, `tests/`, `include/` or `os/` reads one, and no installed `<laya home>/laya-first.json` exists on the c25/c26 media, the staging trees or the box (§8 step 3 is where that field and the pull-time check are scheduled). A design note that asserts a check which does not run is the exact failure this row is supposed to prevent, so the paragraph now states what is true: - the one template check on a routed pull today is the load-time one, against the engine's `config/template-pins.json` — 7 names, three of them these rows; - that check warns unless the row ships `--template-pin-strict` (§9), and it catches one thing: the same name carrying a different template; - the field, the per-row requirement and the pull-time refusal are §8 step 3, and the digests they will need are already measured — `models/pins.json` records 57f1fd00f0… (short), b55ecfbbbc… (prose), 87710339d2… (code), 12827f24b7… (long_doc). Doc only; no code, no table and no operand changed. * docs/laya-first: cite the digests that are in this tree, not one an open PR would add The reviewer's blocker, and it is the same class as the text being fixed: the paragraph said the digests for the four rows "are already measured — models/pins.json records the template of every file we serve and publish", and `models/pins.json` does not exist in this tree (`ls` fails, `git cat-file -e origin/main:models/pins.json` fails). It is engine #1148, open, so a reader on `main` was sent to a path that is not there, and the "already measured" claim rested on an unmerged working tree — including the one digest that exists nowhere else yet (`short`'s 57f1fd00f0…). Now the paragraph says what a reader can check today: - three of the four rows are in `config/template-pins.json` **now** — zaya1-8b-q4_k_m.gguf b55ecfbbbc…, qwen3-coder-30b-a3b-instruct-q4_k_m.gguf 87710339d2…, qwen3.8-27b-ud-q4_k_xl.gguf 12827f24b7… — because they are the same files the load-time check already pins (verified against the tree at this head); - the fourth, `short`'s Qwen3-0.6B-Q8_0.gguf 57f1fd00f0…, is **not in any table in this tree yet**; it is measured, it is added by #1158, and it is recorded with every other served/published file in #1148's models/pins.json — said as such rather than implied to be here. The Q4NX sentence now links docs/npu-chat-template.md, which is in this tree (#1115). Nothing else in the paragraph changed: what runs today, what it catches, and what step 3 adds all verified in the review. --------- Co-authored-by: bong-water-water-bong <bong-water-water-bong@1bit.gg>
…te, not ChatML for every container (#1298) `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 Co-authored-by: x <x@x>
What
A design note for #963 option 2 (render the NPU container's own
chat_template), which that issueleft as an owner decision because it looked like a container-format change plus a new Jinja dependency.
Measured on this box, it is neither — so the decision is much smaller than it looked.
Doc only:
docs/npu-chat-template.md. No code, no behaviour change.The three findings that shrink it
chat_template.jinja; aFastFlowLM pack has
tokenizer_config.json'schat_template. serve: name a model directory's chat template too, from its tokenizer_config.json (closes #1091) #1095 (merged) already reads it to logand pin its sha256, so the substrate exists.
common/jinja/(~6.2 kLOC, the onellama-serverrenders with). Reusing it means no new dependency andthe same renderer our
hrx/cpuroute already trusts.for/if/set+namespace/filters/attribute access/string methods/macros cover all of them;{% generation %}and content-gated branches on a user message appear in none. Sizes 209 B – 16.7 KB.What the note records
app/npu_template.h, one call site (app/forward_serve.cpp:176), ChatMLkept only for a container with no template and anchors, npu: the chat route refuses a container whose tokenizer cannot represent ChatML (measured: 200 with the wrong prompt -> 400 naming the model's specials) #987's 400 kept for neither,
include/import) on top of
common/jinja's input marking, plus the sha line + pin from serve: name a model directory's chat template too, from its tokenizer_config.json (closes #1091) #1095,ChatML-anchored); this is about serving each container the template it declares (e.g. FastFlowLM's
user_system_promptbranch, which the ChatML fallback silently drops) and about the nextnon-ChatML container being servable at all,
common/jinja; fallback order container-template → ChatML onlywith anchors → 400; and a template that fails to render is a 400 naming its sha, never a silent fall
back to ChatML.
Why this lane
The template audit (
AUDIT-1, and the load-time line + pin in #1089/#1095) established what thecontainers actually carry and made the template visible and checkable. This is the other end of the same
seam: the route that runs them.