mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-09 08:21:50 +00:00
perf(desktop): fix "Enter jumps up" on long threads
User reported: after pressing Enter on a long thread, the view jumps up — the just-submitted message disappears below the fold. Confirmed via apps/desktop/scripts/measure-jump.mjs: before: distFromBottom 0 → 49.5px, sticks there permanently after: distFromBottom 0 → ~0 (worst case 4px for one frame) Root cause in useThreadScrollAnchor (thread-virtualizer.tsx): 1. The sticky-bottom logic disarmed on any scroll event where `scrollTop < lastTopRef.current`. That check can't distinguish a user scrolling up from a programmatic `pinToBottom` write that the browser clamped short of bottom (because content also grew in the same frame, so `scrollTop = scrollHeight` lands at `scrollHeight - clientHeight` for the OLD scrollHeight, which is now below the NEW scrollHeight). Result: sticky-bottom disarmed permanently on the user's first submit. 2. There was no synchronous pin tied to React's commit phase. By the time the ResizeObserver fired and re-pinned, the user had already seen ~50ms of "message below the fold" — visually that reads as the view jumping up. Fix: - `programmaticScrollPendingRef` counter tracks scroll events we expect to be ours (one per `pinToBottom` write). The scroll handler skips the disarm check when consuming a pending tick, keeps the arm bit true, and re-pins synchronously if the browser clamped us short of bottom. A depth cap (8) breaks runaway loops in pathological streaming-burst layouts. - `useLayoutEffect` on `groupCount` increase pins BEFORE the browser paints, eliminating the visible ~50ms window between optimistic user-message insert and the RO/scroll-event chain firing. Verified on the long Cloud Shadows thread (7-8 turns, ~11k px tall): all three repro runs now hold within 0–4 px of bottom across the post-Enter transition. Submit latency unchanged (paint 77–107 ms), streaming-typing latency unchanged. Also adds three debug harnesses: - measure-jump.mjs — sample thread scroll across Enter - probe-thread.mjs — dump current thread / scroll state - diag-jump.mjs — intercept scrollTop + RO + mutations across Enter
This commit is contained in:
parent
e18c233c1e
commit
a7e6a4fc0b
4 changed files with 339 additions and 1 deletions
115
apps/desktop/scripts/diag-jump.mjs
Normal file
115
apps/desktop/scripts/diag-jump.mjs
Normal file
|
|
@ -0,0 +1,115 @@
|
|||
// Wrap the thread scroller's properties and observe pin/scroll/RO events
|
||||
// in real time during a submit, then print the timeline.
|
||||
const list = await (await fetch('http://127.0.0.1:9222/json/list')).json()
|
||||
const tgt = list.find(t => t.type === 'page' && t.url.startsWith('http'))
|
||||
const ws = new WebSocket(tgt.webSocketDebuggerUrl)
|
||||
let id = 0
|
||||
const pending = new Map()
|
||||
ws.addEventListener('message', ev => {
|
||||
const m = JSON.parse(ev.data)
|
||||
if (m.id != null && pending.has(m.id)) {
|
||||
pending.get(m.id)(m)
|
||||
pending.delete(m.id)
|
||||
}
|
||||
})
|
||||
await new Promise(r => ws.addEventListener('open', r))
|
||||
const send = (m, p = {}) =>
|
||||
new Promise(r => {
|
||||
const i = ++id
|
||||
pending.set(i, r)
|
||||
ws.send(JSON.stringify({ id: i, method: m, params: p }))
|
||||
})
|
||||
const evalP = async expr => {
|
||||
const r = await send('Runtime.evaluate', { expression: expr, returnByValue: true })
|
||||
if (r.result?.exceptionDetails) throw new Error(r.result.exceptionDetails.text)
|
||||
return r.result.result.value
|
||||
}
|
||||
|
||||
await evalP(`(() => {
|
||||
const v = document.querySelector('[data-slot="aui_thread-viewport"]')
|
||||
if (v) v.scrollTop = v.scrollHeight
|
||||
})()`)
|
||||
await new Promise(r => setTimeout(r, 300))
|
||||
|
||||
await evalP(`(() => {
|
||||
const el = document.querySelector('[data-slot="composer-rich-input"]')
|
||||
el.focus()
|
||||
const r = document.createRange(); r.selectNodeContents(el); r.collapse(false)
|
||||
window.getSelection().removeAllRanges(); window.getSelection().addRange(r)
|
||||
})()`)
|
||||
|
||||
const text = 'short follow-up'
|
||||
for (const c of text) {
|
||||
await send('Input.dispatchKeyEvent', { type: 'char', text: c, unmodifiedText: c })
|
||||
await new Promise(r => setTimeout(r, 10))
|
||||
}
|
||||
await new Promise(r => setTimeout(r, 300))
|
||||
|
||||
// Hook into the viewport scrollTop setter + scroll + RO so we see every event
|
||||
await evalP(`(() => {
|
||||
const v = document.querySelector('[data-slot="aui_thread-viewport"]')
|
||||
const events = []
|
||||
window.__threadEvents = events
|
||||
const t0 = performance.now()
|
||||
const push = (kind, detail) => events.push({ t: performance.now() - t0, kind, ...detail })
|
||||
|
||||
// intercept scrollTop writes
|
||||
const desc = Object.getOwnPropertyDescriptor(Element.prototype, 'scrollTop')
|
||||
Object.defineProperty(v, 'scrollTop', {
|
||||
get() { return desc.get.call(this) },
|
||||
set(val) {
|
||||
push('scrollTop=', { val, fromScrollHeight: this.scrollHeight, stackTop: (new Error()).stack.split('\\n').slice(2, 5).map(s => s.trim()).join(' | ') })
|
||||
desc.set.call(this, val)
|
||||
},
|
||||
configurable: true
|
||||
})
|
||||
|
||||
// scroll event
|
||||
v.addEventListener('scroll', () => {
|
||||
push('scroll', { scrollTop: v.scrollTop, scrollHeight: v.scrollHeight })
|
||||
}, { passive: true, capture: true })
|
||||
|
||||
// RO on the viewport itself
|
||||
const ro = new ResizeObserver((entries) => {
|
||||
for (const e of entries) {
|
||||
push('RO', { target: e.target.getAttribute('data-slot') || e.target.tagName, h: e.contentRect.height })
|
||||
}
|
||||
})
|
||||
ro.observe(v)
|
||||
if (v.firstElementChild) ro.observe(v.firstElementChild)
|
||||
|
||||
// mutationobserver on the viewport
|
||||
const mo = new MutationObserver((muts) => {
|
||||
push('mut', { count: muts.length, added: muts.reduce((s, m) => s + m.addedNodes.length, 0), removed: muts.reduce((s, m) => s + m.removedNodes.length, 0) })
|
||||
})
|
||||
mo.observe(v, { childList: true, subtree: true, characterData: true })
|
||||
|
||||
window.__teardown = () => { ro.disconnect(); mo.disconnect() }
|
||||
return true
|
||||
})()`)
|
||||
|
||||
// fire Enter
|
||||
await send('Input.dispatchKeyEvent', {
|
||||
type: 'rawKeyDown', windowsVirtualKeyCode: 13, key: 'Enter', code: 'Enter', text: '\r', unmodifiedText: '\r'
|
||||
})
|
||||
await send('Input.dispatchKeyEvent', { type: 'keyUp', windowsVirtualKeyCode: 13, key: 'Enter', code: 'Enter' })
|
||||
|
||||
await new Promise(r => setTimeout(r, 1200))
|
||||
|
||||
const events = JSON.parse(await evalP(`JSON.stringify(window.__threadEvents || [])`))
|
||||
console.log(`\n${events.length} events:`)
|
||||
for (const e of events) {
|
||||
const t = String(e.t.toFixed(0)).padStart(5)
|
||||
const { kind, t: _t, ...rest } = e
|
||||
console.log(` ${t}ms ${kind.padEnd(12)} ${JSON.stringify(rest)}`)
|
||||
}
|
||||
|
||||
await evalP(`window.__teardown?.()`)
|
||||
// Cancel running agent
|
||||
await evalP(`(() => {
|
||||
for (const b of document.querySelectorAll('button')) {
|
||||
if ((b.getAttribute('aria-label') || '').toLowerCase().includes('stop')) { b.click(); return 'stopped' }
|
||||
}
|
||||
})()`)
|
||||
|
||||
ws.close()
|
||||
108
apps/desktop/scripts/measure-jump.mjs
Normal file
108
apps/desktop/scripts/measure-jump.mjs
Normal file
|
|
@ -0,0 +1,108 @@
|
|||
// Measure scroll position before and after Enter on a long thread.
|
||||
// The user's complaint: pressing Enter to submit makes the view "jump up".
|
||||
//
|
||||
// Steps:
|
||||
// 1. Scroll to the bottom of the thread
|
||||
// 2. Type a short message
|
||||
// 3. Record scroll position
|
||||
// 4. Hit Enter
|
||||
// 5. Record scroll position every 10ms for 1.5s after Enter
|
||||
// 6. Report deltas
|
||||
//
|
||||
// Usage: node apps/desktop/scripts/measure-jump.mjs
|
||||
|
||||
const list = await (await fetch('http://127.0.0.1:9222/json/list')).json()
|
||||
const tgt = list.find(t => t.type === 'page' && t.url.startsWith('http'))
|
||||
const ws = new WebSocket(tgt.webSocketDebuggerUrl)
|
||||
let id = 0
|
||||
const pending = new Map()
|
||||
ws.addEventListener('message', ev => {
|
||||
const m = JSON.parse(ev.data)
|
||||
if (m.id != null && pending.has(m.id)) {
|
||||
pending.get(m.id)(m)
|
||||
pending.delete(m.id)
|
||||
}
|
||||
})
|
||||
await new Promise(r => ws.addEventListener('open', r))
|
||||
const send = (m, p = {}) =>
|
||||
new Promise(r => {
|
||||
const i = ++id
|
||||
pending.set(i, r)
|
||||
ws.send(JSON.stringify({ id: i, method: m, params: p }))
|
||||
})
|
||||
const evalP = async expr => {
|
||||
const r = await send('Runtime.evaluate', { expression: expr, returnByValue: true })
|
||||
if (r.result?.exceptionDetails) throw new Error(r.result.exceptionDetails.text)
|
||||
return r.result.result.value
|
||||
}
|
||||
|
||||
// Scroll to bottom
|
||||
await evalP(`(() => {
|
||||
const v = document.querySelector('[data-slot="aui_thread-viewport"]')
|
||||
if (v) v.scrollTop = v.scrollHeight
|
||||
})()`)
|
||||
await new Promise(r => setTimeout(r, 300))
|
||||
|
||||
// Focus composer and type
|
||||
await evalP(`(() => {
|
||||
const el = document.querySelector('[data-slot="composer-rich-input"]')
|
||||
el.focus()
|
||||
const r = document.createRange(); r.selectNodeContents(el); r.collapse(false)
|
||||
window.getSelection().removeAllRanges(); window.getSelection().addRange(r)
|
||||
})()`)
|
||||
|
||||
const text = 'short follow-up message'
|
||||
for (const c of text) {
|
||||
await send('Input.dispatchKeyEvent', { type: 'char', text: c, unmodifiedText: c })
|
||||
await new Promise(r => setTimeout(r, 10))
|
||||
}
|
||||
await new Promise(r => setTimeout(r, 300))
|
||||
|
||||
// Set up sampling — sample scroll position every animation frame
|
||||
await evalP(`(() => {
|
||||
const v = document.querySelector('[data-slot="aui_thread-viewport"]')
|
||||
window.__jumpSamples = []
|
||||
window.__jumpStart = performance.now()
|
||||
const tick = () => {
|
||||
if (!v) return
|
||||
window.__jumpSamples.push({
|
||||
t: performance.now() - window.__jumpStart,
|
||||
scrollTop: v.scrollTop,
|
||||
scrollHeight: v.scrollHeight,
|
||||
clientHeight: v.clientHeight,
|
||||
distFromBottom: v.scrollHeight - v.scrollTop - v.clientHeight
|
||||
})
|
||||
if (performance.now() - window.__jumpStart < 2000) {
|
||||
requestAnimationFrame(tick)
|
||||
}
|
||||
}
|
||||
requestAnimationFrame(tick)
|
||||
})()`)
|
||||
|
||||
// Fire Enter
|
||||
await send('Input.dispatchKeyEvent', {
|
||||
type: 'rawKeyDown', windowsVirtualKeyCode: 13, key: 'Enter', code: 'Enter', text: '\r', unmodifiedText: '\r'
|
||||
})
|
||||
await send('Input.dispatchKeyEvent', { type: 'keyUp', windowsVirtualKeyCode: 13, key: 'Enter', code: 'Enter' })
|
||||
|
||||
await new Promise(r => setTimeout(r, 2200))
|
||||
|
||||
const samples = JSON.parse(await evalP(`JSON.stringify(window.__jumpSamples || [])`))
|
||||
console.log(`\n${samples.length} samples over 2s`)
|
||||
console.log(`\n t(ms) scrollTop scrollHeight clientHeight distFromBottom`)
|
||||
let prev = null
|
||||
for (const s of samples) {
|
||||
const marker = prev && Math.abs(s.scrollTop - prev.scrollTop) > 5 ? ' ← jump' : ''
|
||||
console.log(` ${String(s.t.toFixed(0)).padStart(5)} ${String(s.scrollTop).padStart(9)} ${String(s.scrollHeight).padStart(12)} ${String(s.clientHeight).padStart(12)} ${String(s.distFromBottom).padStart(14)}${marker}`)
|
||||
prev = s
|
||||
}
|
||||
|
||||
// Cancel any running agent
|
||||
await evalP(`(() => {
|
||||
for (const b of document.querySelectorAll('button')) {
|
||||
if ((b.getAttribute('aria-label') || '').toLowerCase().includes('stop')) { b.click(); return 'stopped' }
|
||||
}
|
||||
return 'no-stop'
|
||||
})()`).then(r => console.log('\ncancel:', r))
|
||||
|
||||
ws.close()
|
||||
40
apps/desktop/scripts/probe-thread.mjs
Normal file
40
apps/desktop/scripts/probe-thread.mjs
Normal file
|
|
@ -0,0 +1,40 @@
|
|||
// Probe the cloud shadows thread state — count messages, turn pairs,
|
||||
// thread height, composer state
|
||||
const list = await (await fetch('http://127.0.0.1:9222/json/list')).json()
|
||||
const tgt = list.find(t => t.type === 'page' && t.url.startsWith('http'))
|
||||
const ws = new WebSocket(tgt.webSocketDebuggerUrl)
|
||||
let id = 0
|
||||
const pending = new Map()
|
||||
ws.addEventListener('message', ev => {
|
||||
const m = JSON.parse(ev.data)
|
||||
if (m.id != null && pending.has(m.id)) {
|
||||
pending.get(m.id)(m)
|
||||
pending.delete(m.id)
|
||||
}
|
||||
})
|
||||
await new Promise(r => ws.addEventListener('open', r))
|
||||
const send = (m, p = {}) =>
|
||||
new Promise(r => {
|
||||
const i = ++id
|
||||
pending.set(i, r)
|
||||
ws.send(JSON.stringify({ id: i, method: m, params: p }))
|
||||
})
|
||||
|
||||
const r = await send('Runtime.evaluate', {
|
||||
expression: `JSON.stringify({
|
||||
url: location.href,
|
||||
title: document.title,
|
||||
turnPairs: document.querySelectorAll('[data-slot="aui_turn-pair"]').length,
|
||||
assistantMsgs: document.querySelectorAll('[data-slot="aui_assistant-message-root"]').length,
|
||||
userMsgs: document.querySelectorAll('[data-message-role="user"], [data-slot="aui_user-message-root"]').length,
|
||||
totalDomNodes: document.querySelectorAll('*').length,
|
||||
threadViewportScrollHeight: document.querySelector('[data-slot="aui_thread-viewport"]')?.scrollHeight ?? null,
|
||||
threadViewportClientHeight: document.querySelector('[data-slot="aui_thread-viewport"]')?.clientHeight ?? null,
|
||||
threadViewportScrollTop: document.querySelector('[data-slot="aui_thread-viewport"]')?.scrollTop ?? null,
|
||||
composer: !!document.querySelector('[data-slot="composer-rich-input"]'),
|
||||
busy: !!document.querySelector('[aria-label*="Stop"]')
|
||||
})`,
|
||||
returnByValue: true
|
||||
})
|
||||
console.log(JSON.parse(r.result.result.value))
|
||||
ws.close()
|
||||
|
|
@ -1,6 +1,6 @@
|
|||
import { ThreadPrimitive, useAuiEvent, useAuiState } from '@assistant-ui/react'
|
||||
import { useVirtualizer, type Virtualizer } from '@tanstack/react-virtual'
|
||||
import { type ComponentProps, type FC, type ReactNode, useCallback, useEffect, useMemo, useRef } from 'react'
|
||||
import { type ComponentProps, type FC, type ReactNode, useCallback, useEffect, useLayoutEffect, useMemo, useRef } from 'react'
|
||||
|
||||
import { cn } from '@/lib/utils'
|
||||
import { setThreadScrolledUp } from '@/store/thread-scroll'
|
||||
|
|
@ -182,9 +182,25 @@ function useThreadScrollAnchor({ enabled, groupCount, scrollerRef, sessionKey, v
|
|||
// user-driven upward scroll; re-armed when they reach bottom again.
|
||||
const armedRef = useRef(true)
|
||||
const lastTopRef = useRef(0)
|
||||
// Counter that tracks how many scroll events we expect to be ours rather
|
||||
// than the user's. `pinToBottom` writes `el.scrollTop`, which fires an
|
||||
// async `scroll` event; without this guard the on-scroll handler can race
|
||||
// with the programmatic write (because content also grew, the *resulting*
|
||||
// scrollTop can be lower than `lastTopRef` from the previous frame) and
|
||||
// misread the programmatic pin as the user scrolling up — which disarms
|
||||
// sticky-bottom and the user's just-submitted message slides above the
|
||||
// fold. See `apps/desktop/scripts/measure-jump.mjs` for the repro
|
||||
// (distFromBottom 0 → 49 within one frame, sticking forever).
|
||||
const programmaticScrollPendingRef = useRef(0)
|
||||
const prevSessionKeyRef = useRef(sessionKey)
|
||||
const prevGroupCountRef = useRef(0)
|
||||
|
||||
// Track repins-in-a-row to break runaway loops during rapid layout churn.
|
||||
// In healthy paths this drains to zero between frames; we only need the
|
||||
// ceiling for pathological streaming bursts where content height keeps
|
||||
// growing every frame.
|
||||
const inFlightPinDepthRef = useRef(0)
|
||||
|
||||
const pinToBottom = useCallback(() => {
|
||||
const el = scrollerRef.current
|
||||
|
||||
|
|
@ -192,6 +208,8 @@ function useThreadScrollAnchor({ enabled, groupCount, scrollerRef, sessionKey, v
|
|||
return
|
||||
}
|
||||
|
||||
// Hold the disarm gate across the scroll event the next line will fire.
|
||||
programmaticScrollPendingRef.current += 1
|
||||
el.scrollTop = el.scrollHeight
|
||||
lastTopRef.current = el.scrollTop
|
||||
}, [scrollerRef])
|
||||
|
|
@ -228,6 +246,45 @@ function useThreadScrollAnchor({ enabled, groupCount, scrollerRef, sessionKey, v
|
|||
const onScroll = () => {
|
||||
const top = el.scrollTop
|
||||
|
||||
// If this scroll event is the consequence of `pinToBottom` writing
|
||||
// `el.scrollTop`, treat it as ours: never disarm, just consume the
|
||||
// gate. If we landed short of bottom (because content also grew in
|
||||
// the same frame and the browser clamped our scrollTop = scrollHeight
|
||||
// write to the now-stale scrollHeight - clientHeight), schedule
|
||||
// another pin on the next frame. Without this the post-pin scrollTop
|
||||
// gets misread as the user scrolling up, disarming sticky-bottom
|
||||
// permanently and leaving the just-submitted message below the fold.
|
||||
if (programmaticScrollPendingRef.current > 0) {
|
||||
programmaticScrollPendingRef.current -= 1
|
||||
lastTopRef.current = top
|
||||
// Stay armed regardless — sticky-bottom should hold through clamp
|
||||
// races.
|
||||
armedRef.current = true
|
||||
const atBottom = el.scrollHeight - (top + el.clientHeight) <= AT_BOTTOM_THRESHOLD
|
||||
setThreadScrolledUp(!atBottom)
|
||||
|
||||
if (atBottom) {
|
||||
inFlightPinDepthRef.current = 0
|
||||
} else if (inFlightPinDepthRef.current < 8) {
|
||||
// Re-pin synchronously: the browser already laid out for this
|
||||
// scroll event, so reading scrollHeight now gives us the up-to-date
|
||||
// value and writing scrollTop lands us at the actual bottom in the
|
||||
// same frame. Doing this in a rAF causes a 1-frame visual flicker
|
||||
// (distFromBottom briefly nonzero), so we accept one extra
|
||||
// synchronous pin cycle (which goes back through this very
|
||||
// handler with the counter incremented and arm preserved). The
|
||||
// depth guard prevents pathological runaway loops if content
|
||||
// height keeps growing every frame; 8 is generous for any
|
||||
// realistic rendering pattern.
|
||||
inFlightPinDepthRef.current += 1
|
||||
pinToBottom()
|
||||
} else {
|
||||
inFlightPinDepthRef.current = 0
|
||||
}
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
if (top + 1 < lastTopRef.current) {
|
||||
armedRef.current = false
|
||||
}
|
||||
|
|
@ -302,5 +359,23 @@ function useThreadScrollAnchor({ enabled, groupCount, scrollerRef, sessionKey, v
|
|||
}
|
||||
}, [enabled, groupCount, jumpToBottom, sessionKey])
|
||||
|
||||
// Pre-paint pin: when groupCount increases while armed (optimistic user
|
||||
// message insert, streaming assistant turn arriving, etc.), pin BEFORE
|
||||
// the browser commits the layout to screen. Using useLayoutEffect rather
|
||||
// than useEffect so this runs synchronously after React commits the DOM
|
||||
// mutation but before the browser paints. Without this, there's a ~50ms
|
||||
// visual window where the new message sits below the fold while we wait
|
||||
// for the ResizeObserver / scroll event chain to fire and re-pin.
|
||||
const prevGroupCountForLayoutRef = useRef(groupCount)
|
||||
useLayoutEffect(() => {
|
||||
if (!enabled) {
|
||||
return
|
||||
}
|
||||
if (groupCount > prevGroupCountForLayoutRef.current && armedRef.current) {
|
||||
pinToBottom()
|
||||
}
|
||||
prevGroupCountForLayoutRef.current = groupCount
|
||||
}, [enabled, groupCount, pinToBottom])
|
||||
|
||||
useAuiEvent('thread.runStart', jumpToBottom)
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue