Skip to content

npu serve: usage says how many completion tokens were reasoning (refs #2111) - #2131

Merged
bong-water-water-bong merged 1 commit into
mainfrom
cl-review/2111-reasoning-tokens
Nov 11, 2026
Merged

bong-water-water-bong merged 1 commit into
mainfrom
cl-review/2111-reasoning-tokens

Conversation

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator

Refs #2111.

The problem

An empty content with finish_reason: "length" reads like "the model said nothing" when the whole budget went into a thinking block. The issue's table has the case: max_tokens: 8, content: "", completion_tokens: 8. It also produced a wrong bug report (#1998).

On main the text was not lost. Both NPU chat routes keep an unclosed thinking block as message.reasoning_content (AnswerStream::finish on forward-serve, ThinkSplit::flush on the unified route). What the reply lacked was the count that says where the budget went.

What this does

Both NPU chat routes report OpenAI's own field, additively:

"usage": { "completion_tokens": 8, "completion_tokens_details": { "reasoning_tokens": 8 }, ... }
  • forward-serve: each generated token's text is now kept for every request, not only with logprobs. The count runs the same AnswerStream that splits the reply (npu::reasoning_token_count, next to content_mask), so the count, the split and the logprob entries cannot disagree. It applies to non-streamed replies and to the streamed usage chunk. /v1/completions is raw text and gets no details.
  • unified route: the generated ids go through that route's ThinkSplit (think_reasoning_tokens), with the end token excluded as completion_tokens excludes it. The count is 0 when nothing is split (thinking off, or reasoning_format: none).

The rule: a token is reasoning when none of its text reached content and the block was open before it, open after it, or it wrote text into it. Leading tokens that produced nothing join the block if the next token opens it. So these count:

  • both tags;
  • a tag split across tokens;
  • an unclosed block at the end of the budget.

These do not:

  • the answer;
  • tool-call markup;
  • the layout whitespace after a closed block;
  • a token that carries </think> and the first answer text (that one is content, as content_mask already says).

Tests (host)

  • npu_answer_parser_test: +10 arms, all cases ok. They include the issue's 8-of-8 row (and that its content is empty while reasoning holds the text), a closed block, a split tag, a straddling token, a block opened after content, a tool call after a block, an empty reply, and "reasoning tokens and content entries never overlap".
  • think_split_test: +6 arms, all PASS.
  • Mutants of the rule, each rebuilt and run:
mutant result
no "before" (closing tag not counted) FAIL "a closed block …"
no "after" (opening tag of a later block not counted) FAIL "a block opened after content …"
a token that reached content counted FAIL "a token carrying the closing tag and the first answer text is content"
no pending (first half of a split opening tag not counted) FAIL "a tag split over two tokens …"
  • app/forward_serve.cpp and app/unified.cpp compile clean as single-file compiles with the NPU build's flags. tools/copyright.py --check rc 0.

Draft until

I have not yet run it on a device. Before marking this ready I will serve one small thinking model under npu-hold and post the usage object for max_tokens: 8 (expect reasoning_tokens == completion_tokens, content: "") and for a budget that finishes (expect reasoning_tokens < completion_tokens).

Docs: docs/serve.md, next to cached_tokens.

🤖 Generated with Claude Code

…2111)

An empty content with finish_reason "length" read like "the model said nothing" when the whole budget went into a
thinking block (#2111; it produced #1998 as a wrong bug report). The text was already returned, as reasoning_content,
on both routes: what was missing is the count. Both NPU chat routes now report OpenAI's
usage.completion_tokens_details.reasoning_tokens, additively:

- forward-serve: each generated token's text is kept (not only with logprobs), and the count runs the same
  AnswerStream that splits the reply (npu::reasoning_token_count, beside content_mask), non-streamed and in the
  streamed usage chunk. /v1/completions carries no details.
- unified route: the generated ids (end token excluded, as completion_tokens excludes it) through this route's
  ThinkSplit (think_reasoning_tokens); 0 when nothing is split.

A token is reasoning when none of its text reached content and the block was open before it, after it, or it wrote
into it; leading tokens that produced nothing join the block if the next token opens it. So both tags, a split tag,
and an unclosed block at the end of the budget count, while tool-call markup and the layout after a block do not.

Tests: npu_answer_parser_test (+10 arms, incl. the issue's 8-of-8 row and a block opened after content) and
think_split_test (+6). Mutants of the rule (no "before", no "after", content counted, no pending) each fail an arm.
docs/serve.md: what the field means and what to do about it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Nov 11, 2026 •

Copy link
Copy Markdown

PR Reviewer Guide 🔍

(Review updated until commit 32c0386)

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

2111 - Partially compliant

Compliant requirements:

  • Add OpenAI's usage.completion_tokens_details.reasoning_tokens field to chat replies on NPU
  • Distinguish between empty content with finish_reason "length" due to budget running out in thinking block vs. model producing no content
  • Ensure the information is additive and doesn't break existing clients
  • Update documentation to reflect the new field and its meaning

Non-compliant requirements:

  • None

Requires further human verification:

  • None

1998 - Partially compliant

Compliant requirements:

  • None

Non-compliant requirements:

  • Fix empty completions issue on NPU for bonsai2-27b-NPU model
  • Ensure the model actually produces content instead of returning empty strings

Requires further human verification:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Token Counting Logic

The logic for counting reasoning tokens in forward_serve.cpp may have edge cases where tokens that straddle the boundary between content and reasoning are not consistently classified. Specifically, the interaction between last_counted and the final flush logic needs careful review to ensure that tokens like <tool_call>Paris are correctly classified as content rather than reasoning.

        for (size_t i = 0; i < logits.size(); i++) e.tok_text.push_back(e.tok->decode(int(i)));
    }
}
// allowed[i] says token i can be appended without leaving the grammar.  The first
// byte test rejects almost every token cheaply (a JSON object at the start accepts
// a handful of first bytes out of 256), so only the survivors pay a full feed().
// The ids a constrained answer may end on: the container's own eos ids plus <|endoftext|>.
std::vector<int> end_ids = e.eos;
if (e.endoftext >= 0) end_ids.push_back(e.endoftext);
// Ids at or beyond the tokenizer's vocabulary are padding positions, not tokens: decode()
// returns "" for them, which used to make them indistinguishable from a byte fragment and so
// unconditionally allowed (engine#1182: the model drifts into them once the grammar leaves it
// nothing to say, and they emit no text and never end the reply).
const size_t vocab = size_t(e.tok->size());
auto refresh_allowed = [&]() {
    allowed.assign(logits.size(), 0);
    const npu::ByteMask first = gram->next_bytes(grammar_state);
    for (size_t i = 0; i < logits.size() && i < vocab; i++) {
        const std::string& t = e.tok_text[i];
        if (t.empty()) {
            allowed[i] = 1;  // a half-character byte fragment cannot leave the grammar
        } else if (first.has(static_cast<unsigned char>(t[0]))) {
Token Counting Consistency

The think_reasoning_tokens function in unified.cpp uses a similar logic to reasoning_token_count but may have subtle differences in how it handles the final flush and pending tokens. This could lead to inconsistencies between the two counting methods, especially when dealing with tokens that are partially consumed.

int queue_ms_plain = 5000;

// reused: prompt tokens already on the device (a private route's); cache: where from,
// empty when none.
npu::GenerateResult generate(const std::vector<int>& ids, const npu::GenerateOptions& opt, size_t& reused,
                             std::string& cache) {
    reused = 0;
    cache.clear();
    if (lane) return npu::generate(*lane, *model, ids, opt);
    npu::PrivateRequest pr;
    pr.max_tokens = opt.max_tokens;
    pr.repetition_penalty = opt.repetition_penalty;
    pr.penalty_window = opt.penalty_window;
    pr.on_token = opt.on_token;
    auto r = priv->generate(ids, pr);
    npu::GenerateResult out;

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT: READY at 32c0386 — and it fixes the exact ambiguity that made my demo gate's A4 assertion blunt.

+    // engine#2111: each generated token's text, in order, and how many of them were reasoning. -1 = not counte[d]
+    // /v1/completions reply is raw text with no split), so the usage object then carries no details at all.
+    int reasoning_tokens = -1;

Why "empty content + finish_reason: length" needed this

My DEMO-SMOKE.md A4 asserts content is non-empty and not one character repeated, and it exists because Qwen3 thinks by default: the reply can be empty while the model produced plenty, with every token spent inside reasoning_content and finish_reason: "length". Without a count, that case is indistinguishable from "the model said nothing" — and the operator's next move (raise max_tokens? turn thinking off? suspect the model?) depends on which it was. Reporting how many completion tokens were reasoning converts the ambiguity into a number.

Three things that are right in the shape

  • -1 for "not counted" rather than 0 — an absent measurement is not a measurement of zero, which is the distinction this whole day has been about;
  • the /v1/completions note ("raw text with no split … so the usage object then carries no details at all") is the coverage boundary stated where it is read, so nobody infers reasoning_tokens == 0 from a route that could not count it;
  • the per-token pieces are kept in order, so the count is derivable from the same data the text came from rather than counted twice.

Two things to pin

  1. the semantic: does completion_tokens include reasoning tokens, or are they disjoint? OpenAI's shape reports completion_tokens_details.reasoning_tokens as a subset of completion_tokens, and a client that assumes disjointness will compute a wrong "visible tokens" figure. One line, and one arm with reasoning_tokens > 0 and completion_tokens == reasoning_tokens (the pure-thinking reply) versus one where the visible part is non-zero;
  2. make A4 sharper with it: when content is empty and reasoning_tokens > 0, the demo gate's answer should be "SKIP with reason: the budget went to thinking" rather than FAIL on degeneracy — the honest reading, and it needs this field to be sayable. I will update ~/briefs/DEMO-SMOKE.md to prefer it once this is in; flagging it here so the two land consistent.

Nothing else — this is the difference between an empty answer and an unexplained one, and it costs a counter.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

Device check at 32c0386, as promised in the body. One npu-hold ticket (95 s of device time), on this box.

Setup. This branch was built with ONEBIT_NPU=ON and the host path, gcc-16, Release. It served 1bit forward-serve -m ~/models/npu/Qwen3-0.6B/model.q4nx --kernels ~/1bit-MONSTER-iso-build/engine/npu/xclbins on a loopback port. The log said ready … (full ELF) [npu-target=xdna], and temperature: 0 was used for every request. The serve was started by PID inside the ticket and stopped by a trap when the run ended; none was left running.

request finish_reason content reasoning_content completion_tokens reasoning_tokens
max_tokens: 8 (the issue's row) length "" 24 chars 8 8
max_tokens: 200 stop The capital of France is **Paris**. 486 chars 123 113
max_tokens: 40, enable_thinking: false stop The capital of France is **Paris**. — 9 0
max_tokens: 8, streamed, include_usage — — — 8 8

The 8-token row is #2111 exactly: content is empty and finish_reason is length. The reply now says that all 8 tokens were reasoning, and the text is in reasoning_content. The streamed usage chunk carries the same count as the non-streamed reply. With thinking off the count is 0. On a finished answer, the count is 113 of 123: the tags and thinking count, the 10 tokens of layout and answer after the block do not.

Limit: this build has no private route (ONEBIT_NPU_DX=OFF), so the unified route's half is covered by think_split_test (+6 arms) and a compile, not by a device run.

Marking this ready for review.

@bong-water-water-bong
bong-water-water-bong marked this pull request as ready for review November 11, 2026 06:46
@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit 32c0386

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review at 32c0386 — no verdict token (author's lane). I looked for the three ways this could be wrong and it is right in all three, so this is a short review with one convention question.

1. The count cannot drift from the text, because it is computed from the same pieces by the same state machine. reasoning_token_count(pieces, family, tools_sent, starts_in_reasoning) reads the block state before and after each token, so "how many tokens were reasoning" is a property of the stream that was already being split — not a second implementation that has to agree with AnswerStream/ThinkSplit. Both routes call it over stats.pieces, and the comment says so in the place a maintainer will look.

2. A route that cannot count says nothing rather than zero. reasoning_tokens = -1 means not counted, and the field is only added when >= 0 — so a raw /v1/completions reply (no split, no pieces) omits completion_tokens_details instead of asserting reasoning_tokens: 0 about a reply nobody inspected. That distinction — "zero" vs "not measured" — is the same one that makes cancelled ≠ failure in #2077 and 77 (skip) ≠ 0 in #2076; getting it right here by default is worth noting.

3. It reaches streamed replies, which is the case most clients see. The field is added inside the one usage_json builder (:604-619), which both the non-streamed reply (out["usage"] = usage_json(stats), :683) and the streaming tail chunk (tail["usage"] = usage_json(stats), :785) call. Had each path built its own usage object — the usual way these drift — a stream client with content: "" would still have shown nothing, i.e. the bug this PR exists to fix would have survived in the common path.

The arms are adversarial rather than illustrative, which is why I believe the above:
eight → 8 (the reported case: budget 8, all reasoning); <think>,\n,why,</think> → 4 (tags count with the block); <th,ink>,plan,</th,ink>,Paris → 5 (a tag split across token boundaries — the case a naive prefix check gets wrong); <think>,plan,</think>Paris → 2 (block opens and closes inside one piece); Hi.,<think>,x,</think>,Bye → 3 (text before a block); no block → 0; empty → 0.

One convention question, not a defect: tag tokens count as reasoning tokens. That is what makes reasoning_tokens == completion_tokens the true sentence in the reported case, and it is documented in the code — but it is user-visible arithmetic: a client that counts characters of reasoning_content will not see the same number, because the markup tokens are in the count and not in the text. Worth one line in the body (or docs/serve.md) saying that reasoning_tokens counts generated tokens spent inside the block, including the tag tokens, so the next person to reconcile the two numbers knows which convention they are looking at.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT at 32c0386: READY

Counting reasoning tokens separately is the reporting half of #2118's parser fix, and it matters for the same reason: a demo whose usage block reports a total while the answer is a fraction of it reads as a slow model rather than a verbose one.

The comment pins the definition to the mechanism — stats.pieces.push_back(piece); // engine#2111: reasoning_tokens is counted over these — so the number is derived from the same pieces the parser emits, not recomputed by a second rule that could disagree. int reasoning_tokens = -1; as the unset value is also right: -1 is distinguishable from "zero reasoning tokens", which a default of 0 would hide.

think_split.h extracted as its own header means the split is testable apart from the server, and docs/serve.md stating what the usage fields mean is what keeps a caller from guessing.

@bong-water-water-bong
bong-water-water-bong merged commit 539a77f into main Nov 11, 2026
10 checks passed
@bong-water-water-bong
bong-water-water-bong deleted the cl-review/2111-reasoning-tokens branch November 11, 2026 07:04
@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

On the convention question in the 07:01Z review: the merged docs answer it. docs/serve.md:529 says "Both tags of the block count as reasoning; the answer and tool-call markup do not". So reasoning_tokens is generated tokens spent inside the block, tags included, and it will not equal a character or token count of reasoning_content alone. That is deliberate: it is what makes reasoning_tokens == completion_tokens true in #2111's case, where the opening tag was one of the 8 tokens. Thanks for the review.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant