Skip to content

feat(api-server): surface reasoning_content on /v1/chat/completions - #74692

Open
adaofeliz wants to merge 1 commit into
NousResearch:mainfrom
adaofeliz:feat/chat-completions-reasoning-content
Open

feat(api-server): surface reasoning_content on /v1/chat/completions#74692
adaofeliz wants to merge 1 commit into
NousResearch:mainfrom
adaofeliz:feat/chat-completions-reasoning-content

Conversation

@adaofeliz

Copy link
Copy Markdown

Problem

/v1/runs and /v1/responses already relay reasoning.available previews as structured events, but /v1/chat/completions never wired a callback to receive them — so reasoning/thinking text is silently dropped for that endpoint, even though it's the one most OpenAI-SDK clients (Open WebUI, plain openai SDK, etc.) actually use.

Fix

Add a reasoning.available-filtered tool_progress_callback to both the streaming and non-streaming paths of _handle_chat_completions:

  • Streaming: emits delta.reasoning_content chunks — the same field name DeepSeek/Moonshot/OpenRouter already use for thinking traces — as a separate SSE data: line, distinct from delta.content and the existing event: hermes.tool.progress tool-lifecycle events.
  • Non-streaming: accumulates previews into message.reasoning_content on the final JSON response.

Tool-call events are unaffected: the new callback only forwards reasoning.available and ignores every other tool_progress_callback event type, so tool_start_callback/tool_complete_callback still own the tool-lifecycle channel exactly as before (no duplicate emits, per the prior comment this PR updates).

Testing

  • Added test_stream_includes_reasoning_content and test_non_streaming_includes_reasoning_content to tests/gateway/test_api_server.py::TestChatCompletionsEndpoint, mirroring the existing test_stream_includes_tool_progress pattern.
  • python3 -m pytest tests/gateway/test_api_server.py -q → 87 passed (same 7 pre-existing failures as unmodified main, unrelated to this change — missing dotenv/network fixtures in a fresh checkout).
  • ruff check clean on both changed files.
  • Manually verified end-to-end against a running gateway (/v1/runs, which shares _create_agent/_run_agent) that reasoning surfaces without any tool-call leakage.

Note

On an Idempotency-Key cache hit (non-streaming), reasoning_content will be absent even if the original computation had reasoning — the idempotency cache only stores (result, usage), not the side-channel reasoning previews. This mirrors how streaming chunks also aren't replayed on cache hit, so it's a pre-existing limitation of that cache's shape rather than something this PR needs to solve.

/v1/runs and /v1/responses already relay reasoning.available previews
as structured events, but /v1/chat/completions never wired a callback
to receive them, so reasoning/thinking text was silently dropped for
that endpoint — even though it's the one most OpenAI-SDK clients use.

Add a reasoning.available-filtered tool_progress_callback in both the
streaming and non-streaming paths:
- Streaming: emits delta.reasoning_content chunks (same field name
  DeepSeek/Moonshot/OpenRouter use), separate from delta.content and
  the existing hermes.tool.progress tool-lifecycle events.
- Non-streaming: accumulates previews into message.reasoning_content
  on the final response.

Tool-call events are unaffected — the filter only forwards
reasoning.available, so tool_start_callback/tool_complete_callback
still own the tool-lifecycle channel exactly as before.
@alt-glitch alt-glitch added type/feature New feature or request comp/gateway Gateway runner, session dispatch, delivery P3 Low — cosmetic, nice to have labels Jul 30, 2026

@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 tracing the missing chat-completions callback path. The endpoint gap is real, but this implementation captures only the fallback preview channel rather than provider reasoning deltas.

Problems

  • gateway/platforms/api_server.py proposed lines 3932 and 3964 wire tool_progress_callback only. Provider reasoning_content is dispatched via agent/chat_completion_helpers.py:3210-3214AIAgent._fire_reasoning_delta(), which invokes only reasoning_callback in run_agent.py:5831-5842. The API adapter does not currently thread a reasoning_callback through _create_agent (gateway/platforms/api_server.py:2348-2355, 2642-2659), so actual provider thinking deltas remain dropped.
  • The added tests manually emit reasoning.available through tool_progress_callback; they do not cover the reasoning_callback transport that supplies reasoning_content.

Suggested changes

  • Thread reasoning_callback through _run_agent and _create_agent, then use it for the SSE chunks and non-streaming accumulation.
  • Test that callback directly and retain a combined lifecycle assertion so tool events remain single-emitted.

Automated hermes-sweeper review.

agent_task = asyncio.ensure_future(self._run_agent(
user_message=user_message,
conversation_history=history,
ephemeral_system_prompt=system_prompt,

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.

This callback only receives reasoning.available previews. Provider reasoning deltas flow through AIAgent._fire_reasoning_delta()reasoning_callback (run_agent.py:5831-5842), so please thread and use reasoning_callback here as well; otherwise standard reasoning_content streaming remains absent.

@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 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery 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.

3 participants