Skip to content

feat(api-server): stream model reasoning as reasoning_content in chat completions - #52606

Open
ladyada-piclaw wants to merge 1 commit into
NousResearch:mainfrom
ladyada-piclaw:feat/api-server-stream-reasoning-content
Open

feat(api-server): stream model reasoning as reasoning_content in chat completions#52606
ladyada-piclaw wants to merge 1 commit into
NousResearch:mainfrom
ladyada-piclaw:feat/api-server-stream-reasoning-content

Conversation

@ladyada-piclaw

Copy link
Copy Markdown

Summary

The /v1/chat/completions streaming path wired stream_delta_callback, tool_start_callback, and tool_complete_callback, but never reasoning_callback. As a result, a model's reasoning stream (e.g. Claude extended thinking, thinking_deltaagent._fire_reasoning_delta) was silently dropped for OpenAI-compatible frontends.

Open WebUI (and other OpenAI-compatible UIs) render a collapsible "thinking" block from delta.reasoning_content chunks — but nothing on this path emitted them.

Change

Thread reasoning_callback through the existing call chain and route reasoning deltas to a dedicated SSE chunk:

  • _create_agent and _run_agent gain a reasoning_callback parameter, forwarded to AIAgent (which already accepts and fires it via _fire_reasoning_delta).
  • In the streaming branch of _handle_chat_completions, a new _on_reasoning(text) enqueues a tagged ("__reasoning__", text) item (mirroring the existing ("__tool_progress__", …) pattern).
  • _emit gains an elif that renders that tagged item as a chat.completion.chunk carrying delta.reasoning_content.

Visible delta.content, the hermes.tool.progress events, and the end-of-stream sentinel are all unchanged. Empty/None reasoning chunks are skipped.

Real behavior / verification

  • Module imports cleanly; both _create_agent and _run_agent signatures carry reasoning_callback (verified via inspect.signature).
  • End-to-end chain confirmed in source: Anthropic streaming thinking_delta (chat_completion_helpers.py) → agent._fire_reasoning_deltaself.reasoning_callback (assigned in agent_init.py) → _on_reasoning_stream_q_emit.
  • Deterministic unit-level reproduction of the _emit branch confirms a tagged reasoning item serializes to {"delta": {"reasoning_content": "..."}} while normal content still serializes to {"delta": {"content": "..."}}.

Scope / safety

  • No new model tools; agent loop and prompt caching untouched.
  • Additive only (+23 lines, one file); content/tool-progress paths byte-identical when no reasoning is emitted.

… completions

The /v1/chat/completions streaming path wired stream_delta, tool_start, and
tool_complete callbacks but never reasoning_callback, so a model's reasoning
(e.g. Claude extended thinking) was dropped on the floor for OpenAI-compatible
frontends. Open WebUI renders a collapsible "thinking" block from
delta.reasoning_content chunks, but nothing emitted them.

Thread reasoning_callback through _create_agent and _run_agent, add an
_on_reasoning callback in the streaming branch that enqueues a tagged
("__reasoning__", text) item, and route that item in _emit to a
chat.completion.chunk carrying delta.reasoning_content. Visible content,
tool-progress events, and the end-of-stream sentinel are unchanged; empty
reasoning chunks are skipped.

Co-authored-by: ladyada <limor@ladyada.net>
@alt-glitch alt-glitch added type/feature New feature or request comp/gateway Gateway runner, session dispatch, delivery P3 Low — cosmetic, nice to have duplicate This issue or pull request already exists labels Jun 25, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.
Duplicate of #13401 — same mechanism and same file (gateway/platforms/api_server.py): thread reasoning_callback through the /v1/chat/completions streaming path and emit delta.reasoning_content SSE chunks. #13401 is the earlier (2026-04-21) canonical open PR. Broader siblings #39006 and #48024 are related. Maintainer picks one.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the focused Chat Completions reasoning-streaming fix. The premise is confirmed on current main: _handle_chat_completions forwards only text and tool callbacks at gateway/platforms/api_server.py:2311-2321, while _write_sse_chat_completion only special-cases tool-progress tuples at gateway/platforms/api_server.py:2499-2510. The agent-side callback is live: Anthropic thinking_delta reaches _fire_reasoning_delta in agent/chat_completion_helpers.py:2675-2679.

Problems

  • This PR changes only gateway/platforms/api_server.py; gh pr diff 52606 contains no regression test. Add coverage in tests/gateway/test_api_server.py for callback forwarding and the emitted delta.reasoning_content chunk.
  • The supplied discussion points to open #57094 as the broader active implementation, covering this path plus non-streaming Chat and Responses streaming. This PR remains a valid narrow fix, but consolidation should avoid landing overlapping callback plumbing independently.

Suggested changes

  • Mock _run_agent, invoke its received reasoning_callback, and assert reasoning and visible text serialize into their respective delta fields.
  • Add a forwarding-seam test through _run_agent and _create_agent.

This is an automated hermes-sweeper review.

@@ -2007,6 +2020,7 @@ def _on_tool_complete(tool_call_id, function_name, function_args, function_resul
stream_delta_callback=_on_delta,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please add an API-server SSE regression test that invokes this callback through the mocked _run_agent call and verifies it serializes as delta.reasoning_content rather than delta.content; the current PR changes no test files.

@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 15, 2026
@Addonniss

Copy link
Copy Markdown

Thanks for the review. #57094 (disclosure: my PR) already has tests covering both streaming delta.reasoning_content and non-streaming reasoning_content in the response body, plus Responses API support. It's been rebased onto latest main and all 198 tests pass. Happy to close this in favor of #57094 if that helps consolidate.

One ask: could the duplicate label be removed from #57094 so it gets a proper review? #13401 was already closed by its author in favor of #57094.

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

Labels

area/streaming Streaming responses: gateway delivery, provider wire comp/gateway Gateway runner, session dispatch, delivery duplicate This issue or pull request already exists P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants