mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-26 17:38:36 +00:00
fix(desktop): flush stream deltas from a timer, not an animation frame
Once the coalescing floor had already elapsed, the next delta was scheduled with requestAnimationFrame. Chromium pauses rAF for a renderer it considers hidden, and that is not something this code can verify: backgroundThrottling: false and the process-level switches in electron/main.ts cover the blurred and occluded cases, but not a minimized window, a fully off-screen one, or a renderer the compositor has otherwise parked. In those states the callback is accepted and never runs, so a finished answer sits in the queue until some later focus or input event happens to wake a frame — the reply looks stalled, then lands all at once on refocus. Always use a timer. The coalescing cadence is unchanged (the floor above is what enforces it), timers are clamped rather than suspended in background renderers, and disable-background-timer-throttling already opts out of that clamp. The teardown path loses its cancelAnimationFrame branch with it. The regression test parks rAF the way an occluded renderer does and asserts the delta still arrives. It has to send a delta, let it flush, then idle past the floor before the delta under test, because the frame-gated branch was only reachable on that second scheduling — a single-delta version of this test passes on main and proves nothing. Co-authored-by: NetRunner2037 <rerdi92@users.noreply.github.com>
This commit is contained in:
parent
adf47ca832
commit
ad09bf3872
2 changed files with 108 additions and 11 deletions
|
|
@ -251,12 +251,20 @@ export function useMessageStream({
|
|||
flushQueuedDeltas()
|
||||
}
|
||||
|
||||
if (sinceLast >= STREAM_DELTA_FLUSH_MS && typeof window.requestAnimationFrame === 'function') {
|
||||
flushHandleRef.current = window.requestAnimationFrame(runFlush)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
// Always a timer, never requestAnimationFrame. Chromium pauses rAF for a
|
||||
// renderer it considers hidden, and "hidden" is not something this code can
|
||||
// verify: `backgroundThrottling: false` plus the process-level switches in
|
||||
// electron/main.ts cover the blurred and occluded cases, but they don't
|
||||
// cover a minimized window, a fully off-screen one, or a renderer the
|
||||
// compositor has otherwise parked. In those states an rAF-gated flush never
|
||||
// runs, so a finished answer sits in this queue until some later input or
|
||||
// focus event happens to wake a frame — the reply looks stalled, then
|
||||
// arrives all at once on refocus.
|
||||
//
|
||||
// A timer keeps the same coalescing cadence (that's what the floor above is
|
||||
// for) while guaranteeing delivery without user interaction. Timers are
|
||||
// clamped in background renderers rather than suspended, and
|
||||
// disable-background-timer-throttling already opts out of that clamp.
|
||||
flushHandleRef.current = window.setTimeout(runFlush, Math.max(0, STREAM_DELTA_FLUSH_MS - sinceLast))
|
||||
}, [flushQueuedDeltas])
|
||||
|
||||
|
|
@ -277,11 +285,7 @@ export function useMessageStream({
|
|||
useEffect(
|
||||
() => () => {
|
||||
if (flushHandleRef.current !== null && typeof window !== 'undefined') {
|
||||
if (typeof window.cancelAnimationFrame === 'function') {
|
||||
window.cancelAnimationFrame(flushHandleRef.current)
|
||||
} else {
|
||||
window.clearTimeout(flushHandleRef.current)
|
||||
}
|
||||
window.clearTimeout(flushHandleRef.current)
|
||||
}
|
||||
|
||||
flushHandleRef.current = null
|
||||
|
|
|
|||
|
|
@ -0,0 +1,93 @@
|
|||
import { QueryClient } from '@tanstack/react-query'
|
||||
import { act, cleanup, render } from '@testing-library/react'
|
||||
import { useEffect, useRef } from 'react'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import type { ClientSessionState } from '@/app/types'
|
||||
import { createClientSessionState } from '@/lib/chat-runtime'
|
||||
import type { RpcEvent } from '@/types/hermes'
|
||||
|
||||
import { STREAM_DELTA_FLUSH_MS } from './utils'
|
||||
|
||||
import { useMessageStream } from './index'
|
||||
|
||||
const SID = 'stream-session'
|
||||
|
||||
let handleEvent: ((event: RpcEvent) => void) | null = null
|
||||
let states: Map<string, ClientSessionState>
|
||||
|
||||
function Harness() {
|
||||
const activeSessionIdRef = useRef<string | null>(SID)
|
||||
const sessionStateByRuntimeIdRef = useRef(new Map<string, ClientSessionState>())
|
||||
const queryClientRef = useRef(new QueryClient())
|
||||
|
||||
const stream = useMessageStream({
|
||||
activeSessionIdRef,
|
||||
hydrateFromStoredSession: vi.fn(async () => undefined),
|
||||
queryClient: queryClientRef.current,
|
||||
refreshHermesConfig: vi.fn(async () => undefined),
|
||||
refreshSessions: vi.fn(async () => undefined),
|
||||
sessionStateByRuntimeIdRef,
|
||||
updateSessionState: (sessionId, updater) => {
|
||||
const current = sessionStateByRuntimeIdRef.current.get(sessionId) ?? createClientSessionState()
|
||||
const next = updater(current)
|
||||
sessionStateByRuntimeIdRef.current.set(sessionId, next)
|
||||
|
||||
return next
|
||||
}
|
||||
})
|
||||
|
||||
useEffect(() => {
|
||||
handleEvent = stream.handleGatewayEvent
|
||||
states = sessionStateByRuntimeIdRef.current
|
||||
}, [stream.handleGatewayEvent])
|
||||
|
||||
return null
|
||||
}
|
||||
|
||||
describe('stream delta delivery', () => {
|
||||
beforeEach(() => {
|
||||
handleEvent = null
|
||||
states = new Map()
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
cleanup()
|
||||
vi.useRealTimers()
|
||||
vi.restoreAllMocks()
|
||||
})
|
||||
|
||||
it('delivers a delta while animation frames are parked', async () => {
|
||||
// An Electron renderer the compositor has parked: rAF accepts the callback
|
||||
// but never runs it. A frame-gated flush strands the answer here until some
|
||||
// later focus/input event wakes a frame.
|
||||
const rafSpy = vi.spyOn(window, 'requestAnimationFrame').mockImplementation(() => 1)
|
||||
vi.useFakeTimers()
|
||||
|
||||
render(<Harness />)
|
||||
await act(async () => {
|
||||
await Promise.resolve()
|
||||
})
|
||||
|
||||
// Reach the frame-gated branch: it is only taken once the coalescing floor
|
||||
// has already elapsed since the previous flush. Send one delta, let it
|
||||
// flush, then idle past the floor so the NEXT delta schedules immediately.
|
||||
act(() => handleEvent?.({ payload: { text: 'first ' }, session_id: SID, type: 'message.delta' }))
|
||||
await act(async () => {
|
||||
await vi.advanceTimersByTimeAsync(STREAM_DELTA_FLUSH_MS)
|
||||
})
|
||||
await act(async () => {
|
||||
await vi.advanceTimersByTimeAsync(STREAM_DELTA_FLUSH_MS * 2)
|
||||
})
|
||||
|
||||
act(() => handleEvent?.({ payload: { text: 'and the rest' }, session_id: SID, type: 'message.delta' }))
|
||||
|
||||
await act(async () => {
|
||||
await vi.advanceTimersByTimeAsync(STREAM_DELTA_FLUSH_MS)
|
||||
})
|
||||
|
||||
expect(states.get(SID)?.messages.at(-1)?.parts).toEqual([{ type: 'text', text: 'first and the rest' }])
|
||||
// The flush must not have depended on a frame at all.
|
||||
expect(rafSpy).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
Loading…
Add table
Add a link
Reference in a new issue