From bad9fd7e2bc4e317ca9274e1c435727df6d0d580 Mon Sep 17 00:00:00 2001 From: Rod Boev Date: Tue, 23 Jun 2026 17:59:43 -0400 Subject: [PATCH] fix(#4793): harden recycled assistant turns and messages scrollbar drag --- static/ui.js | 12 +- tests/test_issue4793_dom_recycle_hardening.py | 231 ++++++++++++++++++ ...test_live_to_final_anchor_visible_order.py | 14 ++ 3 files changed, 254 insertions(+), 3 deletions(-) create mode 100644 tests/test_issue4793_dom_recycle_hardening.py diff --git a/static/ui.js b/static/ui.js index bdbcfe4f..b33a2daa 100644 --- a/static/ui.js +++ b/static/ui.js @@ -438,6 +438,13 @@ let _messageVirtualScrollSettleTimer=0; let _messageVirtualDeferredMeasurement=null; let _msgNodeRecycleEnabled=false; const _recycleStash=new Map(); +const _recycleResetAttrs=[ + 'data-transparent-turn-collapsed', + 'data-transparent-turn-toggle-bound', + 'data-anchor-scene-live-owner', + 'data-anchor-stream-id', + 'data-live-assistant-turn', +]; let _scrollbarDragActive=false; function _markMessageVirtualScrollActive(){ _messageVirtualScrollActive=true; @@ -3918,7 +3925,7 @@ if(typeof window!=='undefined'){ const el=document.getElementById('messages'); if(!el) return; el.addEventListener('pointerdown',(e)=>{ - if(e.offsetX>=el.clientWidth) _scrollbarDragActive=true; + if(e.target===el&&e.offsetX>=el.clientWidth) _scrollbarDragActive=true; },{passive:true}); window.addEventListener('pointerup',()=>{ if(!_scrollbarDragActive) return; @@ -11526,8 +11533,7 @@ function renderMessages(options){ if(recycled){ const blocks=_assistantTurnBlocks(recycled); if(blocks) blocks.innerHTML=''; - recycled.removeAttribute('data-transparent-turn-collapsed'); - recycled.removeAttribute('data-transparent-turn-toggle-bound'); + for(const attr of _recycleResetAttrs) recycled.removeAttribute(attr); const role=recycled.querySelector('.msg-role.assistant'); if(role) role.outerHTML=_assistantRoleHtml(tsTitle, isTpsDisplayEnabled()?_formatTurnTps(m._turnTps):''); currentAssistantTurn=recycled; diff --git a/tests/test_issue4793_dom_recycle_hardening.py b/tests/test_issue4793_dom_recycle_hardening.py new file mode 100644 index 00000000..13bb45b7 --- /dev/null +++ b/tests/test_issue4793_dom_recycle_hardening.py @@ -0,0 +1,231 @@ +"""Regression coverage for issue #4793 DOM recycle hardening.""" + +import json +import shutil +import subprocess +import tempfile +from pathlib import Path + +import pytest + + +REPO_ROOT = Path(__file__).parent.parent.resolve() +UI_JS_PATH = REPO_ROOT / "static" / "ui.js" +NODE = shutil.which("node") + +pytestmark = pytest.mark.skipif(NODE is None, reason="node not on PATH") + + +def _run_node(source: str) -> dict: + with tempfile.NamedTemporaryFile( + "w", suffix=".cjs", encoding="utf-8", dir=REPO_ROOT, delete=False + ) as script: + script.write(source) + script_path = Path(script.name) + try: + result = subprocess.run( + [NODE, str(script_path)], + cwd=str(REPO_ROOT), + capture_output=True, + text=True, + timeout=30, + ) + finally: + script_path.unlink(missing_ok=True) + if result.returncode != 0: + raise RuntimeError(result.stderr) + return json.loads(result.stdout.strip()) + + +def _js_prefix() -> str: + return f""" +const fs = require('fs'); +const src = fs.readFileSync({json.dumps(str(UI_JS_PATH))}, 'utf8'); +function extractBetween(startMarker, endMarker, startFrom = 0) {{ + const start = src.indexOf(startMarker, startFrom); + if (start < 0) throw new Error(startMarker + ' not found'); + const end = src.indexOf(endMarker, start); + if (end < 0) throw new Error(endMarker + ' not found'); + return src.slice(start, end); +}} +function extractBody(startMarker, endMarker, startFrom = 0) {{ + const start = src.indexOf(startMarker, startFrom); + if (start < 0) throw new Error(startMarker + ' not found'); + const end = src.indexOf(endMarker, start); + if (end < 0) throw new Error(endMarker + ' not found'); + return src.slice(start + startMarker.length, end); +}} +""" + + +def test_recycled_assistant_turn_reuse_clears_live_anchor_attrs(): + script = _js_prefix() + """ +const recycleChunk = extractBetween("if(!currentAssistantTurn){", "const seg=document.createElement('div');"); +const removedAttrs = []; +const role = { + replacement: null, + set outerHTML(value) { this.replacement = value; }, + get outerHTML() { return this.replacement; }, +}; +const recycled = { + dataset: { recycleKey: 'old-key', role: 'old-role', sessionId: 'old-session' }, + classList: { contains(name) { return name === 'assistant-turn'; } }, + removeAttribute(name) { removedAttrs.push(name); }, + querySelector(selector) { return selector === '.msg-role.assistant' ? role : null; }, +}; +const _recycleResetAttrs = [ + 'data-transparent-turn-collapsed', + 'data-transparent-turn-toggle-bound', + 'data-anchor-scene-live-owner', + 'data-anchor-stream-id', + 'data-live-assistant-turn', +]; +const _recycleStash = new Map([[7, recycled]]); +const _msgNodeRecycleEnabled = true; +let currentAssistantTurn = null; +const rawIdx = 7; +const blocks = { innerHTML: 'keep me' }; +const _assistantTurnBlocks = () => blocks; +const inner = { appendChild() {} }; +const tsTitle = 'Turn title'; +const m = { _turnTps: 42 }; +const S = { session: { session_id: 'sess-1' } }; +const _assistantRoleHtml = (title, tps) => `role:${title}:${tps}`; +const isTpsDisplayEnabled = () => true; +const _formatTurnTps = value => `tps:${value}`; +eval(recycleChunk); +console.log(JSON.stringify({ + removedAttrs, + blocksInnerHTML: blocks.innerHTML, + roleHtml: role.replacement, + currentAssistantTurnIsRecycled: currentAssistantTurn === recycled, + dataset: { + role: currentAssistantTurn.dataset.role, + sessionId: currentAssistantTurn.dataset.sessionId, + recycleKey: String(currentAssistantTurn.dataset.recycleKey), + }, +})); +""" + result = _run_node(script) + + assert result["removedAttrs"] == [ + "data-transparent-turn-collapsed", + "data-transparent-turn-toggle-bound", + "data-anchor-scene-live-owner", + "data-anchor-stream-id", + "data-live-assistant-turn", + ] + assert result["currentAssistantTurnIsRecycled"] is True + assert result["dataset"] == { + "role": "assistant", + "sessionId": "sess-1", + "recycleKey": "7", + } + + +def test_recycled_assistant_turn_reuse_keeps_block_clear_and_role_refresh(): + script = _js_prefix() + """ +const recycleChunk = extractBetween("if(!currentAssistantTurn){", "const seg=document.createElement('div');"); +const removedAttrs = []; +const role = { + replacement: null, + set outerHTML(value) { this.replacement = value; }, + get outerHTML() { return this.replacement; }, +}; +const recycled = { + dataset: { recycleKey: 'old-key' }, + classList: { contains(name) { return name === 'assistant-turn'; } }, + removeAttribute(name) { removedAttrs.push(name); }, + querySelector(selector) { return selector === '.msg-role.assistant' ? role : null; }, +}; +const _recycleResetAttrs = [ + 'data-transparent-turn-collapsed', + 'data-transparent-turn-toggle-bound', + 'data-anchor-scene-live-owner', + 'data-anchor-stream-id', + 'data-live-assistant-turn', +]; +const _recycleStash = new Map([[7, recycled]]); +const _msgNodeRecycleEnabled = true; +let currentAssistantTurn = null; +const rawIdx = 7; +const blocks = { innerHTML: 'keep me' }; +const _assistantTurnBlocks = () => blocks; +const inner = { appendChild() {} }; +const tsTitle = 'Turn title'; +const m = { _turnTps: 42 }; +const S = { session: { session_id: 'sess-1' } }; +const _assistantRoleHtml = (title, tps) => `role:${title}:${tps}`; +const isTpsDisplayEnabled = () => true; +const _formatTurnTps = value => `tps:${value}`; +eval(recycleChunk); +console.log(JSON.stringify({ + blocksInnerHTML: blocks.innerHTML, + roleHtml: role.replacement, + currentAssistantTurnIsRecycled: currentAssistantTurn === recycled, + dataset: { + role: currentAssistantTurn.dataset.role, + sessionId: currentAssistantTurn.dataset.sessionId, + recycleKey: String(currentAssistantTurn.dataset.recycleKey), + }, +})); +""" + result = _run_node(script) + + assert result["blocksInnerHTML"] == "" + assert result["roleHtml"] == "role:Turn title:tps:42" + assert result["currentAssistantTurnIsRecycled"] is True + assert result["dataset"] == { + "role": "assistant", + "sessionId": "sess-1", + "recycleKey": "7", + } + + +def test_messages_scrollbar_drag_ignores_bubbled_child_pointerdown(): + script = _js_prefix() + """ +const pointerdownBody = extractBody("el.addEventListener('pointerdown',(e)=>{", "},{passive:true});"); +let _scrollbarDragActive = false; +const el = { clientWidth: 120 }; +eval(`function pointerdownHandler(e){${pointerdownBody}}`); +pointerdownHandler({ target: { clientWidth: 120 }, offsetX: 999 }); +const bubbledArmed = _scrollbarDragActive; +_scrollbarDragActive = false; +pointerdownHandler({ target: el, offsetX: 999 }); +console.log(JSON.stringify({ + bubbledArmed, + directArmed: _scrollbarDragActive, +})); +""" + result = _run_node(script) + + assert result["bubbledArmed"] is False + assert result["directArmed"] is True + + +def test_messages_scrollbar_drag_still_arms_for_direct_messages_press_and_rerenders_on_pointerup(): + script = _js_prefix() + """ +const pointerdownBody = extractBody("el.addEventListener('pointerdown',(e)=>{", "},{passive:true});"); +const pointerupBody = extractBody("window.addEventListener('pointerup',()=>{", "},{passive:true});"); +const rerenders = []; +let _scrollbarDragActive = false; +const el = { clientWidth: 120 }; +function _scheduleMessageVirtualizedRender(force) { + rerenders.push(force); +} +eval(`function pointerdownHandler(e){${pointerdownBody}}`); +eval(`function pointerupHandler(){${pointerupBody}}`); +pointerdownHandler({ target: el, offsetX: 999 }); +const armed = _scrollbarDragActive; +pointerupHandler(); +console.log(JSON.stringify({ + armed, + afterPointerup: _scrollbarDragActive, + rerenders, +})); +""" + result = _run_node(script) + + assert result["armed"] is True + assert result["afterPointerup"] is False + assert result["rerenders"] == [True] diff --git a/tests/test_live_to_final_anchor_visible_order.py b/tests/test_live_to_final_anchor_visible_order.py index 58c18fd7..ff682ed4 100644 --- a/tests/test_live_to_final_anchor_visible_order.py +++ b/tests/test_live_to_final_anchor_visible_order.py @@ -427,6 +427,20 @@ def test_live_anchor_scene_removes_legacy_interim_collapse_toggle(): assert guard_idx < remove_idx < legacy_create_idx +def test_recycled_assistant_turn_clears_live_anchor_attrs_before_role_refresh(): + reset_attrs = UI_JS[UI_JS.index("const _recycleResetAttrs="):UI_JS.index("let _scrollbarDragActive=false;")] + assert "data-anchor-scene-live-owner" in reset_attrs + assert "data-anchor-stream-id" in reset_attrs + assert "data-live-assistant-turn" in reset_attrs + + recycle = _function_body(UI_JS, "renderMessages") + recycle = recycle[recycle.index("if(!currentAssistantTurn){"):] + + loop_idx = recycle.index("for(const attr of _recycleResetAttrs) recycled.removeAttribute(attr);") + refresh_idx = recycle.index("if(role) role.outerHTML=_assistantRoleHtml(tsTitle, isTpsDisplayEnabled()?_formatTurnTps(m._turnTps):'');") + assert loop_idx < refresh_idx + + def test_tool_scene_rows_coalesce_by_logical_tool_call_identity(): rows = _function_body(UI_JS, "_anchorSceneRowsForRendering") key = _function_body(UI_JS, "_anchorSceneToolRowLogicalKey")