Skip to content

fix: keep underscores literal while streaming#2902

Merged
1 commit merged into
nesquena:masterfrom
george-andraws:fix/streaming-underscore-italics
May 27, 2026
Merged

fix: keep underscores literal while streaming#2902
1 commit merged into
nesquena:masterfrom
george-andraws:fix/streaming-underscore-italics

Conversation

@george-andraws
Copy link
Copy Markdown
Contributor

Thinking Path

  • Hermes WebUI uses streaming-markdown for live assistant tokens, then replaces the live DOM with the settled renderMd() output when the turn finishes.
  • renderMd() only treats asterisk emphasis as Markdown emphasis, but streaming-markdown emits separate underscore emphasis tokens.
  • That mismatch made ordinary live text containing _ look italicized until the final settled render corrected it.
  • The smallest durable fix is to keep using smd, but wrap its renderer so underscore emphasis tokens emit literal _ / __ markers while asterisk emphasis still renders normally.

What Changed

  • Wrapped the live smd renderer in _smdRendererWithoutUnderscoreEmphasis().
  • The wrapper neutralizes only ITALIC_UND and STRONG_UND tokens by writing literal underscores.
  • Asterisk-based *emphasis* and **strong** still pass through the base renderer.
  • Added runtime-backed regression coverage in tests/test_streaming_markdown.py using the vendored smd.min.js module.
  • Added a short Unreleased changelog note.

Why It Matters

Live streamed Markdown now matches the final settled renderer for ordinary identifiers and prose such as agent_response, foo_bar, and _not emphasis_. Users no longer see a paragraph temporarily turn italic during a response only to fix itself at completion.

Verification

Passed:

/Users/georgeandraws/.hermes/hermes-agent/venv/bin/python -m pytest tests/test_streaming_markdown.py -q
# 60 passed, 1 warning in 4.57s

/Users/georgeandraws/.hermes/hermes-agent/venv/bin/python -m pytest tests/test_renderer_js_behaviour.py tests/test_renderer_comprehensive.py -q
# 99 passed, 1 warning in 9.31s

git diff --check
# passed

Additional check:

node --input-type=module -e "import * as smd from './static/vendor/smd.min.js'; console.log(JSON.stringify({ITALIC_AST:smd.ITALIC_AST,ITALIC_UND:smd.ITALIC_UND,STRONG_AST:smd.STRONG_AST,STRONG_UND:smd.STRONG_UND}))"
# {"ITALIC_AST":12,"ITALIC_UND":13,"STRONG_AST":14,"STRONG_UND":15}

Full-suite note:

/Users/georgeandraws/.hermes/hermes-agent/venv/bin/python -m pytest tests/ -q
# 6539 passed, 5 skipped, 3 xpassed, 8 subtests passed, 5 failed

The five full-suite failures appear unrelated to this diff:

  • Three skills API tests build URLs with an unescaped local skill name containing a space: Systematic Debugging.
  • Two workspace git tests assume git init creates master, but this machine creates a different default branch.
  • This PR only changes CHANGELOG.md, static/messages.js, and tests/test_streaming_markdown.py.

Risks / Follow-ups

  • Low risk: the wrapper only intercepts smd underscore emphasis token IDs and forwards all other tokens to the existing renderer.
  • A future streaming-markdown upgrade that renames token constants should keep the focused regression red.
  • No new dependencies, build tools, or broader streaming lifecycle changes.

Model Used

  • OpenAI Codex CLI via npx -y @openai/codex exec --full-auto for investigation and implementation.
  • Hermes parent verification with OpenAI Codex provider, model gpt-5.5.

@nesquena-hermes
Copy link
Copy Markdown
Collaborator

Summary

Reading the diff at cron-pr-2902 against master across static/messages.js:1001-1033 and the new test class at tests/test_streaming_markdown.py:271-371: the fix wraps smd.default_renderer(el) (or the fade variant) with a small adapter that intercepts ITALIC_UND / STRONG_UND tokens, pushes a literal _ or __ marker into the DOM, and pops the same marker on end_token. Asterisk tokens still pass through untouched. The wrapper is composed inside _smdNewParser, which is the single chokepoint used by all three live-render entry points (_scheduleRender, the post-tool reattach at messages.js:1333, and the reconnect path at messages.js:1458), so coverage is consistent across normal streaming, post-tool resumes, and reconnect replay.

Code reference

The wrapper at static/messages.js:1009-1033:

function _smdRendererWithoutUnderscoreEmphasis(renderer){
  if(!renderer||!window.smd) return renderer;
  const baseAddToken=renderer.add_token;
  const baseEndToken=renderer.end_token;
  const baseAddText=renderer.add_text;
  const tokenStack=[];
  renderer.add_token=(data,token)=>{
    if(token===window.smd.ITALIC_UND||token===window.smd.STRONG_UND){
      const marker=token===window.smd.STRONG_UND?'__':'_';
      tokenStack.push(marker);
      baseAddText(data,marker);
      return;
    }
    tokenStack.push(null);
    baseAddToken(data,token);
  };
  renderer.end_token=(data)=>{
    const marker=tokenStack.pop();
    if(marker){
      baseAddText(data,marker);
      return;
    }
    baseEndToken(data);
  };
  return renderer;
}

Matches the settled-renderer policy in static/ui.js:2977-2979, which only treats **...**, ***...***, and *...* as emphasis — underscores are never rewritten.

Diagnosis / Recommendation

The shape is correct. A few observations worth flagging:

  1. Stack discipline is right. Pushing null for passthrough tokens keeps the live vs neutralized branches symmetric — end_token always pops exactly once, so nested links/code inside underscore-flanked text still close cleanly through the base renderer. The test test_smd_wrapper_keeps_nested_token_stack enforces this invariant.

  2. The Node-based runtime test is a nice touch. Importing static/vendor/smd.min.js as an ES module and feeding it through a synthetic renderer lets the test prove agent_response, foo_bar, _not emphasis_, and __not strong__ all survive as literals while *emphasis* and **strong** still render. This protects against future smd vendor updates that could ship new underscore token shapes.

  3. One edge case worth a manual sanity check: smd emits ITALIC_UND only when both flanking underscores match its left/right-edge rules — single underscores in the middle of words (agent_response) never enter the emphasis path in the first place. The wrapper correctly handles the case where smd does decide to open an underscore emphasis (e.g. text _emphasis_ text at word boundaries) and then end_tokens it. Worth eyeballing one streaming session that includes Python identifiers mid-sentence to confirm there's no flicker between "rendered as <em> for one tick, then settled to literal" — but the test confirms no <em> ever opens, so this should be tight.

  4. CHANGELOG entry is well-shaped — user-visible behavior change, framed in terms of identifiers and emphasis parity. Good.

Test plan

The new test class runs both static checks (presence of the wrapper, correct symbol use in ITALIC_UND / STRONG_UND, no ITALIC_AST neutralization) and a real smd-module integration. Recommended manual verification:

  • Stream a message containing def my_function(): and foo_bar = 1 and confirm no italics flicker mid-stream.
  • Stream a message that explicitly uses _emphasis_ and confirm both the live and settled renders show literal underscores (matching renderMd()).
  • Stream *italic* and **bold** and confirm both live and settled renders still emphasize.

LGTM as a focused fix. The wrapper is minimal, the testing is solid, and it routes through the existing _smdNewParser chokepoint without touching the settled renderMd() policy.

@nesquena-hermes
Copy link
Copy Markdown
Collaborator

Holding for rebase — base is stale (pre-batch15 master). Once you rebase onto current origin/master (now at v0.51.137 / commit 48a2e79), the CHANGELOG conflict will resolve and this can be re-evaluated.

The actual fix in static/messages.js looks clean — _smdRendererWithoutUnderscoreEmphasis is a focused wrapper. The hold is just on the CHANGELOG conflict.

Thanks @george-andraws!

@nesquena-hermes
Copy link
Copy Markdown
Collaborator

Merged in Release DL / v0.51.140 (stage-batch22 — 5-PR hold-bucket reassessment with PRs #2899 #2902 #2964 #2970 #2986).

Thanks @george-andraws! 🚢

franksong2702 pushed a commit to franksong2702/hermes-webui-fork that referenced this pull request May 27, 2026
# Conflicts:
#	CHANGELOG.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants