mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-27 17:58:07 +00:00
perf(desktop): stop the thread timeline working when nothing can see it
The prompt rail mounts in every chat surface, and a tab group keeps inactive tabs mounted, so a background timeline was stringifying every user prompt's full text on each store update — including on every streamed assistant token, since the selector walked all messages — plus running a scroll listener and a getBoundingClientRect per prompt against a viewport nobody was looking at. It now defers each piece until it can be seen. An inactive pane returns before a single hook is declared (usePaneVisible, the same context the hidden-tab transcript freeze uses), so no subscription is opened at all. An active rail subscribes to prompt IDS rather than prompt text and reads the transcript imperatively only when that signal changes, which takes streaming off the derivation path entirely; an identical derivation hands back the previous array so a filtered-out prompt doesn't restart the measure effect. The offset pass bails below the render threshold instead of measuring a rail that renders null, ahead of the existing following-the-bottom fast path, and the hover popover keeps its shell for the fade but builds its rows on first open.
This commit is contained in:
parent
be00a7176c
commit
91a8fe4a3e
4 changed files with 331 additions and 38 deletions
|
|
@ -1,6 +1,6 @@
|
|||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
import { activeTimelineIndex, deriveTimelineEntries, timelinePreview } from './timeline-data'
|
||||
import { activeTimelineIndex, deriveTimelineEntries, sameTimelineEntries, timelinePreview } from './timeline-data'
|
||||
|
||||
describe('timelinePreview', () => {
|
||||
it('collapses whitespace to a single line', () => {
|
||||
|
|
@ -39,6 +39,33 @@ describe('deriveTimelineEntries', () => {
|
|||
})
|
||||
})
|
||||
|
||||
describe('sameTimelineEntries', () => {
|
||||
const rail = [
|
||||
{ id: 'u1', preview: 'first' },
|
||||
{ id: 'u2', preview: 'second' }
|
||||
]
|
||||
|
||||
it('treats an identical derivation as unchanged, so the memo can reuse it', () => {
|
||||
expect(sameTimelineEntries(rail, [...rail.map(e => ({ ...e }))])).toBe(true)
|
||||
})
|
||||
|
||||
it('detects a changed preview, id, or length', () => {
|
||||
expect(sameTimelineEntries(rail, [rail[0], { id: 'u2', preview: 'edited' }])).toBe(false)
|
||||
expect(sameTimelineEntries(rail, [rail[0], { id: 'u9', preview: 'second' }])).toBe(false)
|
||||
expect(sameTimelineEntries(rail, [rail[0]])).toBe(false)
|
||||
})
|
||||
|
||||
it('is stable when a filtered-out prompt joins the transcript', () => {
|
||||
const withNoise = deriveTimelineEntries([
|
||||
{ id: 'u1', role: 'user', text: 'first' },
|
||||
{ id: 'u2', role: 'user', text: 'second' },
|
||||
{ id: 'u3', role: 'user', text: '[IMPORTANT: Background process 7 finished]' }
|
||||
])
|
||||
|
||||
expect(sameTimelineEntries(rail, withNoise)).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
describe('activeTimelineIndex', () => {
|
||||
it('returns the last prompt scrolled to or above the top edge', () => {
|
||||
expect(activeTimelineIndex([-400, -10, 320])).toBe(1)
|
||||
|
|
|
|||
|
|
@ -46,6 +46,20 @@ export function deriveTimelineEntries(messages: readonly TimelineSourceMessage[]
|
|||
return entries
|
||||
}
|
||||
|
||||
/** Do two derivations describe the same rail? Lets a rebuild hand back the
|
||||
* PREVIOUS array so an unchanged transcript costs zero re-renders. */
|
||||
export function sameTimelineEntries(a: readonly TimelineEntry[], b: readonly TimelineEntry[]): boolean {
|
||||
if (a === b) {
|
||||
return true
|
||||
}
|
||||
|
||||
if (a.length !== b.length) {
|
||||
return false
|
||||
}
|
||||
|
||||
return a.every((entry, index) => entry.id === b[index].id && entry.preview === b[index].preview)
|
||||
}
|
||||
|
||||
/** Last user prompt at/above the viewport top (with slack); else first rendered. */
|
||||
export function activeTimelineIndex(offsets: readonly (number | null)[], slack: number = 8): number {
|
||||
let active = -1
|
||||
|
|
|
|||
|
|
@ -0,0 +1,166 @@
|
|||
import { cleanup, fireEvent, render, screen } from '@testing-library/react'
|
||||
import type { ReactNode } from 'react'
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
/**
|
||||
* The timeline must do NO work it can't currently show. Two gates are proven
|
||||
* here by rendering the real component and counting the work it performs:
|
||||
*
|
||||
* - a background (kept-alive but hidden) tab derives nothing and subscribes
|
||||
* to nothing — the transcript selector is never even called;
|
||||
* - an unhovered rail builds its ticks but not the popover's rows.
|
||||
*
|
||||
* The prompt-id selector is also asserted to be content-blind, which is what
|
||||
* keeps a streaming assistant reply from re-deriving previews per token.
|
||||
*/
|
||||
|
||||
interface FakeMessage {
|
||||
content: unknown
|
||||
id: string
|
||||
role: string
|
||||
}
|
||||
|
||||
const selectorCalls = vi.fn()
|
||||
const transcriptReads = vi.fn()
|
||||
let messages: FakeMessage[] = []
|
||||
|
||||
vi.mock('@assistant-ui/react', () => ({
|
||||
useAui: () => ({
|
||||
thread: () => ({
|
||||
getState: () => {
|
||||
transcriptReads()
|
||||
|
||||
return { messages }
|
||||
}
|
||||
})
|
||||
}),
|
||||
useAuiState: (selector: (state: { thread: { messages: FakeMessage[] } }) => unknown) => {
|
||||
selectorCalls()
|
||||
|
||||
return selector({ thread: { messages } })
|
||||
}
|
||||
}))
|
||||
|
||||
let paneActive = true
|
||||
|
||||
vi.mock('@/components/pane-shell/pane-visibility', () => ({
|
||||
usePaneVisible: () => paneActive
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/haptics', () => ({ triggerHaptic: () => {} }))
|
||||
|
||||
const { ThreadTimeline } = await import('./timeline')
|
||||
|
||||
const userTurn = (id: string, text: string): FakeMessage => ({
|
||||
content: [{ text, type: 'text' }],
|
||||
id,
|
||||
role: 'user'
|
||||
})
|
||||
|
||||
const transcript = (count: number): FakeMessage[] =>
|
||||
Array.from({ length: count }, (_, i) => userTurn(`u${i}`, `prompt ${i}`))
|
||||
|
||||
const renderTimeline = (ui: ReactNode = <ThreadTimeline />) => render(ui)
|
||||
|
||||
afterEach(() => {
|
||||
cleanup()
|
||||
selectorCalls.mockClear()
|
||||
transcriptReads.mockClear()
|
||||
paneActive = true
|
||||
messages = []
|
||||
})
|
||||
|
||||
describe('ThreadTimeline in a background tab', () => {
|
||||
it('renders nothing and never reads the transcript', () => {
|
||||
paneActive = false
|
||||
messages = transcript(6)
|
||||
|
||||
const { container } = renderTimeline()
|
||||
|
||||
expect(container.querySelector('[data-slot="thread-timeline"]')).toBeNull()
|
||||
expect(selectorCalls).not.toHaveBeenCalled()
|
||||
expect(transcriptReads).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('renders the rail once its pane becomes the visible tab', () => {
|
||||
messages = transcript(6)
|
||||
|
||||
const { container } = renderTimeline()
|
||||
|
||||
expect(container.querySelector('[data-slot="thread-timeline"]')).not.toBeNull()
|
||||
expect(selectorCalls).toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
||||
describe('ThreadTimeline popover', () => {
|
||||
it('builds no rows until the rail is hovered', () => {
|
||||
messages = transcript(6)
|
||||
|
||||
const { container } = renderTimeline()
|
||||
const popover = container.querySelector('[data-slot="thread-timeline-popover"]')
|
||||
|
||||
// The shell renders (it owns the fade transition); its rows do not.
|
||||
expect(popover).not.toBeNull()
|
||||
expect(popover?.querySelectorAll('button')).toHaveLength(0)
|
||||
expect(screen.queryByText('prompt 0')).toBeNull()
|
||||
})
|
||||
|
||||
it('builds the rows on hover and keeps them for the close fade', () => {
|
||||
messages = transcript(6)
|
||||
|
||||
const { container } = renderTimeline()
|
||||
const rail = container.querySelector<HTMLElement>('[data-slot="thread-timeline"]')!
|
||||
|
||||
fireEvent.mouseEnter(rail)
|
||||
|
||||
const popover = container.querySelector('[data-slot="thread-timeline-popover"]')
|
||||
expect(popover?.querySelectorAll('button')).toHaveLength(6)
|
||||
|
||||
fireEvent.mouseLeave(rail)
|
||||
|
||||
// Still mounted — the popover fades out, it does not pop out of existence.
|
||||
expect(popover?.querySelectorAll('button')).toHaveLength(6)
|
||||
})
|
||||
})
|
||||
|
||||
describe('ThreadTimeline below the threshold', () => {
|
||||
it('renders nothing for a short thread', () => {
|
||||
messages = transcript(2)
|
||||
|
||||
const { container } = renderTimeline()
|
||||
|
||||
expect(container.querySelector('[data-slot="thread-timeline"]')).toBeNull()
|
||||
})
|
||||
})
|
||||
|
||||
describe('ThreadTimeline while a reply streams', () => {
|
||||
it('does not re-derive the rail as assistant content grows', () => {
|
||||
messages = [...transcript(6), { content: [{ text: 'th', type: 'text' }], id: 'a1', role: 'assistant' }]
|
||||
|
||||
const { rerender } = renderTimeline()
|
||||
const derivations = transcriptReads.mock.calls.length
|
||||
|
||||
// A token lands: the assistant message's content changes, the user prompt
|
||||
// ids do not — so the memo's change signal is untouched and the previews
|
||||
// are never rebuilt.
|
||||
messages = [
|
||||
...messages.slice(0, -1),
|
||||
{ content: [{ text: 'thinking…', type: 'text' }], id: 'a1', role: 'assistant' }
|
||||
]
|
||||
rerender(<ThreadTimeline />)
|
||||
|
||||
expect(transcriptReads.mock.calls.length).toBe(derivations)
|
||||
})
|
||||
|
||||
it('re-derives once a new prompt is sent', () => {
|
||||
messages = transcript(6)
|
||||
|
||||
const { rerender } = renderTimeline()
|
||||
const derivations = transcriptReads.mock.calls.length
|
||||
|
||||
messages = [...messages, userTurn('u6', 'prompt 6')]
|
||||
rerender(<ThreadTimeline />)
|
||||
|
||||
expect(transcriptReads.mock.calls.length).toBeGreaterThan(derivations)
|
||||
})
|
||||
})
|
||||
|
|
@ -1,12 +1,14 @@
|
|||
import { useAuiState } from '@assistant-ui/react'
|
||||
import { useAui, useAuiState } from '@assistant-ui/react'
|
||||
import { type FC, useCallback, useEffect, useMemo, useRef, useState } from 'react'
|
||||
|
||||
import { usePaneVisible } from '@/components/pane-shell/pane-visibility'
|
||||
import { triggerHaptic } from '@/lib/haptics'
|
||||
import { cn } from '@/lib/utils'
|
||||
|
||||
import {
|
||||
activeTimelineIndex,
|
||||
deriveTimelineEntries,
|
||||
sameTimelineEntries,
|
||||
type TimelineEntry,
|
||||
type TimelineSourceMessage
|
||||
} from './timeline-data'
|
||||
|
|
@ -122,26 +124,89 @@ function scrollToPrompt(root: HTMLElement | null, id: string) {
|
|||
jumpScroll(viewport, Math.max(0, top))
|
||||
}
|
||||
|
||||
/** Right-edge prompt rail — hover previews, click to jump. ≥4 user turns only. */
|
||||
/**
|
||||
* Right-edge prompt rail — hover previews, click to jump. ≥4 user turns only.
|
||||
*
|
||||
* Everything here is DEFERRED until it can actually be seen. A chat surface
|
||||
* stays mounted while its tab is in the background (keep-alive, see
|
||||
* pane-visibility.ts), and a background thread keeps streaming, so a naive
|
||||
* timeline would re-derive previews and re-measure prompt offsets all day for
|
||||
* a rail nobody is looking at. Four gates, cheapest first:
|
||||
*
|
||||
* 1. INACTIVE PANE → render null and subscribe to nothing. The transcript
|
||||
* selector, the scroll listener, and the popover markup all stand down.
|
||||
* 2. ACTIVE BUT UNHOVERED → the ticks paint, but the popover's rows are not
|
||||
* built at all; the previews only exist once the pointer opens it.
|
||||
* 3. BELOW THE THRESHOLD → the rail renders null, so the measure effect never
|
||||
* touches layout for it.
|
||||
* 4. FOLLOWING THE BOTTOM → the active prompt is the last one by definition,
|
||||
* answered from data instead of a rect walk (see compute() below).
|
||||
*/
|
||||
export const ThreadTimeline: FC = () => {
|
||||
const sourceSignature = useAuiState(s => {
|
||||
const rows: TimelineSourceMessage[] = []
|
||||
// Cheapest possible gate, and it must come first: an inactive tab returns
|
||||
// before any of the work below is even declared.
|
||||
return usePaneVisible() ? <ActiveThreadTimeline /> : null
|
||||
}
|
||||
|
||||
/** Derived prompt rail for a VISIBLE surface. Split out so the hook body — and
|
||||
* the transcript subscription it opens — never runs for a background tab. */
|
||||
const ActiveThreadTimeline: FC = () => {
|
||||
// Cheap in the selector, expensive only when it changes: the ids alone tell
|
||||
// us whether the RAIL changed. Prompt text is immutable once sent, and an
|
||||
// edit rewinds the transcript (dropping every id after it) and re-appends a
|
||||
// fresh message id — so a preview can never go stale behind a stable id.
|
||||
// Streaming an assistant reply churns that message's content on every token
|
||||
// and leaves this string untouched, which is the whole point.
|
||||
const promptIds = useAuiState(s => {
|
||||
let ids = ''
|
||||
|
||||
for (const message of s.thread.messages) {
|
||||
if (message.role !== 'user') {
|
||||
continue
|
||||
if (message.role === 'user') {
|
||||
ids += `${message.id}\n`
|
||||
}
|
||||
|
||||
rows.push({ id: message.id, role: 'user', text: userPromptText(message.content) })
|
||||
}
|
||||
|
||||
return JSON.stringify(rows)
|
||||
return ids
|
||||
})
|
||||
|
||||
const entries = useMemo(
|
||||
() => deriveTimelineEntries(JSON.parse(sourceSignature) as TimelineSourceMessage[]),
|
||||
[sourceSignature]
|
||||
)
|
||||
// `promptIds` is the change signal; the transcript is read imperatively when
|
||||
// it fires, so the selector above never pays for text extraction. The client
|
||||
// goes through a ref so the memo keys on the SIGNAL alone — an accessor whose
|
||||
// identity churned would otherwise re-derive every render, which is exactly
|
||||
// the streaming cost this is here to avoid.
|
||||
const aui = useAui()
|
||||
const auiRef = useRef(aui)
|
||||
auiRef.current = aui
|
||||
|
||||
const previousRef = useRef<TimelineEntry[]>([])
|
||||
|
||||
const entries = useMemo(() => {
|
||||
const rows: TimelineSourceMessage[] = []
|
||||
|
||||
for (const message of auiRef.current.thread().getState().messages) {
|
||||
if (message.role === 'user') {
|
||||
rows.push({ id: message.id, role: 'user', text: userPromptText(message.content) })
|
||||
}
|
||||
}
|
||||
|
||||
const next = deriveTimelineEntries(rows)
|
||||
|
||||
// Hand back the PREVIOUS array when nothing user-visible moved. Blank and
|
||||
// background-notification prompts are filtered out, so a new id can leave
|
||||
// the rail identical — without this, that re-renders both subtrees and
|
||||
// restarts the measure effect for no visible change.
|
||||
if (sameTimelineEntries(previousRef.current, next)) {
|
||||
return previousRef.current
|
||||
}
|
||||
|
||||
previousRef.current = next
|
||||
|
||||
return next
|
||||
// promptIds is the intentional re-eval TRIGGER, not a value the derivation
|
||||
// reads (the transcript comes off the ref) — same shape as ChatRoutesSurface's
|
||||
// gatewayState memo in app/contrib/controller.tsx.
|
||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
}, [promptIds])
|
||||
|
||||
const [activeIndex, setActiveIndex] = useState(0)
|
||||
const [open, setOpen] = useState(false)
|
||||
|
|
@ -185,9 +250,15 @@ export const ThreadTimeline: FC = () => {
|
|||
useEffect(() => () => window.clearTimeout(closeTimerRef.current), [])
|
||||
|
||||
useEffect(() => {
|
||||
// Below the threshold the rail renders null, so measuring prompt offsets
|
||||
// buys nothing — bail before touching layout at all.
|
||||
if (entries.length < MIN_ENTRIES) {
|
||||
return
|
||||
}
|
||||
|
||||
const viewport = ownViewport(rootRef.current)
|
||||
|
||||
if (!viewport || entries.length === 0) {
|
||||
if (!viewport) {
|
||||
return
|
||||
}
|
||||
|
||||
|
|
@ -279,29 +350,44 @@ const TimelinePopover: FC<{
|
|||
onJump: (id: string) => void
|
||||
open: boolean
|
||||
rowRefs: React.RefObject<(HTMLButtonElement | null)[]>
|
||||
}> = ({ activeIndex, entries, onHover, onJump, open, rowRefs }) => (
|
||||
<div
|
||||
className={cn(
|
||||
POPOVER_SHELL,
|
||||
open ? 'pointer-events-auto opacity-100 translate-x-0' : 'pointer-events-none translate-x-1 opacity-0'
|
||||
)}
|
||||
data-slot="thread-timeline-popover"
|
||||
>
|
||||
{entries.map((entry, index) => (
|
||||
<button
|
||||
aria-label={entry.preview}
|
||||
className={cn(ROW_CLASS, index === activeIndex && 'bg-(--ui-row-active-background) text-foreground')}
|
||||
key={entry.id}
|
||||
onClick={() => onJump(entry.id)}
|
||||
ref={listRef(rowRefs, index)}
|
||||
type="button"
|
||||
{...hoverProps(index, onHover)}
|
||||
>
|
||||
<span className="block w-full min-w-0 truncate font-medium leading-snug text-foreground">{entry.preview}</span>
|
||||
</button>
|
||||
))}
|
||||
</div>
|
||||
)
|
||||
}> = ({ activeIndex, entries, onHover, onJump, open, rowRefs }) => {
|
||||
// The rail is the always-visible part; this list is not built until the
|
||||
// pointer first opens it. The SHELL always renders so the opacity/translate
|
||||
// transition has a node to animate — only the N rows are deferred, and they
|
||||
// stay mounted afterwards so the close fade still has content.
|
||||
const [everOpened, setEverOpened] = useState(open)
|
||||
|
||||
if (open && !everOpened) {
|
||||
setEverOpened(true)
|
||||
}
|
||||
|
||||
return (
|
||||
<div
|
||||
className={cn(
|
||||
POPOVER_SHELL,
|
||||
open ? 'pointer-events-auto opacity-100 translate-x-0' : 'pointer-events-none translate-x-1 opacity-0'
|
||||
)}
|
||||
data-slot="thread-timeline-popover"
|
||||
>
|
||||
{everOpened &&
|
||||
entries.map((entry, index) => (
|
||||
<button
|
||||
aria-label={entry.preview}
|
||||
className={cn(ROW_CLASS, index === activeIndex && 'bg-(--ui-row-active-background) text-foreground')}
|
||||
key={entry.id}
|
||||
onClick={() => onJump(entry.id)}
|
||||
ref={listRef(rowRefs, index)}
|
||||
type="button"
|
||||
{...hoverProps(index, onHover)}
|
||||
>
|
||||
<span className="block w-full min-w-0 truncate font-medium leading-snug text-foreground">
|
||||
{entry.preview}
|
||||
</span>
|
||||
</button>
|
||||
))}
|
||||
</div>
|
||||
)
|
||||
}
|
||||
|
||||
const TimelineTicks: FC<{
|
||||
activeIndex: number
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue