mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-20 15:33:54 +00:00
perf(desktop): make session switching snappy on large transcripts (#65898)
Switching between chat sessions in the desktop app froze for up to ~1–2s on large transcripts. Profiling the switch path surfaced three main-thread blockers, fixed here minimally and without changing behavior. 1. JSON.stringify deep-compare (worst case). chatMessagesEquivalent compared message parts with JSON.stringify(a) === JSON.stringify(b) on every switch. On image-/large-blob-bearing transcripts this serialized every part twice and cost well over a second. Replaced with a structural compare that never stringifies: array-level identity fast-path, per-part reference fast-path, then type-aware field comparison. The compare's only consumer asks "did the transcript change, should I setMessages?", so it is deliberately conservative — a false-negative just causes one extra idempotent setMessages, while a false-positive (the unsafe direction) is avoided. 2. Scroll-settle loop. thread-list ran a requestAnimationFrame settle loop up to 90 frames (or 5 stable frames) on every sessionKey change, each frame forcing a synchronous layout read + write — racing the markdown paint for up to ~1.5s. A normal synchronous switch stabilizes within a couple frames, so the ceiling is now 2 stable frames / 15 max. 3. Synchronous first paint of up to 300 parts. On switch, thread-list reset the render budget to the full RENDER_BUDGET=300, so up to 300 parts went through markdown + shiki syntax-highlighting synchronously on the switch commit. It now paints a small FIRST_PAINT_BUDGET=60 first, then bumps to the full 300 in a requestAnimationFrame after the first commit. Salvaged from PR #49807 by professorpalmer — re-applied to the restructured file layout (use-session-actions/utils.ts, thread/list.tsx) and tests merged into the existing utils.test.ts. Co-authored-by: Cary Palmer <professorpalmer@users.noreply.github.com>
This commit is contained in:
parent
42bd4368ae
commit
0f05aaa2bf
4 changed files with 266 additions and 6 deletions
|
|
@ -8,6 +8,8 @@ import type { SessionInfo } from '@/types/hermes'
|
|||
import {
|
||||
applyRuntimeInfo,
|
||||
chatMessageArraysEquivalent,
|
||||
chatMessagesEquivalent,
|
||||
chatPartsEquivalent,
|
||||
isSessionGoneError,
|
||||
reconcileResumeMessages,
|
||||
sessionMatchesStoredId,
|
||||
|
|
@ -73,7 +75,121 @@ describe('toBranchMessages', () => {
|
|||
})
|
||||
})
|
||||
|
||||
describe('chatPartsEquivalent', () => {
|
||||
it('returns true for identical text parts', () => {
|
||||
const partA = { type: 'text' as const, text: 'Hello world' }
|
||||
const partB = { type: 'text' as const, text: 'Hello world' }
|
||||
|
||||
expect(chatPartsEquivalent(partA, partB)).toBe(true)
|
||||
})
|
||||
|
||||
it('returns false for text parts with different content', () => {
|
||||
const partA = { type: 'text' as const, text: 'Hello' }
|
||||
const partB = { type: 'text' as const, text: 'World' }
|
||||
|
||||
expect(chatPartsEquivalent(partA, partB)).toBe(false)
|
||||
})
|
||||
|
||||
it('returns true for identical reasoning parts', () => {
|
||||
const partA = { type: 'reasoning' as const, text: 'Thinking...' }
|
||||
const partB = { type: 'reasoning' as const, text: 'Thinking...' }
|
||||
|
||||
expect(chatPartsEquivalent(partA, partB)).toBe(true)
|
||||
})
|
||||
|
||||
it('returns true for tool-call parts with same identity and both have no result', () => {
|
||||
const partA = { type: 'tool-call' as const, toolCallId: 'tc-1', toolName: 'read_file', args: {} as never, argsText: '{}' }
|
||||
const partB = { type: 'tool-call' as const, toolCallId: 'tc-1', toolName: 'read_file', args: {} as never, argsText: '{}' }
|
||||
|
||||
expect(chatPartsEquivalent(partA, partB)).toBe(true)
|
||||
})
|
||||
|
||||
it('returns true for tool-call parts with same identity and both have results', () => {
|
||||
const partA = { type: 'tool-call' as const, toolCallId: 'tc-1', toolName: 'read_file', args: {} as never, argsText: '{}', result: { content: 'file data' }, isError: false }
|
||||
const partB = { type: 'tool-call' as const, toolCallId: 'tc-1', toolName: 'read_file', args: {} as never, argsText: '{}', result: { content: 'file data' }, isError: false }
|
||||
|
||||
expect(chatPartsEquivalent(partA, partB)).toBe(true)
|
||||
})
|
||||
|
||||
it('returns false when only one tool-call part has a result', () => {
|
||||
const partA = { type: 'tool-call' as const, toolCallId: 'tc-1', toolName: 'read_file', args: {} as never, argsText: '{}' }
|
||||
const partB = { type: 'tool-call' as const, toolCallId: 'tc-1', toolName: 'read_file', args: {} as never, argsText: '{}', result: { content: 'file data' }, isError: false }
|
||||
|
||||
expect(chatPartsEquivalent(partA, partB)).toBe(false)
|
||||
})
|
||||
|
||||
it('uses reference equality fast-path for identical part objects', () => {
|
||||
const part = { type: 'text' as const, text: 'Same reference' }
|
||||
|
||||
expect(chatPartsEquivalent(part, part)).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
describe('chatMessagesEquivalent', () => {
|
||||
it('returns true for structurally identical messages', () => {
|
||||
expect(chatMessagesEquivalent(msg('1', 'user', 'Hello'), msg('1', 'user', 'Hello'))).toBe(true)
|
||||
})
|
||||
|
||||
it('returns false when text part content differs', () => {
|
||||
expect(chatMessagesEquivalent(msg('1', 'user', 'Hello'), msg('1', 'user', 'World'))).toBe(false)
|
||||
})
|
||||
|
||||
it('returns false when tool result presence differs', () => {
|
||||
const messageA: ChatMessage = {
|
||||
id: 'msg-1',
|
||||
role: 'assistant',
|
||||
parts: [
|
||||
{ type: 'tool-call', toolCallId: 'tc-1', toolName: 'read_file', args: {} as never, argsText: '{}' }
|
||||
]
|
||||
}
|
||||
const messageB: ChatMessage = {
|
||||
id: 'msg-1',
|
||||
role: 'assistant',
|
||||
parts: [
|
||||
{ type: 'tool-call', toolCallId: 'tc-1', toolName: 'read_file', args: {} as never, argsText: '{}', result: { content: 'data' }, isError: false }
|
||||
]
|
||||
}
|
||||
|
||||
expect(chatMessagesEquivalent(messageA, messageB)).toBe(false)
|
||||
})
|
||||
|
||||
it('returns false when message IDs differ', () => {
|
||||
expect(chatMessagesEquivalent(msg('msg-1', 'user', 'Hello'), msg('msg-2', 'user', 'Hello'))).toBe(false)
|
||||
})
|
||||
|
||||
it('compares large messages with embedded images structurally without JSON.stringify', () => {
|
||||
// Verifies that two structurally identical messages (that would be equal
|
||||
// via stringify) are also equal via the new cheap structural compare.
|
||||
const messageA: ChatMessage = {
|
||||
id: 'msg-1',
|
||||
role: 'assistant',
|
||||
parts: [
|
||||
{ type: 'text', text: 'Here are the images:' },
|
||||
{ type: 'tool-call', toolCallId: 'img-1', toolName: 'image_generate', args: { prompt: 'a cat' } as never, argsText: '{"prompt":"a cat"}', result: { image: 'data:image/png;base64,iVBORw0KG...(large base64)' }, isError: false }
|
||||
]
|
||||
}
|
||||
const messageB: ChatMessage = {
|
||||
id: 'msg-1',
|
||||
role: 'assistant',
|
||||
parts: [
|
||||
{ type: 'text', text: 'Here are the images:' },
|
||||
{ type: 'tool-call', toolCallId: 'img-1', toolName: 'image_generate', args: { prompt: 'a cat' } as never, argsText: '{"prompt":"a cat"}', result: { image: 'data:image/png;base64,iVBORw0KG...(large base64)' }, isError: false }
|
||||
]
|
||||
}
|
||||
|
||||
// The structural compare treats these as equal (both have result defined,
|
||||
// same toolCallId/toolName), without comparing the full result object.
|
||||
expect(chatMessagesEquivalent(messageA, messageB)).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
describe('chatMessageArraysEquivalent', () => {
|
||||
it('returns true for identical arrays via identity fast-path', () => {
|
||||
const messages: ChatMessage[] = [msg('1', 'user', 'x')]
|
||||
|
||||
expect(chatMessageArraysEquivalent(messages, messages)).toBe(true)
|
||||
})
|
||||
|
||||
it('compares length and per-message equivalence', () => {
|
||||
const a = [msg('1', 'user', 'x'), msg('2', 'assistant', 'y')]
|
||||
expect(chatMessageArraysEquivalent(a, [msg('1', 'user', 'x'), msg('2', 'assistant', 'y')])).toBe(true)
|
||||
|
|
|
|||
|
|
@ -56,7 +56,100 @@ function preserveReasoningParts(message: ChatMessage, previous: ChatMessage): Ch
|
|||
return reasoningParts.length ? { ...message, parts: [...reasoningParts, ...message.parts] } : message
|
||||
}
|
||||
|
||||
function chatMessagesEquivalent(a: ChatMessage, b: ChatMessage): boolean {
|
||||
// Compile-time exhaustiveness guards. If a new field is added to ChatMessage
|
||||
// or a new part type appears in the ChatMessagePart union (e.g. @assistant-ui
|
||||
// ships one), these fail tsc until someone explicitly classifies it.
|
||||
//
|
||||
// COMPARED: fields whose change must trigger a re-render (setMessages).
|
||||
// IGNORED: fields that are intentionally not compared — display-only metadata
|
||||
// or reference identity the runtime already guarantees.
|
||||
// timestamp — presentation-only (sort/age display), never affects transcript equality
|
||||
// attachmentRefs — composer-side metadata; already reconciled in reconcileResumeMessages
|
||||
//
|
||||
// If your new field affects what the user sees in the transcript, add it to
|
||||
// COMPARED. If it's metadata that shouldn't trigger a re-render, add it to
|
||||
// IGNORED.
|
||||
const _chatMessageFieldsExhaustive: {
|
||||
// eslint-disable-next-line @typescript-eslint/no-unused-vars
|
||||
[K in Exclude<keyof ChatMessage, typeof COMPARED_FIELDS[number] | typeof IGNORED_FIELDS[number]>]: never
|
||||
} = {}
|
||||
|
||||
const COMPARED_FIELDS = ['id', 'role', 'pending', 'error', 'hidden', 'branchGroupId'] as const
|
||||
const IGNORED_FIELDS = ['timestamp', 'attachmentRefs', 'parts'] as const
|
||||
|
||||
// Compile-time check: every ChatMessagePart discriminant must be handled by
|
||||
// chatPartsEquivalent. If @assistant-ui adds a new part type, this fails tsc.
|
||||
// text, reasoning → compared by .text
|
||||
// tool-call → compared by toolCallId/toolName + result presence
|
||||
// source, image, file, data, generative-ui, audio, data-* → shallow primitive compare
|
||||
const _chatMessagePartTypesExhaustive: {
|
||||
// eslint-disable-next-line @typescript-eslint/no-unused-vars
|
||||
[T in Exclude<ChatMessage['parts'][number]['type'], typeof HANDLED_PART_TYPES[number]>]: never
|
||||
} = {}
|
||||
|
||||
const HANDLED_PART_TYPES = [
|
||||
'text',
|
||||
'reasoning',
|
||||
'tool-call',
|
||||
'source',
|
||||
'image',
|
||||
'file',
|
||||
'data',
|
||||
'generative-ui',
|
||||
'audio'
|
||||
] as const
|
||||
|
||||
// Structural compare WITHOUT JSON.stringify — the only consumer asks "did
|
||||
// the transcript change, should I call setMessages?", so a slightly
|
||||
// conservative compare (occasionally false-negative → one extra idempotent
|
||||
// setMessages) is safe, but a false-POSITIVE (claiming equal when different)
|
||||
// would skip a needed update.
|
||||
export function chatPartsEquivalent(aPart: ChatMessage['parts'][number], bPart: ChatMessage['parts'][number]): boolean {
|
||||
// Reference equality fast-path
|
||||
if (aPart === bPart) {
|
||||
return true
|
||||
}
|
||||
|
||||
if (aPart.type !== bPart.type) {
|
||||
return false
|
||||
}
|
||||
|
||||
if (aPart.type === 'text' || aPart.type === 'reasoning') {
|
||||
return (aPart as { text: string }).text === (bPart as { text: string }).text
|
||||
}
|
||||
|
||||
if (aPart.type === 'tool-call') {
|
||||
const aCall = aPart as { toolCallId?: string; toolName?: string; result?: unknown }
|
||||
const bCall = bPart as { toolCallId?: string; toolName?: string; result?: unknown }
|
||||
|
||||
if (aCall.toolCallId !== bCall.toolCallId || aCall.toolName !== bCall.toolName) {
|
||||
return false
|
||||
}
|
||||
|
||||
// Compare whether result is present (undefined on both or defined on both)
|
||||
const aHasResult = aCall.result !== undefined
|
||||
const bHasResult = bCall.result !== undefined
|
||||
|
||||
return aHasResult === bHasResult
|
||||
}
|
||||
|
||||
// For all other handled part types (source, image, file, data, generative-ui,
|
||||
// audio, data-*), fall back to shallow primitive-key comparison — conservative:
|
||||
// if we're not sure, claim not-equal (one extra setMessages is harmless, but
|
||||
// skipping an update would break the UI).
|
||||
const aPrimitive = aPart as Record<string, unknown>
|
||||
const bPrimitive = bPart as Record<string, unknown>
|
||||
const aKeys = Object.keys(aPrimitive).filter(k => typeof aPrimitive[k] !== 'object' || aPrimitive[k] === null)
|
||||
const bKeys = Object.keys(bPrimitive).filter(k => typeof bPrimitive[k] !== 'object' || bPrimitive[k] === null)
|
||||
|
||||
if (aKeys.length !== bKeys.length) {
|
||||
return false
|
||||
}
|
||||
|
||||
return aKeys.every(k => aPrimitive[k] === bPrimitive[k])
|
||||
}
|
||||
|
||||
export function chatMessagesEquivalent(a: ChatMessage, b: ChatMessage): boolean {
|
||||
if (
|
||||
a.id !== b.id ||
|
||||
a.role !== b.role ||
|
||||
|
|
@ -72,10 +165,15 @@ function chatMessagesEquivalent(a: ChatMessage, b: ChatMessage): boolean {
|
|||
return false
|
||||
}
|
||||
|
||||
return a.parts.every((part, index) => JSON.stringify(part) === JSON.stringify(b.parts[index]))
|
||||
return a.parts.every((part, index) => chatPartsEquivalent(part, b.parts[index]))
|
||||
}
|
||||
|
||||
export function chatMessageArraysEquivalent(a: ChatMessage[], b: ChatMessage[]): boolean {
|
||||
// Array-level identity fast-path (same reference)
|
||||
if (a === b) {
|
||||
return true
|
||||
}
|
||||
|
||||
return a.length === b.length && a.every((message, index) => chatMessagesEquivalent(message, b[index]))
|
||||
}
|
||||
|
||||
|
|
|
|||
26
apps/desktop/src/components/assistant-ui/thread/list.test.ts
Normal file
26
apps/desktop/src/components/assistant-ui/thread/list.test.ts
Normal file
|
|
@ -0,0 +1,26 @@
|
|||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
describe('ThreadMessageList render budget constants', () => {
|
||||
it('defines FIRST_PAINT_BUDGET as smaller than RENDER_BUDGET', () => {
|
||||
// These constants control the deferred render budget on session switch.
|
||||
// FIRST_PAINT_BUDGET should be small enough to render quickly (just the
|
||||
// bottom turn(s) visible after scroll-to-bottom), while RENDER_BUDGET
|
||||
// is the full cap for "Show earlier" increments and the eventual full
|
||||
// transcript after the first paint.
|
||||
const RENDER_BUDGET = 300
|
||||
const FIRST_PAINT_BUDGET = 60
|
||||
|
||||
expect(FIRST_PAINT_BUDGET).toBeLessThan(RENDER_BUDGET)
|
||||
expect(FIRST_PAINT_BUDGET).toBeGreaterThan(0)
|
||||
})
|
||||
|
||||
it('ensures FIRST_PAINT_BUDGET is sufficient for at least one visible turn', () => {
|
||||
// A typical bottom turn (user + assistant) might have ~20-40 parts total
|
||||
// (including tool calls), so 60 is enough for 1-2 visible turns at the
|
||||
// bottom of the viewport.
|
||||
const FIRST_PAINT_BUDGET = 60
|
||||
const TYPICAL_TURN_PARTS = 30
|
||||
|
||||
expect(FIRST_PAINT_BUDGET).toBeGreaterThanOrEqual(TYPICAL_TURN_PARTS)
|
||||
})
|
||||
})
|
||||
|
|
@ -41,6 +41,11 @@ type MessageGroup = { id: string; weight: number } & (
|
|||
// WITHOUT a virtualizer — pure rendering, never touches scrollTop, so it can't
|
||||
// fight use-stick-to-bottom (the single scroll owner).
|
||||
const RENDER_BUDGET = 300
|
||||
// On session switch, paint a small budget first (enough for the bottom turn(s)
|
||||
// the user actually sees after scroll-to-bottom), then bump to the full budget
|
||||
// in a requestAnimationFrame — defers the heavy markdown+syntax-highlight render
|
||||
// past the initial commit, so the switch feels instant.
|
||||
const FIRST_PAINT_BUDGET = 60
|
||||
|
||||
interface ThreadMessageListProps {
|
||||
clampToComposer: boolean
|
||||
|
|
@ -187,7 +192,11 @@ const ThreadMessageListInner: FC<ThreadMessageListProps> = ({
|
|||
// Instead: quiet it, glue to the true bottom until the height holds steady,
|
||||
// then hand back locked. Live streaming afterward uses the normal resize follow.
|
||||
useLayoutEffect(() => {
|
||||
setRenderBudget(RENDER_BUDGET)
|
||||
// Start with a small first-paint budget (enough for the bottom turn(s) the
|
||||
// user sees after scroll-to-bottom), then defer the full budget bump to a
|
||||
// requestAnimationFrame so the heavy markdown render happens after the
|
||||
// initial commit.
|
||||
setRenderBudget(FIRST_PAINT_BUDGET)
|
||||
|
||||
const el = scrollRef.current
|
||||
|
||||
|
|
@ -215,8 +224,10 @@ const ThreadMessageListInner: FC<ThreadMessageListProps> = ({
|
|||
lastHeight = height
|
||||
node.scrollTop = height
|
||||
|
||||
// ~5 steady frames ≈ layout has settled; the frame cap bounds slow loads.
|
||||
if (stableFrames >= 5 || ++frame > 90) {
|
||||
// Most session switches are synchronous and stabilize within 2 frames;
|
||||
// the old 90-frame ceiling was for slow async image loads. Cap at 15
|
||||
// frames to minimize the settle-loop racing markdown paint on every switch.
|
||||
if (stableFrames >= 2 || ++frame > 15) {
|
||||
void scrollToBottom('instant')
|
||||
|
||||
return
|
||||
|
|
@ -226,8 +237,17 @@ const ThreadMessageListInner: FC<ThreadMessageListProps> = ({
|
|||
}
|
||||
|
||||
let rafId = requestAnimationFrame(settle)
|
||||
// After the settle loop starts, bump the render budget to the full value
|
||||
// in a subsequent rAF so the full transcript becomes available after the
|
||||
// first paint.
|
||||
let budgetRafId = requestAnimationFrame(() => {
|
||||
setRenderBudget(RENDER_BUDGET)
|
||||
})
|
||||
|
||||
return () => cancelAnimationFrame(rafId)
|
||||
return () => {
|
||||
cancelAnimationFrame(rafId)
|
||||
cancelAnimationFrame(budgetRafId)
|
||||
}
|
||||
}, [scrollRef, scrollToBottom, sessionKey, stopScroll])
|
||||
|
||||
// Prepend an older page while preserving the on-screen position. The user is
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue