* fix(streaming): normalize inline thinking extraction across live and persisted turns (#3599) # Conflicts: # api/streaming.py # static/messages.js # static/ui.js * fix(streaming): code-aware inline-thinking extraction + position-aware unclosed handling Codex deep-review caught two regressions in the leading-only -> full-scan rewrite (both silent data-mangling on the persist/reload path): 1. Code-span unawareness: the scanner only protected triple fences, so a literal <think> in an inline single-backtick code span or an indented (>=4-space/tab) code block got silently extracted into reasoning. Added _inline_thinking_indented_code_at + inline-backtick tracking (Python + the JS twin _thinkingIndentedCodeAt), so all three code contexts now keep thinking tags visible. 2. Unclosed-tag truncation: any unmatched open tag moved the trailing prose into reasoning. Now position-aware — a LEADING unclosed block (cut off mid-thought) is still reasoning (#3455 intent), but an unclosed tag AFTER visible content stays visible so literal typed tags don't truncate prose. Gated partial handling on the previously-unused options.streaming param (live streaming keeps 'still thinking' behavior; persist/reload does not). Updated 2 tests that pinned the buggy behavior + added 4 regression tests (inline-backtick, indented-code, mid-body-unclosed-visible, leading-unclosed- extracted). Updated the node driver harness to include the new helper. Co-authored-by: rodboev <rodboev@users.noreply.github.com> * fix(streaming): recognize fenced code blocks indented 1-3 spaces Codex round-3: a fence indented 1-3 spaces is valid Markdown but the fence detector only matched at column 0, so a literal think tag inside such a fence (not 4+-space indented code either) was still extracted. Both detectors (_inline_thinking_fence_marker_at / _thinkingFenceMarkerAt) now walk back over up to 3 leading spaces to a line start. Added backtick + tilde indented-fence regression tests. Co-authored-by: rodboev <rodboev@users.noreply.github.com> * fix(streaming): O(n) inline-thinking scan + merge separate reasoning on reload Round-4 Codex deep-review caught two real issues in my own fixes: 1. PERF (O(n^2)): the indented-code check (_inline_thinking_indented_code_at / _thinkingIndentedCodeAt) scanned to line boundaries at EVERY character index, plus the leading check sliced+stripped the whole prefix per unclosed tag. On long no-newline content this was quadratic (~8.4s @ 200k, called repeatedly on the streaming path). Replaced with incremental O(1)-per-iteration line state (_line_is_indented_code / _lineIsIndentedCode evaluated only at line starts) + a seen_nonspace flag. 200k now extracts in ~55-140ms. 2. RELOAD reasoning-drop: renderMessages() seeded the shared extractor with '' so a message with BOTH an inline <think> block AND a separate m.reasoning payload showed only the inline part — the separate payload was dropped because the !thinkingText worklog resolution was then skipped. Now seeds with the message's direct reasoning (m.reasoning_content||m.reasoning||...) so the two MERGE (deduped); separate-only reasoning is preserved without promoting it into visible prose. Python + JS twins kept line-for-line parity. Added merge + perf + reload regression tests; updated the reload structure test and the node driver harness for the renamed helper. Co-authored-by: rodboev <rodboev@users.noreply.github.com> * fix(streaming): revert reload reasoning-seed; keep O(n) perf fix Codex round-4 finding #2 (seed renderMessages' inline extractor with m.reasoning so a separate payload merges) turned out to VIOLATE a deliberate architectural invariant pinned by test_issue2565 + test_sprint42: the reload content-extraction path must NOT touch m.reasoning/m.reasoning_content — reasoning metadata is owned exclusively by the Worklog Thinking Card path (_worklogReasoningTextFromMessage / _assistantReasoningPayloadText), never conflated with inline-content extraction (which would risk promoting provider reasoning into final-answer prose). Reverted the ui.js seed to the PR's original `thinkingText` arg. The inline+separate merge is still a genuine extractor capability (exercised by the live streaming path via liveReasoningText) and is covered by a unit test, just not invoked from the reload render path by design. The O(n) perf fix (finding #1) and the code-awareness + position-aware unclosed handling (rounds 1-3) are all retained. Co-authored-by: rodboev <rodboev@users.noreply.github.com> * fix(streaming): only lstrip extracted content when a leading block was removed Codex round-5 catch: the extractor unconditionally lstripped the final content (.lstrip() / .replace(/^\s+/,'')) even when NO thinking block was extracted, so an assistant reply that legitimately starts with an indented code block or blank lines lost its leading whitespace on live display, reload, and persistence. This was a real regression vs master (master returned non-thinking content unchanged). Now track leading_removed (set only when a LEADING thinking block/prefix is actually extracted) and lstrip only in that case. Mid-body / no-thinking content keeps its exact leading whitespace. Python + JS twins kept in parity; added backend regression tests (indented-first preserved, leading-blank preserved, leading-think still strips). Co-authored-by: rodboev <rodboev@users.noreply.github.com> * fix(streaming): reconnect restore prefers raw inflight accumulator Codex round-6 CORE catch: on reconnect, the single-live-message restore used (_liveInflightAssistant.content || ''). Because the PR now splits a leading unclosed <think> into empty content, restoring from the split content dropped the open tag — so a later </think> token leaked into the visible reply and corrupted the live accumulator. Restore from (_fullInflightAssistant || _liveInflightAssistant.content || '') so the raw open tag survives reconnect and the accumulator stays correct. Added a reconnect-restore regression test. Co-authored-by: rodboev <rodboev@users.noreply.github.com> * Release v0.51.335 — Release KY (normalize inline thinking extraction, #3633) Unify inline-thinking (<think>/<|channel>/<|turn|>) extraction across live, reload, and persisted turns (#3599/#3633, @rodboev). Deep-reviewed: Opus + 6 Codex rounds; maintainer fixes resolved every Codex finding — code-awareness (inline-backtick/indented/1-3-space fences keep literal tags visible), position-aware unclosed handling, O(n) line scanning (was O(n^2) on long content), conditional lstrip (preserve leading whitespace when no leading block removed), and a reconnect-restore CORE fix (raw accumulator preferred so an open <think> tag survives reconnect). Python + JS twins in parity. Full suite 8330, Opus SHIP-SAFE, Codex SAFE-TO-SHIP, ESLint/scope-undef/ruff clean. Co-authored-by: rodboev <rodboev@users.noreply.github.com> --------- Co-authored-by: Rod Boev <rod.boev@gmail.com> Co-authored-by: Hermes Agent <hermes-agent@nesquena-hermes.local> Co-authored-by: rodboev <rodboev@users.noreply.github.com>
163 lines
8.0 KiB
Python
163 lines
8.0 KiB
Python
"""
|
|
Sprint 38 Tests: Think-tag stripping with leading whitespace (PR #327).
|
|
|
|
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
|
|
|
|
REPO_ROOT = pathlib.Path(__file__).parent.parent
|
|
UI_JS = (REPO_ROOT / "static" / "ui.js").read_text()
|
|
MSG_JS = (REPO_ROOT / "static" / "messages.js").read_text()
|
|
|
|
|
|
# ── ui.js: static render path ────────────────────────────────────────────────
|
|
|
|
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]
|
|
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_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():
|
|
"""After extracting thinkingText, content must use .replace() not .slice() to remove the tag."""
|
|
# Find the block that handles thinkMatch
|
|
idx = UI_JS.find("if(thinkMatch){")
|
|
assert idx >= 0, "thinkMatch handler block not found"
|
|
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"
|
|
|
|
|
|
def test_gemma_content_removal_uses_replace_not_slice():
|
|
"""Gemma channel token removal must also use .replace() not .slice()."""
|
|
idx = UI_JS.find("if(gemmaMatch){")
|
|
assert idx >= 0, "gemmaMatch handler block not found"
|
|
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"
|
|
|
|
|
|
def test_gemma_turn_regex_in_ui_js():
|
|
"""The Gemma 4 <|turn|>thinking\\n...<turn|> pattern must be extracted from persisted content."""
|
|
# Detection in _messageHasReasoningPayload (correct double-pipe format)
|
|
assert "<\\|turn\\|>thinking" in UI_JS, (
|
|
"ui.js _messageHasReasoningPayload must detect Gemma 4 <|turn|>thinking\\n...<turn|> pattern"
|
|
" (note: double-pipe: <|turn|> not <|turn>)"
|
|
)
|
|
# Extraction block
|
|
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()}"
|
|
)
|
|
|
|
|
|
def test_gemma_turn_content_removal_uses_replace_not_slice():
|
|
"""Gemma 4 turn token removal must use .replace() not .slice()."""
|
|
idx = UI_JS.find("if(gemmaTurnMatch){")
|
|
assert idx >= 0, "gemmaTurnMatch handler block not found in ui.js"
|
|
block = UI_JS[idx:idx+240]
|
|
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():
|
|
"""Browser runtime should use the shared extractor, with a leading-only fallback regex."""
|
|
idx = UI_JS.find("function _messageHasReasoningPayload(m)")
|
|
assert idx >= 0, "_messageHasReasoningPayload function not found in ui.js"
|
|
block = UI_JS[idx:idx+800]
|
|
assert "window._extractInlineThinkingFromContentForRender" in block, (
|
|
"_messageHasReasoningPayload must use the shared extractor in browser runtimes"
|
|
)
|
|
assert "return !!(split&&split.reasoning);" in block, (
|
|
"_messageHasReasoningPayload must treat extracted reasoning as the browser truth source"
|
|
)
|
|
assert "return /^\\s*(?:<think>" in block, (
|
|
"_messageHasReasoningPayload must keep the leading-only regex as a non-browser fallback"
|
|
)
|
|
|
|
|
|
# ── messages.js: streaming render path ───────────────────────────────────────
|
|
|
|
def test_stream_display_uses_shared_inline_thinking_extractor():
|
|
"""_streamDisplay in messages.js must share inline thinking extraction semantics."""
|
|
fn_idx = MSG_JS.find("function _streamDisplay()")
|
|
assert fn_idx >= 0, "_streamDisplay function not found in messages.js"
|
|
fn_end = MSG_JS.find("\n }", fn_idx) + 4
|
|
fn_body = MSG_JS[fn_idx:fn_end]
|
|
assert "_extractInlineThinkingFromContent(_stripXmlToolCalls(assistantText), liveReasoningText, {streaming:true}).content" in fn_body, \
|
|
"_streamDisplay must route through the shared inline thinking extractor"
|
|
|
|
|
|
def test_shared_extractor_scans_known_open_tags():
|
|
"""Shared extractor must still match known provider thinking wrappers."""
|
|
fn_idx = MSG_JS.find("function _extractInlineThinkingFromContent(")
|
|
fn_end = MSG_JS.find("\n}", fn_idx) + 2
|
|
fn_body = MSG_JS[fn_idx:fn_end]
|
|
assert "text.startsWith(candidate.open,index)" in fn_body, \
|
|
"Shared extractor must match complete known thinking open tags"
|
|
|
|
|
|
def test_shared_extractor_hides_partial_tag_prefixes():
|
|
"""The partial-tag guard must hide incomplete provider thinking tags."""
|
|
fn_idx = MSG_JS.find("function _extractInlineThinkingFromContent(")
|
|
fn_end = MSG_JS.find("\n}", fn_idx) + 2
|
|
fn_body = MSG_JS[fn_idx:fn_end]
|
|
assert "candidate.open.startsWith(rest)" in fn_body, \
|
|
"Partial-tag guard must hide incomplete provider thinking tag prefixes"
|
|
|
|
|
|
def test_shared_extractor_trims_visible_content_after_leading_block():
|
|
"""After stripping a leading think block, visible content must trim leading whitespace."""
|
|
fn_idx = MSG_JS.find("function _extractInlineThinkingFromContent(")
|
|
fn_end = MSG_JS.find("\n}", fn_idx) + 2
|
|
fn_body = MSG_JS[fn_idx:fn_end]
|
|
assert "visible.join('').replace(/^\\s+/,'')" in fn_body, \
|
|
"Shared extractor must strip leading whitespace from visible content after extraction"
|
|
|
|
|
|
# ── Regression: existing anchored patterns must be gone ──────────────────────
|
|
|
|
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_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"
|