fix(stream): stop fade-text effect from re-fading the whole message on tool-call rewinds - #6783
Conversation
…ol-call rewinds When the fade-text effect is enabled, Hermes long tasks visibly blink the entire message on every tool call. Root cause: tool-call XML (<function_calls>) appears inline in the assistant text stream, and the unclosed-tag stripping in _stripXmlToolCalls temporarily rewinds displayText to a strict prefix. That rewind zeroed the fade playout (_streamFadeNextText) and tripped _smdWrite's self-heal, which cleared assistantBody.innerHTML and re-created every word as a fresh is-new span — replaying the fade on all visible text at once. Fix: on a detected rewind, shrink the playout cursor to the common prefix instead of resetting to zero, and mute the fade renderer for that common prefix during any rebuild (both the main smd path and the anchor prose incremental node) so only the post-rewind tail animates. Adds regression tests: playout rewind keeps common prefix visible, rebuild mutes the common prefix, and _smdWrite's self-heal sets the silent-prefix window.
|
| Filename | Overview |
|---|---|
| static/messages.js | Updates stream rewind handling and rendered-prefix muting across the main and anchor prose fade paths. |
| tests/test_smooth_text_fade.py | Adds focused regression coverage for rewind playout, rendered-prefix muting, parser rebuilds, markdown/media offsets, and root isolation. |
| tests/test_smd_media_in_stream.py | Extends the extracted JavaScript test harness with the new fade-prefix state. |
Reviews (4): Last reviewed commit: "Merge branch 'master' into fix/fade-text..." | Re-trigger Greptile
SummaryReading all three changed files at PR head 56c9307, plus the corresponding origin/master files, the rewind detection itself is useful. I found one behavior blocker in the new suppression mechanism: the silent-prefix count lives in the raw source-text coordinate space, while both consumers live in rendered-text space. That mismatch means the PR can suppress the first genuinely new words after a rewind. Code referencestatic/messages.js:4350-4366 computes the budget before rebuilding, while renderer add_text receives only rendered text chunks: _streamFadeSilentPrefixChars=_silentCommon;
let silentLeft=_streamFadeSilentPrefixChars||0;
if(silentLeft>0){
silentLeft-=match[0].length;
}The common length includes Markdown delimiters, link destinations, and MEDIA token bytes. Smd does not pass those bytes to add_text: it emits token callbacks or media nodes and passes only visible prose. The same mismatch exists in static/messages.js:4536-4573 and static/messages.js:4864-4889. Diagnosis / recommendationA concrete case is old prose alpha beta rewritten to alpha gamma. The source common prefix includes the four strong-emphasis delimiters, but the renderer consumes only alpha and the separating whitespace. The leftover budget then writes gamma as plain text. That contradicts the intended contract that only the already-visible prefix is muted and the changed tail fades. A common prefix containing a MEDIA token has the same problem because the token becomes a node and does not reach add_text at all. VerificationThe new tests at tests/test_smooth_text_fade.py:589-688 use plain text, and the self-heal test stubs parser writing, so they cannot expose this coordinate mismatch. Add a real vendored-smd regression for strong emphasis, links or code, and MEDIA before the changed tail. Assert the old rendered prefix is plain while the first changed word has exactly one stream-fade-word is-new span. Also cover reduced motion during rebuild followed by motion being enabled, and two concurrent anchor/main parsers to prove suppression cannot cross owners. I did not execute PR-authored tests; this review used read-only source, diff, and assertion inspection. |
Review feedback (maintainer round on 56c9307): the silent-prefix budget was computed in SOURCE-text space (charCodeAt over markdown source), but the fade add_text hook only receives RENDERED prose. Markdown delimiters, link destinations and MEDIA token bytes never reach add_text, so the source-space budget over-muted the first genuinely new word after a rewind (e.g. '**alpha** beta' -> '**alpha** gamma' muted gamma). Fix: snapshot the OLD rendered text before the rebuild (assistantBody textContent / anchor .msg-body textContent), then after parser_write compare it with the NEW rendered text and strip is-new only from spans inside the rendered common prefix (_streamFadeMuteRenderedPrefix). - Rendered-space boundary, scoped to the owning node (no stream-global counter that can survive a rebuild without a fade consumer) - No state left behind on reduced-motion / safe-renderer / empty-text / parser-finalization paths: _rewindPrevRendered is a local variable consumed right after the rebuild - Tests: plain-word mute, markdown/MEDIA byte regression (the blocker), per-root scoping (main vs anchor), updated _smdWrite self-heal test Gate: node -c OK; test_smooth_text_fade + test_smd_media_in_stream + test_svg_audio_video_rendering + test_insights -> 83 passed, 18 subtests.
|
Addressed the coordinate-space blocker — rewind mute now runs in RENDERED-text space. What changed (commit
Tests:
Gate: One note: the strong-emphasis/link/MEDIA-before-changed-tail behavioral test you asked for lives at the unit level ( |
CI lint failure on a6d37b2: tests/test_smooth_text_fade.py:723 F541 f-string without any placeholders. The f-prefix was only used to escape JS braces ({{ }}) — a plain string with literal braces is equivalent and passes the ruff E9+F+B gate.
|
CI lint on a6d37b2 flagged Verified locally: |
…ind (#6783, @silent-reader-cn) (#6866) Co-authored-by: nesquena-hermes <nesquena-hermes@users.noreply.github.com>
|
Shipped in exp-v0.52.186 🎉 Thanks @silent-reader-cn. Gate at the rebased head: Codex SAFE TO SHIP — verified both rebuild paths (the anchor-prose |
…n tool-call rewinds (nesquena#6783) * fix(stream): stop fade-text effect from re-fading whole message on tool-call rewinds When the fade-text effect is enabled, Hermes long tasks visibly blink the entire message on every tool call. Root cause: tool-call XML (<function_calls>) appears inline in the assistant text stream, and the unclosed-tag stripping in _stripXmlToolCalls temporarily rewinds displayText to a strict prefix. That rewind zeroed the fade playout (_streamFadeNextText) and tripped _smdWrite's self-heal, which cleared assistantBody.innerHTML and re-created every word as a fresh is-new span — replaying the fade on all visible text at once. Fix: on a detected rewind, shrink the playout cursor to the common prefix instead of resetting to zero, and mute the fade renderer for that common prefix during any rebuild (both the main smd path and the anchor prose incremental node) so only the post-rewind tail animates. Adds regression tests: playout rewind keeps common prefix visible, rebuild mutes the common prefix, and _smdWrite's self-heal sets the silent-prefix window. * fix(nesquena#6783): compute rewind mute prefix in rendered-text space Review feedback (maintainer round on 56c9307): the silent-prefix budget was computed in SOURCE-text space (charCodeAt over markdown source), but the fade add_text hook only receives RENDERED prose. Markdown delimiters, link destinations and MEDIA token bytes never reach add_text, so the source-space budget over-muted the first genuinely new word after a rewind (e.g. '**alpha** beta' -> '**alpha** gamma' muted gamma). Fix: snapshot the OLD rendered text before the rebuild (assistantBody textContent / anchor .msg-body textContent), then after parser_write compare it with the NEW rendered text and strip is-new only from spans inside the rendered common prefix (_streamFadeMuteRenderedPrefix). - Rendered-space boundary, scoped to the owning node (no stream-global counter that can survive a rebuild without a fade consumer) - No state left behind on reduced-motion / safe-renderer / empty-text / parser-finalization paths: _rewindPrevRendered is a local variable consumed right after the rebuild - Tests: plain-word mute, markdown/MEDIA byte regression (the blocker), per-root scoping (main vs anchor), updated _smdWrite self-heal test Gate: node -c OK; test_smooth_text_fade + test_smd_media_in_stream + test_svg_audio_video_rendering + test_insights -> 83 passed, 18 subtests. * fix(nesquena#6783): drop f-prefix from _fade_fake_node JS block (F541) CI lint failure on a6d37b2: tests/test_smooth_text_fade.py:723 F541 f-string without any placeholders. The f-prefix was only used to escape JS braces ({{ }}) — a plain string with literal braces is equivalent and passes the ruff E9+F+B gate. --------- Co-authored-by: silent-reader-cn <silent-reader-cn@users.noreply.github.com> Co-authored-by: nesquena-hermes <nesquena+hermes@gmail.com>
…ind (nesquena#6783, @silent-reader-cn) (nesquena#6866) Co-authored-by: nesquena-hermes <nesquena-hermes@users.noreply.github.com>
…n tool-call rewinds (nesquena#6783) * fix(stream): stop fade-text effect from re-fading whole message on tool-call rewinds When the fade-text effect is enabled, Hermes long tasks visibly blink the entire message on every tool call. Root cause: tool-call XML (<function_calls>) appears inline in the assistant text stream, and the unclosed-tag stripping in _stripXmlToolCalls temporarily rewinds displayText to a strict prefix. That rewind zeroed the fade playout (_streamFadeNextText) and tripped _smdWrite's self-heal, which cleared assistantBody.innerHTML and re-created every word as a fresh is-new span — replaying the fade on all visible text at once. Fix: on a detected rewind, shrink the playout cursor to the common prefix instead of resetting to zero, and mute the fade renderer for that common prefix during any rebuild (both the main smd path and the anchor prose incremental node) so only the post-rewind tail animates. Adds regression tests: playout rewind keeps common prefix visible, rebuild mutes the common prefix, and _smdWrite's self-heal sets the silent-prefix window. * fix(nesquena#6783): compute rewind mute prefix in rendered-text space Review feedback (maintainer round on 56c9307): the silent-prefix budget was computed in SOURCE-text space (charCodeAt over markdown source), but the fade add_text hook only receives RENDERED prose. Markdown delimiters, link destinations and MEDIA token bytes never reach add_text, so the source-space budget over-muted the first genuinely new word after a rewind (e.g. '**alpha** beta' -> '**alpha** gamma' muted gamma). Fix: snapshot the OLD rendered text before the rebuild (assistantBody textContent / anchor .msg-body textContent), then after parser_write compare it with the NEW rendered text and strip is-new only from spans inside the rendered common prefix (_streamFadeMuteRenderedPrefix). - Rendered-space boundary, scoped to the owning node (no stream-global counter that can survive a rebuild without a fade consumer) - No state left behind on reduced-motion / safe-renderer / empty-text / parser-finalization paths: _rewindPrevRendered is a local variable consumed right after the rebuild - Tests: plain-word mute, markdown/MEDIA byte regression (the blocker), per-root scoping (main vs anchor), updated _smdWrite self-heal test Gate: node -c OK; test_smooth_text_fade + test_smd_media_in_stream + test_svg_audio_video_rendering + test_insights -> 83 passed, 18 subtests. * fix(nesquena#6783): drop f-prefix from _fade_fake_node JS block (F541) CI lint failure on a6d37b2: tests/test_smooth_text_fade.py:723 F541 f-string without any placeholders. The f-prefix was only used to escape JS braces ({{ }}) — a plain string with literal braces is equivalent and passes the ruff E9+F+B gate. --------- Co-authored-by: silent-reader-cn <silent-reader-cn@users.noreply.github.com> Co-authored-by: nesquena-hermes <nesquena+hermes@gmail.com>
…ind (nesquena#6783, @silent-reader-cn) (nesquena#6866) Co-authored-by: nesquena-hermes <nesquena-hermes@users.noreply.github.com>
Summary
With the fade-text effect setting enabled, Hermes long tasks visibly blink/flash the entire message on every tool call: all visible words fade in, disappear, then fade in again.
Root cause
Tool-call XML (
<function_calls>…</function_calls>) appears inline in the assistant text stream._stripXmlToolCallsstrips the tail of an unclosed<function_callsopening tag (regex[\s\S]*$), which temporarily rewindsdisplayTextto a strict prefix of what was already shown. That rewind triggered two cascading rebuilds:_streamFadeNextTextdetected the prefix mismatch and reset the playout cursor to zero._smdWrite's self-heal then saw the new text no longer starting with the written text, clearedassistantBody.innerHTML='', and re-created every word as a freshis-newspan — replaying the fade animation on all visible text at once.Every tool call in a long task hit this, producing the visible flashing loop.
Fix
_streamFadeNextText: on a detected rewind, shrink the playout cursor to the common prefix instead of resetting to zero. The DOM syncs to the shrunken prefix this frame (dropping the rewind tail)._smdWrite+_anchorProseIncrementalNode: when a rebuild is unavoidable, compute the common prefix and set a_streamFadeSilentPrefixCharswindow._streamFadeRenderer.add_text+_streamFadeAppendText: words inside the silent-prefix window are written as plain text nodes (no animation replayed); only the post-rewind tail fades in.Result: already-visible text stays put on tool-call boundaries; only genuinely new words animate.
Tests
Added 3 regression tests in
tests/test_smooth_text_fade.py:_smdWriteself-heal sets the silent-prefix window on rewindVerified:
tests/test_smooth_text_fade.py+test_smd_media_in_stream.py+test_anchor_prose_incremental_finalize.py+test_issue3397_transparent_stream_prose_segments.py— 62 passed. Full suite: 582 passed, 1 pre-existing Windows permission-test failure unrelated to this change.