fix: restore the jump-to-bottom glide and keep user state, selections and scroll anchors in windowed lists
Pause scroll writes while the jump-to-bottom smooth scroll runs; keep rows with pending questions, approvals, recent interactions and selection ends mounted; cache Mermaid results; copy text spanning unmounted rows; anchor the sidebar on id changes; disable native overflow-anchor for windowed lists; write terminal output on the leading edge and before clear; forward remote events every 16 ms; flush streaming text before copy/export; add performance principle P10 (no UX regressions).
This commit is contained in:
@@ -3988,6 +3988,9 @@ export function registerIpcHandlers(
|
||||
let remoteEventBatchClosed = false
|
||||
const remoteEventBatcher = new RemoteEventBatcher<RemoteBatchEvent>({
|
||||
onError: (error) => controller.abort(error),
|
||||
// The batch already coalesced a frame of deltas: send them now rather
|
||||
// than holding them for another public-buffer interval.
|
||||
onTimerFlushed: () => publicEventBuffer.flush(),
|
||||
persist: (entries) =>
|
||||
remoteConversationRecovery
|
||||
? assistantDatabase.appendRemoteConversationTaskEventsBatch({
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
import { RemoteEventBatcher } from './remote-event-batcher'
|
||||
import { REMOTE_EVENT_BATCH_FLUSH_INTERVAL_MS, RemoteEventBatcher } from './remote-event-batcher'
|
||||
|
||||
const provenance = (eventIndex: number) => ({
|
||||
source: 'remote-semantic-transcript' as const,
|
||||
@@ -46,6 +46,49 @@ describe('RemoteEventBatcher', () => {
|
||||
batcher.dispose()
|
||||
})
|
||||
|
||||
it('commits (and so forwards) streamed events within one frame by default', () => {
|
||||
vi.useFakeTimers()
|
||||
expect(REMOTE_EVENT_BATCH_FLUSH_INTERVAL_MS).toBe(16)
|
||||
const persist = vi.fn((entries: readonly unknown[]) => entries.map(() => true))
|
||||
const committed: string[] = []
|
||||
const batcher = new RemoteEventBatcher<string>({ persist })
|
||||
batcher.add(provenance(0), 'a', (event) => committed.push(event))
|
||||
batcher.add(provenance(1), 'b', (event) => committed.push(event))
|
||||
vi.advanceTimersByTime(15)
|
||||
// Nothing is forwarded before it is persisted.
|
||||
expect(committed).toEqual([])
|
||||
vi.advanceTimersByTime(1)
|
||||
expect(persist).toHaveBeenCalledOnce()
|
||||
expect(committed).toEqual(['a', 'b'])
|
||||
batcher.dispose()
|
||||
})
|
||||
|
||||
it('reports a timer flush after its events committed, and not after a failed one', () => {
|
||||
vi.useFakeTimers()
|
||||
const order: string[] = []
|
||||
let fail = false
|
||||
const batcher = new RemoteEventBatcher<string>({
|
||||
persist: (entries) => {
|
||||
if (fail) throw new Error('disk full')
|
||||
return entries.map(() => true)
|
||||
},
|
||||
onTimerFlushed: () => order.push('flushed'),
|
||||
onError: () => order.push('error')
|
||||
})
|
||||
batcher.add(provenance(0), 'a', (event) => order.push(event))
|
||||
vi.advanceTimersByTime(REMOTE_EVENT_BATCH_FLUSH_INTERVAL_MS)
|
||||
expect(order).toEqual(['a', 'flushed'])
|
||||
// Explicit flushes leave forwarding to the caller.
|
||||
batcher.add(provenance(1), 'b', (event) => order.push(event))
|
||||
batcher.flush()
|
||||
expect(order).toEqual(['a', 'flushed', 'b'])
|
||||
fail = true
|
||||
batcher.add(provenance(2), 'c', (event) => order.push(event))
|
||||
vi.advanceTimersByTime(REMOTE_EVENT_BATCH_FLUSH_INTERVAL_MS)
|
||||
expect(order).toEqual(['a', 'flushed', 'b', 'error'])
|
||||
batcher.dispose()
|
||||
})
|
||||
|
||||
it('rethrows a failed commit and stays failed so later checkpoints are never committed', () => {
|
||||
vi.useFakeTimers()
|
||||
const onError = vi.fn()
|
||||
@@ -55,7 +98,7 @@ describe('RemoteEventBatcher', () => {
|
||||
const onCommitted = vi.fn()
|
||||
const batcher = new RemoteEventBatcher<string>({ persist, onError })
|
||||
batcher.add(provenance(0), 'a', onCommitted)
|
||||
vi.advanceTimersByTime(50)
|
||||
vi.advanceTimersByTime(REMOTE_EVENT_BATCH_FLUSH_INTERVAL_MS)
|
||||
expect(onError).toHaveBeenCalledWith(new Error('disk full'))
|
||||
expect(() => batcher.add(provenance(1), 'checkpoint')).toThrow('disk full')
|
||||
expect(() => batcher.flush()).toThrow('disk full')
|
||||
|
||||
@@ -18,6 +18,12 @@ export type RemoteEventBatcherOptions<TEvent> = {
|
||||
persist(entries: readonly RemoteEventBatchEntry<TEvent>[]): readonly boolean[]
|
||||
/** Called when a timer-driven flush fails (for example to abort the run). */
|
||||
onError?(error: unknown): void
|
||||
/**
|
||||
* Called after a timer-driven flush committed (and its `onCommitted`
|
||||
* callbacks ran), e.g. to forward the committed events without waiting for
|
||||
* a second coalescing timer downstream.
|
||||
*/
|
||||
onTimerFlushed?(): void
|
||||
/** Safety flush for buffered events that are not followed by a boundary. */
|
||||
flushIntervalMs?: number
|
||||
maximumEvents?: number
|
||||
@@ -27,7 +33,11 @@ type PendingEntry<TEvent> = RemoteEventBatchEntry<TEvent> & {
|
||||
onCommitted?: RemoteEventCommitted<TEvent>
|
||||
}
|
||||
|
||||
export const REMOTE_EVENT_BATCH_FLUSH_INTERVAL_MS = 50
|
||||
/**
|
||||
* One frame: committed events are forwarded to the renderer only after their
|
||||
* batch commits, so this bounds how long streamed remote text is held back.
|
||||
*/
|
||||
export const REMOTE_EVENT_BATCH_FLUSH_INTERVAL_MS = 16
|
||||
export const REMOTE_EVENT_BATCH_MAXIMUM_EVENTS = 256
|
||||
|
||||
/**
|
||||
@@ -47,6 +57,7 @@ export const REMOTE_EVENT_BATCH_MAXIMUM_EVENTS = 256
|
||||
export class RemoteEventBatcher<TEvent> {
|
||||
private readonly persist: RemoteEventBatcherOptions<TEvent>['persist']
|
||||
private readonly onError: RemoteEventBatcherOptions<TEvent>['onError']
|
||||
private readonly onTimerFlushed: RemoteEventBatcherOptions<TEvent>['onTimerFlushed']
|
||||
private readonly flushIntervalMs: number
|
||||
private readonly maximumEvents: number
|
||||
private pending: PendingEntry<TEvent>[] = []
|
||||
@@ -56,6 +67,7 @@ export class RemoteEventBatcher<TEvent> {
|
||||
constructor(options: RemoteEventBatcherOptions<TEvent>) {
|
||||
this.persist = options.persist
|
||||
this.onError = options.onError
|
||||
this.onTimerFlushed = options.onTimerFlushed
|
||||
this.flushIntervalMs =
|
||||
options.flushIntervalMs ?? REMOTE_EVENT_BATCH_FLUSH_INTERVAL_MS
|
||||
this.maximumEvents =
|
||||
@@ -132,6 +144,7 @@ export class RemoteEventBatcher<TEvent> {
|
||||
this.timer = undefined
|
||||
try {
|
||||
this.flush()
|
||||
this.onTimerFlushed?.()
|
||||
} catch (error) {
|
||||
this.onError?.(error)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user