refactor(reasoning): unify reasoning extraction across providers
Reasoning surfacing was split across three branches in runner.py plus two separate streaming buffers (loop hook and runner progress stream), with three independent display-side gates in the CLI. This collapsed the policy into one source of truth and fixed two real bugs: - Structured `reasoning_content` was suppressed whenever the answer was streamed, because the runner gated emission on `streamed_content`. Providers don't stream `reasoning_content`; it only arrives on the final response, so the answer stream and the reasoning channel are independent. Added `streamed_reasoning` to `AgentHookContext` to track the right bit. - `channels.showReasoning` was subordinated to `sendProgress`. They are orthogonal — turning off progress streaming shouldn't silence reasoning. Reworked the CLI gates accordingly. Single-helper consolidation: - `extract_reasoning(reasoning_content, thinking_blocks, content)` returns `(reasoning_text, cleaned_content)` with a defined fallback order: dedicated field → Anthropic thinking_blocks → inline `<think>`/`<thought>` tags. Models that expose none of these short-circuit to `(None, content)` — zero overhead. - `IncrementalThinkExtractor` replaces the ad-hoc `emit_incremental_think` function and its hand-rolled "emitted cursor" state in both the loop hook and the runner progress stream. Also documented the new `showReasoning` channel option in docs/configuration.md and noted its independence from sendProgress. Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
@@ -227,6 +227,111 @@ async def test_runner_prefers_reasoning_content_over_inline_think():
|
||||
assert emitted_reasoning[0] == "dedicated reasoning field"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_runner_emits_reasoning_content_even_when_answer_was_streamed():
|
||||
"""`reasoning_content` arrives only on the final response; streaming the
|
||||
answer must not suppress it (the answer stream and the reasoning channel
|
||||
are independent — only the reasoning-already-emitted bit matters)."""
|
||||
from nanobot.agent.hook import AgentHook, AgentHookContext
|
||||
from nanobot.agent.runner import AgentRunSpec, AgentRunner
|
||||
|
||||
provider = MagicMock()
|
||||
provider.supports_progress_deltas = True
|
||||
emitted_reasoning: list[str] = []
|
||||
|
||||
async def chat_stream_with_retry(*, on_content_delta=None, **kwargs):
|
||||
if on_content_delta:
|
||||
await on_content_delta("The ")
|
||||
await on_content_delta("answer.")
|
||||
return LLMResponse(
|
||||
content="The answer.",
|
||||
reasoning_content="step-by-step deduction",
|
||||
tool_calls=[],
|
||||
usage={"prompt_tokens": 5, "completion_tokens": 3},
|
||||
)
|
||||
|
||||
provider.chat_stream_with_retry = chat_stream_with_retry
|
||||
tools = MagicMock()
|
||||
tools.get_definitions.return_value = []
|
||||
|
||||
class ReasoningHook(AgentHook):
|
||||
async def emit_reasoning(self, reasoning_content: str | None) -> None:
|
||||
if reasoning_content:
|
||||
emitted_reasoning.append(reasoning_content)
|
||||
|
||||
progress_calls: list[str] = []
|
||||
|
||||
async def _progress(content: str, **_kwargs):
|
||||
progress_calls.append(content)
|
||||
|
||||
runner = AgentRunner(provider)
|
||||
result = await runner.run(AgentRunSpec(
|
||||
initial_messages=[{"role": "user", "content": "question"}],
|
||||
tools=tools,
|
||||
model="test-model",
|
||||
max_iterations=3,
|
||||
max_tool_result_chars=_MAX_TOOL_RESULT_CHARS,
|
||||
hook=ReasoningHook(),
|
||||
stream_progress_deltas=True,
|
||||
progress_callback=_progress,
|
||||
))
|
||||
|
||||
assert result.final_content == "The answer."
|
||||
# The answer must have streamed AND the dedicated reasoning_content must
|
||||
# have been emitted exactly once after the stream completed.
|
||||
assert progress_calls, "answer should have streamed via progress callback"
|
||||
assert emitted_reasoning == ["step-by-step deduction"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_runner_does_not_double_emit_when_inline_think_already_streamed():
|
||||
"""Inline `<think>` blocks streamed incrementally during the answer
|
||||
stream must not be re-emitted from the final response."""
|
||||
from nanobot.agent.hook import AgentHook, AgentHookContext
|
||||
from nanobot.agent.runner import AgentRunSpec, AgentRunner
|
||||
|
||||
provider = MagicMock()
|
||||
provider.supports_progress_deltas = True
|
||||
emitted_reasoning: list[str] = []
|
||||
|
||||
async def chat_stream_with_retry(*, on_content_delta=None, **kwargs):
|
||||
if on_content_delta:
|
||||
await on_content_delta("<think>working...</think>")
|
||||
await on_content_delta("The answer.")
|
||||
return LLMResponse(
|
||||
content="<think>working...</think>The answer.",
|
||||
tool_calls=[],
|
||||
usage={"prompt_tokens": 5, "completion_tokens": 3},
|
||||
)
|
||||
|
||||
provider.chat_stream_with_retry = chat_stream_with_retry
|
||||
tools = MagicMock()
|
||||
tools.get_definitions.return_value = []
|
||||
|
||||
class ReasoningHook(AgentHook):
|
||||
async def emit_reasoning(self, reasoning_content: str | None) -> None:
|
||||
if reasoning_content:
|
||||
emitted_reasoning.append(reasoning_content)
|
||||
|
||||
async def _progress(content: str, **_kwargs):
|
||||
pass
|
||||
|
||||
runner = AgentRunner(provider)
|
||||
result = await runner.run(AgentRunSpec(
|
||||
initial_messages=[{"role": "user", "content": "question"}],
|
||||
tools=tools,
|
||||
model="test-model",
|
||||
max_iterations=3,
|
||||
max_tool_result_chars=_MAX_TOOL_RESULT_CHARS,
|
||||
hook=ReasoningHook(),
|
||||
stream_progress_deltas=True,
|
||||
progress_callback=_progress,
|
||||
))
|
||||
|
||||
assert result.final_content == "The answer."
|
||||
assert emitted_reasoning == ["working..."]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_runner_calls_hooks_in_order():
|
||||
from nanobot.agent.hook import AgentHook, AgentHookContext
|
||||
|
||||
@@ -88,3 +88,26 @@ async def test_non_reasoning_progress_not_affected_by_show_reasoning():
|
||||
|
||||
assert handled is True
|
||||
assert calls == ["working on it..."]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_reasoning_shown_when_send_progress_disabled():
|
||||
"""Reasoning display is governed by `show_reasoning` alone, independent
|
||||
of `send_progress` — the two knobs are orthogonal."""
|
||||
calls: list[str] = []
|
||||
channels_config = SimpleNamespace(
|
||||
send_progress=False, send_tool_hints=False, show_reasoning=True,
|
||||
)
|
||||
msg = SimpleNamespace(
|
||||
content="Let me think about this...",
|
||||
metadata={"_progress": True, "_reasoning": True},
|
||||
)
|
||||
|
||||
with patch(
|
||||
"nanobot.cli.commands._print_cli_reasoning",
|
||||
side_effect=lambda t, th, r=None: calls.append(t),
|
||||
):
|
||||
handled = await commands._maybe_print_interactive_progress(msg, None, channels_config)
|
||||
|
||||
assert handled is True
|
||||
assert calls == ["Let me think about this..."]
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
from nanobot.utils.helpers import extract_think, strip_think
|
||||
from nanobot.utils.helpers import extract_reasoning, extract_think, strip_think
|
||||
|
||||
|
||||
class TestStripThinkTag:
|
||||
@@ -225,3 +225,49 @@ squares = [x**2 for x in range(10)]
|
||||
assert "List comprehensions in Python" in clean
|
||||
assert "<think>" not in clean
|
||||
assert "</think>" not in clean
|
||||
|
||||
|
||||
class TestExtractReasoning:
|
||||
"""Single source of truth for reasoning extraction across all providers."""
|
||||
|
||||
def test_prefers_reasoning_content_and_strips_inline_think(self):
|
||||
# Dedicated field wins; inline tags are still scrubbed from content.
|
||||
reasoning, content = extract_reasoning(
|
||||
"dedicated",
|
||||
None,
|
||||
"<think>inline</think>visible answer",
|
||||
)
|
||||
assert reasoning == "dedicated"
|
||||
assert content == "visible answer"
|
||||
|
||||
def test_falls_back_to_thinking_blocks(self):
|
||||
reasoning, content = extract_reasoning(
|
||||
None,
|
||||
[
|
||||
{"type": "thinking", "thinking": "step 1"},
|
||||
{"type": "thinking", "thinking": "step 2"},
|
||||
{"type": "redacted_thinking"},
|
||||
],
|
||||
"hello",
|
||||
)
|
||||
assert reasoning == "step 1\n\nstep 2"
|
||||
assert content == "hello"
|
||||
|
||||
def test_falls_back_to_inline_think_tags(self):
|
||||
reasoning, content = extract_reasoning(
|
||||
None, None, "<think>plan</think>answer"
|
||||
)
|
||||
assert reasoning == "plan"
|
||||
assert content == "answer"
|
||||
|
||||
def test_no_reasoning_returns_none(self):
|
||||
reasoning, content = extract_reasoning(None, None, "plain answer")
|
||||
assert reasoning is None
|
||||
assert content == "plain answer"
|
||||
|
||||
def test_empty_thinking_blocks_falls_through_to_inline(self):
|
||||
reasoning, content = extract_reasoning(
|
||||
None, [], "<think>plan</think>answer"
|
||||
)
|
||||
assert reasoning == "plan"
|
||||
assert content == "answer"
|
||||
|
||||
Reference in New Issue
Block a user