Skip to content

forward-serve: the chat route renders ChatML for every container, so a non-ChatML model is prompted with text its tokenizer cannot accept (33 vs 15 tokens, measured) #963

Open

Description

@bong-water-water-bong

Found and measured while running the Zyphra family on the NPU route (~/briefs/T1BP-ZYPHRA-STATE.md, 2026-11-03 21:0x).
Not a regression — app/npu_chatml.h was introduced deliberately by #832 for the Qwen3 lane — but the route applies it to
every container, so a model whose tokenizer is not ChatML is silently prompted with the wrong text and every chat-route
number for it is meaningless.

Where

app/forward_serve.cpp:167 (origin/main c43ada7f):

        if (chat) prompt = npu::chatml(body.at("messages"), npu::thinking_from_body(body));

npu::chatml() (app/npu_chatml.h:44) emits <|im_start|>{role}\n{content}<|im_end|>\n … <|im_start|>assistant\n
unconditionally. Nothing reads the container's own chat template; the other byte-level knobs (add_bos_token,
add_eos_token) do come from config.json (:724), so the container already carries per-model behaviour — the template is
the only one that is hardcoded.

Measurement (Zyphra/ZR1-1.5B, Qwen2ForCausalLM weights, DeepSeek-style specials <|begin▁of▁sentence|>)

Same weights, same request, chat route vs llama.cpp's own template:

Options, cheapest first

  1. Honesty, small (~30 lines): when the container's tokenizer has no ChatML anchors (no <|im_start|>/<|im_end|> among
    its added/special tokens in config.json's tokenizer metadata), the chat route answers 400 naming the model's real
    specials and pointing at /v1/completions with token ids (forward-serve: /v1/completions takes the prompt as token ids, so a route parity run measures the forward #938). Turns silent wrongness into a visible refusal; changes no
    working Qwen3/Qwen2-Chat row.
  2. Real fix (needs a decision): pack the model's chat_template from tokenizer_config.json into the container and render
    it. That is a Jinja subset ({% for %}, {% if %}, filters) — small for the templates we actually ship, but a new
    dependency-shaped piece of the container format, so it belongs to an owner call rather than a night.
  3. Documentation-only: say in docs/npu.md that the NPU chat route is ChatML-only. Cheapest, leaves the trap in place.

Filed rather than patched because (1) is a behaviour change on a route other lanes serve from, i.e. an owner call.

Activity

  1. bong-water-water-bong commented on Nov 4, 2026

    @bong-water-water-bong
    CollaboratorAuthor

    Claiming this: it is this lane's own code, and I am taking option 1 tonight

    app/npu_chatml.h came from #832, which is this lane's (pi-npufeat, the Qwen3 answer/logprobs work). So the
    hardcoded ChatML is my footprint, not a legacy quirk: I introduced it with a comment that justified it with "this
    file parses and masks every answer as AnswerFamily::qwen3" — true of the parser, and not a licence to prompt
    another family's model with Qwen3's template. That is the part I got wrong.

    Tonight's branch is 1bit/npu-963-chatml-guard: option 1, the refusal, because it is the smallest change that
    stops the route answering plausibly from the wrong prompt, and because none of it touches a container that carries
    the anchors. Option 2 (pack chat_template and render a Jinja subset) stays yours — it changes the container
    format, which is a decision, not a night's patch. Option 3 alone would leave the trap armed, so it is not what I am
    proposing; the docs get the paragraph either way.

    Reproduced the tokenizer half, device-free (no ticket needed, no device opened)

    Same request shape through npu::chatml() — one user message, hi — tokenized by each container's own
    tokenizer.json. Two rows, and the second one is the mechanism:

    | container | rendered prompt | <|im_start|> alone | chatml_supported() |
    |---|---:|---|---|
    | Zyphra/ZR1-1.5B (DeepSeek specials) | 23 tokens | 6 pieces | no |
    | Qwen3-0.6B (ChatML) | 9 tokens | 1 token, id 151644 | yes |

    Your 33-vs-15 is your request's shape (and llama.cpp's own template on the other side); mine is one message and the
    Qwen3 render as the yardstick. Direction and cause are the same: the marker is not a token in that vocabulary, so
    the prompt the model reads is not the prompt the caller wrote.

    What the guard does, and what it does not claim

    The check is token_id("<|im_start|>") >= 0 && token_id("<|im_end|>") >= 0 on the container's own tokenizer, done
    once at load, so a request pays nothing for it. A container without them gets a 400 on
    /v1/chat/completions naming the model's own bos/eos specials and pointing at /v1/completions with token ids
    (#938) — that route is untouched and is the correct way to serve such a container, since the caller can render the
    model's own template and send ids. /v1/systemone renders the d3 pack's own template and is unaffected.

    Deliberately not claimed: anchors present is a necessary condition, not proof the container's template is
    ChatML. A model could carry both and use something else, and only option 2 closes that.

    Served rows (400 on the chat route, prompt: [ids] still 200, Qwen3 unchanged) go on the PR with the ticket
    ledger. Nothing merges tonight, so this cannot change any lane's measurements before the owner sees it.

  2. bong-water-water-bong commented on Nov 4, 2026

    @bong-water-water-bong
    CollaboratorAuthor

    PR #987 — 1bit/npu-963-chatml-guard at 3cca1fa0: option 1, the refusal. Served rows below are from the same
    reproducer before and after, not from the prose.

    Reproduced first on the pre-change binary (my two app/ files reverted to eda7542e, strings … | grep -c 'chat route disabled' = 0 to prove which binary it was), then the same requests on the fixed one. Container A = Qwen3-0.6B as
    shipped; container B = the same 0.6B weights/plan with Zyphra/ZR1-1.5B's tokenizer.json, tokenizer_config.json
    and its bos/eos ids, since the ZR1 container has no full_i8 ELF set on this box to bring its own forward up.

    row (host GEMM, 1 request each) before after
    A chat (ChatML container — must not move) 200, prompt_tokens 9 200, prompt_tokens 9
    B chat 200, `content: "< ", prompt_tokens` 23
    B chat, stream: true 200 SSE, `delta.content: "< "`
    B /v1/completions, prompt: [ids] 200 200 — the way out is untouched
    stderr at load (nothing) one chat route disabled: … line

    That prompt_tokens: 23 from the old binary is exactly the 23 the device-free tokenizer measurement gives for the same
    request, so the served and tokenizer halves are one fact seen twice; the Qwen3 container's 9 is unchanged from before
    the change.

    Tickets: two, -l npufeat, queued 522 s / 252 s and held 16 s / 22 s, both released cleanly and both commands
    exited 0 well inside -t 12; amdxdna ERROR lines 0 in both windows. ctest -R 'npu_chatml|npu_sampling' passes,
    copyright.py --check rc 0.

    Two things stated in the PR rather than glossed: the check is a necessary condition (a container can carry both
    anchors and still use another template — option 2 is what closes that, and it stays yours as a container-format
    decision), and app/unified.cpp:112 has its own chatml() for the llama.cpp lane, so the same trap exists there for a
    non-ChatML GGUF. That file is not this lane's and I did not touch it; flagging it rather than leaving it unsaid.

  3. bong-water-water-bong commented on Nov 5, 2026

    @bong-water-water-bong
    CollaboratorAuthor

    Status, so this does not read as unstarted: option 1 is shipped and option 3 is documented; only option 2 is left, and it
    is the one that needs a decision.

    Option 2 (render the container's own chat_template) is not started on purpose: as this issue says, it is a Jinja
    subset plus a new field in the container format — i.e. a container-format decision, not a night's patch. I have put it in
    front of the orchestrator as an owner call with the cheap scoping (which templates we actually ship, and whether the
    template travels in the container or is rendered at import time). If the answer is yes, the shape I would build is: the
    template text carried per container, a small renderer with the tags our shipped models use ({% for %}, {% if %},
    {{ }}, eos_token/bos_token, add_generation_prompt), the ChatML path kept as the fallback for containers without a
    template, and a device-free test per template family.

    Nothing here blocks the Qwen2/Qwen3 rows: they are ChatML and unaffected.

  4. bong-water-water-bong commented on Nov 5, 2026

    @bong-water-water-bong
    CollaboratorAuthor

    Option 2 now has a measured design note instead of an estimate: #1115 (docs/npu-chat-template.md).

    Three of its stated costs turn out not to exist on this box:

    1. No container-format change is needed — the containers already carry the template: an imported OGA pack has chat_template.jinja (/tmp/844-out-clean: 2561 B, 8aa40ce145adb73c…), a FastFlowLM pack carries it as tokenizer_config.json's chat_template (~/Qwen3-0.6B-NPU2). serve: name a model directory's chat template too, from its tokenizer_config.json (closes #1091) #1095 (merged) already reads exactly that file to log and pin its sha256.
    2. No new dependency — the pinned llama.cpp fork vendors a C++ Jinja engine in common/jinja/ (~6.2 kLOC, upstream's own, the renderer llama-server uses). Reusing it is the same renderer our hrx/cpu route trusts.
    3. The templates use little of Jinja — census over the appliance catalogue's 24: for/if/set+namespace/filters/attribute access/string methods/macros cover all; {% generation %} and content-gated branches in none; sizes 209 B – 16.7 KB.

    The note also records that no shipped NPU row is blocked today (all four catalogue containers are ChatML-anchored, so #987's refusal never fires for them) — the value is serving each container the template it declares (FastFlowLM's user_system_prompt branch is silently dropped by the ChatML fallback today) and being able to serve a non-ChatML container at all.

    It asks for three decisions: reuse common/jinja; fallback order container-template → ChatML-only-with-anchors → 400; and a template that fails to render being a 400 naming its sha rather than a silent fall back. If (1) is yes, the implementation is one PR.

  5. bong-water-water-bong commented on Nov 6, 2026

    @bong-water-water-bong
    CollaboratorAuthor

    Disposition: parked on an owner decision — it is not blocked on work.

    Unpark: say yes/no to option 2. If yes, #1115 is the brief and the existing chatml_ok/chatml_refusal tests are the gate.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions