fix(#4793): harden recycled assistant turns and messages scrollbar drag
This commit is contained in:
Rod Boev
parent
6f39bb6d08
commit
bad9fd7e2b
3 changed files with 254 additions and 3 deletions
12
static/ui.js
12
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;
|
||||
|
|
|
|||
231
tests/test_issue4793_dom_recycle_hardening.py
Normal file
231
tests/test_issue4793_dom_recycle_hardening.py
Normal file
|
|
@ -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]
|
||||
|
|
@ -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")
|
||||
|
|
|
|||
Loading…
Reference in a new issue