diff --git a/CHANGELOG.md b/CHANGELOG.md index d4bf740be..99b1bcebf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,11 @@ ## [Unreleased] +## [v0.51.335] — 2026-06-08 — Release KY (normalize inline thinking extraction) + +### Fixed +- **Inline reasoning traces are extracted consistently across live, reload, and persisted turns.** Inline-thinking providers (MiniMax-M3, Gemma, OpenAI-compat, Ollama Cloud) that emit `` (or `<|channel>`/`<|turn|>` variants) anywhere in the response now have those traces moved into the Thinking Card uniformly — live, on reload, and in the saved session file — instead of leaving them in the visible answer or bloating the persisted content. Literal thinking tags inside code (inline `` `` ``, fenced blocks, or indented code) stay visible, leading whitespace is preserved when no thinking block is removed, and an unclosed tag only collapses into reasoning when it leads the message. (#3599, #3633, @rodboev) + ## [v0.51.334] — 2026-06-08 — Release KX (new-message cue when scrolled up) ### Added diff --git a/api/streaming.py b/api/streaming.py index 49bd5881a..024e3c2f0 100644 --- a/api/streaming.py +++ b/api/streaming.py @@ -1489,47 +1489,194 @@ def _build_native_multimodal_message(workspace_ctx: str, msg_text: str, attachme return parts if image_count else workspace_ctx + msg_text -def _split_thinking_from_content(raw_content, existing_reasoning=''): - """Split a single LEADING block out of assistant content. +_INLINE_THINKING_TAG_PAIRS = ( + ('', ''), + ('<|channel>thought\n', ''), + ('<|turn|>thinking\n', ''), +) - Server-side twin of the JS ``_splitThinkFromContent`` (static/messages.js). - Inline-thinking providers (e.g. MiniMax-M3, OpenAI-compat) leave the thinking - trace inside the saved ``m['content']``, bloating session files 30-50% and - bypassing the ``m['reasoning']`` field the thinking card reads on reload - (#3455). This extracts exactly ONE leading block (after lstrip) — matching the - live renderer's _streamDisplay/_parseStreamState semantics — so a closed - ``...`` that appears MID-BODY (e.g. a literal tag in a fenced - code block) stays visible content and is never moved into reasoning, and a - partial/unclosed block is left intact. - Returns ``(content, reasoning)``. ``reasoning`` merges ``existing_reasoning`` - (e.g. from a separate on_reasoning stream) with the extracted block. +def _inline_thinking_fence_marker_at(text, index): + # A fenced code block opener may be indented up to 3 spaces in Markdown + # (4+ spaces is an indented code block, handled separately). The marker is + # only a fence when it sits at the start of a line (after optional 1-3 + # spaces of indentation). + if index > 0 and text[index - 1] != '\n': + # Allow up to 3 leading spaces: walk back over spaces to a line start. + back = index - 1 + spaces = 0 + while back >= 0 and text[back] == ' ' and spaces < 3: + back -= 1 + spaces += 1 + if not (back < 0 or text[back] == '\n'): + return '' + if text.startswith('```', index): + return '```' + if text.startswith('~~~', index): + return '~~~' + return '' + + +def _line_is_indented_code(text, line_start): + """True when the line beginning at `line_start` is a markdown indented code + block line (>=4 leading spaces or a leading tab, and not blank). `line_start` + must be the index of the first character of the line. O(1)-ish: only inspects + the line's leading characters, not the whole document (the per-character + variant was O(n^2) on long no-newline content — #3633 Codex perf catch).""" + if line_start >= len(text): + return False + if text[line_start] == '\t': + # A leading tab is indented code only if the line isn't otherwise blank. + nl = text.find('\n', line_start) + seg = text[line_start:(nl if nl != -1 else len(text))] + return bool(seg.strip()) + if text.startswith(' ', line_start): + nl = text.find('\n', line_start) + seg = text[line_start:(nl if nl != -1 else len(text))] + return bool(seg.strip()) + return False + + +def _merge_inline_thinking_reasoning(existing_reasoning, extracted_parts): + out = str(existing_reasoning or '').strip() + for part in extracted_parts or (): + item = str(part or '').strip() + if not item: + continue + if not out: + out = item + continue + if out == item or any(existing.strip() == item for existing in out.split('\n\n')): + continue + out = out + '\n\n' + item + return out + + +def _extract_inline_thinking_from_content(raw_content, existing_reasoning='', *, streaming=False): + """Split inline thinking blocks out of assistant content. + + Code-aware: thinking tags inside a triple-fence (``` / ~~~), an inline + single-backtick code span, or an indented (>=4-space / tab) code block are + LEFT VISIBLE — they are literal text a user typed/pasted, not a real thinking + trace. (#3633 deep-review / Codex catch: the earlier full-scan version only + protected triple fences, so a literal `` in an inline code span got + silently extracted.) + + ``streaming`` gates partial/unclosed-block handling: during live streaming an + unmatched open tag means "still thinking" and its tail is shown as reasoning; + on the persist/reload path (streaming=False) an unclosed tag is LEFT VISIBLE + so prose after a literal ```` is never silently truncated on save. """ text = '' if raw_content is None else str(raw_content) if not text: - return text, (existing_reasoning or '') - # Leading-only, single block — same three tag pairs as the JS helper. - _pairs = ( - ('', ''), - ('<|channel>thought\n', ''), - ('<|turn|>thinking\n', ''), + return text, str(existing_reasoning or '').strip() + visible = [] + extracted = [] + cursor = 0 + index = 0 + fence = '' + in_backtick = False + length = len(text) + # Incremental, O(1)-per-iteration line state (the previous per-character line + # scan made the whole pass O(n^2) on long no-newline content — #3633 Codex + # perf catch). `line_is_indented_code` is recomputed only at a line start. + line_is_indented_code = _line_is_indented_code(text, 0) + # Whether any non-whitespace char appeared in text[:index] — the cheap + # equivalent of the old `text[:index].strip() != ''` leading check. + seen_nonspace = False + # Whether a LEADING thinking block/prefix was removed — only then do we + # lstrip the final content (so a reply that legitimately starts with + # indented code / whitespace and has NO leading thinking wrapper keeps its + # leading whitespace — #3633 Codex catch). + leading_removed = False + while index < length: + ch = text[index] + if index > 0 and text[index - 1] == '\n': + line_is_indented_code = _line_is_indented_code(text, index) + marker = _inline_thinking_fence_marker_at(text, index) + if marker: + fence = '' if fence == marker else (fence or marker) + # Inline single-backtick code span toggles on each lone backtick that is + # not part of a triple fence. Only tracked outside a triple fence. + if not fence and not marker and ch == '`': + in_backtick = not in_backtick + in_code = bool(fence) or in_backtick or line_is_indented_code + if not in_code: + pair = None + for open_tag, close_tag in _INLINE_THINKING_TAG_PAIRS: + if text.startswith(open_tag, index): + pair = (open_tag, close_tag) + break + if pair: + open_tag, close_tag = pair + close_index = text.find(close_tag, index + len(open_tag)) + if close_index == -1: + # Unclosed open tag. A LEADING unclosed block (nothing + # visible before it) is a genuine thinking trace that got + # cut off / persisted mid-thought → reasoning (master #3455 + # leading-only intent, and the live-stream "still thinking" + # case). An unclosed tag AFTER visible content on the persist + # path is almost always a literal typed tag — leave it (and + # the prose after it) visible so nothing is silently + # truncated (#3633 Codex catch). During live streaming any + # unmatched open tag is treated as in-progress thinking. + leading = not seen_nonspace + if not streaming and not leading: + break + if leading: + leading_removed = True + visible.append(text[cursor:index]) + partial = text[index + len(open_tag):] + if partial: + extracted.append(partial) + cursor = length + index = length + break + visible.append(text[cursor:index]) + extracted.append(text[index + len(open_tag):close_index]) + if not seen_nonspace: + leading_removed = True + seen_nonspace = True # the extracted tag span is non-whitespace + index = close_index + len(close_tag) + cursor = index + continue + if streaming: + matched_partial = False + for open_tag, _close_tag in _INLINE_THINKING_TAG_PAIRS: + rest = text[index:] + if len(rest) < len(open_tag) and open_tag.startswith(rest): + if not seen_nonspace: + leading_removed = True + visible.append(text[cursor:index]) + cursor = length + index = length + matched_partial = True + break + if matched_partial or index >= length: + break + if not ch.isspace(): + seen_nonspace = True + index += 1 + if cursor < length: + visible.append(text[cursor:]) + content = ''.join(visible) + if leading_removed: + content = content.lstrip() + reasoning = _merge_inline_thinking_reasoning(existing_reasoning, extracted) + return content, reasoning + + +def _split_thinking_from_content(raw_content, existing_reasoning=''): + """Split inline thinking blocks out of assistant content for persistence. + + Persistence path: streaming=False, so an unclosed tag stays visible content + (a partial block only means "still thinking" during a live stream). + """ + return _extract_inline_thinking_from_content( + raw_content, + existing_reasoning=existing_reasoning, + streaming=False, ) - trimmed = text.lstrip() - extracted = '' - remaining = text - for open_tag, close_tag in _pairs: - if not trimmed.startswith(open_tag): - continue - ci = trimmed.find(close_tag, len(open_tag)) - if ci == -1: - break # partial open — leave intact - extracted = trimmed[len(open_tag):ci] - remaining = trimmed[ci + len(close_tag):].lstrip() - break - if not extracted: - return raw_content, (existing_reasoning or '') - final_reasoning = (existing_reasoning + '\n\n' + extracted) if existing_reasoning else extracted - return remaining, final_reasoning def _strip_thinking_markup(text: str) -> str: @@ -6570,14 +6717,11 @@ def _run_agent_streaming( # memory until the next turn's save, and the last-turn thinking card # is lost when the user reloads immediately after a response. # - # #3455: also split any inline leading block out of the saved + # #3455/#3599: split inline thinking blocks out of the saved # assistant content into m['reasoning'] (server-side twin of the JS # _splitThinkFromContent). Inline-thinking providers (e.g. MiniMax-M3) # otherwise leave the thinking trace in m['content'], bloating the # persisted session file 30-50% and bypassing the thinking card. The - # split is leading-only/single-block so mid-body literal tags (e.g. in - # a fenced code block) stay visible content. - # # #3587: use per-message segments so intermediate assistant turns # (before tool calls) each receive their own reasoning trace rather # than all reasoning being written only to the last assistant message. diff --git a/static/messages.js b/static/messages.js index 5ae2daca8..e8fb707b7 100644 --- a/static/messages.js +++ b/static/messages.js @@ -106,6 +106,154 @@ let _selectedTextReplyBtn=null; let _selectedTextReplyText=''; let _selectedTextReplyRaf=0; const _persistentStateToastSeen=new Set(); +const _thinkPairs=[ + {open:'',close:''}, + {open:'<|channel>thought\n',close:''}, + {open:'<|turn|>thinking\n',close:''} +]; + +function _thinkingFenceMarkerAt(text, index){ + // A fenced code block opener may be indented up to 3 spaces in Markdown + // (4+ spaces is an indented code block, handled separately). Only treat the + // marker as a fence when it sits at a line start after optional 1-3 spaces. + if(index>0&&text[index-1]!=='\n'){ + let back=index-1, spaces=0; + while(back>=0&&text[back]===' '&&spaces<3){back--;spaces++;} + if(!(back<0||text[back]==='\n')) return ''; + } + if(text.startsWith('```',index)) return '```'; + if(text.startsWith('~~~',index)) return '~~~'; + return ''; +} + +function _lineIsIndentedCode(text, lineStart){ + // True when the line beginning at lineStart is a markdown indented code block + // line (>=4 leading spaces or a leading tab, and not blank). lineStart must be + // the first char of the line. Only inspects the line's leading chars, not the + // whole document (the per-character variant was O(n^2) on long no-newline + // content — #3633 Codex perf catch). + if(lineStart>=text.length) return false; + if(text[lineStart]==='\t'||text.startsWith(' ',lineStart)){ + let nl=text.indexOf('\n',lineStart); + if(nl===-1) nl=text.length; + return text.slice(lineStart,nl).trim()!==''; + } + return false; +} + +function _mergeInlineThinkingReasoning(existingReasoning, extractedParts){ + let out=String(existingReasoning||'').trim(); + (Array.isArray(extractedParts)?extractedParts:[]).forEach(function(part){ + const item=String(part||'').trim(); + if(!item) return; + if(!out){out=item;return;} + if(out===item||out.split('\n\n').some(function(existing){return existing.trim()===item;})) return; + out += '\n\n' + item; + }); + return out; +} + +function _extractInlineThinkingFromContent(rawContent, existingReasoning, options){ + // Code-aware extraction (must mirror api/streaming.py + // _extract_inline_thinking_from_content): thinking tags inside a triple-fence, + // an inline single-backtick code span, or an indented code block are LEFT + // VISIBLE. options.streaming gates partial/unclosed handling — only during a + // live stream does an unmatched open tag mean "still thinking"; on the + // reload/render path an unclosed tag stays visible content (#3633 Codex catch). + const streaming=!!(options&&options.streaming); + const text=String(rawContent||''); + if(!text){ + const reasoning=String(existingReasoning||'').trim(); + return {reasoning,content:text,thinkingText:reasoning,displayText:text,inThinking:false}; + } + const visible=[]; + const extracted=[]; + let cursor=0; + let index=0; + let fence=''; + let inBacktick=false; + let inThinking=false; + // Incremental O(1)-per-iteration line state + seen-nonspace flag (the previous + // per-character line scan + slice(0,index).trim() were O(n^2) on long + // no-newline content — #3633 Codex perf catch). + let lineIsIndentedCode=_lineIsIndentedCode(text,0); + let seenNonspace=false; + // Only lstrip the final content when a LEADING thinking block/prefix was + // removed — a reply that legitimately starts with indented code / whitespace + // and has no leading thinking wrapper keeps its leading whitespace (#3633 + // Codex catch). + let leadingRemoved=false; + while(index0&&text[index-1]==='\n') lineIsIndentedCode=_lineIsIndentedCode(text,index); + const marker=_thinkingFenceMarkerAt(text,index); + if(marker) fence=(fence===marker)?'':(fence||marker); + if(!fence&&!marker&&ch==='`') inBacktick=!inBacktick; + const inCode=!!fence||inBacktick||lineIsIndentedCode; + if(!inCode){ + let pair=null; + for(const candidate of _thinkPairs){ + if(text.startsWith(candidate.open,index)){pair=candidate;break;} + } + if(pair){ + const closeIndex=text.indexOf(pair.close,index+pair.open.length); + if(closeIndex===-1){ + // Unclosed open tag. A LEADING unclosed block (nothing visible before + // it) is a genuine thinking trace cut off mid-thought → reasoning + // (master #3455 leading-only intent + live "still thinking"). An + // unclosed tag AFTER visible content on the reload/render path is + // almost always a literal typed tag — leave it (and following prose) + // visible so nothing is silently truncated (#3633 Codex catch). + const leading=!seenNonspace; + if(!streaming&&!leading) break; + if(leading) leadingRemoved=true; + visible.push(text.slice(cursor,index)); + const partial=text.slice(index+pair.open.length); + if(partial) extracted.push(partial); + inThinking=true; + cursor=text.length; + index=text.length; + break; + } + visible.push(text.slice(cursor,index)); + extracted.push(text.slice(index+pair.open.length,closeIndex)); + if(!seenNonspace) leadingRemoved=true; + seenNonspace=true; + index=closeIndex+pair.close.length; + cursor=index; + continue; + } + if(streaming){ + let matchedPartial=false; + for(const candidate of _thinkPairs){ + const rest=text.slice(index); + if(rest.length=text.length) break; + } + } + if(ch.trim()!=='') seenNonspace=true; + index++; + } + if(cursor1 ? (_fullInflightAssistant || _joinedInflightSegments) : (_liveInflightAssistant - ? (_liveInflightAssistant.content || '') + ? (_fullInflightAssistant || _liveInflightAssistant.content || '') : _fullInflightAssistant)) : ''; const _lastLiveReasoning = reconnecting @@ -1104,13 +1252,6 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ // On reconnect, the assistantBody already has partial smd-rendered content. // We clear it on first new token and restart the parser from the reconnect point. let _smdReconnect=reconnecting; - // Thinking tag patterns for streaming display - const _thinkPairs=[ - {open:'',close:''}, - {open:'<|channel>thought\n',close:''}, - {open:'<|turn|>thinking\n',close:''} // Gemma 4 - ]; - function _isActiveSession(){ return !!(S.session&&S.session.session_id===activeSid); } @@ -1268,30 +1409,7 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ // reasoning channel, which would otherwise bloat the persisted session // message by 30-50% and miss the m.reasoning field used by the thinking card. function _splitThinkFromContent(rawContent, existingReasoning){ - const text=String(rawContent||''); - if(!text) return {reasoning:existingReasoning||'', content:text}; - // Extract exactly ONE leading think block (after lstrip), matching the - // streaming renderer's _streamDisplay/_parseStreamState semantics EXACTLY — - // both strip only the first leading block. A closed ... that - // appears MID-BODY is, by the renderer's definition, visible content (e.g. a - // literal tag inside a fenced code block); a whole-body scan would silently - // move it into m.reasoning. And looping multiple leading blocks here (when the - // renderer strips only one) would make persisted/reload content diverge from - // the live stream. So: leading, single, partial-open left intact (#3455 review, Codex). - let extracted=''; - let remaining=text; - const trimmed=text.trimStart(); - for(const {open,close} of _thinkPairs){ - if(!trimmed.startsWith(open)) continue; - const ci=trimmed.indexOf(close,open.length); - if(ci===-1) break; // partial open — leave intact for the live renderer - extracted=trimmed.slice(open.length,ci); - remaining=trimmed.slice(ci+close.length).replace(/^\s+/,''); - break; - } - if(!extracted) return {reasoning:existingReasoning||'', content:rawContent}; - const finalReasoning=existingReasoning?existingReasoning+'\n\n'+extracted:extracted; - return {reasoning:finalReasoning, content:remaining}; + return _extractInlineThinkingFromContent(rawContent, existingReasoning, {streaming:false}); } function syncInflightAssistantMessage(){ const inflight=INFLIGHT[activeSid]; @@ -1541,57 +1659,10 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ return s.trim(); } function _streamDisplay(){ - const raw=_stripXmlToolCalls(assistantText); - // Always run think-block stripping even when reasoningText is populated. - // Some providers emit reasoning content via on_reasoning AND wrap it in - // tags in the token stream — the early-return caused the thinking - // card and main response to show identical content (closes #852). - for(const {open,close} of _thinkPairs){ - // Trim leading whitespace before checking for the open tag — some models - // (e.g. MiniMax) emit newlines before . - const trimmed=raw.trimStart(); - if(trimmed.startsWith(open)){ - const ci=trimmed.indexOf(close,open.length); - if(ci!==-1){ - // Thinking block complete — strip it, show the rest - return trimmed.slice(ci+close.length).replace(/^\s+/,''); - } - // Still inside thinking block — show placeholder - return ''; - } - // Hide partial tag prefixes while streaming so users don't see - // `p&&(p.type==='thinking'||p.type==='reasoning')); + if(typeof window!=='undefined'&&typeof window._extractInlineThinkingFromContentForRender==='function'){ + const split=window._extractInlineThinkingFromContentForRender(String(m.content||''),''); + return !!(split&&split.reasoning); + } return /^\s*(?:[\s\S]*?<\/think>|<\|channel\|?>thought\n?[\s\S]*?|<\|turn\|>thinking\n[\s\S]*?)/.test(String(m.content||'')); } function _isAssistantEmptyPlaceholderContent(m, content){ @@ -6285,6 +6289,10 @@ function _assistantReasoningPayloadText(m){ return parts.join('\n').trim(); } const text=String(m.content||''); + if(typeof window!=='undefined'&&typeof window._extractInlineThinkingFromContentForRender==='function'){ + const split=window._extractInlineThinkingFromContentForRender(text,''); + if(split&&String(split.reasoning||'').trim()) return String(split.reasoning).trim(); + } // Extract a LEADING thinking block even when visible answer text follows it // (e.g. "4"). The matching display-content stripper // (_stripLeadingAssistantThinkingMarkup) is non-anchored, so the extractor must @@ -6315,7 +6323,14 @@ function _assistantVisibleContentForReasoningCompare(m){ if(Array.isArray(content)){ content=content.filter(p=>p&&p.type==='text').map(p=>p.text||p.content||'').join('\n'); } - if(typeof content==='string') content=_stripLeadingAssistantThinkingMarkup(content); + if(typeof content==='string'){ + if(typeof window!=='undefined'&&typeof window._extractInlineThinkingFromContentForRender==='function'){ + const split=window._extractInlineThinkingFromContentForRender(content,''); + content=split&&typeof split.content==='string'?split.content:_stripLeadingAssistantThinkingMarkup(content); + } else { + content=_stripLeadingAssistantThinkingMarkup(content); + } + } if(_isMarkerOnlyAssistantCompressionMessage(m)){ content='**Error:** No response received after context compression. Please retry.'; } @@ -8072,23 +8087,30 @@ function renderMessages(options){ if(Array.isArray(content)){ content=content.filter(p=>p&&p.type==='text').map(p=>p.text||p.content||'').join('\n'); } - if(!thinkingText && typeof content==='string'){ - const thinkMatch=content.match(/^\s*([\s\S]*?)<\/think>\s*/); - if(thinkMatch){ - content=content.replace(/^\s*[\s\S]*?<\/think>\s*/,'').trimStart(); - } - if(!thinkingText){ - // Historical name "gemmaMatch" refers to MiniMax <|channel>thought format. - const gemmaMatch=content.match(/^\s*<\|channel\|?>thought\n?([\s\S]*?)\s*/); - if(gemmaMatch){ - content=content.replace(/^\s*<\|channel\|?>thought\n?[\s\S]*?\s*/,'').trimStart(); + if(typeof content==='string'){ + if(typeof window!=='undefined'&&typeof window._extractInlineThinkingFromContentForRender==='function'){ + const split=window._extractInlineThinkingFromContentForRender(content, thinkingText); + thinkingText=split.reasoning||thinkingText; + content=split.content; + }else if(!thinkingText){ + const thinkMatch=content.match(/^\s*([\s\S]*?)<\/think>\s*/); + if(thinkMatch){ + thinkingText=thinkMatch[1].trim(); + content=content.replace(/^\s*[\s\S]*?<\/think>\s*/,'').trimStart(); } - } - if(!thinkingText){ - // Gemma 4 uses asymmetric <|turn|>thinking\n... delimiters. - const gemmaTurnMatch=content.match(/^\s*<\|turn\|>thinking\n([\s\S]*?)\s*/); - if(gemmaTurnMatch){ - content=content.replace(/^\s*<\|turn\|>thinking\n[\s\S]*?\s*/,'').trimStart(); + if(!thinkingText){ + const gemmaMatch=content.match(/^\s*<\|channel\|?>thought\n?([\s\S]*?)\s*/); + if(gemmaMatch){ + thinkingText=gemmaMatch[1].trim(); + content=content.replace(/^\s*<\|channel\|?>thought\n?[\s\S]*?\s*/,'').trimStart(); + } + } + if(!thinkingText){ + const gemmaTurnMatch=content.match(/^\s*<\|turn\|>thinking\n([\s\S]*?)\s*/); + if(gemmaTurnMatch){ + thinkingText=gemmaTurnMatch[1].trim(); + content=content.replace(/^\s*<\|turn\|>thinking\n[\s\S]*?\s*/,'').trimStart(); + } } } } diff --git a/tests/test_issue3401_deep_review_fixes.py b/tests/test_issue3401_deep_review_fixes.py index 233157a40..6973f3f07 100644 --- a/tests/test_issue3401_deep_review_fixes.py +++ b/tests/test_issue3401_deep_review_fixes.py @@ -147,3 +147,23 @@ def test_settled_worklog_rebuild_not_gated_on_idle_only(): "the worklog-rebuild block must not be gated on the bare !S.busy form" ) + + +def test_reconnect_restore_prefers_raw_inflight_accumulator(): + """#3633 Codex CORE catch: on reconnect, the single-live-message restore must + prefer the RAW inflight accumulator (_fullInflightAssistant = lastAssistantText) + over the SPLIT live message content. Since the PR now splits a leading unclosed + into empty content, restoring from _liveInflightAssistant.content alone + would drop the open tag — so a later token would leak into the visible + reply and corrupt the accumulator. The raw text keeps the open tag intact.""" + body = MESSAGES_JS + assert "_fullInflightAssistant || _liveInflightAssistant.content || ''" in body, ( + "single-live-message reconnect restore must be " + "(_fullInflightAssistant || _liveInflightAssistant.content || '') so the " + "raw open tag survives reconnect" + ) + # The buggy form (split content preferred first) must be gone. + assert "? (_liveInflightAssistant.content || '')\n" not in body, ( + "reconnect restore must not prefer the split live content over the raw " + "inflight accumulator" + ) diff --git a/tests/test_issue3455_think_block_extraction.py b/tests/test_issue3455_think_block_extraction.py index ffc19cf8f..827031fe9 100644 --- a/tests/test_issue3455_think_block_extraction.py +++ b/tests/test_issue3455_think_block_extraction.py @@ -49,6 +49,10 @@ def _extract_block(src: str, marker: str) -> str: _DRIVER = """ %s %s +%s +%s +%s +%s const args = JSON.parse(process.argv[2]); process.stdout.write(JSON.stringify(_splitThinkFromContent(args.raw, args.existing || ''))); """ @@ -59,9 +63,13 @@ def driver(tmp_path_factory): if shutil.which("node") is None: pytest.skip("node not available") pairs = _extract_block(MESSAGES_JS, "const _thinkPairs=") + fence = _extract_block(MESSAGES_JS, "function _thinkingFenceMarkerAt(") + indented = _extract_block(MESSAGES_JS, "function _lineIsIndentedCode(") + merge = _extract_block(MESSAGES_JS, "function _mergeInlineThinkingReasoning(") + extract = _extract_block(MESSAGES_JS, "function _extractInlineThinkingFromContent(") fn = _extract_block(MESSAGES_JS, "function _splitThinkFromContent(") p = tmp_path_factory.mktemp("think3455") / "driver.js" - p.write_text(_DRIVER % (pairs, fn), encoding="utf-8") + p.write_text(_DRIVER % (pairs, fence, indented, merge, extract, fn), encoding="utf-8") return str(p) @@ -85,14 +93,11 @@ def test_think_at_start_extracted(driver): assert r["reasoning"] == "my reasoning" -def test_content_before_think_is_not_extracted(driver): - """Renderer-matching: a think block is only extracted at the LEADING position. - A that appears after real prose is, by the renderer's definition, - visible content and must be left in m.content (not moved to reasoning).""" +def test_content_before_think_is_extracted(driver): + """#3599: inline providers can emit visible prose before a complete think block.""" r = _split(driver, "Real prefix mid tail") - # Not leading -> nothing extracted, content fully preserved. - assert r["content"] == "Real prefix mid tail" - assert r["reasoning"] == "" + assert r["content"] == "Real prefix tail" + assert r["reasoning"] == "mid" def test_closed_literal_think_in_code_block_preserved(driver): @@ -106,12 +111,11 @@ def test_closed_literal_think_in_code_block_preserved(driver): assert "visible literal" in r["content"] -def test_unclosed_think_left_intact(driver): - """Streaming-safe: a partial/unclosed block is not extracted (the live - renderer hides it); content must not be dropped.""" +def test_unclosed_think_hidden_into_reasoning(driver): + """Streaming-safe: a partial/unclosed block is hidden from visible content.""" r = _split(driver, "still thinking...") - assert r["content"] == "still thinking..." - assert r["reasoning"] == "" + assert r["content"] == "" + assert r["reasoning"] == "still thinking..." def test_existing_reasoning_is_merged_not_overwritten(driver): @@ -120,26 +124,34 @@ def test_existing_reasoning_is_merged_not_overwritten(driver): assert r["reasoning"] == "from on_reasoning stream\n\nextra" -def test_single_leading_block_extracted_matches_renderer(driver): - """Only ONE leading think block is extracted — matching _streamDisplay/ - _parseStreamState which strip a single leading block. A second consecutive - block stays in content so persisted state never diverges from the live stream.""" +def test_existing_reasoning_substring_does_not_drop_block(driver): + r = _split(driver, "plananswer", existing="planning the approach carefully") + assert r["content"] == "answer" + assert r["reasoning"] == "planning the approach carefully\n\nplan" + + +def test_multiple_blocks_extracted(driver): + """#3599: multiple complete inline blocks move to reasoning together.""" r = _split(driver, "abthe answer") - assert r["content"] == "bthe answer" - assert r["reasoning"] == "a" + assert r["content"] == "the answer" + assert r["reasoning"] == "a\n\nb" -def test_block_after_content_not_extracted(driver): - """A think block that follows visible content stays in content (renderer only - strips leading blocks).""" +def test_block_after_content_extracted(driver): + """#3599: complete inline blocks after visible content are reasoning too.""" r = _split(driver, "leadanswer trailing more") - assert r["content"] == "answer trailing more" - assert r["reasoning"] == "lead" + assert r["content"] == "answer more" + assert r["reasoning"] == "lead\n\ntrailing" -def test_lookalike_tag_in_code_not_extracted(driver): +def test_lookalike_tag_without_close_after_content_stays_visible(driver): + """#3633 deep-review (Codex catch): a literal token used mid-sentence + and never closed is NOT a thinking trace — it must stay visible, not get the + rest of the line swallowed into reasoning. (A LEADING unclosed block is still + treated as reasoning; see test_unclosed_think_hidden_into_reasoning.)""" r = _split(driver, "use as a literal token, never closed") assert r["content"] == "use as a literal token, never closed" + assert r["reasoning"] == "" def test_empty_content(driver): @@ -176,14 +188,20 @@ class TestBackendThinkSplitParity: assert content == raw assert reasoning == "" - def test_unclosed_left_intact(self): - assert self._sp("still...") == ("still...", "") + def test_unclosed_hidden_into_reasoning(self): + assert self._sp("still...") == ("", "still...") def test_existing_reasoning_merged(self): assert self._sp("newans", "prior") == ("ans", "prior\n\nnew") - def test_single_leading_block_only(self): - assert self._sp("abend") == ("bend", "a") + def test_multiple_blocks_extracted(self): + assert self._sp("abend") == ("end", "a\n\nb") + + def test_substring_reasoning_is_not_dropped(self): + assert self._sp("plananswer", "planning the approach carefully") == ( + "answer", + "planning the approach carefully\n\nplan", + ) def test_empty(self): assert self._sp("") == ("", "") @@ -193,3 +211,47 @@ class TestBackendThinkSplitParity: content, reasoning = self._sp(None) assert content in (None, "") assert reasoning == "" + + # ── #3633 deep-review (Codex catch): code-awareness + unclosed-position ── + def test_inline_backtick_code_span_preserved(self): + """A literal inside an inline single-backtick code span is code, + not a thinking trace — it must stay visible (the earlier full-scan only + protected triple fences).""" + raw = "Use the `foo` tag in your prompt." + assert self._sp(raw) == (raw, "") + + def test_indented_code_block_preserved(self): + """A literal inside a >=4-space indented code block must stay + visible.""" + raw = "Example:\n\n foo\n\ndone" + assert self._sp(raw) == (raw, "") + + def test_mid_body_unclosed_stays_visible(self): + """An unclosed AFTER visible content (a literal typed tag) must + NOT truncate the following prose on the persist path.""" + assert self._sp("answerstill thinking") == ( + "answerstill thinking", + "", + ) + + def test_leading_unclosed_still_extracted(self): + """A LEADING unclosed block (cut off mid-thought) is still reasoning.""" + assert self._sp("still thinking") == ("", "still thinking") + + def test_indented_fence_1_3_spaces_preserved(self): + """A fenced code block indented 1-3 spaces is still a fence (valid + Markdown), so a literal think tag inside it stays visible.""" + backtick = "text\n ```\n lit\n ```\nend" + assert self._sp(backtick) == (backtick, "") + tilde = "text\n ~~~html\n lit\n ~~~\nend" + assert self._sp(tilde) == (tilde, "") + + def test_leading_whitespace_preserved_when_no_thinking_removed(self): + """#3633 Codex catch: content is only lstripped when a LEADING thinking + block/prefix was actually removed. A reply that legitimately starts with + an indented code block or blank lines (and has no leading thinking + wrapper) keeps its leading whitespace.""" + assert self._sp(" indented code\nmore") == (" indented code\nmore", "") + assert self._sp("\n\n hi") == ("\n\n hi", "") + # ...but a leading thinking block still strips the whitespace after it. + assert self._sp("r answer") == ("answer", "r") diff --git a/tests/test_issue3599_inline_thinking_extraction.py b/tests/test_issue3599_inline_thinking_extraction.py new file mode 100644 index 000000000..59f61b65f --- /dev/null +++ b/tests/test_issue3599_inline_thinking_extraction.py @@ -0,0 +1,152 @@ +from pathlib import Path + +from api.streaming import _split_thinking_from_content + + +REPO = Path(__file__).resolve().parents[1] +MESSAGES_JS = (REPO / "static" / "messages.js").read_text(encoding="utf-8") +UI_JS = (REPO / "static" / "ui.js").read_text(encoding="utf-8") +WORKSPACE_JS = (REPO / "static" / "workspace.js").read_text(encoding="utf-8") + + +def _function_body(src: str, signature: str) -> str: + start = src.index(signature) + brace = src.index("{", start) + depth = 0 + for i in range(brace, len(src)): + if src[i] == "{": + depth += 1 + elif src[i] == "}": + depth -= 1 + if depth == 0: + return src[start : i + 1] + raise AssertionError(f"function body not found: {signature}") + + +def test_split_clean_leading_think_block(): + content, reasoning = _split_thinking_from_content("plan\nanswer") + + assert content == "answer" + assert reasoning == "plan" + + +def test_split_extracts_non_leading_complete_block(): + content, reasoning = _split_thinking_from_content("visible before hidden visible after") + + assert "" not in content + assert "visible before" in content + assert "visible after" in content + assert reasoning == "hidden" + + +def test_split_extracts_multiple_complete_blocks(): + content, reasoning = _split_thinking_from_content("onetwo final") + + assert content == "final" + assert reasoning == "one\n\ntwo" + + +def test_split_keeps_fenced_code_literal_think_visible(): + raw = "```html\nliteral\n```\nanswer" + content, reasoning = _split_thinking_from_content(raw) + + assert content == raw + assert reasoning == "" + + +def test_split_merges_existing_reasoning_without_duplicate(): + content, reasoning = _split_thinking_from_content("sameanswer", "same") + + assert content == "answer" + assert reasoning == "same" + + +def test_split_merges_existing_reasoning_with_new_inline_block(): + content, reasoning = _split_thinking_from_content("inlineanswer", "separate") + + assert content == "answer" + assert reasoning == "separate\n\ninline" + + +def test_reasoning_only_content_survives_reload_source_fields(): + content, reasoning = _split_thinking_from_content("only reasoning") + + assert content == "" + assert reasoning == "only reasoning" + + +def test_unclosed_inline_thinking_after_content_stays_visible_on_persist(): + """#3633 deep-review (Codex catch): on the PERSIST path an unclosed think tag + that appears AFTER visible content is almost always a literal typed tag, so + the prose after it must NOT be silently truncated into reasoning. A LEADING + unclosed block (cut off mid-thought) is still treated as reasoning.""" + # Mid-body unclosed → stays fully visible, nothing moved to reasoning. + content, reasoning = _split_thinking_from_content("answerstill thinking") + assert content == "answerstill thinking" + assert reasoning == "" + + # Leading unclosed → genuine cut-off thinking trace, moves to reasoning. + lead_content, lead_reasoning = _split_thinking_from_content("still thinking") + assert lead_content == "" + assert lead_reasoning == "still thinking" + + +def test_messages_js_live_and_persist_paths_share_extractor(): + stream_display = _function_body(MESSAGES_JS, "function _streamDisplay") + parse_state = _function_body(MESSAGES_JS, "function _parseStreamState") + split_persist = _function_body(MESSAGES_JS, "function _splitThinkFromContent") + + assert "_extractInlineThinkingFromContent(_stripXmlToolCalls(assistantText), liveReasoningText, {streaming:true}).content" in stream_display + assert "return _extractInlineThinkingFromContent(_stripXmlToolCalls(assistantText), liveReasoningText, {streaming:true});" in parse_state + assert "return _extractInlineThinkingFromContent(rawContent, existingReasoning, {streaming:false});" in split_persist + assert "window._extractInlineThinkingFromContentForRender" in MESSAGES_JS + assert "_thinkingFenceMarkerAt" in MESSAGES_JS + + +def test_render_messages_uses_shared_extractor_on_reload(): + render = _function_body(UI_JS, "function renderMessages") + + assert "window._extractInlineThinkingFromContentForRender(content, thinkingText)" in render + assert "thinkingText=split.reasoning||thinkingText" in render + assert "content=split.content" in render + + +def test_inline_and_separate_reasoning_merge_not_drop(): + """#3633: the extractor MERGES inline + an explicitly-passed separate reasoning + payload (deduped) rather than dropping either. (The reload render path itself + deliberately does NOT seed m.reasoning into this extractor — that separation + is pinned by test_issue2565; the merge capability is exercised by the live + streaming path which passes liveReasoningText.)""" + content, reasoning = _split_thinking_from_content("inlineanswer", "separate") + assert content == "answer" + assert reasoning == "separate\n\ninline" + + # Identical inline + separate dedupe to one. + content2, reasoning2 = _split_thinking_from_content("sameanswer", "same") + assert content2 == "answer" + assert reasoning2 == "same" + + # Separate-only (no inline tag) is preserved and content is untouched + # (no promotion of reasoning into visible prose). + content3, reasoning3 = _split_thinking_from_content("plain answer", "separate") + assert content3 == "plain answer" + assert reasoning3 == "separate" + + +def test_extraction_is_linear_on_long_no_newline_content(): + """#3633 Codex perf catch: the indented-code / leading checks must not be + O(n^2). A 200k-char no-newline message must extract well under a second.""" + import time + + big = "x" * 200_000 + "answer" + start = time.time() + content, reasoning = _split_thinking_from_content(big) + elapsed = time.time() - start + assert content == big + assert reasoning == "" + assert elapsed < 1.0, f"extraction took {elapsed:.2f}s — likely quadratic" + + +def test_timeout_wrapper_remains_out_of_scope(): + assert "Request timed out. Please try again." in WORKSPACE_JS + assert "AbortController" in WORKSPACE_JS diff --git a/tests/test_issue852_thinking_card_mirror.py b/tests/test_issue852_thinking_card_mirror.py index a39694bb5..6fad1034e 100644 --- a/tests/test_issue852_thinking_card_mirror.py +++ b/tests/test_issue852_thinking_card_mirror.py @@ -17,6 +17,12 @@ def _read(name): return open(os.path.join(_SRC, name), encoding="utf-8").read() +def _inline_extractor_body(js): + start = js.index("function _extractInlineThinkingFromContent(") + end = js.index("if(typeof window", start) + return js[start:end] + + class TestStreamDisplayStripsThinkBlocksAlways: def test_early_return_on_reasoning_text_is_gone(self): @@ -33,18 +39,15 @@ class TestStreamDisplayStripsThinkBlocksAlways: ) def test_think_pair_stripping_still_runs(self): - """The `_thinkPairs` stripping loop must still be present so the - fix actually strips think blocks.""" + """The shared inline extractor must still strip think blocks.""" js = _read("static/messages.js") m = re.search(r'function _streamDisplay\(\)\{.*?\n \}', js, re.DOTALL) assert m fn = m.group(0) - assert "_thinkPairs" in fn, ( - "_streamDisplay must iterate _thinkPairs to strip think blocks" - ) - assert "trimmed.startsWith(open)" in fn, ( - "the think-block stripping must check for the open tag" - ) + assert "_extractInlineThinkingFromContent" in fn + helper = _inline_extractor_body(js) + assert "_thinkPairs" in helper + assert "text.startsWith(candidate.open,index)" in helper def test_still_handles_incomplete_think_tag_partial_prefix(self): """Existing behaviour preserved: partial `= 0, "_messageHasReasoningPayload function not found in ui.js" - block = UI_JS[idx:idx+500] + 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*(?:" in block, ( - "_messageHasReasoningPayload must only detect leading provider thinking wrappers" + "_messageHasReasoningPayload must keep the leading-only regex as a non-browser fallback" ) # ── messages.js: streaming render path ─────────────────────────────────────── -def test_stream_display_trims_before_startswith(): - """_streamDisplay in messages.js must call .trimStart() before .startsWith() check.""" +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 "trimStart()" in fn_body, \ - "_streamDisplay must call trimStart() to handle models that emit leading whitespace before " + assert "_extractInlineThinkingFromContent(_stripXmlToolCalls(assistantText), liveReasoningText, {streaming:true}).content" in fn_body, \ + "_streamDisplay must route through the shared inline thinking extractor" -def test_stream_display_uses_trimmed_for_startswith(): - """_streamDisplay must check trimmed.startsWith(open), not raw.startsWith(open).""" - fn_idx = MSG_JS.find("function _streamDisplay()") - fn_end = MSG_JS.find("\n }", fn_idx) + 4 +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 "trimmed.startsWith(open)" in fn_body, \ - "_streamDisplay must use trimmed.startsWith(open) not raw.startsWith(open)" + assert "text.startsWith(candidate.open,index)" in fn_body, \ + "Shared extractor must match complete known thinking open tags" -def test_stream_display_partial_tag_uses_trimmed(): - """The partial-tag guard in _streamDisplay must also use trimmed, not raw.""" - fn_idx = MSG_JS.find("function _streamDisplay()") - fn_end = MSG_JS.find("\n }", fn_idx) + 4 +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 "open.startsWith(trimmed)" in fn_body, \ - "Partial-tag guard must use open.startsWith(trimmed) not open.startsWith(raw)" + assert "candidate.open.startsWith(rest)" in fn_body, \ + "Partial-tag guard must hide incomplete provider thinking tag prefixes" -def test_stream_display_trims_return_after_close(): - """After stripping a completed think block, _streamDisplay must trim leading whitespace from the result.""" - fn_idx = MSG_JS.find("function _streamDisplay()") - fn_end = MSG_JS.find("\n }", fn_idx) + 4 +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] - # The return after finding close must strip whitespace from the result - assert ".replace(/^" in fn_body and "s+/,'')" in fn_body, \ - "_streamDisplay must strip leading whitespace from content after the closing think tag" + 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 ──────────────────────