fix: preserve scroll on stream completion
This commit is contained in:
BIN
docs/pr-media/1690/scroll-preserved-after-completion.png
Normal file
BIN
docs/pr-media/1690/scroll-preserved-after-completion.png
Normal file
Binary file not shown.
|
After Width: | Height: | Size: 58 KiB |
@@ -932,7 +932,7 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){
|
||||
// No-reply guard (#373): if agent returned nothing, show inline error
|
||||
if(!S.messages.some(m=>m.role==='assistant'&&String(m.content||'').trim())&&!assistantText){removeThinking();S.messages.push({role:'assistant',content:'**No response received.** Check your API key and model selection.'});}
|
||||
if(isSessionViewed) _markSessionViewed(completedSid, completedSession.message_count ?? S.messages.length);
|
||||
syncTopbar();renderMessages();loadDir('.');
|
||||
syncTopbar();renderMessages({preserveScroll:true});loadDir('.');
|
||||
// TTS auto-read: speak the last assistant response if enabled (#499)
|
||||
if(typeof autoReadLastAssistant==='function') setTimeout(()=>autoReadLastAssistant(), 300);
|
||||
}
|
||||
@@ -1038,7 +1038,7 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){
|
||||
S.messages.push({role:'assistant',content:'**Error:** An error occurred. Check server logs.'});
|
||||
}
|
||||
_markSessionViewed(activeSid, S.messages.length);
|
||||
renderMessages();
|
||||
renderMessages({preserveScroll:true});
|
||||
}else if(typeof trackBackgroundError==='function'){
|
||||
const _errTitle=(typeof _allSessions!=='undefined'&&_allSessions.find(s=>s.session_id===activeSid)||{}).title||null;
|
||||
try{const d=JSON.parse(e.data);trackBackgroundError(activeSid,_errTitle,d.message||'Error');}
|
||||
@@ -1113,13 +1113,13 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){
|
||||
S.messages=(data.session.messages||[]).filter(m=>m&&m.role);
|
||||
clearLiveToolCards();if(!assistantText)removeThinking();
|
||||
_markSessionViewed(activeSid, data.session.message_count ?? S.messages.length);
|
||||
renderMessages();
|
||||
renderMessages({preserveScroll:true});
|
||||
}
|
||||
}catch(_){
|
||||
// Fallback to local cancel message if API fails
|
||||
if(S.session&&S.session.session_id===activeSid){
|
||||
clearLiveToolCards();if(!assistantText)removeThinking();
|
||||
S.messages.push({role:'assistant',content:'*Task cancelled.*'});renderMessages();
|
||||
S.messages.push({role:'assistant',content:'*Task cancelled.*'});renderMessages({preserveScroll:true});
|
||||
_markSessionViewed(activeSid, S.messages.length);
|
||||
}
|
||||
}
|
||||
@@ -1169,7 +1169,7 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){
|
||||
S.toolCalls=[];
|
||||
}
|
||||
if(isSessionViewed) _markSessionViewed(completedSid, session.message_count ?? S.messages.length);
|
||||
syncTopbar();renderMessages();
|
||||
syncTopbar();renderMessages({preserveScroll:true});
|
||||
}
|
||||
_queueDrainSid=activeSid;renderSessionList();setBusy(false);setComposerStatus('');
|
||||
return true;
|
||||
@@ -1192,7 +1192,7 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){
|
||||
if(S.session&&S.session.session_id===activeSid){
|
||||
S.activeStreamId=null;
|
||||
clearLiveToolCards();if(!assistantText)removeThinking();
|
||||
S.messages.push({role:'assistant',content:'**Error:** Connection lost'});renderMessages();
|
||||
S.messages.push({role:'assistant',content:'**Error:** Connection lost'});renderMessages({preserveScroll:true});
|
||||
_markSessionViewed(activeSid, S.messages.length);
|
||||
}else{
|
||||
if(typeof trackBackgroundError==='function'){
|
||||
@@ -1223,7 +1223,7 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){
|
||||
removeThinking();
|
||||
_queueDrainSid=activeSid;setBusy(false);
|
||||
setComposerStatus('');
|
||||
renderMessages();
|
||||
renderMessages({preserveScroll:true});
|
||||
renderSessionList();
|
||||
}
|
||||
return;
|
||||
@@ -1980,7 +1980,7 @@ function startBackgroundPolling(parentSid, taskId, prompt){
|
||||
delete _bgPollTimers[taskId];
|
||||
const msg={role:'assistant',content:`**${t('bg_label')}** ${prompt.slice(0,80)}\n\n${res.answer||t('bg_no_answer')}`,'_background':true,_ts:Date.now()/1000};
|
||||
S.messages.push(msg);
|
||||
renderMessages();
|
||||
renderMessages({preserveScroll:true});
|
||||
showToast(t('bg_complete'));
|
||||
return;
|
||||
}
|
||||
|
||||
26
static/ui.js
26
static/ui.js
@@ -4014,7 +4014,23 @@ function clearMessageRenderCache(){
|
||||
_sessionHtmlCacheSid=null;
|
||||
}
|
||||
|
||||
function renderMessages(){
|
||||
function _scrollAfterMessageRender(preserveScroll){
|
||||
// Terminal stream renders can happen after S.activeStreamId is cleared.
|
||||
// In that case, preserveScroll asks the normal pin-state helper to decide:
|
||||
// pinned users stay at bottom; users who manually scrolled up stay put.
|
||||
if(preserveScroll){
|
||||
scrollIfPinned();
|
||||
return;
|
||||
}
|
||||
if(S.activeStreamId){
|
||||
scrollIfPinned();
|
||||
return;
|
||||
}
|
||||
scrollToBottom();
|
||||
}
|
||||
|
||||
function renderMessages(options){
|
||||
const preserveScroll=!!(options&&options.preserveScroll);
|
||||
const inner=$('msgInner');
|
||||
const sid=S.session?S.session.session_id:null;
|
||||
const msgCount=S.messages.length;
|
||||
@@ -4039,7 +4055,7 @@ function renderMessages(){
|
||||
inner.innerHTML=cached.html;
|
||||
_sessionHtmlCacheSid=sid;
|
||||
_wireMessageWindowLoadEarlierButton();
|
||||
if(S.activeStreamId){scrollIfPinned();}else{scrollToBottom();}
|
||||
_scrollAfterMessageRender(preserveScroll);
|
||||
requestAnimationFrame(()=>{highlightCode();addCopyButtons();loadDiffInline();loadCsvInline();loadExcalidrawInline();loadPdfInline();loadHtmlInline();renderMermaidBlocks();renderKatexBlocks();});
|
||||
requestAnimationFrame(()=>{highlightCode();addCopyButtons();initTreeViews();loadPdfInline();loadHtmlInline();renderMermaidBlocks();renderKatexBlocks();});
|
||||
if(typeof _initMediaPlaybackObserver==='function') _initMediaPlaybackObserver();
|
||||
@@ -4574,11 +4590,7 @@ function renderMessages(){
|
||||
// Only force-scroll when not actively streaming — mid-stream re-renders
|
||||
// (tool completion, session switch) must not override the user's scroll position.
|
||||
// scrollIfPinned() respects _scrollPinned, so it's a no-op if user scrolled up.
|
||||
if(S.activeStreamId){
|
||||
scrollIfPinned();
|
||||
} else {
|
||||
scrollToBottom();
|
||||
}
|
||||
_scrollAfterMessageRender(preserveScroll);
|
||||
// Apply syntax highlighting after DOM is built
|
||||
requestAnimationFrame(()=>{highlightCode();addCopyButtons();loadDiffInline();loadCsvInline();loadExcalidrawInline();loadPdfInline();loadHtmlInline();renderMermaidBlocks();renderKatexBlocks();});
|
||||
requestAnimationFrame(()=>{highlightCode();addCopyButtons();initTreeViews();loadPdfInline();loadHtmlInline();renderMermaidBlocks();renderKatexBlocks();});
|
||||
|
||||
75
tests/test_issue1690_scroll_completion.py
Normal file
75
tests/test_issue1690_scroll_completion.py
Normal file
@@ -0,0 +1,75 @@
|
||||
from pathlib import Path
|
||||
|
||||
REPO = Path(__file__).resolve().parents[1]
|
||||
UI_JS = (REPO / "static" / "ui.js").read_text(encoding="utf-8")
|
||||
MESSAGES_JS = (REPO / "static" / "messages.js").read_text(encoding="utf-8")
|
||||
SESSIONS_JS = (REPO / "static" / "sessions.js").read_text(encoding="utf-8")
|
||||
|
||||
|
||||
def _function_body(src: str, name: str) -> str:
|
||||
start = src.index(f"function {name}")
|
||||
brace = src.index("{", start)
|
||||
depth = 0
|
||||
for i in range(brace, len(src)):
|
||||
if src[i] == "{":
|
||||
depth += 1
|
||||
elif src[i] == "}":
|
||||
depth -= 1
|
||||
if depth == 0:
|
||||
return src[start : i + 1]
|
||||
raise AssertionError(f"function {name} body not found")
|
||||
|
||||
|
||||
def _event_listener_body(src: str, event_name: str) -> str:
|
||||
needle = f"source.addEventListener('{event_name}'"
|
||||
start = src.index(needle)
|
||||
brace = src.index("{", start)
|
||||
depth = 0
|
||||
for i in range(brace, len(src)):
|
||||
if src[i] == "{":
|
||||
depth += 1
|
||||
elif src[i] == "}":
|
||||
depth -= 1
|
||||
if depth == 0:
|
||||
return src[start : i + 1]
|
||||
raise AssertionError(f"event listener {event_name!r} body not found")
|
||||
|
||||
|
||||
def test_terminal_done_render_preserves_manual_scroll_after_active_stream_is_cleared():
|
||||
done_block = _event_listener_body(MESSAGES_JS, "done")
|
||||
|
||||
clear_idx = done_block.index("S.activeStreamId=null")
|
||||
render_idx = done_block.index("renderMessages({preserveScroll:true})")
|
||||
|
||||
assert clear_idx < render_idx, (
|
||||
"the done handler should clear stream liveness before the final render, "
|
||||
"but must pass preserveScroll so renderMessages does not infer bottom-pin "
|
||||
"from S.activeStreamId alone"
|
||||
)
|
||||
|
||||
|
||||
def test_render_messages_preserve_scroll_option_uses_user_pin_state_not_stream_liveness():
|
||||
render_body = _function_body(UI_JS, "renderMessages")
|
||||
scroll_helper = _function_body(UI_JS, "_scrollAfterMessageRender")
|
||||
|
||||
assert "function renderMessages(options)" in render_body
|
||||
assert "const preserveScroll=!!(options&&options.preserveScroll);" in render_body
|
||||
assert "_scrollAfterMessageRender(preserveScroll);" in render_body
|
||||
assert "if(preserveScroll){\n scrollIfPinned();\n return;\n }" in scroll_helper
|
||||
assert "if(S.activeStreamId){\n scrollIfPinned();\n return;\n }" in scroll_helper
|
||||
|
||||
|
||||
def test_cached_render_path_uses_same_scroll_policy_as_fresh_render():
|
||||
render_body = _function_body(UI_JS, "renderMessages")
|
||||
cached_branch = render_body[render_body.index("if(sid&&sid!==_sessionHtmlCacheSid") : render_body.index("const compressionState=")]
|
||||
|
||||
assert "_scrollAfterMessageRender(preserveScroll);" in cached_branch
|
||||
assert "if(S.activeStreamId){scrollIfPinned();}else{scrollToBottom();}" not in cached_branch
|
||||
|
||||
|
||||
def test_session_switch_and_idle_session_load_keep_default_bottom_pin_behavior():
|
||||
load_session = _function_body(SESSIONS_JS, "loadSession")
|
||||
idle_branch = load_session[load_session.index("}else{\n S.busy=false;") : load_session.index("// Sync context usage indicator")]
|
||||
|
||||
assert "syncTopbar();renderMessages();" in idle_branch
|
||||
assert "preserveScroll:true" not in idle_branch
|
||||
@@ -25,18 +25,23 @@ class TestScrollPinningFix:
|
||||
instead when S.activeStreamId is set.
|
||||
"""
|
||||
# Find renderMessages function
|
||||
rm_start = UI_JS.find("function renderMessages()")
|
||||
rm_start = UI_JS.find("function renderMessages(")
|
||||
assert rm_start != -1, "renderMessages() not found in ui.js"
|
||||
rm_end = UI_JS.find("\nfunction ", rm_start + 1)
|
||||
rm_body = UI_JS[rm_start:rm_end]
|
||||
helper_start = UI_JS.find("function _scrollAfterMessageRender")
|
||||
assert helper_start != -1, "renderMessages scroll helper not found in ui.js"
|
||||
helper_end = UI_JS.find("\nfunction ", helper_start + 1)
|
||||
helper_body = UI_JS[helper_start:helper_end]
|
||||
|
||||
# Must check activeStreamId before deciding which scroll fn to call
|
||||
assert "activeStreamId" in rm_body, (
|
||||
assert "activeStreamId" in helper_body, (
|
||||
"renderMessages() must check S.activeStreamId before scrolling — "
|
||||
"unconditional scrollToBottom() overrides user scroll position (#677)"
|
||||
)
|
||||
# scrollIfPinned must be called inside renderMessages (stream path)
|
||||
assert "scrollIfPinned()" in rm_body, (
|
||||
# scrollIfPinned must be called through the renderMessages scroll policy (stream path)
|
||||
assert "_scrollAfterMessageRender(preserveScroll);" in rm_body
|
||||
assert "scrollIfPinned()" in helper_body, (
|
||||
"renderMessages() must call scrollIfPinned() during streaming (#677)"
|
||||
)
|
||||
|
||||
|
||||
@@ -24,7 +24,7 @@ def test_load_earlier_expands_local_window_before_server_pagination_and_preserve
|
||||
|
||||
|
||||
def test_windowed_render_keeps_streaming_and_tool_activity_anchored_to_rendered_messages():
|
||||
assert "if(S.activeStreamId){\n scrollIfPinned();" in UI_JS
|
||||
assert "_scrollAfterMessageRender(preserveScroll);" in UI_JS
|
||||
assert "const assistantIdxs=[...assistantSegments.keys()].sort((a,b)=>a-b);" in UI_JS
|
||||
assert "if(aIdx<assistantIdxs[0]) continue;" in UI_JS
|
||||
assert "const renderedAssistantIdxs=[...assistantSegments.keys()].sort((a,b)=>a-b);" in UI_JS
|
||||
|
||||
@@ -302,7 +302,7 @@ def test_hidden_active_done_still_updates_current_pane_but_not_read_state():
|
||||
viewed_const_idx = done_block.find("const isSessionViewed=_isSessionActivelyViewed(activeSid);")
|
||||
active_guard_idx = done_block.find("if(isActiveSession){", viewed_const_idx)
|
||||
session_update_idx = done_block.find("S.session=d.session", active_guard_idx)
|
||||
render_idx = done_block.find("renderMessages()", active_guard_idx)
|
||||
render_idx = done_block.find("renderMessages(", active_guard_idx)
|
||||
load_dir_idx = done_block.find("loadDir('.')", active_guard_idx)
|
||||
mark_viewed_idx = done_block.find("if(isSessionViewed) _markSessionViewed(completedSid", active_guard_idx)
|
||||
|
||||
|
||||
@@ -436,11 +436,12 @@ def test_done_handler_sets_busy_false_before_renderMessages(cleanup_test_session
|
||||
stream_end_idx = src.find("source.addEventListener('stream_end'", done_idx)
|
||||
assert stream_end_idx >= 0, "stream_end listener after done handler not found"
|
||||
done_block = src[done_idx:stream_end_idx]
|
||||
# S.busy=false must appear before renderMessages() within the done handler
|
||||
# S.busy=false must appear before the terminal render call within the done handler.
|
||||
busy_pos = done_block.find("S.busy=false;")
|
||||
render_pos = done_block.find("renderMessages()")
|
||||
render_pos = done_block.find("renderMessages(")
|
||||
assert busy_pos >= 0, "done handler must set S.busy=false before renderMessages()"
|
||||
assert busy_pos < render_pos, f"S.busy=false (pos {busy_pos}) must come before renderMessages() (pos {render_pos})"
|
||||
assert render_pos >= 0, "done handler must call renderMessages after settling state"
|
||||
assert busy_pos < render_pos, f"S.busy=false (pos {busy_pos}) must come before renderMessages (pos {render_pos})"
|
||||
|
||||
|
||||
# ── R14: send() uses stale modelSelect.value instead of session model ────────
|
||||
|
||||
Reference in New Issue
Block a user