feat: add support for displaying thinking/reasoning blocks in chat

[TaraTheStar]

This PR adds support for displaying thinking/reasoning blocks in the chat interface. It handles both standard tags and Gemma 4-style channel tokens, providing a clean 'Thinking...' placeholder during streaming and stripping/displaying the reasoning once completed.

[nesquena]

Full Review: PR #181 — thinking/reasoning block display

Thanks @TaraTheStar — nice continuation of the reasoning display work. This cleanly covers the inline-tag formats that PR #169 didn't handle.

Security Audit

Clean. Streaming path strips tags before passing to renderMd() (which escapes HTML). History path extracts thinking text into thinkingText which is rendered via ${esc(thinkingText)} in the existing <pre> block. "Thinking..." placeholder is hardcoded with no user data. No XSS vectors.

Code Review

Both paths are well-implemented:

• Streaming (_streamDisplay()): correctly buffers, detects open/closed thinking blocks, shows placeholder while thinking, strips completed blocks. The startsWith check is the right conservative choice — avoids false positives on <think> in code examples.
• History (renderMessages()): ^-anchored regex is consistent with streaming path. slice() to remove matched block is correct.

The three reasoning formats are now fully covered:

1. Structured content arrays (Claude, o3) — existing
2. Top-level m.reasoning (Hermes) — PR #169
3. Inline <think> / <|channel>thought tags (DeepSeek, QwQ, Gemma 4) — this PR

Tests

474 passed, 0 regressions.

Verdict

Approved. Clean, focused, safe. Ready to merge.

[nesquena-hermes]

Thanks for this PR, @TaraTheStar! Adding inline <think> tag support is a meaningful feature — DeepSeek, QwQ, and Gemma 4 users all benefit. The approach is solid and fits naturally alongside the existing structured thinking block support (Claude extended thinking, o3 reasoning). I reviewed the diff carefully against current master and ran the full test suite.

---

Test results

499/499 pass on this branch. ✅

---

What works well

Streaming path (messages.js)

The _thinkPairs array and _streamDisplay() function are clean. The logic — show empty string while inside an open thinking block, strip the block once closed and show the remainder — is exactly right for the streaming case. The isThinking flag driving the Thinking… placeholder is a nice touch and matches the visual style already in the codebase.

History/reload path (ui.js)

Adding the inline tag parse after the existing structured block checks (Array.isArray(content) + m.reasoning) is the right order of precedence. Structured blocks (from Claude's API) should take priority, and they do.

The regex ^<think>([\s\S]*?)<\/think>\s* correctly anchors to the start of the string, uses non-greedy match for the content, and strips trailing whitespace before the answer. The content = content.slice(thinkMatch[0].length) correctly advances past the block so the answer renders clean.

---

Issues to address

1. Streaming: partial <think> tag briefly visible during streaming

The _streamDisplay() function only hides content once it startsWith(open) with the full opening tag. If tokens arrive one or two characters at a time, the user will briefly see <thi, <thin, <think rendered in the message bubble before the full <think> token arrives and the function switches to hiding mode.

This is a minor UX issue — it resolves itself within a few tokens — but it can look odd. A simple fix is to also hide content when it starts with a prefix of the open tag:

function _streamDisplay(){
  let txt=assistantText;
  for(const {open,close} of _thinkPairs){
    if(txt.startsWith(open)){
      const ci=txt.indexOf(close,open.length);
      if(ci!==-1) return txt.slice(ci+close.length).replace(/^\s+/,'');
      return '';  // inside open block
    }
    // Partial open tag — hide until we know what this is
    if(open.startsWith(txt)) return '';
  }
  return txt;
}

The added line if(open.startsWith(txt)) return '' suppresses display whenever the accumulated text is a valid prefix of any open tag (e.g. <, <t, <th, etc.). Since these are highly specific tokens (<think>, <|channel>), false positives in normal text would be vanishingly rare and resolve within one frame anyway.

---

2. Forward-compatibility with #177 (i18n)

Once PR #177 (i18n/Chinese localization) merges, i18n.js will define a global t() function. Two things in this PR will need minor fixes at that point:

• function _streamDisplay() uses let t = assistantText — this local variable shadows the global t(). It works fine today (no global t on master), but it's a latent footgun. Renaming to let txt = assistantText or let raw = assistantText avoids the collision entirely.

• The Thinking\u2026 placeholder string in _scheduleRender is hardcoded in English. After #177 merges, it should use t('thinking') consistent with how the thinking card label in ui.js uses it. (The i18n branch already has t('thinking') defined.)

These don't need to block this PR if it merges first — but flagging so whoever merges doesn't forget the follow-up.

---

3. No test coverage for the new parsing logic

The JS logic runs client-side so the pytest suite doesn't cover it, which is expected. But it's worth noting that both the _streamDisplay() function and the renderMessages() inline parser have no automated tests. If you wanted to add any, the tests/ directory already has a pattern for JS behavior assertions via the test server. Not a blocker, just a suggestion for future work.

---

Summary

The core feature is correct and well-implemented. The one concrete code issue worth fixing before merge is the partial tag flicker (#1). The let t rename (#2) is low-effort and worth doing now to avoid the merge headache later. No security concerns, no regressions, tests pass.

[nesquena-hermes]

PR #181 Review — feat: add support for displaying thinking/reasoning blocks in chat

Verdict: CHANGES REQUESTED (minor — most issues are already addressed by the follow-up #182, but two ui.js edge cases remain unaddressed)

---

Summary

This PR adds client-side rendering of thinking/reasoning blocks for models that emit inline think tags (<think>...</think> for DeepSeek/QwQ, <|channel>thought\n...<channel|> for Gemma 4) in two places:

• static/messages.js — streaming path: show a "Thinking…" placeholder while a think block is in progress, suppress the think block from the live stream bubble once completed
• static/ui.js — history/render path: parse the tag from the stored message content, extract reasoning into the collapsible thinking card UI that already existed for structured Claude/o3 reasoning blocks

The feature is broadly correct, well-scoped, and the approach (extending the existing thinking-card infrastructure) is the right one. Security handling is sound for the static render path. The main issues are one unfixed streaming edge case in messages.js and two logical edge cases in the ui.js regex parsing.

---

File-by-file findings

static/messages.js — Streaming path

M1 — Partial-tag flash during streaming (BUG, medium severity)
_streamDisplay (lines 116–128) only hides content if assistantText starts with a complete <think> or <|channel>thought\n token. If the model has only emitted <thi, <think, <|cha, etc. so far, _streamDisplay returns that partial fragment as-is, which gets piped straight into assistantBody.innerHTML = renderMd(txt). Users will briefly see raw partial tag fragments flashing in the bubble before the full open-tag arrives.

This is the exact issue that PR #182 patches by adding the guard:

if (open.startsWith(raw)) return '';

Since #182 is already approved, this is a documentation note only — but it means #181 should not be merged standalone without #182.

M2 — Variable shadow let t = assistantText
_streamDisplay assigns let t = assistantText but never mutates t — assistantText is read-only captured from the outer scope. The let is unused mutability; const would be clearer. Minor, but #182 renames it to const raw which is an improvement.

M3 — Single-block-only assumption
_streamDisplay uses Array.from(_thinkPairs).find() semantics (stops after first matching pair). This is intentional and consistent with how the history path works, but it means if a model somehow emits two consecutive think blocks in a single message (unusual but theoretically possible), only the first will be suppressed during streaming. The second block's open tag will be visible as live content. Low impact; documenting for awareness.

---

static/ui.js — History/render path

U1 — Second think block leaks as raw <think> text in rendered content (BUG, low severity)
Lines 509–520: the regex /^<think>([\s\S]*?)<\/think>\s*/ correctly strips the first think block at position 0. However, if a message contains multiple think blocks — e.g.:

<think>first reasoning</think>middle<think>second reasoning</think>final

After stripping the first block, content becomes middle<think>second reasoning</think>final. The second <think>...</think> block is not caught by the if (!thinkingText) guard (because thinkingText is now set) and passes through to renderMd(). The renderMd() pre-pass does not strip <think> tags (only <strong>, <b>, <em>, <i>, <code>, <br> are allowlisted), so the raw tags will be visible as escaped literal text <think> in the bubble. Not a security issue because renderMd escapes unknown tags, but the UX is broken for multi-block messages.

U2 — Leading whitespace before <think> breaks extraction (minor)
The regex anchors to ^, so a message like \n<think>...</think>answer will not match. The outer thinkingText guard continues to false, so the raw <think> block stays in content and is rendered as escaped text. Whether models emit a leading newline before the tag depends on the model — DeepSeek-R1 generally does not, but this is worth a defensive ^\s* anchor or stripping content before the match. Low impact in practice.

U3 — Gemma 4 match order quirk
Lines 514–519: the Gemma match is inside if (!thinkingText), which runs only when the <think> match already failed. This is correct and intentional. No bug, just noting it's fine.

U4 — Thinking card uses esc(thinkingText) — XSS safe ✅
Line 527: <pre>${esc(thinkingText)}</pre> — the esc() helper (ui.js line 5) escapes & < > " ' so model-controlled reasoning content cannot inject HTML. Verified via browser console XSS probe — <script> tags and onerror attributes in think content are properly escaped and do not execute.

U5 — content mutation without reassignment guard
Lines 511–518: content is mutated directly (sliced) after extraction. The variable was either a string or an already-joined string at this point (the Array path at line 498–500 has already joined it). This is fine, but the mutation is somewhat fragile — if a future refactor moves code above this block, it could break. A let displayContent = content; ... content = displayContent.slice(...) pattern would be safer. Minor style concern.

---

Security

• XSS in thinking card content: Safe. esc() is applied to thinkingText before insertion into innerHTML. Confirmed clean via browser probe.
• XSS via streaming render (renderMd): The streaming path passes _streamDisplay() output directly to renderMd(). renderMd() does not fully sanitize HTML — it converts specific safe tags and escapes unknown tags, but unknown tags inside already-fenced code blocks are stashed and restored. For the thinking-display path specifically: once the think block is stripped, only the post-block text goes to renderMd(), which is normal assistant output already handled identically to any other streamed message. No new attack surface here.
• No server-side changes — this is purely a client-side display feature. No new API endpoints, no new data stored.

---

Tests

No new automated tests cover the thinking/reasoning display parsing. The test suite has 499 passing tests (19 fail due to pre-existing server config issues unrelated to this PR) and zero tests in tests/ exercise the JavaScript parsing logic for <think> or <|channel>thought patterns.

This is acceptable for a UI-only change in a JS-heavy frontend with no existing JS unit test infrastructure, but a note for the future: the _streamDisplay and regex extraction logic has enough edge cases (partial tags, multi-block, leading whitespace) that jest/vitest unit tests would catch regressions.

---

Streaming correctness

The rAF-throttled render path in _scheduleRender is preserved correctly. The _streamDisplay() call is inside the rAF callback, reading from assistantText at frame time — this is correct and avoids stale closure issues.

The isThinking check (!txt && assistantText.length > 0) correctly distinguishes "nothing received yet" from "thinking in progress" and shows the styled placeholder only when content exists but is being suppressed. ✅

---

CSS

The thinking card styles (lines 806–815 in style.css) already existed prior to this PR and are not modified here. No new CSS conflicts introduced.

---

Overall assessment

The feature is conceptually sound and the implementation is clean. The critical partial-tag streaming flash (M1) is already resolved by #182. The two remaining issues in ui.js (U1 multi-block leak as escaped text, U2 leading-whitespace anchor) are low-severity UX edge cases that don't affect security but could produce confusing output for certain model responses.

Recommendation: Merge #181 only in conjunction with #182 (or after squash-merging both). If merging independently, U1 and U2 should be addressed first.

---

Reviewed by automated end-to-end review pass: diff analysis, static code review, browser QA (XSS probes, think-card render verification, streaming logic unit testing in-browser), test suite run.

[nesquena-hermes]

Merged, thanks!