mirror of
https://github.com/nesquena/hermes-webui.git
synced 2026-07-16 20:50:18 +00:00
Release v0.51.230 — Release GX (stage-p14 — extract <think> to m.reasoning #3455 + LLM Wiki last-writer #1257) (#3466)
* Release v0.51.230 (stage-p14): extract <think> to m.reasoning #3455 + LLM Wiki last-writer (#1257) Salvage of #3455 (@gsurenull): dropped the stale api/config.py bits (MiniMax-M3 + SCHEMA_VERSION 3->4 — both already on master via #3374). Kept the two genuine fixes: (1) _splitThinkFromContent persist-path extraction of inline <think> blocks into m.reasoning (fixes 30-50% session bloat for reasoning-only providers like MiniMax-M3); (2) LLM Wiki status Last-writer 3-tier fallback (was always 'Not available' since #1257). Added 9 Node-driven think-split regression tests (data-loss guards: content-before/after preserved, unclosed blocks intact, lookalike tags not extracted). * fix(#3455): renderer-matching think extraction + wiki symlink/bounded-read guards (Codex review) Codex review of stage-p14 found 3 SILENT bugs, all fixed: (1) DATA-LOSS: _splitThinkFromContent's Pass-2 whole-body scan extracted a CLOSED literal <think>...</think> from visible prose/code (e.g. inside a fenced code block) into m.reasoning, emptying it — more aggressive than the renderer (which only strips LEADING blocks). Removed Pass 2; extraction now matches _streamDisplay semantics (leading-only, loop captures consecutive leading blocks). +fenced-code regression test. (2) PRIVACY: _llm_wiki_last_writer followed symlinked .md pages resolving OUTSIDE the wiki (is_file follows symlinks), leaking external frontmatter. Now requires resolved path under wiki_root. +symlink-containment regression test. (3) CONTRACT/PERF: replaced full read_text() with bounded line-by-line reads (frontmatter block only / capped log-heading scan), never page bodies. * fix(#3455): think-split is leading-single (renderer-matching) + fix 2 stale source-match tests Codex re-review finding #2: looping consecutive leading blocks diverged from the renderer (_streamDisplay/_parseStreamState strip ONE leading block). Now extracts exactly one leading block. Also updated 2 tests that asserted pre-split implementation strings: test_live_stream_tokens_persist (content:assistantText -> content:split.content, invariant preserved) and the consecutive-blocks test. NOTE: Codex finding #1 (client-only split doesn't persist server-side) is a separate architectural decision pending Nathan. * feat(#3455): split inline <think> server-side before s.save() so persisted file is compacted (Codex #1) Codex finding #1: the think-split was client-only, so the SAVED session file still carried inline <think> blocks (bloat) — the fix only compacted the browser copy. Added _split_thinking_from_content (api/streaming.py), a server-side twin of the JS helper with identical leading-only/single-block semantics, applied to the final assistant message before s.save() (extended the existing reasoning-persist block). Merges with on_reasoning-stream reasoning. +8 backend-parity regression tests covering the mid-body-code-block data-loss guard, unclosed-intact, single-leading, none-content. * test: update 3 save-path source-assertion tests for #3455 server-side think-split The backend think-split (api/streaming.py reasoning-persist block) changed the literal code shape + grew the pre-save block, breaking 8 source-assertion tests that anchor on it: - test_sprint42: assert _rm['reasoning']=_reasoning_text -> now _merged_reasoning/_existing_reasoning + _split_thinking_from_content present (intent preserved: reasoning persisted before save). - test_pr1318 (6) + test_pr1341: re-anchored the locator from the changed 'if _reasoning_text and s.messages:' line to the stable 'Persist reasoning trace in the session' comment marker; bumped the 1341 byte-distance limit 15000->16000 (the test self-documents bumping on legit pre-save growth). All behavioral invariants (reasoning persisted + context fields before save) unchanged. --------- Co-authored-by: nesquena-hermes <[email protected]>
This commit is contained in:
nesquena-hermes
@@ -3,6 +3,12 @@
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
## [v0.51.230] — 2026-06-03 — Release GX (stage-p14 — extract <think> blocks to m.reasoning + LLM Wiki last-writer)
|
||||
|
||||
### Fixed
|
||||
- Assistant message `<think>…</think>` blocks are now extracted into `m.reasoning` instead of being stored inline in `m.content` — **both client-side (streaming/inflight state) and server-side at save time**. Reasoning-only providers such as `MiniMax-M3` (OpenAI-compat) previously left the thinking trace inside the assistant content, bloating persisted session files by 30–50% and bypassing the `m.reasoning` field the thinking card reads on reload. A new `_splitThinkFromContent()` (in `static/messages.js`) and its server-side twin `_split_thinking_from_content()` (in `api/streaming.py`, applied to the final assistant message before `s.save()`) extract a single **leading** block (after lstrip) for all three known tag pairs, matching the live renderer's `_streamDisplay`/`_parseStreamState` semantics exactly: a closed `<think>…</think>` that appears mid-body (e.g. a literal tag inside a fenced code block) stays visible content and is never moved into reasoning, a partial/unclosed block is left intact, and any pre-existing `m.reasoning` (from a separate `on_reasoning` stream) is preserved/merged. So the persisted session file — not just the in-browser copy — is compacted on reload (#3455 part 1, @gsurenull).
|
||||
- The LLM Wiki status panel's `Last writer` field is now populated (it always showed `Not available` since the panel shipped in #1257). The reader uses a 3-tier fallback — most-recent page frontmatter (`updated_by`/`writer`/`author`), the most recent `log.md` action verb, then a static `ai-agent` fallback — and reads only frontmatter + log headings, never page bodies, preserving the private-safe status contract (#3455 part 2, @gsurenull; closes #1257).
|
||||
|
||||
## [v0.51.229] — 2026-06-03 — Release GW (stage-p13 — /model never silently snaps a versioned name to a -tier variant)
|
||||
|
||||
### Fixed
|
||||
|
||||
+91
-2
@@ -3494,6 +3494,94 @@ def _llm_wiki_page_files(wiki_path: Path) -> list[Path]:
|
||||
return pages
|
||||
|
||||
|
||||
def _llm_wiki_last_writer(wiki_path: Path, page_files: list[Path]) -> str:
|
||||
"""Best-effort last-writer detection for the LLM Wiki status card.
|
||||
|
||||
Closes the gap left by the original panel (commit 2684d6fa, Issue #1257):
|
||||
the field was reserved as ``"last_writer": None`` with no reader wired up,
|
||||
so the UI always rendered "Not available". This helper makes the field
|
||||
useful without breaking the private-safe contract (reads only one line
|
||||
of frontmatter and one line of log.md headings, never page bodies).
|
||||
|
||||
Priority:
|
||||
1. Most-recently-modified page frontmatter ``updated_by`` / ``writer`` /
|
||||
``author`` (case-insensitive).
|
||||
2. Most recent ``log.md`` heading of the form
|
||||
``## [YYYY-MM-DD] <action> | subject`` — returns
|
||||
``"ai-agent (<action>)"`` so the user can see ingest vs update.
|
||||
3. Static fallback ``"ai-agent"`` so the UI never shows "Not available"
|
||||
for a configured wiki.
|
||||
"""
|
||||
# #3455 review (Codex): resolve the wiki root once and require every file we
|
||||
# read to stay under it, so a symlinked .md page can't leak frontmatter from
|
||||
# outside the wiki. Also read bounded line-by-line (frontmatter only / log
|
||||
# headings only), never full page bodies, per the private-safe status contract.
|
||||
try:
|
||||
wiki_root = wiki_path.resolve()
|
||||
except Exception:
|
||||
wiki_root = wiki_path
|
||||
|
||||
def _within_wiki(p: Path) -> bool:
|
||||
try:
|
||||
return p.resolve().is_relative_to(wiki_root)
|
||||
except Exception:
|
||||
return False
|
||||
|
||||
# Priority 1: most recent page frontmatter (resolved-path must stay in-wiki)
|
||||
latest_page: Path | None = None
|
||||
latest_mtime = -1.0
|
||||
for candidate in page_files:
|
||||
if not _within_wiki(candidate):
|
||||
continue # skip symlinks resolving outside the wiki
|
||||
try:
|
||||
mtime = candidate.stat().st_mtime
|
||||
except Exception:
|
||||
continue
|
||||
if mtime > latest_mtime:
|
||||
latest_mtime = mtime
|
||||
latest_page = candidate
|
||||
if latest_page is not None:
|
||||
try:
|
||||
with open(latest_page, encoding="utf-8", errors="replace") as fh:
|
||||
first = fh.readline()
|
||||
if first.strip() == "---":
|
||||
# Read only the frontmatter block, bounded to a small line cap.
|
||||
for _ in range(200):
|
||||
line = fh.readline()
|
||||
if line == "" or line.strip() == "---":
|
||||
break # EOF or end of frontmatter — never touch the body
|
||||
stripped = line.strip()
|
||||
lower = stripped.lower()
|
||||
for key in ("updated_by", "writer", "author"):
|
||||
if lower.startswith(f"{key}:"):
|
||||
value = stripped.split(":", 1)[1].strip()
|
||||
if value:
|
||||
return value
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
# Priority 2: log.md last entry action verb (heading lines only, bounded)
|
||||
log_path = wiki_path / "log.md"
|
||||
if _within_wiki(log_path) and log_path.is_file():
|
||||
try:
|
||||
with open(log_path, encoding="utf-8", errors="replace") as fh:
|
||||
for _ in range(5000): # cap the heading scan
|
||||
line = fh.readline()
|
||||
if line == "":
|
||||
break
|
||||
stripped = line.strip()
|
||||
if not stripped.startswith("## [") or "|" not in stripped:
|
||||
continue
|
||||
tail = stripped.split("]", 1)[1].strip() if "]" in stripped else ""
|
||||
action = tail.split()[0] if tail else "update"
|
||||
return f"ai-agent ({action})"
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
# Priority 3: never return None / "Not available" for a configured wiki
|
||||
return "ai-agent"
|
||||
|
||||
|
||||
def _build_llm_wiki_status() -> dict:
|
||||
"""Return private-safe LLM Wiki status metadata without reading page bodies."""
|
||||
try:
|
||||
@@ -3506,7 +3594,7 @@ def _build_llm_wiki_status() -> dict:
|
||||
"page_count": 0,
|
||||
"raw_source_count": 0,
|
||||
"last_updated": None,
|
||||
"last_writer": None,
|
||||
"last_writer": "ai-agent",
|
||||
"path_configured": path_configured,
|
||||
"path_source": path_source,
|
||||
"toggle_available": False,
|
||||
@@ -3538,6 +3626,7 @@ def _build_llm_wiki_status() -> dict:
|
||||
"page_count": len(page_files),
|
||||
"raw_source_count": _llm_wiki_count_files(wiki_path / "raw"),
|
||||
"last_updated": _llm_wiki_safe_iso(latest),
|
||||
"last_writer": _llm_wiki_last_writer(wiki_path, page_files),
|
||||
})
|
||||
return base
|
||||
except Exception as exc:
|
||||
@@ -3549,7 +3638,7 @@ def _build_llm_wiki_status() -> dict:
|
||||
"page_count": 0,
|
||||
"raw_source_count": 0,
|
||||
"last_updated": None,
|
||||
"last_writer": None,
|
||||
"last_writer": "ai-agent",
|
||||
"path_configured": False,
|
||||
"path_source": "unknown",
|
||||
"toggle_available": False,
|
||||
|
||||
+66
-4
@@ -1303,6 +1303,49 @@ 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 <think> block out of assistant content.
|
||||
|
||||
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
|
||||
``<think>...</think>`` 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.
|
||||
"""
|
||||
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 = (
|
||||
('<think>', '</think>'),
|
||||
('<|channel>thought\n', '<channel|>'),
|
||||
('<|turn|>thinking\n', '<turn|>'),
|
||||
)
|
||||
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:
|
||||
"""Remove common reasoning/thinking wrappers from model text."""
|
||||
if not text:
|
||||
@@ -5996,11 +6039,30 @@ def _run_agent_streaming(
|
||||
# Must run BEFORE s.save() — otherwise the mutation lives only in
|
||||
# memory until the next turn's save, and the last-turn thinking card
|
||||
# is lost when the user reloads immediately after a response.
|
||||
if _reasoning_text and s.messages:
|
||||
#
|
||||
# #3455: also split any inline leading <think> block 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.
|
||||
if s.messages:
|
||||
for _rm in reversed(s.messages):
|
||||
if isinstance(_rm, dict) and _rm.get('role') == 'assistant':
|
||||
_rm['reasoning'] = _reasoning_text
|
||||
break
|
||||
if not (isinstance(_rm, dict) and _rm.get('role') == 'assistant'):
|
||||
continue
|
||||
_existing_reasoning = _reasoning_text or _rm.get('reasoning') or ''
|
||||
_content = _rm.get('content')
|
||||
if isinstance(_content, str) and _content:
|
||||
_new_content, _merged_reasoning = _split_thinking_from_content(
|
||||
_content, _existing_reasoning
|
||||
)
|
||||
_rm['content'] = _new_content
|
||||
if _merged_reasoning:
|
||||
_rm['reasoning'] = _merged_reasoning
|
||||
elif _existing_reasoning:
|
||||
_rm['reasoning'] = _existing_reasoning
|
||||
break
|
||||
try:
|
||||
_turn_duration_seconds = max(0.0, time.time() - float(_turn_started_at))
|
||||
except Exception:
|
||||
|
||||
+51
-3
@@ -835,6 +835,40 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){
|
||||
const clean=_stripLiveVisibleAssistantEchoFromThinking(liveReasoningText, visibleInterimSnippets);
|
||||
return clean || 'Thinking…';
|
||||
}
|
||||
// Split a content string into {reasoning, content} by extracting any <think>...
|
||||
// blocks (or other known reasoning-tag pairs). If reasoning is already
|
||||
// populated on the message (e.g. from a separate on_reasoning stream), the
|
||||
// inline blocks are stripped but the existing reasoning field is preserved.
|
||||
// Provider-bug workaround: M3 (and similar reasoning models) emit the
|
||||
// thinking inline in the OpenAI-compat content stream instead of a separate
|
||||
// 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 <think>...</think> 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};
|
||||
}
|
||||
function syncInflightAssistantMessage(){
|
||||
const inflight=INFLIGHT[activeSid];
|
||||
if(!inflight) return;
|
||||
@@ -845,14 +879,17 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){
|
||||
if(msg&&msg.role==='assistant'&&msg._live){assistantIdx=i;break;}
|
||||
}
|
||||
const ts=Date.now()/1000;
|
||||
// Split inline <think> blocks into m.reasoning so the persisted inflight
|
||||
// state stays compact and the thinking card has a proper source field.
|
||||
const split=_splitThinkFromContent(assistantText, reasoningText);
|
||||
if(assistantIdx>=0){
|
||||
inflight.messages[assistantIdx].content=assistantText;
|
||||
inflight.messages[assistantIdx].reasoning=reasoningText||undefined;
|
||||
inflight.messages[assistantIdx].content=split.content;
|
||||
inflight.messages[assistantIdx].reasoning=split.reasoning||undefined;
|
||||
inflight.messages[assistantIdx]._ts=inflight.messages[assistantIdx]._ts||ts;
|
||||
_throttledPersist();
|
||||
return;
|
||||
}
|
||||
inflight.messages.push({role:'assistant',content:assistantText,reasoning:reasoningText||undefined,_live:true,_ts:ts});
|
||||
inflight.messages.push({role:'assistant',content:split.content,reasoning:split.reasoning||undefined,_live:true,_ts:ts});
|
||||
_throttledPersist();
|
||||
}
|
||||
function ensureAssistantRow(force=false){
|
||||
@@ -1927,6 +1964,17 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){
|
||||
const lastAsst=[...S.messages].reverse().find(m=>m.role==='assistant');
|
||||
// Persist reasoning trace so thinking card survives page reload
|
||||
if(reasoningText&&lastAsst&&!lastAsst.reasoning) lastAsst.reasoning=reasoningText;
|
||||
// Strip any inline <think> blocks still embedded in the server-side
|
||||
// content (M3 OpenAI-compat doesn't separate reasoning). Move them
|
||||
// to m.reasoning so the persisted session stays compact and the
|
||||
// thinking card has a proper source field on reload.
|
||||
if(lastAsst && typeof lastAsst.content === 'string' && lastAsst.content){
|
||||
const split=_splitThinkFromContent(lastAsst.content, lastAsst.reasoning);
|
||||
if(split.content!==lastAsst.content){
|
||||
lastAsst.content=split.content;
|
||||
if(split.reasoning) lastAsst.reasoning=split.reasoning;
|
||||
}
|
||||
}
|
||||
// Stamp _ts on the last assistant message if it has no timestamp
|
||||
if(lastAsst&&!lastAsst._ts&&!lastAsst.timestamp) lastAsst._ts=Date.now()/1000;
|
||||
if(d.usage){
|
||||
|
||||
@@ -40,7 +40,9 @@ def test_llm_wiki_status_reads_synthetic_fixture_without_exposing_content(tmp_pa
|
||||
assert status["page_count"] == 2
|
||||
assert status["raw_source_count"] == 1
|
||||
assert status["last_updated"] is not None
|
||||
assert status["last_writer"] is None
|
||||
# log.md in the fixture has a "## [2026-05-04] update | ..." heading,
|
||||
# so the new last-writer reader must surface that action verb.
|
||||
assert status["last_writer"] == "ai-agent (update)"
|
||||
assert status["toggle_available"] is False
|
||||
assert status["docs_url"].endswith("/research-llm-wiki")
|
||||
serialized = repr(status)
|
||||
@@ -98,3 +100,40 @@ def test_insights_panel_fetches_and_renders_llm_wiki_status_card():
|
||||
assert "wiki-status-card" in style_src
|
||||
assert "raw/" in panels_src
|
||||
assert "recent_entries" not in panels_src
|
||||
|
||||
|
||||
def test_last_writer_reads_frontmatter(tmp_path):
|
||||
"""#3455 part 2: the Last writer field reads page frontmatter updated_by/writer/author."""
|
||||
import api.routes as routes
|
||||
|
||||
wiki = tmp_path / "wiki"
|
||||
_write(wiki / "entities" / "a.md", "---\ntitle: A\nupdated_by: alice\n---\nbody\n")
|
||||
pages = routes._llm_wiki_page_files(wiki)
|
||||
assert routes._llm_wiki_last_writer(wiki, pages) == "alice"
|
||||
|
||||
|
||||
def test_last_writer_rejects_symlink_outside_wiki(tmp_path):
|
||||
"""#3455 review (Codex): a symlinked .md page resolving OUTSIDE the wiki must
|
||||
not be read — its frontmatter must never leak into the status card."""
|
||||
import api.routes as routes
|
||||
|
||||
outside = tmp_path / "outside"
|
||||
outside.mkdir(parents=True, exist_ok=True)
|
||||
secret = outside / "private.md"
|
||||
secret.write_text("---\nauthor: outside-secret\n---\nprivate body\n", encoding="utf-8")
|
||||
|
||||
wiki = tmp_path / "wiki" / "entities"
|
||||
wiki.mkdir(parents=True, exist_ok=True)
|
||||
link = wiki / "linked.md"
|
||||
try:
|
||||
link.symlink_to(secret)
|
||||
except (OSError, NotImplementedError):
|
||||
import pytest
|
||||
pytest.skip("symlinks not supported on this platform")
|
||||
|
||||
wiki_root = tmp_path / "wiki"
|
||||
pages = routes._llm_wiki_page_files(wiki_root)
|
||||
writer = routes._llm_wiki_last_writer(wiki_root, pages)
|
||||
# The external symlink's frontmatter must NOT surface; falls back to ai-agent.
|
||||
assert writer != "outside-secret"
|
||||
assert "outside-secret" not in writer
|
||||
|
||||
@@ -0,0 +1,195 @@
|
||||
"""#3455 — _splitThinkFromContent persist-path regression tests.
|
||||
|
||||
The think-block extraction runs at PERSIST time (inflight state + SSE `done`
|
||||
finalization), moving inline <think>…</think> reasoning out of m.content into
|
||||
m.reasoning. Because it rewrites persisted assistant content, the critical
|
||||
invariant is that it NEVER loses real content: content before/after a think
|
||||
block survives, partial/unclosed blocks are left intact for the live renderer,
|
||||
and lookalike tags in code are not falsely extracted.
|
||||
|
||||
Drives the live JS via Node (same harness style as the #3368/#1188 suites) so
|
||||
the test exercises the shipped function, not a Python re-implementation.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import shutil
|
||||
import subprocess
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
REPO = Path(__file__).resolve().parents[1]
|
||||
MESSAGES_JS = (REPO / "static" / "messages.js").read_text(encoding="utf-8")
|
||||
|
||||
|
||||
def _extract_block(src: str, marker: str) -> str:
|
||||
"""Extract a brace-balanced JS block starting at `marker` (a `const x=[` or
|
||||
`function name(`)."""
|
||||
start = src.index(marker)
|
||||
# find first opening bracket of the block ( '[' for the array, '{' for the fn )
|
||||
i = start
|
||||
while src[i] not in "[{":
|
||||
i += 1
|
||||
opener = src[i]
|
||||
closer = "]" if opener == "[" else "}"
|
||||
depth = 0
|
||||
j = i
|
||||
while j < len(src):
|
||||
if src[j] == opener:
|
||||
depth += 1
|
||||
elif src[j] == closer:
|
||||
depth -= 1
|
||||
if depth == 0:
|
||||
return src[start:j + 1]
|
||||
j += 1
|
||||
raise AssertionError(f"unbalanced block for {marker!r}")
|
||||
|
||||
|
||||
_DRIVER = """
|
||||
%s
|
||||
%s
|
||||
const args = JSON.parse(process.argv[2]);
|
||||
process.stdout.write(JSON.stringify(_splitThinkFromContent(args.raw, args.existing || '')));
|
||||
"""
|
||||
|
||||
|
||||
@pytest.fixture(scope="module")
|
||||
def driver(tmp_path_factory):
|
||||
if shutil.which("node") is None:
|
||||
pytest.skip("node not available")
|
||||
pairs = _extract_block(MESSAGES_JS, "const _thinkPairs=")
|
||||
fn = _extract_block(MESSAGES_JS, "function _splitThinkFromContent(")
|
||||
p = tmp_path_factory.mktemp("think3455") / "driver.js"
|
||||
p.write_text(_DRIVER % (pairs, fn), encoding="utf-8")
|
||||
return str(p)
|
||||
|
||||
|
||||
def _split(driver, raw, existing=""):
|
||||
out = subprocess.run(
|
||||
["node", driver, json.dumps({"raw": raw, "existing": existing})],
|
||||
capture_output=True, text=True, check=True,
|
||||
)
|
||||
return json.loads(out.stdout)
|
||||
|
||||
|
||||
def test_plain_content_untouched(driver):
|
||||
r = _split(driver, "Hello world, no thinking here.")
|
||||
assert r["content"] == "Hello world, no thinking here."
|
||||
assert r["reasoning"] == ""
|
||||
|
||||
|
||||
def test_think_at_start_extracted(driver):
|
||||
r = _split(driver, "<think>my reasoning</think>The visible answer")
|
||||
assert r["content"] == "The visible answer"
|
||||
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 <think> that appears after real prose is, by the renderer's definition,
|
||||
visible content and must be left in m.content (not moved to reasoning)."""
|
||||
r = _split(driver, "Real prefix <think>mid</think> tail")
|
||||
# Not leading -> nothing extracted, content fully preserved.
|
||||
assert r["content"] == "Real prefix <think>mid</think> tail"
|
||||
assert r["reasoning"] == ""
|
||||
|
||||
|
||||
def test_closed_literal_think_in_code_block_preserved(driver):
|
||||
"""#3455 review (Codex data-loss): a closed literal <think>...</think> inside
|
||||
a fenced code block (visible content, not leading) must NOT be extracted into
|
||||
reasoning — the whole-body scan that did this is removed."""
|
||||
raw = "```html\n<think>visible literal</think>\n```"
|
||||
r = _split(driver, raw)
|
||||
assert r["content"] == raw, "fenced-code closed think tag must stay in content"
|
||||
assert r["reasoning"] == ""
|
||||
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."""
|
||||
r = _split(driver, "<think>still thinking...")
|
||||
assert r["content"] == "<think>still thinking..."
|
||||
assert r["reasoning"] == ""
|
||||
|
||||
|
||||
def test_existing_reasoning_is_merged_not_overwritten(driver):
|
||||
r = _split(driver, "<think>extra</think>answer", existing="from on_reasoning stream")
|
||||
assert r["content"] == "answer"
|
||||
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."""
|
||||
r = _split(driver, "<think>a</think><think>b</think>the answer")
|
||||
assert r["content"] == "<think>b</think>the answer"
|
||||
assert r["reasoning"] == "a"
|
||||
|
||||
|
||||
def test_block_after_content_not_extracted(driver):
|
||||
"""A think block that follows visible content stays in content (renderer only
|
||||
strips leading blocks)."""
|
||||
r = _split(driver, "<think>lead</think>answer <think>trailing</think> more")
|
||||
assert r["content"] == "answer <think>trailing</think> more"
|
||||
assert r["reasoning"] == "lead"
|
||||
|
||||
|
||||
def test_lookalike_tag_in_code_not_extracted(driver):
|
||||
r = _split(driver, "use <think> as a literal token, never closed")
|
||||
assert r["content"] == "use <think> as a literal token, never closed"
|
||||
|
||||
|
||||
def test_empty_content(driver):
|
||||
r = _split(driver, "")
|
||||
assert r["content"] == ""
|
||||
assert r["reasoning"] == ""
|
||||
|
||||
|
||||
def test_think_only_message(driver):
|
||||
r = _split(driver, "<think>only thinking</think>")
|
||||
assert r["content"] == ""
|
||||
assert r["reasoning"] == "only thinking"
|
||||
|
||||
|
||||
# ── Backend parity: api/streaming._split_thinking_from_content ──────────────
|
||||
# #3455 review (Codex): the split must also run server-side before s.save() so
|
||||
# the PERSISTED session file is compacted (the client-only split left the saved
|
||||
# file bloated). The backend helper must match the JS semantics exactly.
|
||||
|
||||
class TestBackendThinkSplitParity:
|
||||
def _sp(self, raw, existing=""):
|
||||
from api.streaming import _split_thinking_from_content
|
||||
return _split_thinking_from_content(raw, existing)
|
||||
|
||||
def test_plain_untouched(self):
|
||||
assert self._sp("Hello world") == ("Hello world", "")
|
||||
|
||||
def test_leading_extracted(self):
|
||||
assert self._sp("<think>r</think>The answer") == ("The answer", "r")
|
||||
|
||||
def test_mid_body_code_block_preserved(self):
|
||||
raw = "```html\n<think>visible literal</think>\n```"
|
||||
content, reasoning = self._sp(raw)
|
||||
assert content == raw
|
||||
assert reasoning == ""
|
||||
|
||||
def test_unclosed_left_intact(self):
|
||||
assert self._sp("<think>still...") == ("<think>still...", "")
|
||||
|
||||
def test_existing_reasoning_merged(self):
|
||||
assert self._sp("<think>new</think>ans", "prior") == ("ans", "prior\n\nnew")
|
||||
|
||||
def test_single_leading_block_only(self):
|
||||
assert self._sp("<think>a</think><think>b</think>end") == ("<think>b</think>end", "a")
|
||||
|
||||
def test_empty(self):
|
||||
assert self._sp("") == ("", "")
|
||||
|
||||
def test_none_content(self):
|
||||
# Defensive: non-string content must not crash.
|
||||
content, reasoning = self._sp(None)
|
||||
assert content in (None, "")
|
||||
assert reasoning == ""
|
||||
@@ -26,7 +26,7 @@ STREAMING = Path(__file__).resolve().parent.parent / "api" / "streaming.py"
|
||||
def _persistence_block():
|
||||
"""Return the source range covering the post-merge per-turn save block."""
|
||||
src = STREAMING.read_text(encoding="utf-8")
|
||||
start = src.find("if _reasoning_text and s.messages:")
|
||||
start = src.find("Persist reasoning trace in the session")
|
||||
assert start != -1, "Reasoning trace marker not found in streaming.py"
|
||||
end = src.find("\n s.save()", start)
|
||||
assert end != -1, "s.save() not found after the reasoning trace marker"
|
||||
|
||||
@@ -32,20 +32,21 @@ def test_streaming_persists_context_fields_on_session_before_save():
|
||||
|
||||
# Find the post-merge save block — anchored on the unique reasoning trace
|
||||
# marker right above the persistence block.
|
||||
block_start = src.find("if _reasoning_text and s.messages:")
|
||||
block_start = src.find("Persist reasoning trace in the session")
|
||||
assert block_start != -1, "Reasoning-trace marker not found in streaming.py"
|
||||
|
||||
# Save call follows shortly after
|
||||
save_call = src.find("\n s.save()", block_start)
|
||||
assert save_call != -1, "s.save() not found after the post-merge marker"
|
||||
# Limit bumped to 15000 by the #3256/#3263 default-only context_length guard
|
||||
# plus its dual-gate consistency fixes (recompute persisted stale cap +
|
||||
# rescale threshold_tokens). The pre-save block legitimately grew here. NOTE:
|
||||
# this byte-distance assertion is itself brittle (it must be bumped whenever a
|
||||
# legitimate pre-save mutation block is added) — a structural check (presence
|
||||
# of s.save() shortly after the post-merge marker) would be more durable; left
|
||||
# as a follow-up. Earlier limits: 9000 (cancellation guards) → 13000 (#3263 v1).
|
||||
assert save_call - block_start < 15000, (
|
||||
# Limit bumped to 16000 by #3455 (server-side <think> split added to the
|
||||
# pre-save reasoning-persist block, + the anchor moved to the comment marker
|
||||
# which sits a few lines above the former `if` anchor). The pre-save block
|
||||
# legitimately grew here. NOTE: this byte-distance assertion is itself brittle
|
||||
# (it must be bumped whenever a legitimate pre-save mutation block is added) — a
|
||||
# structural check (presence of s.save() shortly after the post-merge marker)
|
||||
# would be more durable; left as a follow-up. Earlier limits: 9000 (cancellation
|
||||
# guards) → 13000 (#3263 v1) → 15000 (#3256/#3263 dual-gate).
|
||||
assert save_call - block_start < 16000, (
|
||||
"s.save() should be close to the post-merge marker — block expanded unexpectedly. "
|
||||
"If you've added a new pre-save mutation block here, bump this limit."
|
||||
)
|
||||
|
||||
@@ -571,8 +571,14 @@ def test_live_stream_tokens_persist_partial_assistant_for_session_switch(cleanup
|
||||
messages_src = (REPO_ROOT / "static/messages.js").read_text()
|
||||
ui_src = (REPO_ROOT / "static/ui.js").read_text()
|
||||
|
||||
assert "content:assistantText" in messages_src, \
|
||||
"messages.js must persist the partial assistant text into INFLIGHT state"
|
||||
# #3455: the persisted partial assistant content is now the think-split content
|
||||
# (inline <think> moved to m.reasoning), so the push uses content:split.content
|
||||
# where split=_splitThinkFromContent(assistantText, ...). The invariant — partial
|
||||
# assistant text is mirrored into INFLIGHT state — is unchanged.
|
||||
assert "content:split.content" in messages_src, \
|
||||
"messages.js must persist the (think-split) partial assistant text into INFLIGHT state"
|
||||
assert "_splitThinkFromContent(assistantText" in messages_src, \
|
||||
"the persisted partial must be derived from the live assistantText"
|
||||
assert "_live:true" in messages_src, \
|
||||
"messages.js must mark the persisted in-flight assistant row so renderMessages can re-anchor it"
|
||||
assert "syncInflightAssistantMessage();" in messages_src, \
|
||||
|
||||
@@ -695,8 +695,15 @@ def test_streaming_persists_reasoning_in_session():
|
||||
assert "Persist reasoning trace in the session so it survives reload" in src, \
|
||||
"Reasoning persistence comment not found in streaming.py"
|
||||
|
||||
assert "_rm['reasoning'] = _reasoning_text" in src, \
|
||||
"Code to set _rm['reasoning'] not found in streaming.py"
|
||||
# #3455: reasoning is now persisted via the think-split path — either the
|
||||
# merged reasoning (inline <think> + on_reasoning stream) or the existing
|
||||
# _reasoning_text when content has no leading block. Both set _rm['reasoning'].
|
||||
assert "_rm['reasoning'] = _merged_reasoning" in src, \
|
||||
"Code to set the last assistant message's reasoning (merged think-split) not found"
|
||||
assert "_split_thinking_from_content(" in src, \
|
||||
"server-side think-split must run before save (#3455)"
|
||||
assert "_rm['reasoning'] = _existing_reasoning" in src, \
|
||||
"the no-think-block branch must still persist _reasoning_text into the assistant message"
|
||||
|
||||
# Persistence block must come BEFORE raw_session assignment
|
||||
persist_idx = src.index("Persist reasoning trace in the session")
|
||||
|
||||
Reference in New Issue
Block a user