[bong-water-water-bong]

Refs #1786 (the Qwen3.5-2B-NPU2 row; follows #2098, which is merged). Backlog, not a release blocker.

What was wrong, on the NPU

FastFlowLM's Qwen3.5-2B-NPU2 served with its own template, thinking on, "What is the capital of France? Answer with one word.", run to finish=stop, main with #2098:

content            (   7 chars): '\n\nParis'
reasoning_content  ( 553 chars): 'Thinking Process:\n\n1.  **Analyze the Request:**…'

and on the ChatML path, where the model thinks nothing, every answer in the #1785 rows came back as '\n\n\n\nItem 3 is', preceded by a streamed reasoning_content chunk that was two newlines (data: {"choices":[{"delta":{"reasoning_content":"\n\n"},…).

A model writes <think>\n…\n</think>\n\n and then its answer. Those newlines are layout. The splitter returned them as text on both sides of the block.

The change

AnswerStream drops the whitespace that leads the reasoning, and the whitespace that leads the content when that content follows a reasoning block. A block that holds only whitespace produces no reasoning at all. Whitespace inside either text is kept; content with no reasoning before it is returned exactly as written; content that came before a block keeps the space after the block.

This is the rule app/think_split.h already applies on the in-process route (its header: "whitespace around the block is dropped"), so the two routes return the same strings. parse_answer, the streamed path and content_mask share the one splitter, so they change together.

After, same request, binary from this branch

plain :  content (5 chars): 'Paris'    reasoning_content (553 chars): 'Thinking Process:\n\n1.  **Analyze the Request:**…'
stream:  content (5 chars): 'Paris'    reasoning_content (553 chars): (the same)

Two existing expectations change

Two arms in tests/npu_sampling_test.cpp pinned the newline after the opening tag as part of the reasoning. They assert the new rule now:

• qwen3: "<think>\nOkay, the user asked." gave reasoning "\nOkay, the user asked.", now "Okay, the user asked.";
• gemma4: "<|channel>thought\nlet me think<channel|>Done." gave "\nlet me think", now "let me think".

That is a visible change to reasoning_content for every thinking model on this route: its first character is no longer the newline the template puts after the tag. A client that depended on that newline would see a difference; I know of none, and the in-process route never sent it.

Tests

tests/npu_answer_parser_test.cpp, 11 new checks: the newline after the opener and the blank line after the closer are in neither text; an empty block gives no reasoning and the answer starts at its first character; content with no reasoning before it is untouched; content before a block keeps the following space; inner whitespace is kept; an answer that is only layout after the block is empty content; streamed, an empty block and its layout produce no chunk and the first chunk is the answer's first word; the per-token mask gives layout tokens no content entry; a tool call after the block and a newline is still a call; without tools the marker after the block is kept whole. all cases ok, npu_sampling: ok.

tools/copyright.py --check rc 0. Docs: docs/npu-chat-template.md, "Whitespace around the block is layout".

Limits

• Device run: one container, one question, the rendered-template path. The ChatML empty-block case is covered by the unit tests, not re-run on the device at this head.
• Trailing whitespace of the reasoning (the newline before </think>) is left as it is, as on the in-process route.
• "reasoning_format": "none" was not exercised.

🤖 Generated with Claude Code

[bong-water-water-bong]

ready for review at 8a14ad6

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 1786 - Partially compliant Compliant requirements: Every FastFlowLM/*-NPU2 model source imports and serves on --device npu 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 Decode tok/s and time-to-first-token measured under npu-hold Non-compliant requirements: The row names the kernel set and the engine commit that produced it Requires further human verification: The row names the kernel set and the engine commit that produced it 1785 - Partially compliant Compliant requirements: F5 — a single-turn "flash" serving mode on the NPU route (W8 / FLM 1.0.12 parity) It gains a single-turn mode with a fixed 1k context, clean state per request (no KV, recurrent or scratch carry-over between requests), and the system prompt prefilled once and reused It reuses the existing cache mechanism from PR npu: multi-prompt prefix cache (#650) — re-landing #738, which merged into its base branch and never reached main #745 It demonstrates that the per-request reset is complete (KV + recurrent state + scratch) Non-compliant requirements: It measures TTFT at 1k prompt tokens and decode tok/s, 3 identical requests returning identical bytes Requires further human verification: It measures TTFT at 1k prompt tokens and decode tok/s, 3 identical requests returning identical bytes

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

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Whitespace trimming logic The new whitespace trimming logic in AnswerStream::emit may inadvertently drop valid content if the first non-whitespace character is not properly detected. Specifically, the logic assumes that if text.find_first_not_of(" \t\r\n") returns std::string::npos, then the entire text is whitespace, which is correct, but the subsequent text.erase(0, b) call could be problematic if b is not correctly calculated. This could lead to incorrect trimming of content that starts with whitespace. const size_t b = text.find_first_not_of(" \t\r\n"); if (b == std::string::npos) return; // all whitespace so far: nothing to send yet text.erase(0, b); State management for trimming flags The logic for setting trim_content_ in AnswerStream::feed when a reasoning block closes may not be robust. If the content after the reasoning block is empty or only whitespace, it could lead to incorrect behavior in the next block or content handling. The logic should ensure that trim_content_ is reset appropriately when content is actually encountered. if (mode_ == Mode::reasoning && !saw_text_) trim_content_ = true;

[bong-water-water-bong]

VERDICT at 8a14ad6: READY

Whitespace at a reasoning boundary is layout, not content — and getting that wrong is visible to a user in a way most parser bugs are not: a stray blank line or a swallowed space at the seam between [think] and the answer is the first thing anyone notices in a demo.

The shape is right: emit() takes (is_reasoning, raw) and appends to an ordered vector<pair<bool, string>>, so the emitted segments keep their order and their reasoning/answer tag together rather than being reassembled by position. if (raw.empty()) return in front of the normalisation keeps an empty segment from becoming a whitespace-only one — the failure that would otherwise make "the model said nothing" indistinguishable from "the model said a space".

Lands with tests/npu_answer_parser_test.cpp and the sampling-test wiring, so the seam is pinned rather than eyeballed.

[bong-water-water-bong]

VERDICT: READY at 8a14ad6 — fresh configure rc 0, built and ran npu_answer_parser_test: 49 checks ok, all cases pass.

The defect is a one-word bug with a visible symptom, and the evidence is the served text itself:

content            (   7 chars): '\n\nParis'
reasoning_content  ( 553 chars): 'Thinking Process:\n\n1. **Analyze the Request:**…'

The model writes <think>\n…\n</think>\n\n and then its answer; those newlines are layout, and the splitter returned them as content. On the ChatML path (where the model thinks nothing) every answer came back as '\n\n\n\nItem 3 is'. For a demo that shows answers on screen, a leading \n\n is the difference between "it answered Paris" and "it answered a blank block and then Paris" — and for a streaming client it is a chunk of pure whitespace before the first word.

The arms I would have asked for are the ones present:

streamed, an empty block and its layout produce no chunk; the first chunk is the answer's first word
layout tokens after the block get no content entry; the answer's own do
a call after the block and a newline is a call with null content
the first text after the block is kept whole, whatever it starts with

The last one is the subtle half and the reason this is a parser change rather than a trim(): stripping leading whitespace globally would eat the answer's own indentation (a code block, a nested list) — the rule has to be "layout around the block, not layout in the answer", and that arm is what distinguishes the two.

One ask: an arm where the answer itself starts with whitespace that is meaningful — a fenced code block whose first line is indented (' def f():') or a nested list. The kept whole, whatever it starts with arm covers text, but not the case where the first characters are exactly the ones a lazy implementation would strip. It is the same distinction as template vs content in the earlier slices, one level down.

Scope of my check: fresh configure (rc 0), built the parser test in a scratch dir, ran it (49 checks). I did not run a serve: the served-text evidence above is the PR's own, from FastFlowLM's Qwen3.5-2B-NPU2 and the #1785 ChatML rows.

[bong-water-water-bong]

VERDICT: READY at 8a14ad6 — and the important part is the third clause: "Content with no reasoning before it is not touched." That is the property that keeps this a layout fix rather than a text mutation, and it is the one a future refactor will drop.

Three asks:

1. Test the leading-whitespace rule on a chunk boundary. "the whitespace that LEADS the reasoning is dropped" is unambiguous for a single chunk; the risk is a stream where the reasoning arrives as "\n" then "thought…" — the leading whitespace is the first chunk's entirety, so dropping it means dropping a chunk. If emit() can be handed an empty-after-trim chunk, the test should cover it: the stream must produce no empty reasoning chunk rather than a zero-length one (a zero-length chunk is what makes clients render an empty thinking bubble).
2. Say what happens to trailing whitespace after the reasoning block — the comment states the leading rule and the content-leading rule; <think>…</think>\n\nanswer has trailing whitespace between block and answer, which is the "leads the content" case (covered), but <think>…</think>\n\n at end of generation is a third case. Unspecified means implementation-defined, so name it: dropped, kept, or not reachable.
3. The cross-route identity is the reason this PR exists ("so the two routes return the same strings") — so the test should compare against app/think_split.h's output for the same input, not against hand-written expectations. A hand-written expectation drifts from the in-process route silently; a comparison to the declared source of truth cannot.

And one thing to note in the body: this makes the streaming route match the in-process route, which means a client that has only ever seen one of them will see different output after this lands. That is the intended change, but it is the kind of change that should be one sentence in the release notes rather than a discovery.