diff --git a/PR_NOTES.md b/PR_NOTES.md index 1f5699a8..72cbee2a 100644 --- a/PR_NOTES.md +++ b/PR_NOTES.md @@ -20,16 +20,16 @@ Adds an opt-in `Fade text effect` preference for live assistant responses. When - Uses Hermes' existing incremental `streaming-markdown` parser with a custom renderer instead of full markdown re-renders. - Only newly appended words are wrapped and animated. - Animated spans are replaced with plain text by a delegated `animationend` handler, avoiding long-lived wrapper buildup without per-word listeners. -- Reduced-motion preference is cached with a media-query listener instead of checked for every appended text node. +- Reduced-motion preference is cached with a media-query listener instead of checked for every appended text node, and terminal stream paths remove the listener. - Unsafe streamed `href`/`src` values are blocked in the fade renderer `set_attr` path. - On `done`, fade mode drains buffered text, ends the parser to flush pending markdown, and waits for the final fade/stagger window before the final `renderMessages()` replacement. ## Performance/readability tuning -- Normal fade duration: `160ms` -- Word stagger: `12ms` -- Done drain wait cap: `220ms` -- Visual playback is capped at `100 wps`. +- Normal fade duration: `200ms` +- Word stagger: `16ms` +- Done drain wait cap: `320ms` +- Visual playback is capped at `160 wps`. - Reveals at most `2 words/frame`, or `3 words/frame` only with very large backlog. - Reveal steps pause briefly after sentence punctuation and paragraph breaks so very fast models feel less overwhelming. - Target word counts reuse the already-visible word count plus backlog count, avoiding repeated full-response word scans on each tick. @@ -46,7 +46,7 @@ git diff --check Latest focused result: ```text -68 passed +69 passed ``` ## Manual QA suggested diff --git a/static/messages.js b/static/messages.js index c1198cf0..12d1525c 100644 --- a/static/messages.js +++ b/static/messages.js @@ -538,6 +538,7 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ let _streamFadeHoldUntilMs=0; let _streamFadeReduceMotionMql=null; let _streamFadeReduceMotion=false; + let _streamFadeReduceMotionOnChange=null; const _STREAM_FADE_MS=200; const _STREAM_FADE_STAGGER_MS=16; const _STREAM_FADE_DONE_MAX_MS=320; @@ -727,12 +728,19 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ if(!_streamFadeReduceMotionMql){ _streamFadeReduceMotionMql=window.matchMedia('(prefers-reduced-motion: reduce)'); _streamFadeReduceMotion=!!_streamFadeReduceMotionMql.matches; - const onChange=e=>{_streamFadeReduceMotion=!!e.matches;}; - try{_streamFadeReduceMotionMql.addEventListener('change',onChange);} - catch(_){try{_streamFadeReduceMotionMql.addListener(onChange);}catch(_){}} + _streamFadeReduceMotionOnChange=e=>{_streamFadeReduceMotion=!!e.matches;}; + try{_streamFadeReduceMotionMql.addEventListener('change',_streamFadeReduceMotionOnChange);} + catch(_){try{_streamFadeReduceMotionMql.addListener(_streamFadeReduceMotionOnChange);}catch(_){}} } return _streamFadeReduceMotion; } + function _streamFadeCleanupReduceMotionListener(){ + if(!_streamFadeReduceMotionMql||!_streamFadeReduceMotionOnChange) return; + try{_streamFadeReduceMotionMql.removeEventListener('change',_streamFadeReduceMotionOnChange);} + catch(_){try{_streamFadeReduceMotionMql.removeListener(_streamFadeReduceMotionOnChange);}catch(_){}} + _streamFadeReduceMotionMql=null; + _streamFadeReduceMotionOnChange=null; + } function _streamFadeBindCleanup(el){ if(!el||el._streamFadeCleanupBound) return; el._streamFadeCleanupBound=true; @@ -1245,6 +1253,7 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ // can reintroduce a stale thinking card or duplicate content. _streamFinalized=true; _cancelAnimationFramePendingStreamRender(); + _streamFadeCleanupReduceMotionListener(); if(typeof finalizeThinkingCard==='function') finalizeThinkingCard(); // Finalize smd parser — flushes any remaining buffered markdown state // and runs Prism + copy buttons on the live segment before the DOM is replaced @@ -1489,6 +1498,7 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ if(_persistTimer){clearTimeout(_persistTimer);_persistTimer=null;} _streamFinalized=true; _cancelAnimationFramePendingStreamRender(); + _streamFadeCleanupReduceMotionListener(); _smdEndParser(); if(typeof finalizeThinkingCard==='function') finalizeThinkingCard(); // Application-level error sent explicitly by the server (rate limit, crash, etc.) @@ -1575,6 +1585,7 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ if(_persistTimer){clearTimeout(_persistTimer);_persistTimer=null;} _streamFinalized=true; _cancelAnimationFramePendingStreamRender(); + _streamFadeCleanupReduceMotionListener(); _smdEndParser(); if(typeof finalizeThinkingCard==='function') finalizeThinkingCard(); source.close(); @@ -1669,6 +1680,7 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ if(_persistTimer){clearTimeout(_persistTimer);_persistTimer=null;} _streamFinalized=true; _cancelAnimationFramePendingStreamRender(); + _streamFadeCleanupReduceMotionListener(); if(typeof finalizeThinkingCard==='function') finalizeThinkingCard(); _clearOwnerInflightState(); _closeSource(); diff --git a/tests/test_smooth_text_fade.py b/tests/test_smooth_text_fade.py index 8aadea62..c984d1e7 100644 --- a/tests/test_smooth_text_fade.py +++ b/tests/test_smooth_text_fade.py @@ -77,9 +77,9 @@ let _streamFadeLatestAnimationEndAt=0; let _streamFadeAppendOffset=0; let _streamFadeVisibleWords=0; let _streamFadeHoldUntilMs=0; -const _STREAM_FADE_MS=160; -const _STREAM_FADE_STAGGER_MS=12; -const _STREAM_FADE_DONE_MAX_MS=220; +const _STREAM_FADE_MS=200; +const _STREAM_FADE_STAGGER_MS=16; +const _STREAM_FADE_DONE_MAX_MS=320; const performance={performance_stub}; {helpers} """ @@ -187,7 +187,7 @@ def test_stream_fade_css_is_opacity_only_and_hides_live_cursor(): [ "@keyframes stream-fade-word-in", ".stream-fade-word.is-new", - "var(--stream-fade-ms,160ms) cubic-bezier(.2,.7,.2,1)", + "var(--stream-fade-ms,240ms) cubic-bezier(.2,.7,.2,1)", "prefers-reduced-motion: reduce", ".msg-body.stream-fade-active > :last-child::after", "display:none", @@ -197,6 +197,14 @@ def test_stream_fade_css_is_opacity_only_and_hides_live_cursor(): assert ".stream-fade-active .stream-fade-word{display:inline;}" in fade_css +def test_stream_fade_reduced_motion_listener_is_cleaned_up_on_terminal_paths(): + assert "_streamFadeReduceMotionOnChange" in MESSAGES_JS + assert "function _streamFadeCleanupReduceMotionListener()" in MESSAGES_JS + assert "removeEventListener('change',_streamFadeReduceMotionOnChange)" in MESSAGES_JS + assert "removeListener(_streamFadeReduceMotionOnChange)" in MESSAGES_JS + assert MESSAGES_JS.count("_streamFadeCleanupReduceMotionListener();") >= 4 + + def test_stream_fade_playout_handles_fast_models_without_paragraph_pops(): script = ( fade_helper_script()