[bong-water-water-bong]

Refs #1786 (the GPT-OSS-20B-NPU2 row).

What was wrong. On --device npu, GPT-OSS-20B-NPU2 loads and answers, but forward-serve parsed its reply as qwen3 (answer markup=qwen3 at load). Measured reply to "What is the capital of France? Answer with one word.":

content='<|channel|>analysis<|message|>The user asks: "What is the capital of France? Answer with one word." The answer is "Paris". So we answer "Paris".'  reasoning=''

Change.

• app/npu_answer_parser.h: a third family, harmony. It is picked when the chat template writes <|channel|>analysis<|message|>; the pack's template does this five times, and no other family's template carries <|channel|>. Analysis is reasoning and final is content. The message headers (<|start|>assistant, <|channel|>final<|message|>, a commentary preamble's header, a rendered <|return|>) are dropped outside a call. to=functions.NAME … <|message|>{args}<|call|> is a tool call in both header orders, the template's and the model card's. Arguments that are not a JSON object go back into content unchanged.
• Two scanner fixes the new tags needed:
  • A tag may start with a byte other than < (the recipient to=functions.).
  • A whole tag at the end of the buffer is no longer split by holding back a tail that also starts another tag. <|start|>assistant ends in the t of to=, so a byte-at-a-time stream sent the header out as text.
• The load line prints family_name(), so it says harmony.
• docs/npu-chat-template.md: a table row and a paragraph.

Tests (tests/npu_sampling_test.cpp):

• test_harmony_answer_parser: the device reply byte for byte, whole and fed one byte at a time; a final header with no <|end|>; no analysis channel; both call orders; no tools; bad JSON.
• A template-detection case.
• harmony added to the every-tag, every-split-point arm.

Each scanner fix was checked by removing it: without the < widening the four call checks fail, and without the holdback rule the byte-at-a-time check fails.

ctest -j 6 without the device tests: 273/273 passed (skips as on main).

🤖 Generated with Claude Code

[bong-water-water-bong]

ready for review at ea878e2

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 1786 - Partially compliant Compliant requirements: It imports with zero gaps and its self-check passes against a named reference It serves on --device npu It answers a known-answer prompt with the model's own chat template, 3 identical requests returning identical text Non-compliant requirements: Decode tok/s and time-to-first-token measured under npu-hold The row names the kernel set and the engine commit that produced it Requires further human verification: Decode tok/s and time-to-first-token measured under npu-hold The row names the kernel set and the engine commit that produced it

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

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Tag parsing edge case The change to holdback function in npu_answer_parser.h attempts to fix a specific edge case where a tag ending in the buffer could be incorrectly split by holding back a tail that also starts another tag. This fix is important for ensuring correct parsing of the harmony markup, especially when processing byte-by-byte streams. However, the fix relies on a specific condition that checks if a tag ends the buffer and compares it against the tag's length, which could be fragile if not all edge cases are covered. // A whole tag that ends the buffer and reaches back past the held tail is that tag, not the start of another: // `<|start|>assistant` ends in the `t` that opens `to=functions.`, and holding the `t` back split the tag, so // neither half matched and the header went out as text (byte-at-a-time stream, harmony). for (const char* tag : tags) { const size_t n = std::char_traits<char>::length(tag); if (n > hold && buf.size() >= n && buf.compare(buf.size() - n, n, tag) == 0) return 0; } return hold; Harmony call parsing robustness The close_harmony_call function in npu_answer_parser.h attempts to parse tool calls in the harmony format. It uses a try-catch block to parse JSON arguments, but the logic for extracting the function name and arguments could be more robust. Specifically, the function name extraction relies on finding the first space or < character, which might not be sufficient for all valid function names, potentially leading to incorrect parsing of tool calls. bool close_harmony_call() { static const std::string msg = "<|message|>"; const size_t end = call_text_.find_first_of(" <"); const size_t m = call_text_.rfind(msg); if (end == 0 || end == std::string::npos || m == std::string::npos || m < end) return false; const std::string name = call_text_.substr(0, end); for (const char c : name) if (!(std::isalnum(static_cast<unsigned char>(c)) || c == '_' || c == '-' || c == '.')) return false; try { const nlohmann::json j = nlohmann::json::parse(call_text_.substr(m + msg.size())); if (!j.is_object()) return false; ToolCall c; c.id = "call_" + std::to_string(calls_.size()); c.name = name; c.arguments = j.dump(); calls_.push_back(std::move(c)); return true; } catch (const std::exception&) { return false; } }

[bong-water-water-bong]

VERDICT at ea878e2: READY

Reviewed at ea878e276aa04f17150c291990d8f357cc0caf78. It is on main (0 behind) and merges clean. The diff is 4 files, +152/−9.

Checked

• Detection. harmony is picked only when the template writes <|channel|>analysis<|message|>. That string is in no other family's template, so qwen3, gemma4 and the rest detect as before.
• The other families are unaffected.
  • The widened scanner test is safe[i] == '<' || safe[i] == tags_.call_open[0], and call_open[0] is < for every family except harmony.
  • drops_ is empty for non-harmony families.
  • The new holdback rule returns 0 only when a whole tag ends the buffer, and the scanner then matches that tag whole.
  • The every-tag, every-split-point arm, which covers all families, still passes.
• The call parser. close_harmony_call() takes the name up to the first space or <, checks its characters, and parses the JSON after the last <|message|>. A non-object or malformed body goes back into content unchanged.
• Tests. npu_sampling_test reports ok, and tools/copyright.py --check returns rc 0.

My mutants (each restored, and the test passes again)

mutant	result
`<	channel
the harmony template detection removed	FAIL "a template that writes the analysis channel is harmony"

Together with the body's two scanner mutants, the parser, the stream path and the detection are each pinned.

The measured device reply (content='<|channel|>analysis<|message|>…', before) is the author's run on GPT-OSS-20B-NPU2. I did not re-run it on the NPU; the test reproduces that reply byte for byte.

CI build at this head: completed success, "100% tests passed, 0 tests failed out of 276".

[bong-water-water-bong]

VERDICT: READY at ea878e2 — a third answer-parser family, chosen by the template's own markup, with the split-boundary arm I have been asking of this class all night.

The symptom is the class, and the measurement is the raw reply

"forward-serve parsed its reply as qwen3 (answer markup=qwen3 at load)" — so on --device npu GPT-OSS's harmony markup went straight into content:

content='<|channel|>analysis<|message|>The user asks: "What is the capital of France? …"'

That is the same family as #2064 (<|im_end|> served as text), #2172 (tools never reached the template) and #2144 (a third source of the stop-token class): the model is fine, the parser chose the wrong family, and the failure is visible only if someone reads the served text rather than the logits. answer markup=qwen3 at load is the line that says which choice was made — and it is why this was findable at all.

The fix is picked from the artefact, not from a table

"a third family, harmony. It is picked when the chat template writes <|channel|>analysi[s]…"

Selecting the parser by what the template actually emits is the right discriminator — the same conclusion #2242/#2254/#2261 reached for config keys: the container tells you which rules apply, and a hardcoded per-model table rots the moment a family gains a variant.

And the boundary case is handled explicitly

"A whole tag at the end of the buffer is no longer split by holding back a tail that also starts another tag."

That is the streaming hazard in its precise form: to avoid emitting half a tag you hold back a tail, and the bug is holding back too much — a complete tag at the buffer's end whose tail also begins another. Fixing it in npu_answer_parser.h and testing it "whole and fed one byte at a time" is the right pair: a byte-at-a-time feed is the only way to hit every split point, and test_harmony_answer_parser uses the device reply byte for byte rather than a synthetic string, so the fixture is the artefact.

Two asks

1. state what the parser does versus strips — content now presumably excludes the markup; say whether the analysis channel is dropped or surfaced separately (a reasoning_content field). The distinction matters for clients: dropping it makes the reply clean and loses information; surfacing it is a schema change. Either is fine; the PR should say which;
2. answer markup=harmony on the load line is the observable — worth one clause in docs/npu.md, because the next wrong-family bug will be diagnosed by grepping that line, and a reader who does not know it exists will debug the model instead.