Skip to content

feat(cli): say when a generation went entirely to the reasoning channel - #1721

Merged
inureyes merged 1 commit into
mainfrom
feat/reasoning-channel-notice
Sep 9, 2026
Merged

inureyes merged 1 commit into
mainfrom
feat/reasoning-channel-notice

Conversation

@inureyes

@inureyes inureyes commented Sep 9, 2026

Copy link
Copy Markdown
Member

Why

mlxcel generate suppresses the <think> channel by default. When a reasoning model's generation ends before the channel closes, every token is reasoning, the content channel is empty, and the CLI printed a blank line followed by its timing line. The model is working; the screen says nothing, and a blank screen is indistinguishable from a broken checkpoint or a broken patch.

That reading was one step away twice in one day. First on glm-4.1v-9b-thinking-4bit. Then on nvidia-nemotron-3-nano-30b-a3b-4bit, where the blank was the safety half of the granitemoehybrid RMS-norm A/B and attributing it to the arm under test would have inverted the verdict.

What changed

reasoning_stream::is_reasoning_only(generated_text, saw_visible_text, show_reasoning) names the condition: tokens were generated, none reached the content channel, and reasoning is suppressed. Both output paths consume it. The one-shot generate flow prints [All N generated tokens went to the reasoning channel; the content channel is empty. Re-run with --show-reasoning to see them.] in place of the blank. The chat REPL had the same hole on every turn and now prints the same line there. With --show-reasoning the predicate is false, because the text is already on screen.

scripts/ab_output_equality.sh is the output half of an A/B, run so that three recurring errors are not available. Sampling: a checkpoint's own generation_config.json can enable it with no flag from the caller, and then the files being compared are two samples rather than two implementations, so an untouched arm reads as a failure and a broken one can pass. The script passes --temp 0 to both arms and never takes it from the caller. The reasoning channel: comparing an empty content channel against an empty content channel passes while comparing no tokens at all, so both arms run with --show-reasoning. Attribution: a difference only belongs to the arm if the baseline reproduces itself, so the baseline runs twice as a control and a disagreement there reports INCONCLUSIVE (exit 2), pointing at the teacher-forced logit trace instead.

docs/benchmarks.md gains "The output half of an A/B" alongside the four conditions for judging a change that moves the numbers.

Validation

M1 Ultra, nvidia-nemotron-3-nano-30b-a3b-4bit at --temp 0. At -n 20 the generation truncates inside the channel and the new line prints; at -n 64 the channel closes, **Paris**. prints, and the line stays silent; non-thinking qwen2.5-7b-instruct-4bit stays silent. Three of three conditions behave, including both negatives.

The script was falsified against test doubles rather than only exercised: two identical binaries report EQUAL, a wrapper that alters the prompt reports DIFFERS, and a wrapper that alternates its prompt between invocations reports INCONCLUSIVE.

Five unit tests cover the predicate (unclosed thought with no answer, answer present, --show-reasoning on, nothing generated, whitespace-only content). cargo test -p mlxcel --lib reasoning_stream:: is 28 passing, cargo fmt --all -- --check is clean, and cargo clippy -p mlxcel --all-targets is clean. Workspace clippy is red on main for two unrelated lints in mlxcel-core, fixed separately in #1720.

A reasoning model whose generation ends before `</think>` has an empty content channel, and the default rendering suppresses reasoning, so `mlxcel generate` printed a blank line and its timing line. That is the model working normally, but on screen it is indistinguishable from a broken checkpoint or a broken patch. It cost an afternoon twice in one day: glm-4.1v-9b-thinking-4bit, then nvidia-nemotron-3-nano-30b-a3b-4bit while it was the safety half of the granitemoehybrid RMS-norm A/B, where reading the blank as breakage would have inverted the verdict.

`reasoning_stream::is_reasoning_only` names the condition and both output paths print it, the one-shot `generate` flow and the chat REPL, which had the same blank-turn hole on every turn.

`scripts/ab_output_equality.sh` covers the other half. It forces `--temp 0` (a checkpoint's own `generation_config.json` can enable sampling, making the comparison two samples rather than two implementations) and `--show-reasoning` on both arms, and runs the baseline twice as a control so an unstable checkpoint reports INCONCLUSIVE instead of DIFFERS.

Validated on M1 Ultra: nemotron at `-n 20` prints the line, at `-n 64` stays silent, non-thinking qwen2.5-7b stays silent; the script reports EQUAL, DIFFERS and INCONCLUSIVE against three test doubles.
@inureyes
inureyes merged commit 397adfe into main Sep 9, 2026
13 checks passed
bebekim added a commit to bebekim/mlxcel that referenced this pull request Sep 10, 2026
A reasoning model whose generation exhausts max_tokens or its
reasoning_budget before closing <think> leaves `content` empty with no
signal that anything else was coming. lablup#1721 named this condition for
the CLI's generate/chat REPL; lablup#467 logs it server-side, but only for
prompts that primed an open thinking block. Neither reaches the HTTP
API, which is what most real integrations actually hit.

Add reasoning_only: Option<bool> to ChatMessage and Delta, additive
and omitted unless true, set via a generalized check (any request,
primed or not) that reuses reasoning_stream::is_reasoning_only.

Closes lablup#1745
Refs lablup#467
Refs lablup#1721

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016fmmt7cCRCmiwmTbWrEq8x
inureyes pushed a commit that referenced this pull request Sep 11, 2026
Adds an additive `reasoning_only: true` to chat completion responses, streaming and non-streaming, when a generation produced tokens but `content` came back empty because everything stayed in the reasoning channel. That case was previously silent and indistinguishable from a clean empty response. The field is never `Some(false)` and is omitted otherwise, so existing clients see an unchanged wire shape.

Hardening on top closed two false positives. `is_reasoning_only` only means "raw text non-empty and shaped content empty", which never checks that reasoning happened, and two ordinary responses match it.

A streamed tool call is the bigger one. `FilterState::ToolCall` suppresses the whole payload from `delta.content`, so a model answering with nothing but a tool call, the ordinary shape, reaches the finish chunk with `saw_content` false and a non-empty `result.text`, which carried `reasoning_only: true` beside `finish_reason: "tool_calls"`. The non-streaming path already excluded its tool-calls arm by hand, so the two surfaces disagreed on the same turn. The second is output of nothing but structural markers, which `clean_structural_tokens` reduces to empty content with no thinking block to extract, leaving `reasoning_content` absent while still claiming reasoning-only; Gemma 4 emits exactly that when a request carries no tools.

Both paths now require that reasoning reached the client. Non-streaming gates on the shaped `reasoning_content`; streaming tracks `saw_reasoning_content` over the same finalized chunk batch as `saw_content`, flush included, and the finish-time decision moved into `stream_reasoning_only` so its conditions are testable without a live model. This cannot suppress a true positive: `deepseek` and `auto` populate `reasoning_content`, while `none` and `deepseek-legacy` keep thoughts in `content`, which is then non-empty and fails the emptiness check anyway.

Validated on a real checkpoint (`mlx-community/Qwen3.5-4B-MLX-4bit`) by the author for the original feature. The hardening adds 31 targeted tests; reverting each gate separately fails exactly its own test. 2,922 `server::` and `reasoning_stream::` tests pass under `test-fast`, with fmt and clippy clean.

Closes #1745

Refs #467, #1721
@inureyes
inureyes deleted the feat/reasoning-channel-notice branch October 6, 2026 12:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant