Skip to content

feat: surface reasoning-only truncation on the chat completions API - #1746

Merged
inureyes merged 2 commits into
lablup:mainfrom
bebekim:feat/surface-reasoning-only-to-api
Sep 11, 2026
Merged

feat: surface reasoning-only truncation on the chat completions API#1746
inureyes merged 2 commits into
lablup:mainfrom
bebekim:feat/surface-reasoning-only-to-api

Conversation

@bebekim

@bebekim bebekim commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds an additive reasoning_only: true field to chat completion responses (both streaming and non-streaming) when a generation produced tokens but content came back empty because everything stayed in the reasoning channel — today that case is silent and indistinguishable from a clean, intentionally empty response.

Related issues

Closes #1745
Refs #467
Refs #1721

(Neither #467 nor #1721 is a full duplicate: #467's fix only logs server-side, and only when the prompt itself primed an open thinking block; #1721 only fixed the CLI's generate/chat display. This generalizes past both to cover the far more common shape — a plain, unprimed request that opens its own <think> block and exhausts its token budget — and surfaces it to the actual API response, not just a log or a terminal.)

Type of change

  • feat — new user-visible feature

Test plan

  • cargo fmt --all -- --check
  • cargo clippy --workspace --all-targets --features metal,accelerate -- -D warnings
  • cargo test --workspace --profile test-fast --features metal,accelerate --no-fail-fast -- --test-threads=1 (full workspace: 10,605 passed, 0 failed)
  • cargo deny check
  • Validated with a real checkpoint: mlx-community/Qwen3.5-4B-MLX-4bit. Built baseline and patched release binaries from the same commit (stash/build/pop, mirroring ab_output_equality.sh's own pattern) and compared live server responses:
    • Truncated mid-thought, non-streaming: before → content:"", finish_reason:"length", no signal. After → same, plus "reasoning_only": true.
    • Truncated mid-thought, streaming: before → delta:{} on the finish chunk. After → delta:{"reasoning_only":true}.
    • Real completion (--reasoning-budget 40, reaches finish_reason:"stop"): field correctly absent in both paths, before and after — no regression on the common case.
  • N/A — no arithmetic/generation-numerics touched (pure response-shaping), so the teacher-forced logit trace doesn't apply here.

Notes for reviewers

  • Design choice: an additive boolean field rather than overloading finish_reason, matching the precedent already set for reasoning_content — existing OpenAI-schema clients that validate finish_reason as an enum are unaffected.
  • reasoning_stream::is_reasoning_only (added for feat(cli): say when a generation went entirely to the reasoning channel #1721) is reused as-is with show_reasoning: false fixed at both server call sites — the server never suppresses reasoning into a hidden channel, so passing false correctly fires whenever content is empty and generation happened, independent of priming.
  • Non-streaming: threaded through the two response arms where empty content is actually anomalous; deliberately not attached in the tool-calls-succeeded arm, where empty content is normal/expected by OpenAI convention.
  • Streaming: added StreamCallbackState::saw_content, updated from a single check over the finalized per-token chunk batch (chunk_carries_content) plus the end-of-stream flush, read once at the terminal chunk.
  • primed_thinking_unclosed keeps its original job (driving strip_unclosed_primed_thinking's content-emptying) — only the log that used to key off it directly was generalized.

Checklist

  • PR title uses a Conventional Commits prefix (feat:)
  • One logical change per PR
  • Updated docs/: N/A — no existing doc describes the chat completions response schema field-by-field
  • Updated // Used by: ... — extended is_reasoning_only's doc comment to name the new server call site
  • No secrets, credentials, or .env files committed

@cla-assistant

cla-assistant Bot commented Sep 10, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

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
@bebekim
bebekim force-pushed the feat/surface-reasoning-only-to-api branch from f4c8b34 to f6295ec Compare September 10, 2026 03:36
@inureyes inureyes added status:review Under review type:enhancement New features, capabilities, or significant additions priority:medium Medium priority area:inference Generation, sampling, decoding (incl. speculative, DRY) labels Sep 11, 2026
The new field asserts that a generation's output stayed in the reasoning channel, but both paths decided it from `is_reasoning_only` alone, which only means "raw text non-empty and shaped content empty". Two ordinary responses match that with no reasoning having happened.

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, ends with `saw_content` false and a non-empty `result.text`. The terminal chunk then 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: `reasoning_content` absent, yet still reported reasoning-only. Gemma 4 emits exactly this when a request carries no tools.

Both paths now require that reasoning reached the client. The non-streaming check gates on the shaped `reasoning_content`; the streaming side tracks `saw_reasoning_content` over the same finalized chunk batch as `saw_content`, flush included, and the finish-time decision moves into `stream_reasoning_only` so its three 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.

Reverting each gate separately fails exactly its own test and no others. fmt and clippy pass.

Refs lablup#1745, lablup#1746
@inureyes

Copy link
Copy Markdown
Member

Hardening pass on this PR

Reviewed against #1745 (and the #467 / #1721 framing). The design holds: an additive Option<bool> that is never Some(false), reusing is_reasoning_only so the CLI and the API agree, and excluding the non-streaming tool-calls arm were all the right calls. The type-level work, the skip_serializing_if omission, and the end-of-stream flush accounting are correct as authored. One commit on top, 88816432, closes two false positives.

Streaming tool calls were flagged as reasoning-only

is_reasoning_only reduces to "raw text non-empty and shaped content empty", which never checks that reasoning happened. FilterState::ToolCall suppresses the whole tool-call 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. That chunk carried reasoning_only: true beside finish_reason: "tool_calls". You excluded exactly this arm in the non-streaming path and called it out in the PR body; the streaming path had no equivalent, so the two surfaces disagreed on the same turn.

Marker-only output claimed a channel it never used

clean_structural_tokens empties content for a generation containing no thinking block at all. A Gemma 4 turn that is nothing but <turn|> / <channel|> / <|tool_call> markers, which its own doc comment says happens when a request carries no tools, leaves reasoning_content None and still reported reasoning-only.

Fix

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 saw_content already used, flush included. The finish-time decision moved into stream_reasoning_only, so its three conditions are testable without a live model. This cannot suppress a true positive: deepseek / auto populate reasoning_content, and none / deepseek-legacy keep thoughts in content, which is then non-empty and fails the emptiness check anyway.

Validation

31 targeted tests pass. Reverting each gate separately fails exactly its own test and no others. cargo fmt --all -- --check and cargo clippy -p mlxcel --lib --tests -- -D warnings both exit 0.

Not re-run here: your real-checkpoint check against mlx-community/Qwen3.5-4B-MLX-4bit. This box is Linux/CUDA, and the gates added are pure response-shaping with no generation numerics, so the live comparison you recorded still stands. Worth one streamed tool-call request against a tools-capable checkpoint before merge if you have one handy.

Security and performance: no auth, input-validation, or allocation behavior touched; the added work is one bool per chunk batch on a path that already scanned it.

Refs #1745

@inureyes
inureyes merged commit 4cfb08d into lablup:main Sep 11, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:inference Generation, sampling, decoding (incl. speculative, DRY) priority:medium Medium priority status:review Under review type:enhancement New features, capabilities, or significant additions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(server): surface reasoning-only truncation on /v1/chat/completions when a generation ends inside an open thinking block

2 participants