fix(gateway): stream real reasoning_content instead of answer echo - #75562
fix(gateway): stream real reasoning_content instead of answer echo#75562huanshan5195 wants to merge 3 commits into
Conversation
teknium1
left a comment
There was a problem hiding this comment.
Thanks for tracing both the fake reasoning fallback and the dropped callback path. The current-main premise is valid: agent/conversation_loop.py:5529-5545 publishes answer content as reasoning.available, while gateway/platforms/api_server.py:2653-2656 does not pass reasoning_callback into AIAgent.
Problems
gateway/platforms/api_server.py:6245emits every callback chunk asreasoning.available. That is fallback/snapshot semantics:ui-tui/src/app/turnController.ts:715-723accepts only the first available text, whereas:766-785appendsreasoning.deltachunks. The incremental Runs contract should usereasoning.delta, as related PR #15169 does.- The diff adds no regression tests. In particular, it does not prove multi-chunk Runs ordering, Chat
delta.reasoning_contentseparation, or the_run_agent→_create_agent→AIAgentcallback forwarding chain.
Suggested changes
- Emit
reasoning.deltafor Runs and add an endpoint test covering multiple chunks beforerun.completed. - Add focused forwarding and Chat SSE serialization regression tests in
tests/gateway/.
This is an automated hermes-sweeper review.
| return | ||
| try: | ||
| loop.call_soon_threadsafe(_put_event_if_active, { | ||
| "event": "reasoning.available", |
There was a problem hiding this comment.
reasoning_callback is invoked per provider chunk, but reasoning.available is the one-shot fallback event: the existing UI keeps only its first nonempty value. Emit reasoning.delta here so the Runs stream preserves every real reasoning chunk, matching #15169's contract.
|
Thanks for the review - both points addressed in 92dfdd8. 1. Runs event type ( 2. Regression tests
6 passed; On overlap with #15169: this PR now aligns with its |
92dfdd8 to
ba28884
Compare
The /v1/runs and /v1/chat/completions SSE paths showed the model's answer content (truncated to 500 chars) as the "thinking" block, so the reasoning panel was an exact echo of the response. Two coupled fixes: 1. agent/conversation_loop.py: top-level agents no longer emit reasoning.available from assistant_message.content. That fallback was meant for subagent delegation (a child's content IS the parent's reasoning surface) but was also firing for top-level agents, duplicating the answer as a fake chain-of-thought. 2. gateway/platforms/api_server.py: wire the agent's reasoning_callback (already fired by _fire_reasoning_delta from reasoning_content/reasoning deltas of GLM/DeepSeek/Kimi/Qwen thinking models) through to the SSE transports: /v1/runs emits reasoning.available events with the real thinking text; /v1/chat/completions emits delta.reasoning_content chunks. Non-thinking models never fire the callback, so no spurious reasoning block is shown. Tested with GLM-5.2 via /v1/runs: reasoning panel now shows the model's chain-of-thought, distinct from the answer; non-thinking models show no reasoning block. Python syntax verified (py_compile).
…nt separation Address review on NousResearch#75562: - /v1/runs: _reasoning_cb now emits `reasoning.delta` (incremental) instead of `reasoning.available` (one-shot snapshot). The snapshot path (event_cb `reasoning.available` at api_server.py:6047, and the chat tool_progress `reasoning.available` at :3557) is intentionally preserved for post-run/legacy consumers; only the live callback wiring switches to `reasoning.delta`, matching the NousResearch#15169 contract and ui-tui/src/app/turnController.ts:766-785 delta-appending behavior. Without this, turnController.ts:715-723 kept only the first non-empty `reasoning.available` value and dropped every subsequent real chunk. - Add tests/gateway/test_api_server_reasoning.py covering: * _create_agent forwards reasoning_callback through to AIAgent (and defaults to None for back-compat) - the _run_agent -> _create_agent -> AIAgent forwarding chain. * Multi-chunk reasoning.delta ordering is preserved before run.completed over /v1/runs; None/empty payloads are suppressed. * reasoning_callback errors do not break the run stream. * /v1/chat/completions streams delta.reasoning_content separated from delta.content, with reasoning preceding content (the answer-echo regression this PR fixes). * Non-thinking models (reasoning_callback wired but never fired) emit no reasoning_content, so no spurious reasoning block is shown.
ba28884 to
8e24a55
Compare
|
Updated for the review: branch rebased onto current main (0 behind, conflict resolved — the stream queue is now On the two points:
|
…stream real reasoning_content)
What & Why
When a thinking model (GLM-5.2, DeepSeek-V4, Kimi, Qwen) answers through the HTTP gateway, the reasoning panel shows the model's answer text (truncated to 500 chars) instead of its real chain-of-thought. Two coupled bugs cause this; this PR fixes the parts that #15169 does not cover.
Bug 1 — this PR (root cause of #60634)
conversation_loop.pyemitsreasoning.availablefromassistant_message.content[:500]for all agents, including top-level ones. That fallback was designed for subagent delegation (a child's content IS the parent's reasoning surface), but a missing_delegate_depth > 0guard lets top-level agents publish their own answer as a fake "thinking" block.This is the bug users reported in #60634 ("reasoning text is identical to the assistant answer").
Bug 2 — covered by #15169
_create_agent/_run_agentnever forwardreasoning_callbacktoAIAgent, so_fire_reasoning_deltafires into a no-op and real reasoning deltas are silently dropped. #15169 fixes this for the/v1/runsSSE path.Scope — complementary to #15169
conversation_loop.pyanswer-echo guard/v1/chat/completionsdelta.reasoning_content_create_agent/_run_agentreasoning_callbackplumbing/v1/runsreasoning.deltaSSE eventsThe
/v1/runs_reasoning_cb→reasoning.deltawiring is included here so the full callback chain is testable end-to-end, but #15169 is the primary vehicle for that path. If maintainers merge #15169 first, this PR can rebase onto it and drop the/v1/runsblock — onlyconversation_loop.py+/v1/chat/completions+ tests remain.Under #15169 alone, a top-level agent would still publish its answer as a fake thinking block (Bug 1 is untouched). That's the gap this PR closes.
Changes
agent/conversation_loop.py(+12/-12)Added
_delegate_depth > 0guard so only subagents relay their content as reasoning. Top-level agents no longer emitreasoning.availablefromassistant_message.content. Subagent delegation behavior (_thinkingfirst-line relay) is preserved unchanged.gateway/platforms/api_server.py(+46/-1)_create_agent/_run_agentacceptreasoning_callback=Noneand forward it toAIAgent— shared plumbing that both/v1/runs(fix(api_server): stream reasoning.delta events over /v1/runs SSE #15169) and/v1/chat/completions(this PR) depend on./v1/chat/completions(streaming):_on_reasoningqueues("__reasoning__", text);_emitforwards it asdelta.reasoning_contentSSE chunks, matching the field OpenAI-compatible clients read. Reasoning streams before content, separate fromdelta.content./v1/runs:_reasoning_cbemitsreasoning.deltaevents (aligns with fix(api_server): stream reasoning.delta events over /v1/runs SSE #15169's contract; can be dropped if fix(api_server): stream reasoning.delta events over /v1/runs SSE #15169 merges first).Non-thinking models never fire
reasoning_callback, so no spurious reasoning block appears.tests/gateway/test_api_server_reasoning.py(new, 374 lines, 6 tests)_create_agentforwardsreasoning_callbacktoAIAgent(defaults toNonefor back-compat).reasoning.deltaordering preserved beforerun.completed;None/empty payloads suppressed.reasoning_callbackerrors do not break the run stream./v1/chat/completionsstreamsdelta.reasoning_contentseparated fromdelta.content, reasoning preceding content (the answer-echo regression).reasoning_content.Relationship to other PRs / issues
/v1/runsreasoning.deltawiring. This PR is complementary: it fixesconversation_loop.py(Bug 1) and adds/v1/chat/completionssupport, which fix(api_server): stream reasoning.delta events over /v1/runs SSE #15169 does not touch.Happy to rebase onto #15169 or merge into a single vehicle — maintainers' call.
How to test
/v1/chat/completions(stream=true). A client readingdelta.reasoning_contentshould see real reasoning, distinct fromdelta.content.reasoning_contentchunks should appear.pytest tests/gateway/test_api_server_reasoning.py— 6 passed.tests/gateway/test_api_server_runs.py— 16 passed (no regression).Platforms tested