mirror of
https://github.com/stablyai/orca.git
synced 2026-10-22 16:03:47 +00:00
fix(sidebar): stop the agent row's tool step from flickering on every hook (#11075)
This commit is contained in:
@@ -13,6 +13,7 @@ import type { DashboardAgentRow as DashboardAgentRowData } from './useDashboardD
|
||||
import { getAgentRowPrimaryText } from '@/lib/agent-row-primary-text'
|
||||
import { useAgentRowConversationName } from './use-agent-row-conversation-name'
|
||||
import { lastEnteredDoneAt } from './agent-finished-timestamp'
|
||||
import { useSettledAgentToolStep } from '@/hooks/use-settled-agent-tool-step'
|
||||
|
||||
// Why: narrow the dashboard's rollup states to shared dot states, defaulting unknowns to 'idle' so a row never crashes.
|
||||
function asDotState(state: AgentStatusState | 'idle'): AgentDotState {
|
||||
@@ -146,8 +147,11 @@ const DashboardAgentRow = React.memo(function DashboardAgentRow({
|
||||
const model = agent.entry.model?.trim() ?? ''
|
||||
// Why: gate tool fields on 'working' — a stale tool line on a done row reads as still-running.
|
||||
const isWorking = agent.state === 'working'
|
||||
const toolName = isWorking ? (agent.entry.toolName?.trim() ?? '') : ''
|
||||
const toolInput = isWorking ? (agent.entry.toolInput?.trim() ?? '') : ''
|
||||
// Why: hooks rewrite the pair several times a second; hold it for a readable dwell (#11075).
|
||||
const { toolName, toolInput } = useSettledAgentToolStep(
|
||||
isWorking ? (agent.entry.toolName?.trim() ?? '') : '',
|
||||
isWorking ? (agent.entry.toolInput?.trim() ?? '') : ''
|
||||
)
|
||||
const lastAssistantMessage = agent.entry.lastAssistantMessage?.trim() ?? ''
|
||||
const isInterrupted = agent.entry.interrupted === true
|
||||
const lineage = agent.lineage
|
||||
|
||||
+122
@@ -0,0 +1,122 @@
|
||||
// @vitest-environment happy-dom
|
||||
|
||||
import { act } from 'react'
|
||||
import { createRoot, type Root } from 'react-dom/client'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import type { DashboardAgentRow as DashboardAgentRowData } from '@/components/dashboard/useDashboardData'
|
||||
import { AGENT_TOOL_STEP_DWELL_MS } from '@/hooks/use-settled-agent-tool-step'
|
||||
import { CompactAgentRow } from './worktree-card-compact-agent-row'
|
||||
|
||||
globalThis.IS_REACT_ACT_ENVIRONMENT = true
|
||||
|
||||
const PANE_KEY = 'tab-1:leaf-1'
|
||||
|
||||
function cursorAgentRow(toolName: string, toolInput: string): DashboardAgentRowData {
|
||||
return {
|
||||
paneKey: PANE_KEY,
|
||||
tab: { id: 'tab-1', worktreeId: 'wt-1' },
|
||||
agentType: 'cursor',
|
||||
rowSource: 'live',
|
||||
state: 'working',
|
||||
startedAt: 1000,
|
||||
entry: {
|
||||
prompt: 'Refactor the intake flow',
|
||||
state: 'working',
|
||||
paneKey: PANE_KEY,
|
||||
updatedAt: 1000,
|
||||
stateStartedAt: 1000,
|
||||
stateHistory: [],
|
||||
agentType: 'cursor',
|
||||
toolName,
|
||||
toolInput
|
||||
}
|
||||
} as unknown as DashboardAgentRowData
|
||||
}
|
||||
|
||||
let container: HTMLDivElement
|
||||
let root: Root
|
||||
|
||||
function renderRow(agent: DashboardAgentRowData): void {
|
||||
act(() => {
|
||||
root.render(
|
||||
<CompactAgentRow agent={agent} now={2000} onActivate={vi.fn()} cacheTimerActive={false} />
|
||||
)
|
||||
})
|
||||
}
|
||||
|
||||
function rowText(): string {
|
||||
return container.querySelector('.compact-agent-row')?.textContent?.trim() ?? ''
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers()
|
||||
container = document.createElement('div')
|
||||
document.body.appendChild(container)
|
||||
root = createRoot(container)
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
act(() => {
|
||||
root.unmount()
|
||||
})
|
||||
container.remove()
|
||||
vi.useRealTimers()
|
||||
})
|
||||
|
||||
describe('compact sidebar agent row tool-step flicker (#11075)', () => {
|
||||
it('does not repaint the row on every hook while a cursor turn runs', () => {
|
||||
renderRow(cursorAgentRow('Read', 'src/a.ts'))
|
||||
const settled = rowText()
|
||||
expect(settled).toContain('Read: src/a.ts')
|
||||
|
||||
// A cursor turn emits pre/post tool hooks several times a second. Every one
|
||||
// of these rewrote the row's single truncated line before the fix.
|
||||
const burst: [string, string][] = [
|
||||
['Grep', 'TODO'],
|
||||
['Edit', 'src/b.ts'],
|
||||
['Bash', 'pnpm test'],
|
||||
['Read', 'src/c.ts']
|
||||
]
|
||||
for (const [toolName, toolInput] of burst) {
|
||||
act(() => {
|
||||
vi.advanceTimersByTime(60)
|
||||
})
|
||||
renderRow(cursorAgentRow(toolName, toolInput))
|
||||
expect(rowText()).toBe(settled)
|
||||
}
|
||||
|
||||
act(() => {
|
||||
vi.advanceTimersByTime(AGENT_TOOL_STEP_DWELL_MS)
|
||||
})
|
||||
// The newest step lands; 'Grep: TODO', 'Edit: src/b.ts', and 'Bash: pnpm test'
|
||||
// were never painted at all.
|
||||
expect(rowText()).toContain('Read: src/c.ts')
|
||||
})
|
||||
|
||||
it('keeps the prompt visible the whole time', () => {
|
||||
renderRow(cursorAgentRow('Read', 'src/a.ts'))
|
||||
for (const [toolName, toolInput] of [
|
||||
['Grep', 'TODO'],
|
||||
['Edit', 'src/b.ts']
|
||||
] as [string, string][]) {
|
||||
act(() => {
|
||||
vi.advanceTimersByTime(60)
|
||||
})
|
||||
renderRow(cursorAgentRow(toolName, toolInput))
|
||||
expect(rowText()).toContain('Refactor the intake flow')
|
||||
}
|
||||
})
|
||||
|
||||
it('settles on the newest tool step once the burst stops', () => {
|
||||
renderRow(cursorAgentRow('Read', 'src/a.ts'))
|
||||
act(() => {
|
||||
vi.advanceTimersByTime(40)
|
||||
})
|
||||
renderRow(cursorAgentRow('Bash', 'pnpm build'))
|
||||
act(() => {
|
||||
vi.advanceTimersByTime(AGENT_TOOL_STEP_DWELL_MS)
|
||||
})
|
||||
|
||||
expect(rowText()).toContain('Bash: pnpm build')
|
||||
})
|
||||
})
|
||||
@@ -10,6 +10,7 @@ import { translate } from '@/i18n/i18n'
|
||||
import { getAgentRowPrimaryText } from '@/lib/agent-row-primary-text'
|
||||
import { useAgentRowConversationName } from '@/components/dashboard/use-agent-row-conversation-name'
|
||||
import { lastEnteredDoneAt } from '@/components/dashboard/agent-finished-timestamp'
|
||||
import { useSettledAgentToolStep, type AgentToolStep } from '@/hooks/use-settled-agent-tool-step'
|
||||
import CacheTimer, { usePromptCacheCountdownForPane } from './CacheTimer'
|
||||
|
||||
function formatShortTimeAgo(ts: number, now: number): string {
|
||||
@@ -36,19 +37,15 @@ function getCompactAgentPrimary(
|
||||
return prompt || agentStateLabel(getAgentDotState(agent))
|
||||
}
|
||||
|
||||
function getCompactAgentSecondary(agent: DashboardAgentRowData): string {
|
||||
function getCompactAgentSecondary(agent: DashboardAgentRowData, toolStep: AgentToolStep): string {
|
||||
if (agent.entry.interrupted === true) {
|
||||
return 'Interrupted by user'
|
||||
}
|
||||
if (agent.state === 'working') {
|
||||
const toolName = agent.entry.toolName?.trim() ?? ''
|
||||
const toolInput = agent.entry.toolInput?.trim() ?? ''
|
||||
if (toolName && toolInput) {
|
||||
return `${toolName}: ${toolInput}`
|
||||
}
|
||||
if (toolName) {
|
||||
return toolName
|
||||
}
|
||||
if (toolStep.toolName && toolStep.toolInput) {
|
||||
return `${toolStep.toolName}: ${toolStep.toolInput}`
|
||||
}
|
||||
if (toolStep.toolName) {
|
||||
return toolStep.toolName
|
||||
}
|
||||
const lastAssistantMessage = agent.entry.lastAssistantMessage?.trim()
|
||||
if (lastAssistantMessage) {
|
||||
@@ -123,7 +120,15 @@ export const CompactAgentRow = React.memo(function CompactAgentRow({
|
||||
const conversationName = useAgentRowConversationName(agent)
|
||||
const primary = getCompactAgentPrimary(agent, conversationName)
|
||||
const isLineageChild = agent.lineage?.depth === 1
|
||||
const secondary = getCompactAgentSecondary(agent)
|
||||
// Why: gate on 'working' — a stale tool step on a done row reads as still-running.
|
||||
const isWorking = agent.state === 'working'
|
||||
// Why: the tool step shares this row's one truncated line with the prompt, so a
|
||||
// per-hook rewrite reads as the identity itself flickering (#11075).
|
||||
const toolStep = useSettledAgentToolStep(
|
||||
isWorking ? (agent.entry.toolName?.trim() ?? '') : '',
|
||||
isWorking ? (agent.entry.toolInput?.trim() ?? '') : ''
|
||||
)
|
||||
const secondary = getCompactAgentSecondary(agent, toolStep)
|
||||
const model = agent.entry.model?.trim() ?? ''
|
||||
const shortTime = getCompactAgentTime(agent, now)
|
||||
const cacheTimer = usePromptCacheCountdownForPane(agent.paneKey, cacheTimerActive)
|
||||
|
||||
@@ -0,0 +1,189 @@
|
||||
// @vitest-environment happy-dom
|
||||
|
||||
import { act, renderHook } from '@testing-library/react'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { AGENT_TOOL_STEP_DWELL_MS, useSettledAgentToolStep } from './use-settled-agent-tool-step'
|
||||
|
||||
type StepProps = { name: string; input: string }
|
||||
|
||||
function renderStep(initialProps: StepProps) {
|
||||
return renderHook(({ name, input }: StepProps) => useSettledAgentToolStep(name, input), {
|
||||
initialProps
|
||||
})
|
||||
}
|
||||
|
||||
function passDwell(): void {
|
||||
act(() => {
|
||||
vi.advanceTimersByTime(AGENT_TOOL_STEP_DWELL_MS)
|
||||
})
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers()
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
vi.useRealTimers()
|
||||
})
|
||||
|
||||
describe('useSettledAgentToolStep', () => {
|
||||
it('shows the mounted step without waiting for a dwell', () => {
|
||||
const { result } = renderStep({ name: 'Read', input: 'src/main.ts' })
|
||||
|
||||
expect(result.current).toEqual({ toolName: 'Read', toolInput: 'src/main.ts' })
|
||||
})
|
||||
|
||||
it('holds the mounted step for one dwell before the first rewrite lands', () => {
|
||||
const { result, rerender } = renderStep({ name: '', input: '' })
|
||||
|
||||
act(() => {
|
||||
rerender({ name: 'Read', input: 'src/main.ts' })
|
||||
})
|
||||
expect(result.current).toEqual({ toolName: '', toolInput: '' })
|
||||
|
||||
passDwell()
|
||||
expect(result.current).toEqual({ toolName: 'Read', toolInput: 'src/main.ts' })
|
||||
})
|
||||
|
||||
it('coalesces a burst of hook rewrites to the newest step', () => {
|
||||
const { result, rerender } = renderStep({ name: 'Read', input: 'a.ts' })
|
||||
passDwell()
|
||||
|
||||
act(() => {
|
||||
rerender({ name: 'Read', input: 'b.ts' })
|
||||
})
|
||||
expect(result.current).toEqual({ toolName: 'Read', toolInput: 'b.ts' })
|
||||
|
||||
for (const [name, input] of [
|
||||
['Grep', 'TODO'],
|
||||
['Edit', 'c.ts'],
|
||||
['Bash', 'pnpm test']
|
||||
] as [string, string][]) {
|
||||
act(() => {
|
||||
vi.advanceTimersByTime(80)
|
||||
rerender({ name, input })
|
||||
})
|
||||
// Three rewrites inside the dwell paint nothing.
|
||||
expect(result.current).toEqual({ toolName: 'Read', toolInput: 'b.ts' })
|
||||
}
|
||||
|
||||
passDwell()
|
||||
// Only the newest lands — the two intermediates are never painted.
|
||||
expect(result.current).toEqual({ toolName: 'Bash', toolInput: 'pnpm test' })
|
||||
})
|
||||
|
||||
it('always lands the final step once the burst stops', () => {
|
||||
const { result, rerender } = renderStep({ name: 'Read', input: 'a.ts' })
|
||||
passDwell()
|
||||
|
||||
act(() => {
|
||||
rerender({ name: 'Read', input: 'b.ts' })
|
||||
})
|
||||
act(() => {
|
||||
vi.advanceTimersByTime(50)
|
||||
rerender({ name: 'Bash', input: 'pnpm build' })
|
||||
})
|
||||
passDwell()
|
||||
|
||||
expect(result.current).toEqual({ toolName: 'Bash', toolInput: 'pnpm build' })
|
||||
})
|
||||
|
||||
it('never pairs one step name with another step input', () => {
|
||||
const seen: string[] = []
|
||||
const { rerender } = renderHook(
|
||||
({ name, input }: StepProps) => {
|
||||
const step = useSettledAgentToolStep(name, input)
|
||||
seen.push(`${step.toolName}|${step.toolInput}`)
|
||||
return step
|
||||
},
|
||||
{ initialProps: { name: 'Read', input: 'a.ts' } }
|
||||
)
|
||||
passDwell()
|
||||
|
||||
act(() => {
|
||||
rerender({ name: 'Bash', input: 'pnpm test' })
|
||||
})
|
||||
act(() => {
|
||||
vi.advanceTimersByTime(40)
|
||||
rerender({ name: 'Grep', input: 'needle' })
|
||||
})
|
||||
passDwell()
|
||||
|
||||
const validPairs = new Set(['Read|a.ts', 'Bash|pnpm test', 'Grep|needle'])
|
||||
expect(seen.every((pair) => validPairs.has(pair))).toBe(true)
|
||||
})
|
||||
|
||||
it('coalesces a transient clear away when a new step arrives inside the dwell', () => {
|
||||
// Why: cursor postToolUseFailure clears the pair for one event; painting that
|
||||
// blank frame is what swings the row between unrelated text (#11075).
|
||||
const { result, rerender } = renderStep({ name: 'Read', input: 'a.ts' })
|
||||
passDwell()
|
||||
|
||||
act(() => {
|
||||
rerender({ name: 'Bash', input: 'pnpm test' })
|
||||
})
|
||||
act(() => {
|
||||
vi.advanceTimersByTime(30)
|
||||
rerender({ name: '', input: '' })
|
||||
})
|
||||
act(() => {
|
||||
vi.advanceTimersByTime(30)
|
||||
rerender({ name: 'Edit', input: 'fix.ts' })
|
||||
})
|
||||
expect(result.current).toEqual({ toolName: 'Bash', toolInput: 'pnpm test' })
|
||||
|
||||
passDwell()
|
||||
expect(result.current).toEqual({ toolName: 'Edit', toolInput: 'fix.ts' })
|
||||
})
|
||||
|
||||
it('lands a clear that outlives the dwell so a finished row drops its tool step', () => {
|
||||
const { result, rerender } = renderStep({ name: 'Read', input: 'a.ts' })
|
||||
passDwell()
|
||||
|
||||
act(() => {
|
||||
rerender({ name: 'Bash', input: 'pnpm test' })
|
||||
})
|
||||
act(() => {
|
||||
vi.advanceTimersByTime(30)
|
||||
rerender({ name: '', input: '' })
|
||||
})
|
||||
passDwell()
|
||||
|
||||
expect(result.current).toEqual({ toolName: '', toolInput: '' })
|
||||
})
|
||||
|
||||
it('keeps repainting on schedule instead of restarting the dwell on every rewrite', () => {
|
||||
const { result, rerender } = renderStep({ name: 'Read', input: 'a.ts' })
|
||||
passDwell()
|
||||
|
||||
// A hook every 50ms for well over one dwell must still land a fresh step.
|
||||
for (let elapsed = 0; elapsed <= AGENT_TOOL_STEP_DWELL_MS * 2; elapsed += 50) {
|
||||
act(() => {
|
||||
vi.advanceTimersByTime(50)
|
||||
rerender({ name: 'Bash', input: `step-${elapsed}` })
|
||||
})
|
||||
}
|
||||
|
||||
expect(result.current.toolName).toBe('Bash')
|
||||
expect(result.current.toolInput).toMatch(/^step-\d+$/)
|
||||
})
|
||||
|
||||
it('clears its pending dwell on unmount', () => {
|
||||
const clearTimeoutSpy = vi.spyOn(globalThis, 'clearTimeout')
|
||||
const { rerender, unmount } = renderStep({ name: 'Read', input: 'a.ts' })
|
||||
passDwell()
|
||||
|
||||
act(() => {
|
||||
rerender({ name: 'Bash', input: 'pnpm test' })
|
||||
})
|
||||
act(() => {
|
||||
vi.advanceTimersByTime(20)
|
||||
rerender({ name: 'Grep', input: 'needle' })
|
||||
})
|
||||
unmount()
|
||||
|
||||
expect(clearTimeoutSpy).toHaveBeenCalled()
|
||||
expect(() => vi.advanceTimersByTime(AGENT_TOOL_STEP_DWELL_MS)).not.toThrow()
|
||||
clearTimeoutSpy.mockRestore()
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,56 @@
|
||||
import { useEffect, useRef, useState } from 'react'
|
||||
|
||||
export type AgentToolStep = {
|
||||
toolName: string
|
||||
toolInput: string
|
||||
}
|
||||
|
||||
/**
|
||||
* Minimum time a tool step stays on screen before the next one may replace it.
|
||||
* STYLEGUIDE.md: feedback that lives under ~100ms "reads as a glitch"; agents
|
||||
* emit several hooks per second, so the row needs a readable floor.
|
||||
*/
|
||||
export const AGENT_TOOL_STEP_DWELL_MS = 800
|
||||
|
||||
/**
|
||||
* Holds an agent's `toolName`/`toolInput` pair steady for a minimum dwell so the
|
||||
* row reads instead of flickering (#11075). Steps arriving inside the dwell are
|
||||
* coalesced to the newest one, so the pair is always atomic and the last value
|
||||
* always lands. Hook events themselves are untouched — this is display only, and
|
||||
* every other consumer still sees each event.
|
||||
*/
|
||||
export function useSettledAgentToolStep(
|
||||
toolName: string,
|
||||
toolInput: string,
|
||||
dwellMs: number = AGENT_TOOL_STEP_DWELL_MS
|
||||
): AgentToolStep {
|
||||
const [settled, setSettled] = useState<AgentToolStep>(() => ({ toolName, toolInput }))
|
||||
const paintedAtRef = useRef<number | null>(null)
|
||||
|
||||
useEffect(() => {
|
||||
// Why: mount already painted a step, so its dwell starts here — otherwise the
|
||||
// first rewrite after mount lands instantly and the row still flickers once.
|
||||
paintedAtRef.current ??= Date.now()
|
||||
if (toolName === settled.toolName && toolInput === settled.toolInput) {
|
||||
return
|
||||
}
|
||||
const paint = (): void => {
|
||||
paintedAtRef.current = Date.now()
|
||||
setSettled({ toolName, toolInput })
|
||||
}
|
||||
// Why: an ABSOLUTE deadline off the last paint. Rescheduling on the next hook
|
||||
// therefore re-aims at the same instant instead of restarting the dwell, which
|
||||
// is what lets this effect own its own cleanup without starving the row.
|
||||
const remainingMs = paintedAtRef.current + dwellMs - Date.now()
|
||||
if (remainingMs <= 0) {
|
||||
paint()
|
||||
return
|
||||
}
|
||||
const timer = setTimeout(paint, remainingMs)
|
||||
return () => {
|
||||
clearTimeout(timer)
|
||||
}
|
||||
}, [toolName, toolInput, dwellMs, settled])
|
||||
|
||||
return settled
|
||||
}
|
||||
Reference in New Issue
Block a user