From 8e7d8bef6a36e7b2159d8697c12ea2333dc4338b Mon Sep 17 00:00:00 2001 From: hlg Date: Mon, 20 Apr 2026 16:28:26 +0800 Subject: [PATCH] fix(utils): handle malformed think tags and channel markers in strip_think MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some models / Ollama renderers occasionally emit tokenizer-level template leaks that the existing regexes miss: 1. Malformed opening tags with no closing `>`, running straight into user-facing content — e.g. `[\s\S]*?` and `^\s*[\s\S]*$` patterns both require `>`, so these leak into rendered messages. 2. Harmony-style channel markers like `` / `<|channel|>` at the start of a response. 3. Orphan `` / `` closing tags left behind when only the opener was consumed upstream. Handles each case conservatively: - Malformed `/]`). Explicit ASCII class instead of `\w` because Python's Unicode `\w` matches CJK and would defeat the primary fix. - Orphan closing tags and channel markers are stripped **only at the start or end of the text**. `strip_think` is also applied before persisting history (memory.py), so mid-text stripping would silently rewrite transcripts where the tokens themselves are discussed. Preserves: ``, ``, ``, ``, ``, ``, literal `` `` `` / `` `` `` inside prose or code blocks. Adds 16 new regression tests covering both the leak cases and the preserved-prose cases. --- nanobot/utils/helpers.py | 73 ++++++++++++++++++++++++++----- tests/utils/test_strip_think.py | 77 +++++++++++++++++++++++++++++++-- 2 files changed, 136 insertions(+), 14 deletions(-) diff --git a/nanobot/utils/helpers.py b/nanobot/utils/helpers.py index 6c3849ef..74c80c11 100644 --- a/nanobot/utils/helpers.py +++ b/nanobot/utils/helpers.py @@ -15,12 +15,48 @@ from loguru import logger def strip_think(text: str) -> str: - """Remove thinking blocks and any unclosed trailing tag.""" + """Remove thinking blocks, unclosed trailing tags, and tokenizer-level + template leaks occasionally emitted by some models (notably Gemma 4's + Ollama renderer). + + Covers: + 1. Well-formed `...` and `...` blocks. + 2. Streaming prefixes where the block is never closed. + 3. *Malformed* opening tags missing the `>` — e.g. `` / `<|channel|>` + **at the start of the text** — conservative to avoid eating + explanatory prose that mentions these tokens. + 5. Orphan closing tags `` / `` **at the very start + or end of the text** only, for the same reason. + + Since this is also applied before persisting to history (memory.py), + the edge-only stripping of (4) and (5) is deliberate: stripping those + tokens mid-text would silently rewrite any message where a user or the + assistant discusses the tokens themselves. + """ + # Well-formed blocks first. text = re.sub(r"[\s\S]*?", "", text) text = re.sub(r"^\s*[\s\S]*$", "", text) - # Gemma 4 and similar models use ... blocks text = re.sub(r"[\s\S]*?", "", text) text = re.sub(r"^\s*[\s\S]*$", "", text) + # Malformed opening tags: `` / `/` — 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 `/])", "", text) + text = re.sub(r"/])", "", text) + # Edge-only orphan closing tags (start or end of text). + text = re.sub(r"^\s*\s*", "", text) + text = re.sub(r"\s*\s*$", "", text) + text = re.sub(r"^\s*\s*", "", text) + text = re.sub(r"\s*\s*$", "", text) + # Edge-only channel markers (harmony / Gemma 4 variant leaks). + text = re.sub(r"^\s*<\|?channel\|?>\s*", "", text) return text.strip() @@ -37,7 +73,9 @@ def detect_image_mime(data: bytes) -> str | None: return None -def build_image_content_blocks(raw: bytes, mime: str, path: str, label: str) -> list[dict[str, Any]]: +def build_image_content_blocks( + raw: bytes, mime: str, path: str, label: str +) -> list[dict[str, Any]]: """Build native image blocks plus a short text label.""" b64 = base64.b64encode(raw).decode() return [ @@ -83,6 +121,7 @@ _TOOL_RESULTS_DIR = ".nanobot/tool-results" _TOOL_RESULT_RETENTION_SECS = 7 * 24 * 60 * 60 _TOOL_RESULT_MAX_BUCKETS = 32 + def safe_filename(name: str) -> str: """Replace unsafe path characters with underscores.""" return _UNSAFE_CHARS.sub("_", name).strip() @@ -258,9 +297,9 @@ def split_message(content: str, max_len: int = 2000) -> list[str]: break cut = content[:max_len] # Try to break at newline first, then space, then hard break - pos = cut.rfind('\n') + pos = cut.rfind("\n") if pos <= 0: - pos = cut.rfind(' ') + pos = cut.rfind(" ") if pos <= 0: pos = max_len chunks.append(content[:pos]) @@ -404,7 +443,7 @@ def build_status_content( max_completion_tokens: int = 8192, ) -> str: """Build a human-readable runtime status snapshot. - + Args: search_usage_text: Optional pre-formatted web search usage string (produced by SearchUsageInfo.format()). When provided @@ -423,7 +462,11 @@ def build_status_content( # Budget mirrors Consolidator formula: ctx_window - max_completion - _SAFETY_BUFFER ctx_budget = max(ctx_total - int(max_completion_tokens) - 1024, 1) ctx_pct = min(int((context_tokens_estimate / ctx_budget) * 100), 999) if ctx_budget > 0 else 0 - ctx_used_str = f"{context_tokens_estimate // 1000}k" if context_tokens_estimate >= 1000 else str(context_tokens_estimate) + ctx_used_str = ( + f"{context_tokens_estimate // 1000}k" + if context_tokens_estimate >= 1000 + else str(context_tokens_estimate) + ) ctx_total_str = f"{ctx_total // 1000}k" if ctx_total > 0 else "n/a" token_line = f"\U0001f4ca Tokens: {last_in} in / {last_out} out" if cached and last_in: @@ -439,12 +482,13 @@ def build_status_content( ] if search_usage_text: lines.append(search_usage_text) - return "\n".join(lines) + return "\n".join(lines) def sync_workspace_templates(workspace: Path, silent: bool = False) -> list[str]: """Sync bundled templates to workspace. Only creates missing files.""" from importlib.resources import files as pkg_files + try: tpl = pkg_files("nanobot") / "templates" except Exception: @@ -470,15 +514,22 @@ def sync_workspace_templates(workspace: Path, silent: bool = False) -> list[str] if added and not silent: from rich.console import Console + for name in added: Console().print(f" [dim]Created {name}[/dim]") # Initialize git for memory version control try: from nanobot.utils.gitstore import GitStore - gs = GitStore(workspace, tracked_files=[ - "SOUL.md", "USER.md", "memory/MEMORY.md", - ]) + + gs = GitStore( + workspace, + tracked_files=[ + "SOUL.md", + "USER.md", + "memory/MEMORY.md", + ], + ) gs.init() except Exception: logger.warning("Failed to initialize git store for {}", workspace) diff --git a/tests/utils/test_strip_think.py b/tests/utils/test_strip_think.py index 5828c6d1..1eda89eb 100644 --- a/tests/utils/test_strip_think.py +++ b/tests/utils/test_strip_think.py @@ -1,5 +1,3 @@ -import pytest - from nanobot.utils.helpers import strip_think @@ -48,7 +46,7 @@ class TestStripThinkFalsePositive: assert strip_think(text) == text def test_code_block_think_tag_preserved(self): - text = "Example:\n```\ntext = re.sub(r\"[\\s\\S]*\", \"\", text)\n```\nDone." + text = 'Example:\n```\ntext = re.sub(r"[\\s\\S]*", "", text)\n```\nDone.' assert strip_think(text) == text def test_backtick_thought_tag_preserved(self): @@ -63,3 +61,76 @@ class TestStripThinkFalsePositive: def test_prefix_unclosed_thought_still_stripped(self): assert strip_think("reasoning without closing") == "" + + +class TestStripThinkMalformedLeaks: + """Regression: Gemma 4's Ollama renderer occasionally emits a tag name + with no closing '>', running straight into the user-facing content + (e.g. `' and + let these through.""" + + def test_malformed_think_no_gt_chinese(self): + assert strip_think("` is a valid tag name variant; must not match. + assert strip_think("content") == "content" + + def test_self_closing_preserved(self): + assert strip_think("ok") == "ok" + assert strip_think("ok") == "ok" + + def test_orphan_closing_think_at_end_stripped(self): + # Typical leak: model opens `` without closing; we strip the + # opener from the start, leaving an orphan `` at the end. + assert strip_think("answer") == "answer" + + def test_orphan_closing_think_at_start_stripped(self): + assert strip_think("answer") == "answer" + + def test_channel_marker_at_start_stripped(self): + # Harmony / Gemma 4 channel markers leak at the start of a response. + assert strip_think("喷泉策略：09:00 开启") == ("喷泉策略：09:00 开启") + assert strip_think("<|channel|>answer") == "answer" + + +class TestStripThinkConservativePreserve: + """Regression: the malformed-tag / orphan cleanup must NOT touch + legitimate prose or code that mentions these tokens literally, otherwise + `strip_think` (which runs before history is persisted, memory.py) will + silently rewrite the conversation transcript.""" + + def test_think_dash_variant_preserved(self): + assert strip_think("bar") == "bar" + + def test_think_underscore_variant_preserved(self): + assert strip_think("bar") == "bar" + + def test_think_numeric_variant_preserved(self): + assert strip_think("bar") == "bar" + + def test_think_namespaced_variant_preserved(self): + assert strip_think("bar") == "bar" + + def test_literal_close_think_in_prose_preserved(self): + # Mid-prose references to `` in backticks or plain text must + # not be stripped; edge-only regex protects this. + text = "Use `` to close a thinking block." + assert strip_think(text) == text + + def test_literal_channel_marker_in_prose_preserved(self): + text = "The Harmony spec uses `<|channel|>` and `` markers." + assert strip_think(text) == text + + def test_literal_channel_marker_in_code_block_preserved(self): + text = "Example:\n```\nif line.startswith(''):\n skip()\n```" + assert strip_think(text) == text