mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-31 19:16:29 +00:00
fix(desktop): measure reasoning per block instead of per turn
The timer registry hands every caller of a key the same origin, and every reasoning block in a turn was keyed `reasoning:<messageId>`. So the second and third blocks measured from the first one's start and each reported the running total as its own duration — the "6s, 6s, 16s" down a single turn. Key per block, and move the measurement into `useMeasuredDuration`, which keeps the number beside the origin that produced it. The thread virtualizes, so the component that watched a block finish is usually gone by the time anyone scrolls back to read it; component state forgot the duration on unmount and the row fell back to having none. A block that genuinely was never watched running — history from an earlier app session, or reasoning that arrived already complete — still has no duration to report, and now says "Thought" rather than sitting in the present tense at a turn that ended. Also drops the run summary's aggregate +N/−M: a run can no longer contain a file edit, so it was always zero. Each edit carries its own count on its card.
This commit is contained in:
Brooklyn Nicholson
parent
f08b0e5606
commit
b1f4b95761
8 changed files with 282 additions and 120 deletions
|
|
@ -9,7 +9,7 @@ import { type ComponentProps, type FC, type ReactNode, useEffect, useRef, useSta
|
|||
import { ClarifyTool } from '@/components/assistant-ui/clarify-tool'
|
||||
import { MarkdownText, MarkdownTextContent } from '@/components/assistant-ui/markdown-text'
|
||||
import { ToolFallback, ToolGroupSlot } from '@/components/assistant-ui/tool/fallback'
|
||||
import { formatElapsed, useElapsedSeconds } from '@/components/chat/activity-timer'
|
||||
import { formatElapsed, useElapsedSeconds, useMeasuredDuration } from '@/components/chat/activity-timer'
|
||||
import { ActivityTimerText } from '@/components/chat/activity-timer-text'
|
||||
import { GeneratedImage } from '@/components/chat/generated-image-result'
|
||||
import { SCAFFOLD_LABEL_CLASS, SCAFFOLD_META_CLASS, ScaffoldRow } from '@/components/chat/scaffold-row'
|
||||
|
|
@ -57,7 +57,9 @@ const ThinkingDisclosure: FC<{
|
|||
children: ReactNode
|
||||
messageRunning?: boolean
|
||||
pending?: boolean
|
||||
timerKey?: string
|
||||
// Required: the block's duration is remembered against this key, so a
|
||||
// component that mounts after the block finished can still report it.
|
||||
timerKey: string
|
||||
}> = ({ children, messageRunning = false, pending = false, timerKey }) => {
|
||||
const { t } = useI18n()
|
||||
// `null` = no explicit user toggle yet, defer to the streaming default.
|
||||
|
|
@ -66,6 +68,7 @@ const ThinkingDisclosure: FC<{
|
|||
// explicit toggle wins from then on.
|
||||
const [userOpen, setUserOpen] = useState<boolean | null>(null)
|
||||
const elapsed = useElapsedSeconds(pending, timerKey)
|
||||
const thoughtFor = useMeasuredDuration(pending, timerKey)
|
||||
const scrollRef = useRef<HTMLDivElement | null>(null)
|
||||
const contentRef = useRef<HTMLDivElement | null>(null)
|
||||
const enterRef = useEnterAnimation(messageRunning, timerKey)
|
||||
|
|
@ -73,28 +76,11 @@ const ThinkingDisclosure: FC<{
|
|||
const open = userOpen ?? pending
|
||||
const isPreview = pending && userOpen === null
|
||||
|
||||
// How long the model thought is only knowable by having watched it happen —
|
||||
// nothing in the persisted turn records it. So freeze the number the moment
|
||||
// this block finishes in front of us, and stay quiet on a rehydrated turn
|
||||
// rather than reporting whatever a timer that never ran would say.
|
||||
const [watching, setWatching] = useState(false)
|
||||
const [thoughtFor, setThoughtFor] = useState<null | number>(null)
|
||||
|
||||
useEffect(() => {
|
||||
if (pending) {
|
||||
setWatching(true)
|
||||
} else if (watching) {
|
||||
setWatching(false)
|
||||
setThoughtFor(elapsed)
|
||||
}
|
||||
}, [elapsed, pending, watching])
|
||||
|
||||
// Three ways a finished block can report itself. With a measured duration it
|
||||
// says so, unless the timer's whole seconds round it to "0s" — accurate and
|
||||
// useless — in which case it just says it was quick. With no duration at all
|
||||
// (rehydrated history, or reasoning that arrived already complete so we never
|
||||
// saw it run) it still has to read as finished; a turn that ended must not go
|
||||
// on saying "Thinking".
|
||||
// it still has to read as finished; a turn that ended must not go on saying
|
||||
// "Thinking".
|
||||
let thoughtLabel = t.assistant.thread.thinking
|
||||
|
||||
if (!pending) {
|
||||
|
|
@ -213,7 +199,15 @@ const ReasoningAccordionGroup: FC<{ children?: ReactNode; endIndex: number; star
|
|||
}
|
||||
|
||||
return (
|
||||
<ThinkingDisclosure messageRunning={messageRunning} pending={pending} timerKey={`reasoning:${messageId}`}>
|
||||
// Keyed per block, not per message: the timer registry hands every caller
|
||||
// of a key the same origin, so a turn that thinks three separate times used
|
||||
// to measure the second and third blocks from the first one's start and
|
||||
// report the running total as each block's duration.
|
||||
<ThinkingDisclosure
|
||||
messageRunning={messageRunning}
|
||||
pending={pending}
|
||||
timerKey={`reasoning:${messageId}:${startIndex}`}
|
||||
>
|
||||
{children}
|
||||
</ThinkingDisclosure>
|
||||
)
|
||||
|
|
|
|||
|
|
@ -606,7 +606,8 @@ describe('assistant-ui streaming renderer', () => {
|
|||
const { container } = render(<ReasoningHarness />)
|
||||
const ui = within(container)
|
||||
|
||||
fireEvent.click(ui.getByRole('button', { name: /thinking/i }))
|
||||
// Settled, so the header is past tense — a running block says "Thinking".
|
||||
fireEvent.click(ui.getByRole('button', { name: /thought/i }))
|
||||
|
||||
expect(container.querySelector('[data-slot="aui_reasoning-text"]')?.textContent).toBe(
|
||||
'The user is asking what this file is.'
|
||||
|
|
|
|||
|
|
@ -30,7 +30,6 @@ import { ZoomableImage } from '@/components/chat/zoomable-image'
|
|||
import { Button } from '@/components/ui/button'
|
||||
import { Codicon } from '@/components/ui/codicon'
|
||||
import { CopyButton } from '@/components/ui/copy-button'
|
||||
import { DiffCount } from '@/components/ui/diff-count'
|
||||
import { DisclosureCaret } from '@/components/ui/disclosure-caret'
|
||||
import { FadeText } from '@/components/ui/fade-text'
|
||||
import { FileTypeIcon } from '@/components/ui/file-type-icon'
|
||||
|
|
@ -68,7 +67,7 @@ import {
|
|||
type ToolStatus,
|
||||
type ToolTitleAction
|
||||
} from './fallback-model'
|
||||
import { isToolCallPart, type RunSummary, summarizeToolRun } from './run-summary'
|
||||
import { isToolCallPart, summarizeToolRun } from './run-summary'
|
||||
|
||||
// `true` when a ToolEntry is rendered inside an embedding wrapper that owns
|
||||
// the per-row chrome (timer / preview). The flat ToolGroupSlot sets this
|
||||
|
|
@ -784,10 +783,9 @@ function ToolRunTicker({ children }: { children: ReactNode }) {
|
|||
)
|
||||
}
|
||||
|
||||
// The one grey line that stands in for a run of tool calls once it has
|
||||
// settled — "Edited wiring.tsx, explored 3 files +6 −4". While the run is
|
||||
// live the same line narrates it in the present tense and the rows stay
|
||||
// visible below, so nothing the user is waiting on hides behind a chevron.
|
||||
// The one grey line that stands in for a run of tool calls — "Explored 3
|
||||
// files, ran 5 commands". Live, it narrates in the present tense above the
|
||||
// ticker and offers no toggle, since there is nothing settled to unfold yet.
|
||||
function ToolRunHeader({
|
||||
live,
|
||||
onToggle,
|
||||
|
|
@ -797,15 +795,14 @@ function ToolRunHeader({
|
|||
live: boolean
|
||||
onToggle?: () => void
|
||||
open: boolean
|
||||
summary: RunSummary
|
||||
summary: string
|
||||
}) {
|
||||
return (
|
||||
<div data-tool-summary="">
|
||||
<ScaffoldRow onToggle={onToggle} open={open}>
|
||||
<FadeText className={cn(SCAFFOLD_LABEL_CLASS, 'truncate')}>
|
||||
{live ? <span className="shimmer">{summary.text}</span> : summary.text}
|
||||
{live ? <span className="shimmer">{summary}</span> : summary}
|
||||
</FadeText>
|
||||
<DiffCount added={summary.added} className="text-[0.625rem]" removed={summary.removed} />
|
||||
</ScaffoldRow>
|
||||
</div>
|
||||
)
|
||||
|
|
@ -817,7 +814,7 @@ interface ToolRunState {
|
|||
live: boolean
|
||||
/** A call still awaiting a result that could be the one blocking on approval. */
|
||||
pendingApprovalTool: boolean
|
||||
summary: RunSummary
|
||||
summary: string
|
||||
}
|
||||
|
||||
// assistant-ui compares selector results with `Object.is` and calls the
|
||||
|
|
@ -830,6 +827,7 @@ function useToolRun(startIndex: number, endIndex: number): ToolRunState {
|
|||
return useAuiState(state => {
|
||||
const parts = state.message.parts
|
||||
const tools = parts.slice(Math.max(0, startIndex), endIndex + 1).filter(isToolCallPart)
|
||||
|
||||
// A missing result only means "still working" while this run is the tail of
|
||||
// a running message — the same qualification ToolEntry puts on a row's
|
||||
// pending state. A turn that ends, or an agent that moves on to later
|
||||
|
|
@ -838,6 +836,7 @@ function useToolRun(startIndex: number, endIndex: number): ToolRunState {
|
|||
// live run deliberately withholds its toggle.
|
||||
const live =
|
||||
selectMessageRunning(state) && endIndex >= parts.length - 1 && tools.some(tool => tool.result === undefined)
|
||||
|
||||
const signature = tools
|
||||
.map(tool => `${tool.toolCallId}:${tool.result === undefined ? 0 : 1}`)
|
||||
.concat(String(live))
|
||||
|
|
|
|||
|
|
@ -6,61 +6,48 @@ function tool(toolName: string, args: Record<string, unknown> = {}, result?: unk
|
|||
return { args, result, toolCallId: `${toolName}-${Math.random()}`, toolName }
|
||||
}
|
||||
|
||||
const edited = (path: string, diff = '') => tool('write_file', { path }, { path, inline_diff: diff })
|
||||
const read = (path: string) => tool('read_file', { path }, { content: '' })
|
||||
const searched = (query: string) => tool('search_files', { query }, { hits: [] })
|
||||
const ran = (command: string) => tool('terminal', { command }, { exit_code: 0 })
|
||||
|
||||
const settled = (tools: ToolCallLike[]) => summarizeToolRun(tools, false)
|
||||
const running = (tools: ToolCallLike[]) => summarizeToolRun(tools, true)
|
||||
|
||||
// A run only ever holds ephemeral activity: reads, searches, commands. File
|
||||
// edits and other cards are split out before a run is summarized, so there is
|
||||
// no "Edited …" clause to test here — that work shows as its own diff card.
|
||||
describe('summarizeToolRun', () => {
|
||||
it('names a lone edit and counts the rest', () => {
|
||||
expect(settled([edited('src/use-preview-routing.ts'), read('a.ts'), read('b.ts'), read('c.ts')]).text).toBe(
|
||||
'Edited use-preview-routing.ts, explored 3 files'
|
||||
it('names a lone target and counts the rest', () => {
|
||||
expect(settled([searched('toolRuns'), read('a.ts'), read('b.ts'), read('c.ts')])).toBe('Explored 4 files')
|
||||
})
|
||||
|
||||
it('orders clauses explore then run regardless of call order', () => {
|
||||
expect(settled([ran('ls'), read('a.ts'), read('b.ts'), ran('pwd'), ran('id')])).toBe(
|
||||
'Explored 2 files, ran 3 commands'
|
||||
)
|
||||
})
|
||||
|
||||
it('orders clauses edit, explore, run regardless of call order', () => {
|
||||
expect(
|
||||
settled([ran('ls'), read('a.ts'), edited('src/attachments.tsx'), read('b.ts'), ran('pwd'), ran('id')]).text
|
||||
).toBe('Edited attachments.tsx, explored 2 files, ran 3 commands')
|
||||
})
|
||||
|
||||
it('counts commands rather than naming them once they have run', () => {
|
||||
expect(settled([ran('git status')]).text).toBe('Ran 1 command')
|
||||
expect(settled([read('status.ts'), ran('a'), ran('b'), ran('c'), ran('d'), ran('e')]).text).toBe(
|
||||
expect(settled([ran('git status')])).toBe('Ran 1 command')
|
||||
expect(settled([read('status.ts'), ran('a'), ran('b'), ran('c'), ran('d'), ran('e')])).toBe(
|
||||
'Explored status.ts, ran 5 commands'
|
||||
)
|
||||
})
|
||||
|
||||
it('counts a multi-file edit', () => {
|
||||
expect(settled([edited('a.tsx'), edited('b.tsx'), read('c.ts')]).text).toBe('Edited 2 files, explored c.ts')
|
||||
})
|
||||
|
||||
it('puts the running category in the present tense and leaves the rest past', () => {
|
||||
expect(running([edited('a.tsx'), tool('write_file', { path: 'b.tsx' }), ran('x'), ran('y')]).text).toBe(
|
||||
'Editing 2 files, ran 2 commands'
|
||||
expect(running([read('a.ts'), tool('read_file', { path: 'b.ts' }), ran('x'), ran('y')])).toBe(
|
||||
'Exploring 2 files, ran 2 commands'
|
||||
)
|
||||
})
|
||||
|
||||
it('names the command that is still running', () => {
|
||||
expect(running([tool('terminal', { command: 'npm run typecheck' })]).text).toMatch(/^Running /)
|
||||
expect(running([tool('terminal', { command: 'npm run typecheck' })])).toMatch(/^Running /)
|
||||
})
|
||||
|
||||
// A turn can end — or the agent can simply move on — with a call that never
|
||||
// got a result. The run is history at that point and has to read as history,
|
||||
// or it narrates work that stopped happening and never offers its toggle.
|
||||
it('reads a run the turn left unresolved as finished', () => {
|
||||
expect(settled([read('a.ts'), tool('search_files', { query: 'toolRuns' })]).text).toBe('Explored 2 files')
|
||||
})
|
||||
|
||||
it('sums diff stats across the edits in the run', () => {
|
||||
const summary = settled([edited('a.tsx', '--- a\n+++ b\n+one\n+two\n-old'), edited('b.tsx', '+three'), ran('ls')])
|
||||
|
||||
expect(summary).toMatchObject({ added: 3, removed: 1 })
|
||||
})
|
||||
|
||||
it('reports no diff stats for a run that changed nothing', () => {
|
||||
expect(settled([read('a.ts'), ran('ls')])).toMatchObject({ added: 0, removed: 0 })
|
||||
expect(settled([read('a.ts'), tool('search_files', { query: 'toolRuns' })])).toBe('Explored 2 files')
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -1,13 +1,6 @@
|
|||
import { summarizeShellCommand } from '@/lib/summarize-command'
|
||||
|
||||
import {
|
||||
countDiffLineStats,
|
||||
fileEditBasename,
|
||||
firstStringField,
|
||||
inlineDiffFromResult,
|
||||
isFileEditTool,
|
||||
parseMaybeObject
|
||||
} from './fallback-model'
|
||||
import { fileEditBasename, firstStringField, isFileEditTool, parseMaybeObject } from './fallback-model'
|
||||
|
||||
/**
|
||||
* The little a summary needs from a tool call, stated structurally so both
|
||||
|
|
@ -27,12 +20,6 @@ export function isToolCallPart<T extends { type: string }>(part: T): part is Ext
|
|||
|
||||
type RunCategory = 'delegate' | 'edit' | 'explore' | 'other' | 'run'
|
||||
|
||||
export interface RunSummary {
|
||||
added: number
|
||||
removed: number
|
||||
text: string
|
||||
}
|
||||
|
||||
// Clause order is fixed so the same run always reads the same way, whichever
|
||||
// category happens to be live.
|
||||
const CATEGORY_ORDER: readonly RunCategory[] = ['edit', 'explore', 'run', 'delegate', 'other']
|
||||
|
|
@ -101,24 +88,6 @@ function toolTarget(tool: ToolCallLike): string {
|
|||
return path ? fileEditBasename(path) : firstStringField(args, ['query', 'url'])
|
||||
}
|
||||
|
||||
function diffStats(tools: readonly ToolCallLike[]): { added: number; removed: number } {
|
||||
let added = 0
|
||||
let removed = 0
|
||||
|
||||
for (const tool of tools) {
|
||||
if (!isFileEditTool(tool.toolName)) {
|
||||
continue
|
||||
}
|
||||
|
||||
const stats = countDiffLineStats(inlineDiffFromResult(tool.result))
|
||||
|
||||
added += stats.added
|
||||
removed += stats.removed
|
||||
}
|
||||
|
||||
return { added, removed }
|
||||
}
|
||||
|
||||
/**
|
||||
* One clause per category. A category holding a single thing says what it was
|
||||
* ("Edited wiring.tsx"); anything else counts ("explored 3 files"). A settled
|
||||
|
|
@ -143,16 +112,20 @@ function lowerFirst(text: string): string {
|
|||
|
||||
/**
|
||||
* Collapse a run of tool calls into the single grey line that stands in for it
|
||||
* — "Edited wiring.tsx, explored 3 files, ran 5 commands". The category holding
|
||||
* the still-running tool speaks in the present tense so a live run reads as
|
||||
* work in progress rather than work already done.
|
||||
* — "Explored 3 files, ran 5 commands". The category holding the still-running
|
||||
* tool speaks in the present tense so a live run reads as work in progress
|
||||
* rather than work already done.
|
||||
*
|
||||
* Whether the run is `live` is the caller's to say, not something readable off
|
||||
* the calls: a call can be left without a result by a turn that ended or an
|
||||
* agent that moved on, and a run like that has to read as finished rather than
|
||||
* narrate work that stopped happening.
|
||||
*
|
||||
* A run only ever holds ephemeral activity — file edits and other cards are
|
||||
* split out before this sees them (`splitRunItems`), so there is no aggregate
|
||||
* diff to report here; each edit carries its own +N/−M on its card.
|
||||
*/
|
||||
export function summarizeToolRun(tools: readonly ToolCallLike[], live: boolean): RunSummary {
|
||||
export function summarizeToolRun(tools: readonly ToolCallLike[], live: boolean): string {
|
||||
const running = live ? tools.find(isPending) : undefined
|
||||
const liveCategory = running ? toolCategory(running.toolName) : null
|
||||
|
||||
|
|
@ -175,8 +148,5 @@ export function summarizeToolRun(tools: readonly ToolCallLike[], live: boolean):
|
|||
return group ? [clause(category, group, category === liveCategory)] : []
|
||||
})
|
||||
|
||||
return {
|
||||
...diffStats(tools),
|
||||
text: clauses.map((text, index) => (index === 0 ? text : lowerFirst(text))).join(', ')
|
||||
}
|
||||
return clauses.map((text, index) => (index === 0 ? text : lowerFirst(text))).join(', ')
|
||||
}
|
||||
|
|
|
|||
|
|
@ -184,8 +184,8 @@ function failedOnlyMessage(): ThreadMessage {
|
|||
} as ThreadMessage
|
||||
}
|
||||
|
||||
// Two settled tools in a row — an edit plus a read — so the run earns a
|
||||
// summary line and collapses.
|
||||
// Two settled activity calls in a row, so the run earns a summary line and
|
||||
// collapses behind it.
|
||||
function settledRunMessage(): ThreadMessage {
|
||||
return {
|
||||
id: 'assistant-settled-run',
|
||||
|
|
@ -193,7 +193,60 @@ function settledRunMessage(): ThreadMessage {
|
|||
content: [
|
||||
{
|
||||
type: 'tool-call',
|
||||
toolCallId: 'patch-1',
|
||||
toolCallId: 'read-2',
|
||||
toolName: 'read_file',
|
||||
args: { path: '/repo/src/wiring.tsx' },
|
||||
argsText: JSON.stringify({ path: '/repo/src/wiring.tsx' }),
|
||||
result: { content: 'export const Wiring = () => null' }
|
||||
},
|
||||
{
|
||||
type: 'tool-call',
|
||||
toolCallId: 'term-3',
|
||||
toolName: 'terminal',
|
||||
args: { command: 'ls -la' },
|
||||
argsText: JSON.stringify({ command: 'ls -la' }),
|
||||
result: { exit_code: 0, stdout: 'wiring.tsx' }
|
||||
}
|
||||
],
|
||||
status: { type: 'complete', reason: 'stop' },
|
||||
createdAt,
|
||||
metadata: {
|
||||
unstable_state: null,
|
||||
unstable_annotations: [],
|
||||
unstable_data: [],
|
||||
steps: [],
|
||||
custom: {}
|
||||
}
|
||||
} as ThreadMessage
|
||||
}
|
||||
|
||||
// Activity, an edit, then more activity — all adjacent, so assistant-ui hands
|
||||
// the whole stretch over as one group. The edit is the deliverable and has to
|
||||
// survive that as its own card.
|
||||
function editBetweenRunsMessage(): ThreadMessage {
|
||||
return {
|
||||
id: 'assistant-edit-between-runs',
|
||||
role: 'assistant',
|
||||
content: [
|
||||
{
|
||||
type: 'tool-call',
|
||||
toolCallId: 'read-5',
|
||||
toolName: 'read_file',
|
||||
args: { path: '/repo/src/a.ts' },
|
||||
argsText: JSON.stringify({ path: '/repo/src/a.ts' }),
|
||||
result: { content: 'a' }
|
||||
},
|
||||
{
|
||||
type: 'tool-call',
|
||||
toolCallId: 'search-3',
|
||||
toolName: 'search_files',
|
||||
args: { query: 'toolRuns' },
|
||||
argsText: JSON.stringify({ query: 'toolRuns' }),
|
||||
result: { hits: [] }
|
||||
},
|
||||
{
|
||||
type: 'tool-call',
|
||||
toolCallId: 'patch-2',
|
||||
toolName: 'patch',
|
||||
args: { path: '/repo/src/wiring.tsx' },
|
||||
argsText: JSON.stringify({ path: '/repo/src/wiring.tsx' }),
|
||||
|
|
@ -201,11 +254,19 @@ function settledRunMessage(): ThreadMessage {
|
|||
},
|
||||
{
|
||||
type: 'tool-call',
|
||||
toolCallId: 'read-2',
|
||||
toolCallId: 'read-6',
|
||||
toolName: 'read_file',
|
||||
args: { path: '/repo/src/status.tsx' },
|
||||
argsText: JSON.stringify({ path: '/repo/src/status.tsx' }),
|
||||
result: { content: 'export const Status = () => null' }
|
||||
args: { path: '/repo/src/b.ts' },
|
||||
argsText: JSON.stringify({ path: '/repo/src/b.ts' }),
|
||||
result: { content: 'b' }
|
||||
},
|
||||
{
|
||||
type: 'tool-call',
|
||||
toolCallId: 'term-4',
|
||||
toolName: 'terminal',
|
||||
args: { command: 'ls' },
|
||||
argsText: JSON.stringify({ command: 'ls' }),
|
||||
result: { exit_code: 0 }
|
||||
}
|
||||
],
|
||||
status: { type: 'complete', reason: 'stop' },
|
||||
|
|
@ -326,18 +387,17 @@ afterEach(() => {
|
|||
})
|
||||
|
||||
describe('settled tool run', () => {
|
||||
it('collapses to a summary line naming the work and its diff', async () => {
|
||||
it('collapses to a summary line naming the work', async () => {
|
||||
const { container } = render(<GroupHarness message={settledRunMessage()} />)
|
||||
|
||||
expect(await screen.findByText('Edited wiring.tsx, explored status.tsx')).toBeTruthy()
|
||||
expect(await screen.findByText('Explored wiring.tsx, ran 1 command')).toBeTruthy()
|
||||
expect(container.querySelectorAll('[data-tool-row]')).toHaveLength(0)
|
||||
expect(screen.getByText('1', { selector: '.text-\\(--ui-green\\) *' })).toBeTruthy()
|
||||
})
|
||||
|
||||
it('expands to the underlying rows when the summary is clicked', async () => {
|
||||
const { container } = render(<GroupHarness message={settledRunMessage()} />)
|
||||
|
||||
fireEvent.click(await screen.findByText('Edited wiring.tsx, explored status.tsx'))
|
||||
fireEvent.click(await screen.findByText('Explored wiring.tsx, ran 1 command'))
|
||||
|
||||
await waitFor(() => {
|
||||
expect(container.querySelectorAll('[data-tool-row]').length).toBeGreaterThan(0)
|
||||
|
|
@ -354,6 +414,31 @@ describe('settled tool run', () => {
|
|||
})
|
||||
})
|
||||
|
||||
// A diff is what the user reviews, so it is never what gets summarized away.
|
||||
// It stays on screen at the point in the turn where it happened, with the
|
||||
// activity either side of it collapsing around it.
|
||||
describe('a file edit among ordinary activity', () => {
|
||||
it('stays visible between the two runs it interrupted', async () => {
|
||||
const { container } = render(<GroupHarness message={editBetweenRunsMessage()} />)
|
||||
|
||||
await screen.findByText('Explored 2 files')
|
||||
|
||||
const shape = [...container.querySelectorAll('[data-tool-summary],[data-tool-row]')].map(node =>
|
||||
node.hasAttribute('data-tool-summary') ? 'summary' : 'row'
|
||||
)
|
||||
|
||||
expect(shape).toEqual(['summary', 'row', 'summary'])
|
||||
})
|
||||
|
||||
it('keeps the diff itself on screen rather than behind the summary', async () => {
|
||||
const { container } = render(<GroupHarness message={editBetweenRunsMessage()} />)
|
||||
|
||||
await waitFor(() => {
|
||||
expect(container.querySelector('[data-tool-row][data-file-edit]')).not.toBeNull()
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe('live tool run', () => {
|
||||
it('keeps its rows on screen instead of hiding them behind the summary', async () => {
|
||||
const { container } = render(<GroupHarness message={groupedPendingMessage()} />)
|
||||
|
|
@ -425,7 +510,7 @@ describe('flat tool list approval surfacing', () => {
|
|||
|
||||
const dismiss = await screen.findByLabelText('Dismiss')
|
||||
|
||||
expect(container.querySelectorAll('[data-slot="tool-block"]').length).toBeGreaterThan(1)
|
||||
expect(container.querySelectorAll('[data-slot="tool-block"]').length).toBeGreaterThan(0)
|
||||
|
||||
fireEvent.click(dismiss)
|
||||
|
||||
|
|
@ -449,13 +534,13 @@ describe('flat tool list approval surfacing', () => {
|
|||
|
||||
first.unmount()
|
||||
|
||||
const { container } = render(<GroupHarness message={completedOnlyMessage()} />)
|
||||
render(<GroupHarness message={completedOnlyMessage()} />)
|
||||
|
||||
// The row is the only thing this message renders, so staying dismissed
|
||||
// means nothing comes back — including its dismiss control.
|
||||
await waitFor(() => {
|
||||
expect(container.querySelectorAll('[data-slot="tool-block"]').length).toBeGreaterThan(0)
|
||||
expect(screen.queryByLabelText('Dismiss')).toBeNull()
|
||||
})
|
||||
|
||||
expect(screen.queryByLabelText('Dismiss')).toBeNull()
|
||||
})
|
||||
|
||||
it('lets failed tool rows be dismissed', async () => {
|
||||
|
|
|
|||
|
|
@ -1,7 +1,7 @@
|
|||
import { act, render, screen } from '@testing-library/react'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import { __resetElapsedTimerRegistryForTests, useElapsedSeconds } from './activity-timer'
|
||||
import { __resetElapsedTimerRegistryForTests, useElapsedSeconds, useMeasuredDuration } from './activity-timer'
|
||||
|
||||
function Probe({ active, since, timerKey }: { active: boolean; since?: number; timerKey?: string }) {
|
||||
const elapsed = useElapsedSeconds(active, timerKey, since)
|
||||
|
|
@ -9,6 +9,12 @@ function Probe({ active, since, timerKey }: { active: boolean; since?: number; t
|
|||
return <span data-testid="elapsed">{elapsed}</span>
|
||||
}
|
||||
|
||||
function DurationProbe({ active, timerKey }: { active: boolean; timerKey: string }) {
|
||||
const measured = useMeasuredDuration(active, timerKey)
|
||||
|
||||
return <span data-testid="measured">{measured === null ? 'unknown' : measured}</span>
|
||||
}
|
||||
|
||||
describe('useElapsedSeconds', () => {
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers()
|
||||
|
|
@ -67,3 +73,84 @@ describe('useElapsedSeconds', () => {
|
|||
expect(screen.getByTestId('elapsed').textContent).toBe('0')
|
||||
})
|
||||
})
|
||||
|
||||
describe('useMeasuredDuration', () => {
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers()
|
||||
vi.setSystemTime(new Date('2026-01-01T00:00:00.000Z'))
|
||||
__resetElapsedTimerRegistryForTests()
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
vi.useRealTimers()
|
||||
__resetElapsedTimerRegistryForTests()
|
||||
})
|
||||
|
||||
it('has nothing to report until it has watched something finish', () => {
|
||||
render(<DurationProbe active timerKey="reasoning:m1:0" />)
|
||||
|
||||
act(() => {
|
||||
vi.advanceTimersByTime(4_000)
|
||||
})
|
||||
|
||||
expect(screen.getByTestId('measured').textContent).toBe('unknown')
|
||||
})
|
||||
|
||||
it('freezes the duration at the moment the thing finishes', () => {
|
||||
const { rerender } = render(<DurationProbe active timerKey="reasoning:m1:0" />)
|
||||
|
||||
act(() => {
|
||||
vi.advanceTimersByTime(4_000)
|
||||
})
|
||||
|
||||
rerender(<DurationProbe active={false} timerKey="reasoning:m1:0" />)
|
||||
|
||||
expect(screen.getByTestId('measured').textContent).toBe('4')
|
||||
|
||||
// Time keeps passing; the block is over and its duration must not creep up
|
||||
// with it.
|
||||
act(() => {
|
||||
vi.advanceTimersByTime(9_000)
|
||||
})
|
||||
|
||||
expect(screen.getByTestId('measured').textContent).toBe('4')
|
||||
})
|
||||
|
||||
// The thread virtualizes, so the component that watched a block finish is
|
||||
// usually gone by the time anyone scrolls back to read it.
|
||||
it('remembers the duration for a component that mounts after the fact', () => {
|
||||
const first = render(<DurationProbe active timerKey="reasoning:m1:0" />)
|
||||
|
||||
act(() => {
|
||||
vi.advanceTimersByTime(6_000)
|
||||
})
|
||||
|
||||
first.rerender(<DurationProbe active={false} timerKey="reasoning:m1:0" />)
|
||||
first.unmount()
|
||||
|
||||
render(<DurationProbe active={false} timerKey="reasoning:m1:0" />)
|
||||
|
||||
expect(screen.getByTestId('measured').textContent).toBe('6')
|
||||
})
|
||||
|
||||
it('measures each key separately', () => {
|
||||
const first = render(<DurationProbe active timerKey="reasoning:m1:0" />)
|
||||
|
||||
act(() => {
|
||||
vi.advanceTimersByTime(3_000)
|
||||
})
|
||||
|
||||
first.rerender(<DurationProbe active={false} timerKey="reasoning:m1:0" />)
|
||||
first.unmount()
|
||||
|
||||
const second = render(<DurationProbe active timerKey="reasoning:m1:7" />)
|
||||
|
||||
act(() => {
|
||||
vi.advanceTimersByTime(2_000)
|
||||
})
|
||||
|
||||
second.rerender(<DurationProbe active={false} timerKey="reasoning:m1:7" />)
|
||||
|
||||
expect(screen.getByTestId('measured').textContent).toBe('2')
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -5,6 +5,10 @@ import { useEffect, useRef, useState } from 'react'
|
|||
// anonymous timers (no key) start fresh each mount.
|
||||
const startedAtByKey = new Map<string, number>()
|
||||
|
||||
// Durations of things that have already finished, kept beside the origins that
|
||||
// measured them. See `useMeasuredDuration`.
|
||||
const durationByKey = new Map<string, number>()
|
||||
|
||||
function startedAt(key?: string): number {
|
||||
if (!key) {
|
||||
return Date.now()
|
||||
|
|
@ -71,6 +75,41 @@ export function useElapsedSeconds(active = true, timerKey?: string, since?: numb
|
|||
return elapsed
|
||||
}
|
||||
|
||||
/**
|
||||
* How long something took, measured by watching it finish and remembered
|
||||
* afterwards. `null` until it has been watched at least once.
|
||||
*
|
||||
* Some durations exist nowhere but in the watching. A reasoning block is the
|
||||
* case this was written for: the persisted turn records the text the model
|
||||
* thought, never how long it spent thinking it, so the only way to know is to
|
||||
* have been there. Watching alone isn't enough either — the thread virtualizes,
|
||||
* so the component that saw a block finish is usually gone by the time anyone
|
||||
* scrolls back to read it. Keeping the number in the same registry as the
|
||||
* timer's origin lets it outlive the component that measured it.
|
||||
*
|
||||
* A block that was never watched running — history loaded from an earlier app
|
||||
* session, or reasoning that arrived already complete — has no duration and
|
||||
* says so, rather than reporting a timer that never ran.
|
||||
*/
|
||||
export function useMeasuredDuration(active: boolean, timerKey: string): null | number {
|
||||
const elapsed = useElapsedSeconds(active, timerKey)
|
||||
const [watching, setWatching] = useState(false)
|
||||
const [measured, setMeasured] = useState<null | number>(() => durationByKey.get(timerKey) ?? null)
|
||||
|
||||
useEffect(() => {
|
||||
if (active) {
|
||||
setWatching(true)
|
||||
} else if (watching) {
|
||||
setWatching(false)
|
||||
durationByKey.set(timerKey, elapsed)
|
||||
setMeasured(elapsed)
|
||||
}
|
||||
}, [active, elapsed, timerKey, watching])
|
||||
|
||||
return measured
|
||||
}
|
||||
|
||||
export function __resetElapsedTimerRegistryForTests() {
|
||||
startedAtByKey.clear()
|
||||
durationByKey.clear()
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue