fix: normalize thinking tags in reasoning output
This commit is contained in:
@@ -34,6 +34,7 @@ from nanobot.utils.helpers import (
|
||||
extract_reasoning,
|
||||
find_legal_message_start,
|
||||
maybe_persist_tool_result,
|
||||
strip_reasoning_tags,
|
||||
strip_think,
|
||||
truncate_text,
|
||||
)
|
||||
@@ -778,8 +779,10 @@ class AgentRunner:
|
||||
async def _thinking(delta: str) -> None:
|
||||
if not delta:
|
||||
return
|
||||
context.streamed_reasoning = True
|
||||
await hook.emit_reasoning(delta)
|
||||
delta = strip_reasoning_tags(delta)
|
||||
if delta:
|
||||
context.streamed_reasoning = True
|
||||
await hook.emit_reasoning(delta)
|
||||
|
||||
async def _stream_recover() -> None:
|
||||
await hook.on_stream_end(context, resuming=True)
|
||||
|
||||
@@ -71,7 +71,8 @@ def strip_think(text: str) -> str:
|
||||
Ollama renderer).
|
||||
|
||||
Covers:
|
||||
1. Well-formed `<think>...</think>` and `<thought>...</thought>` blocks.
|
||||
1. Well-formed `<think>...</think>`, `<thinking>...</thinking>`,
|
||||
and `<thought>...</thought>` blocks.
|
||||
2. Streaming prefixes where the block is never closed.
|
||||
3. *Malformed* opening tags missing the `>` — e.g. `<think广场…`. The
|
||||
model sometimes emits the tag name directly followed by user-facing
|
||||
@@ -80,8 +81,8 @@ def strip_think(text: str) -> str:
|
||||
4. Harmony-style channel markers like `<channel|>` / `<|channel|>`
|
||||
**at the start of the text** — conservative to avoid eating
|
||||
explanatory prose that mentions these tokens.
|
||||
5. Orphan closing tags `</think>` / `</thought>` **at the very start
|
||||
or end of the text** only, for the same reason.
|
||||
5. Orphan closing tags `</think>` / `</thinking>` / `</thought>`
|
||||
**at the very start or end of the text** only, for the same reason.
|
||||
6. Trailing partial control tags split across stream chunks, such as
|
||||
`<thi`, `<thin`, or `<tho`.
|
||||
|
||||
@@ -93,19 +94,27 @@ def strip_think(text: str) -> str:
|
||||
# Well-formed blocks first.
|
||||
text = re.sub(r"<think>[\s\S]*?</think>", "", text)
|
||||
text = re.sub(r"^\s*<think>[\s\S]*$", "", text)
|
||||
text = re.sub(r"<thinking>[\s\S]*?</thinking>", "", text)
|
||||
text = re.sub(r"^\s*<thinking>[\s\S]*$", "", text)
|
||||
text = re.sub(r"<thought>[\s\S]*?</thought>", "", text)
|
||||
text = re.sub(r"^\s*<thought>[\s\S]*$", "", text)
|
||||
# Malformed opening tags: `<think` / `<thought` where the next char is
|
||||
# Self-closing `<thinking/>` is an empty marker, not user-visible text.
|
||||
text = re.sub(r"^\s*<thinking/>\s*", "", text)
|
||||
text = re.sub(r"\s*<thinking/>\s*$", "", text)
|
||||
# Malformed opening tags: `<think` / `<thinking` / `<thought` where the next char is
|
||||
# NOT one that could continue a valid tag / identifier name. Explicitly
|
||||
# listing ASCII tag-name chars (letters, digits, `_`, `-`, `:`) plus
|
||||
# `>` / `/` — we can't use `\w` here because in Python's default
|
||||
# Unicode regex mode it matches CJK characters too, which would defeat
|
||||
# the primary fix for `<think广场…` leaks.
|
||||
text = re.sub(r"<think(?![A-Za-z0-9_\-:>/])", "", text)
|
||||
text = re.sub(r"<thinking(?![A-Za-z0-9_\-:>/])", "", text)
|
||||
text = re.sub(r"<thought(?![A-Za-z0-9_\-:>/])", "", text)
|
||||
# Edge-only orphan closing tags (start or end of text).
|
||||
text = re.sub(r"^\s*</think>\s*", "", text)
|
||||
text = re.sub(r"\s*</think>\s*$", "", text)
|
||||
text = re.sub(r"^\s*</thinking>\s*", "", text)
|
||||
text = re.sub(r"\s*</thinking>\s*$", "", text)
|
||||
text = re.sub(r"^\s*</thought>\s*", "", text)
|
||||
text = re.sub(r"\s*</thought>\s*$", "", text)
|
||||
# Edge-only channel markers (harmony / Gemma 4 variant leaks).
|
||||
@@ -113,7 +122,7 @@ def strip_think(text: str) -> str:
|
||||
# Stream chunks may end in the middle of a control tag. Strip only known
|
||||
# control-token prefixes at the very end.
|
||||
partial_control_tag = (
|
||||
r"</?(?:t|th|thi|thin|think|tho|thou|thoug|though|thought)>?"
|
||||
r"</?(?:t|th|thi|thin|think|thinki|thinkin|thinking|tho|thou|thoug|though|thought)>?"
|
||||
r"|<\|?(?:c|ch|cha|chan|chann|channe|channel)(?:\|?>?)?"
|
||||
)
|
||||
text = re.sub(rf"(?:{partial_control_tag})$", "", text)
|
||||
@@ -121,8 +130,17 @@ def strip_think(text: str) -> str:
|
||||
return text.strip()
|
||||
|
||||
|
||||
def strip_reasoning_tags(text: str) -> str:
|
||||
"""Remove wrapper tags from text that is already known to be reasoning."""
|
||||
text = re.sub(r"^\s*<(?:think|thinking|thought)/>\s*", "", text)
|
||||
text = re.sub(r"\s*<(?:think|thinking|thought)/>\s*$", "", text)
|
||||
text = re.sub(r"^\s*<(?:think|thinking|thought)>\s*", "", text)
|
||||
text = re.sub(r"\s*</(?:think|thinking|thought)>\s*$", "", text)
|
||||
return text.strip()
|
||||
|
||||
|
||||
def extract_think(text: str) -> tuple[str | None, str]:
|
||||
"""Extract thinking content from inline ``<think>`` / ``<thought>`` blocks.
|
||||
"""Extract thinking content from inline thinking tags.
|
||||
|
||||
Returns ``(thinking_text, cleaned_text)``. Only closed blocks are
|
||||
extracted; unclosed streaming prefixes are stripped from the cleaned
|
||||
@@ -131,6 +149,8 @@ def extract_think(text: str) -> tuple[str | None, str]:
|
||||
parts: list[str] = []
|
||||
for m in re.finditer(r"<think>([\s\S]*?)</think>", text):
|
||||
parts.append(m.group(1).strip())
|
||||
for m in re.finditer(r"<thinking>([\s\S]*?)</thinking>", text):
|
||||
parts.append(m.group(1).strip())
|
||||
for m in re.finditer(r"<thought>([\s\S]*?)</thought>", text):
|
||||
parts.append(m.group(1).strip())
|
||||
thinking = "\n\n".join(parts) if parts else None
|
||||
@@ -194,10 +214,10 @@ def extract_reasoning(
|
||||
final answer.
|
||||
"""
|
||||
if reasoning_content:
|
||||
return reasoning_content, strip_think(content) if content else content
|
||||
return strip_reasoning_tags(reasoning_content), strip_think(content) if content else content
|
||||
if thinking_blocks:
|
||||
parts = [
|
||||
tb.get("thinking", "")
|
||||
strip_reasoning_tags(tb.get("thinking", ""))
|
||||
for tb in thinking_blocks
|
||||
if isinstance(tb, dict) and tb.get("type") == "thinking"
|
||||
]
|
||||
@@ -520,7 +540,11 @@ def build_assistant_message(
|
||||
if tool_calls:
|
||||
msg["tool_calls"] = tool_calls
|
||||
if reasoning_content is not None or thinking_blocks:
|
||||
msg["reasoning_content"] = reasoning_content if reasoning_content is not None else ""
|
||||
msg["reasoning_content"] = (
|
||||
strip_reasoning_tags(reasoning_content)
|
||||
if reasoning_content is not None
|
||||
else ""
|
||||
)
|
||||
if thinking_blocks:
|
||||
msg["thinking_blocks"] = thinking_blocks
|
||||
return msg
|
||||
|
||||
@@ -369,3 +369,39 @@ async def test_runner_streams_native_thinking_deltas_without_post_hoc_dup():
|
||||
|
||||
assert result.final_content == "done"
|
||||
assert hook.emitted == ["part1", "part2"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_runner_strips_thinking_tags_from_native_thinking_deltas():
|
||||
from nanobot.agent.runner import AgentRunner, AgentRunSpec
|
||||
|
||||
provider = MagicMock()
|
||||
|
||||
async def chat_stream_with_retry(
|
||||
*, on_content_delta=None, on_thinking_delta=None, **kwargs
|
||||
):
|
||||
if on_thinking_delta:
|
||||
await on_thinking_delta("<thinking>")
|
||||
await on_thinking_delta("Preparing final response")
|
||||
await on_thinking_delta("</thinking>")
|
||||
if on_content_delta:
|
||||
await on_content_delta("done")
|
||||
return LLMResponse(content="done", tool_calls=[], usage={})
|
||||
|
||||
provider.chat_stream_with_retry = chat_stream_with_retry
|
||||
tools = MagicMock()
|
||||
tools.get_definitions.return_value = []
|
||||
|
||||
hook = _StreamRecordingHook()
|
||||
runner = AgentRunner(provider)
|
||||
result = await runner.run(AgentRunSpec(
|
||||
initial_messages=[{"role": "user", "content": "q"}],
|
||||
tools=tools,
|
||||
model="test-model",
|
||||
max_iterations=3,
|
||||
max_tool_result_chars=_MAX_TOOL_RESULT_CHARS,
|
||||
hook=hook,
|
||||
))
|
||||
|
||||
assert result.final_content == "done"
|
||||
assert hook.emitted == ["Preparing final response"]
|
||||
|
||||
@@ -1,4 +1,9 @@
|
||||
from nanobot.utils.helpers import extract_reasoning, extract_think, strip_think
|
||||
from nanobot.utils.helpers import (
|
||||
extract_reasoning,
|
||||
extract_think,
|
||||
strip_reasoning_tags,
|
||||
strip_think,
|
||||
)
|
||||
|
||||
|
||||
class TestStripThinkTag:
|
||||
@@ -27,6 +32,15 @@ class TestStripThinkTag:
|
||||
def test_self_closing_tag_not_matched(self):
|
||||
assert strip_think("<thought/>some text") == "<thought/>some text"
|
||||
|
||||
def test_thinking_alias_closed_tag(self):
|
||||
assert strip_think("Hello <thinking>reasoning</thinking> World") == "Hello World"
|
||||
|
||||
def test_thinking_alias_unclosed_trailing_tag(self):
|
||||
assert strip_think("<thinking>ongoing...") == ""
|
||||
|
||||
def test_self_closing_thinking_marker_at_start_stripped(self):
|
||||
assert strip_think("<thinking/>some text") == "some text"
|
||||
|
||||
def test_normal_text_unchanged(self):
|
||||
assert strip_think("Just normal text") == "Just normal text"
|
||||
|
||||
@@ -165,6 +179,12 @@ class TestExtractThink:
|
||||
assert thinking == "reasoning content"
|
||||
assert clean == "Hello World"
|
||||
|
||||
def test_single_thinking_block(self):
|
||||
text = "Hello <thinking>reasoning content</thinking> World"
|
||||
thinking, clean = extract_think(text)
|
||||
assert thinking == "reasoning content"
|
||||
assert clean == "Hello World"
|
||||
|
||||
def test_multiple_think_blocks(self):
|
||||
text = "A<think>first</think>B<thought>second</thought>C"
|
||||
thinking, clean = extract_think(text)
|
||||
@@ -230,6 +250,24 @@ squares = [x**2 for x in range(10)]
|
||||
class TestExtractReasoning:
|
||||
"""Single source of truth for reasoning extraction across all providers."""
|
||||
|
||||
def test_strips_tags_from_dedicated_reasoning_content(self):
|
||||
reasoning, content = extract_reasoning(
|
||||
"<thinking>Preparing final response",
|
||||
None,
|
||||
"visible answer",
|
||||
)
|
||||
assert reasoning == "Preparing final response"
|
||||
assert content == "visible answer"
|
||||
|
||||
def test_self_closing_thinking_marker_in_reasoning_content(self):
|
||||
reasoning, content = extract_reasoning(
|
||||
"<thinking/>Preparing final response",
|
||||
None,
|
||||
"visible answer",
|
||||
)
|
||||
assert reasoning == "Preparing final response"
|
||||
assert content == "visible answer"
|
||||
|
||||
def test_prefers_reasoning_content_and_strips_inline_think(self):
|
||||
# Dedicated field wins; inline tags are still scrubbed from content.
|
||||
reasoning, content = extract_reasoning(
|
||||
@@ -271,3 +309,21 @@ class TestExtractReasoning:
|
||||
)
|
||||
assert reasoning == "plan"
|
||||
assert content == "answer"
|
||||
|
||||
|
||||
class TestStripReasoningTags:
|
||||
|
||||
def test_unclosed_thinking_wrapper_keeps_reasoning_body(self):
|
||||
assert strip_reasoning_tags("<thinking>Preparing final response") == (
|
||||
"Preparing final response"
|
||||
)
|
||||
|
||||
def test_self_closing_thinking_marker_keeps_reasoning_body(self):
|
||||
assert strip_reasoning_tags("<thinking/>Preparing final response") == (
|
||||
"Preparing final response"
|
||||
)
|
||||
|
||||
def test_closing_thinking_wrapper_removed(self):
|
||||
assert strip_reasoning_tags("Preparing final response</thinking>") == (
|
||||
"Preparing final response"
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user