Merge pull request #2213 into stage-351

Preserve literal thinking tags in assistant messages (franksong2702, fixes #2152)

CHANGELOG.md

@@ -4,6 +4,8 @@
 
 ### Fixed
 
+- **Issue #2152** — Literal discussions of reasoning tags such as `<think>` and `</think>` no longer disappear from saved or re-rendered assistant messages. WebUI now treats `<think>...</think>`, MiniMax `<|channel>thought...<channel|>`, and Gemma 4 `<|turn|>thinking...<turn|>` blocks as hidden reasoning metadata only when the wrapper is the first non-whitespace content in the response; provider wrappers with leading whitespace still strip as before.
+
 - **PR #2191** by @lucasrc (auth refactor 1/3) — Thread-safe login rate limiter (new `_LOGIN_ATTEMPTS_LOCK`) + PBKDF2 key separation (new `_pbkdf2_key()` reading `.pbkdf2_key` separately from `_signing_key()` reading `.signing_key` — previously both shared `.signing_key`, a key-reuse anti-pattern across HMAC and PBKDF2 primitives) + transparent migration in `verify_password()` that re-salts legacy hashes with the new key on next successful login. 241-line regression suite covering the lock + migration paths. Split from earlier #2167 per maintainer review request.
 
 - **PR #2192** by @lucasrc (auth refactor 2/3, depends on #2191) — Invalidate password-hash cache when password changes via the Settings panel. The PR #2191 cache lives for the process lifetime, but `save_settings({'_set_password': ...})` could mutate `settings.json.password_hash` without telling the auth module — leaving the cache stale and verifying against the old password until restart. Now `save_settings()` calls `_invalidate_password_hash_cache()` on both `_set_password` and `_clear_password` paths. 52-line regression suite + `verify_password()` simplified to rely on the new hook instead of doing the invalidation itself.

api/streaming.py

@@ -689,9 +689,12 @@ def _strip_thinking_markup(text: str) -> str:
     if not text:
         return ''
     s = str(text)
-    s = re.sub(r'<think>.*?</think>', ' ', s, flags=re.IGNORECASE | re.DOTALL)
-    s = re.sub(r'<\|channel\|>thought.*?<channel\|>', ' ', s, flags=re.IGNORECASE | re.DOTALL)
-    s = re.sub(r'<\|turn\|>thinking\n.*?<turn\|>', ' ', s, flags=re.IGNORECASE | re.DOTALL)  # Gemma 4
+    # Treat provider thinking wrappers as metadata only when they lead the
+    # response. Literal discussion of these tags later in normal prose should
+    # stay visible (#2152).
+    s = re.sub(r'^\s*<think>.*?</think>\s*', ' ', s, flags=re.IGNORECASE | re.DOTALL)
+    s = re.sub(r'^\s*<\|channel\|?>thought\n?.*?<channel\|>\s*', ' ', s, flags=re.IGNORECASE | re.DOTALL)
+    s = re.sub(r'^\s*<\|turn\|>thinking\n.*?<turn\|>\s*', ' ', s, flags=re.IGNORECASE | re.DOTALL)  # Gemma 4
     s = re.sub(r'^\s*(the|ther)\s+user\s+is\s+asking.*$', ' ', s, flags=re.IGNORECASE | re.MULTILINE)
     # Strip plain-text thinking preambles from models that don't use <think> tags (e.g. Qwen3).
     # These appear as the very first sentence of the assistant response and are not useful as titles.

static/ui.js

@@ -4236,7 +4236,7 @@ function _messageHasReasoningPayload(m){
   if(!m||m.role!=='assistant') return false;
   if(m.reasoning) return true;
   if(Array.isArray(m.content)) return m.content.some(p=>p&&(p.type==='thinking'||p.type==='reasoning'));
-  return /<think>[\s\S]*?<\/think>|<\|channel>thought\n[\s\S]*?<channel\|>|<\|turn\|>thinking\n[\s\S]*?<turn\|>/.test(String(m.content||''));
+  return /^\s*(?:<think>[\s\S]*?<\/think>|<\|channel\|?>thought\n?[\s\S]*?<channel\|>|<\|turn\|>thinking\n[\s\S]*?<turn\|>)/.test(String(m.content||''));
 }
 function _formatTurnTps(value){
   const n=Number(value);
@@ -5101,25 +5101,25 @@ function renderMessages(options){
     }
     if(!thinkingText && m.reasoning) thinkingText=m.reasoning;
     if(!thinkingText && typeof content==='string'){
-      const thinkMatch=content.match(/<think>([\s\S]*?)<\/think>/);
+      const thinkMatch=content.match(/^\s*<think>([\s\S]*?)<\/think>\s*/);
       if(thinkMatch){
         thinkingText=thinkMatch[1].trim();
-        content=content.replace(/<think>[\s\S]*?<\/think>\s*/,'').trimStart();
+        content=content.replace(/^\s*<think>[\s\S]*?<\/think>\s*/,'').trimStart();
       }
       if(!thinkingText){
         // Historical name "gemmaMatch" refers to MiniMax <|channel>thought format.
-        const gemmaMatch=content.match(/<\|channel>thought\n([\s\S]*?)<channel\|>/);
+        const gemmaMatch=content.match(/^\s*<\|channel\|?>thought\n?([\s\S]*?)<channel\|>\s*/);
         if(gemmaMatch){
           thinkingText=gemmaMatch[1].trim();
-          content=content.replace(/<\|channel>thought\n[\s\S]*?<channel\|>\s*/,'').trimStart();
+          content=content.replace(/^\s*<\|channel\|?>thought\n?[\s\S]*?<channel\|>\s*/,'').trimStart();
         }
       }
       if(!thinkingText){
         // Gemma 4 uses asymmetric <|turn|>thinking\n...<turn|> delimiters.
-        const gemmaTurnMatch=content.match(/<\|turn\|>thinking\n([\s\S]*?)<turn\|>/);
+        const gemmaTurnMatch=content.match(/^\s*<\|turn\|>thinking\n([\s\S]*?)<turn\|>\s*/);
         if(gemmaTurnMatch){
           thinkingText=gemmaTurnMatch[1].trim();
-          content=content.replace(/<\|turn\|>thinking\n[\s\S]*?<turn\|>\s*/,'').trimStart();
+          content=content.replace(/^\s*<\|turn\|>thinking\n[\s\S]*?<turn\|>\s*/,'').trimStart();
         }
       }
     }

tests/test_issue607.py

@@ -64,6 +64,36 @@ class TestGemma4ThinkingTokenStrip:
         result = _strip_thinking_markup(raw)
         assert result == "Answer"
 
+    def test_mid_sentence_think_tags_are_preserved(self):
+        """Literal discussion of <think> tags in visible prose must survive (#2152)."""
+        raw = "The literal tags <think> and </think> describe reasoning markup."
+        result = _strip_thinking_markup(raw)
+        assert result == raw
+
+    def test_mid_sentence_closed_think_block_is_preserved(self):
+        """Only leading provider wrappers are stripped; later tag-looking text stays visible."""
+        raw = "Use <think>scratchpad</think> as an example in the answer."
+        result = _strip_thinking_markup(raw)
+        assert result == raw
+
+    def test_mid_sentence_channel_tokens_are_preserved(self):
+        """MiniMax-style channel markers later in prose are visible content, not metadata."""
+        raw = "Use <|channel>thought to start reasoning and <channel|> to finish."
+        result = _strip_thinking_markup(raw)
+        assert result == raw
+
+    def test_mid_sentence_gemma4_tokens_are_preserved(self):
+        """Gemma 4 thinking markers later in prose are visible content, not metadata."""
+        raw = "Use <|turn|>thinking to start reasoning and <turn|> to finish."
+        result = _strip_thinking_markup(raw)
+        assert result == raw
+
+    def test_minimax_channel_without_second_pipe_still_stripped_at_start(self):
+        """The client-facing MiniMax <|channel>thought shape is stripped when leading."""
+        raw = "<|channel>thought\nSome reasoning<channel|>Answer"
+        result = _strip_thinking_markup(raw)
+        assert result == "Answer"
+
 
 class TestGemma4TitleLeakDetection:
     """Verify _looks_invalid_generated_title catches Gemma 4 leak."""

tests/test_sprint38.py

@@ -5,7 +5,6 @@ Covers the static render path (ui.js regex logic, verified against the JS source
 and the streaming render path (messages.js _streamDisplay logic).
 """
 import pathlib
-import re
 
 REPO_ROOT = pathlib.Path(__file__).parent.parent
 UI_JS     = (REPO_ROOT / "static" / "ui.js").read_text()
@@ -14,24 +13,25 @@ MSG_JS    = (REPO_ROOT / "static" / "messages.js").read_text()
 
 # ── ui.js: static render path ────────────────────────────────────────────────
 
-def test_think_regex_has_no_anchor():
-    """The <think> regex in ui.js must not use a ^ anchor so leading whitespace is allowed."""
+def test_think_regex_is_leading_only_after_optional_whitespace():
+    """The <think> regex in ui.js must anchor after optional whitespace."""
     # Find the thinkMatch line by locating the .match( call on that line
     idx = UI_JS.find("const thinkMatch=content.match(")
     assert idx >= 0, "thinkMatch line not found in ui.js"
     line = UI_JS[idx:idx+100]
-    # The regex must NOT start with ^ right after the opening /
-    assert "/^<think>" not in line and "(/^" not in line, \
-        f"thinkMatch regex must not use ^ anchor — found: {line.strip()}"
+    assert "/^\\s*<think>" in line, \
+        f"thinkMatch regex must only match leading <think> blocks after whitespace — found: {line.strip()}"
+    assert "/^<think>" not in line, \
+        f"thinkMatch regex must still allow leading whitespace — found: {line.strip()}"
 
 
-def test_gemma_regex_has_no_anchor():
-    """The Gemma channel-token regex in ui.js must not use a ^ anchor."""
-    match = re.search(r'const gemmaMatch=content\.match\((/[^/]+/)\)', UI_JS)
-    assert match, "gemmaMatch line not found in ui.js"
-    pattern = match.group(1)
-    assert not pattern.startswith('/^'), \
-        f"gemmaMatch regex must not use ^ anchor — got {pattern}"
+def test_gemma_regex_is_leading_only_after_optional_whitespace():
+    """The MiniMax channel-token regex in ui.js must anchor after optional whitespace."""
+    idx = UI_JS.find("const gemmaMatch=content.match(")
+    assert idx >= 0, "gemmaMatch line not found in ui.js"
+    line = UI_JS[idx:idx+140]
+    assert "/^\\s*<\\|channel\\|?>thought\\n?" in line, \
+        f"gemmaMatch regex must only match leading channel blocks after whitespace — found: {line.strip()}"
 
 
 def test_think_content_removal_uses_replace_not_slice():
@@ -42,6 +42,8 @@ def test_think_content_removal_uses_replace_not_slice():
     block = UI_JS[idx:idx+200]
     assert "content.replace(" in block, \
         "ui.js must use content.replace() to remove <think> block (not .slice())"
+    assert "content.replace(/^\\s*<think>" in block, \
+        "ui.js must remove only leading <think> blocks after optional whitespace"
     assert ".trimStart()" in block, \
         "ui.js must call .trimStart() on content after removing the <think> block"
 
@@ -53,6 +55,8 @@ def test_gemma_content_removal_uses_replace_not_slice():
     block = UI_JS[idx:idx+200]
     assert "content.replace(" in block, \
         "ui.js must use content.replace() to remove Gemma channel block (not .slice())"
+    assert "content.replace(/^\\s*<\\|channel\\|?>thought\\n?" in block, \
+        "ui.js must remove only leading Gemma channel blocks after optional whitespace"
     assert ".trimStart()" in block, \
         "ui.js must call .trimStart() on content after removing the Gemma channel block"
 
@@ -65,11 +69,11 @@ def test_gemma_turn_regex_in_ui_js():
         " (note: double-pipe: <|turn|> not <|turn>)"
     )
     # Extraction block
-    match = re.search(r'const gemmaTurnMatch=content\.match\((/[^/]+/)\)', UI_JS)
-    assert match, "gemmaTurnMatch line not found in ui.js"
-    pattern = match.group(1)
-    assert not pattern.startswith('/^'), (
-        f"gemmaTurnMatch regex must not use ^ anchor — got {pattern}"
+    idx = UI_JS.find("const gemmaTurnMatch=content.match(")
+    assert idx >= 0, "gemmaTurnMatch line not found in ui.js"
+    line = UI_JS[idx:idx+140]
+    assert "/^\\s*<\\|turn\\|>thinking\\n" in line, (
+        f"gemmaTurnMatch regex must only match leading Gemma 4 blocks after whitespace — found: {line.strip()}"
     )
 
 
@@ -81,11 +85,24 @@ def test_gemma_turn_content_removal_uses_replace_not_slice():
     assert "content.replace(" in block, (
         "ui.js must use content.replace() to remove Gemma 4 turn block (not .slice())"
     )
+    assert "content.replace(/^\\s*<\\|turn\\|>thinking\\n" in block, (
+        "ui.js must remove only leading Gemma 4 turn blocks after optional whitespace"
+    )
     assert ".trimStart()" in block, (
         "ui.js must call .trimStart() on content after removing the Gemma 4 turn block"
     )
 
 
+def test_message_reasoning_payload_detection_is_leading_only():
+    """Persisted literal tag discussion later in content must not create a thinking card."""
+    idx = UI_JS.find("function _messageHasReasoningPayload(m)")
+    assert idx >= 0, "_messageHasReasoningPayload function not found in ui.js"
+    block = UI_JS[idx:idx+500]
+    assert "return /^\\s*(?:<think>" in block, (
+        "_messageHasReasoningPayload must only detect leading provider thinking wrappers"
+    )
+
+
 # ── messages.js: streaming render path ───────────────────────────────────────
 
 def test_stream_display_trims_before_startswith():
@@ -128,13 +145,13 @@ def test_stream_display_trims_return_after_close():
 
 # ── Regression: existing anchored patterns must be gone ──────────────────────
 
-def test_no_anchored_think_regex_in_ui_js():
-    """The old anchored regex /^<think>/ must not exist in ui.js."""
+def test_no_strictly_anchored_think_regex_in_ui_js():
+    """The old /^<think>/ shape must not return; leading whitespace remains supported."""
     assert "/^<think>" not in UI_JS, \
         "Old anchored /^<think>/ regex still present in ui.js — fix not applied"
 
 
-def test_no_anchored_gemma_regex_in_ui_js():
-    """The old anchored Gemma regex must not exist in ui.js."""
+def test_no_strictly_anchored_gemma_regex_in_ui_js():
+    """The old /^<|channel>/ shape must not return; leading whitespace remains supported."""
     assert "/^<|channel>" not in UI_JS, \
         "Old anchored /^<|channel>/ regex still present in ui.js — fix not applied"