mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-04-25 00:51:20 +00:00
fix(tui): render tool trail before anchored inline diffs
Inline diff segments were anchored relative to assistant narration, but the turn details pane still rendered after streamSegments. On completion that put the diff before the tool telemetry that produced it. When a turn has anchored diff segments, commit the accumulated thinking/tool trail as a pre-diff trail message, then render the diff and final summary.
This commit is contained in:
parent
6f1eed3968
commit
de596aca1c
3 changed files with 64 additions and 32 deletions
|
|
@ -175,14 +175,16 @@ describe('createGatewayEventHandler', () => {
|
||||||
|
|
||||||
onEvent({ payload: { text: 'patch applied' }, type: 'message.complete' } as any)
|
onEvent({ payload: { text: 'patch applied' }, type: 'message.complete' } as any)
|
||||||
|
|
||||||
// Three transcript messages: pre-tool narration → diff (kind='diff',
|
// Four transcript messages: pre-tool narration → tool trail → diff
|
||||||
// so MessageLine gives it blank-line breathing room) → post-tool
|
// (kind='diff', so MessageLine gives it blank-line breathing room) →
|
||||||
// narration. The final message does NOT contain a diff.
|
// post-tool narration. The final message does NOT contain a diff.
|
||||||
expect(appended).toHaveLength(3)
|
expect(appended).toHaveLength(4)
|
||||||
expect(appended[0]?.text).toBe('Editing the file')
|
expect(appended[0]?.text).toBe('Editing the file')
|
||||||
expect(appended[1]).toMatchObject({ kind: 'diff', text: block })
|
expect(appended[1]).toMatchObject({ kind: 'trail' })
|
||||||
expect(appended[2]?.text).toBe('patch applied')
|
expect(appended[1]?.tools?.[0]).toContain('Patch')
|
||||||
expect(appended[2]?.text).not.toContain('```diff')
|
expect(appended[2]).toMatchObject({ kind: 'diff', text: block })
|
||||||
|
expect(appended[3]?.text).toBe('patch applied')
|
||||||
|
expect(appended[3]?.text).not.toContain('```diff')
|
||||||
})
|
})
|
||||||
|
|
||||||
it('drops the diff segment when the final assistant text narrates the same diff', () => {
|
it('drops the diff segment when the final assistant text narrates the same diff', () => {
|
||||||
|
|
@ -209,12 +211,13 @@ describe('createGatewayEventHandler', () => {
|
||||||
onEvent({ payload: { inline_diff: raw, summary: 'patched', tool_id: 'tool-1' }, type: 'tool.complete' } as any)
|
onEvent({ payload: { inline_diff: raw, summary: 'patched', tool_id: 'tool-1' }, type: 'tool.complete' } as any)
|
||||||
onEvent({ payload: { text: 'done' }, type: 'message.complete' } as any)
|
onEvent({ payload: { text: 'done' }, type: 'message.complete' } as any)
|
||||||
|
|
||||||
// diff segment first (kind='diff'), final narration second
|
// Tool trail first, then diff segment (kind='diff'), then final narration.
|
||||||
expect(appended).toHaveLength(2)
|
expect(appended).toHaveLength(3)
|
||||||
expect(appended[0]?.kind).toBe('diff')
|
expect(appended[0]?.kind).toBe('trail')
|
||||||
expect(appended[0]?.text).not.toContain('┊ review diff')
|
expect(appended[1]?.kind).toBe('diff')
|
||||||
expect(appended[0]?.text).toContain('--- a/foo.ts')
|
expect(appended[1]?.text).not.toContain('┊ review diff')
|
||||||
expect(appended[1]?.text).toBe('done')
|
expect(appended[1]?.text).toContain('--- a/foo.ts')
|
||||||
|
expect(appended[2]?.text).toBe('done')
|
||||||
})
|
})
|
||||||
|
|
||||||
it('drops the diff segment when assistant writes its own ```diff fence', () => {
|
it('drops the diff segment when assistant writes its own ```diff fence', () => {
|
||||||
|
|
@ -242,16 +245,17 @@ describe('createGatewayEventHandler', () => {
|
||||||
} as any)
|
} as any)
|
||||||
onEvent({ payload: { text: 'done' }, type: 'message.complete' } as any)
|
onEvent({ payload: { text: 'done' }, type: 'message.complete' } as any)
|
||||||
|
|
||||||
// Two segments: the diff block (kind='diff', no tool row) and the final
|
// Tool row is now placed before the diff, so telemetry does not render
|
||||||
// narration (tool row belongs here since pendingSegmentTools carries
|
// below the patch that came from that tool.
|
||||||
// across the flushStreamingSegment call).
|
expect(appended).toHaveLength(3)
|
||||||
expect(appended).toHaveLength(2)
|
expect(appended[0]?.kind).toBe('trail')
|
||||||
expect(appended[0]?.kind).toBe('diff')
|
expect(appended[0]?.tools?.[0]).toContain('Review Diff')
|
||||||
expect(appended[0]?.text).toContain('```diff')
|
expect(appended[0]?.tools?.[0]).not.toContain('--- a/foo.ts')
|
||||||
expect(appended[0]?.tools ?? []).toEqual([])
|
expect(appended[1]?.kind).toBe('diff')
|
||||||
expect(appended[1]?.text).toBe('done')
|
expect(appended[1]?.text).toContain('```diff')
|
||||||
expect(appended[1]?.tools?.[0]).toContain('Review Diff')
|
expect(appended[1]?.tools ?? []).toEqual([])
|
||||||
expect(appended[1]?.tools?.[0]).not.toContain('--- a/foo.ts')
|
expect(appended[2]?.text).toBe('done')
|
||||||
|
expect(appended[2]?.tools ?? []).toEqual([])
|
||||||
})
|
})
|
||||||
|
|
||||||
it('shows setup panel for missing provider startup error', () => {
|
it('shows setup panel for missing provider startup error', () => {
|
||||||
|
|
|
||||||
|
|
@ -33,6 +33,12 @@ const diffSegmentBody = (msg: Msg): null | string => {
|
||||||
return m ? m[1]! : null
|
return m ? m[1]! : null
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const insertBeforeFirstDiff = (segments: Msg[], msg: Msg): Msg[] => {
|
||||||
|
const index = segments.findIndex(segment => segment.kind === 'diff')
|
||||||
|
|
||||||
|
return index < 0 ? [...segments, msg] : [...segments.slice(0, index), msg, ...segments.slice(index)]
|
||||||
|
}
|
||||||
|
|
||||||
export interface InterruptDeps {
|
export interface InterruptDeps {
|
||||||
appendMessage: (msg: Msg) => void
|
appendMessage: (msg: Msg) => void
|
||||||
gw: { request: <T = unknown>(method: string, params?: Record<string, unknown>) => Promise<T> }
|
gw: { request: <T = unknown>(method: string, params?: Record<string, unknown>) => Promise<T> }
|
||||||
|
|
@ -292,16 +298,30 @@ class TurnController {
|
||||||
return body === null || (!finalHasOwnDiffFence && !finalText.includes(body))
|
return body === null || (!finalHasOwnDiffFence && !finalText.includes(body))
|
||||||
})
|
})
|
||||||
|
|
||||||
const finalMessages = [...segments]
|
const hasDiffSegment = segments.some(msg => msg.kind === 'diff')
|
||||||
|
const detailsBelongBeforeDiff = hasDiffSegment && (tools.length > 0 || Boolean(savedReasoning))
|
||||||
|
const finalMessages = detailsBelongBeforeDiff
|
||||||
|
? insertBeforeFirstDiff(segments, {
|
||||||
|
kind: 'trail',
|
||||||
|
role: 'system',
|
||||||
|
text: '',
|
||||||
|
thinking: savedReasoning || undefined,
|
||||||
|
thinkingTokens: savedReasoning ? savedReasoningTokens : undefined,
|
||||||
|
toolTokens: savedToolTokens || undefined,
|
||||||
|
...(tools.length && { tools })
|
||||||
|
})
|
||||||
|
: [...segments]
|
||||||
|
|
||||||
if (finalText) {
|
if (finalText) {
|
||||||
finalMessages.push({
|
finalMessages.push({
|
||||||
role: 'assistant',
|
role: 'assistant',
|
||||||
text: finalText,
|
text: finalText,
|
||||||
thinking: savedReasoning || undefined,
|
...(!detailsBelongBeforeDiff && {
|
||||||
thinkingTokens: savedReasoning ? savedReasoningTokens : undefined,
|
thinking: savedReasoning || undefined,
|
||||||
toolTokens: savedToolTokens || undefined,
|
thinkingTokens: savedReasoning ? savedReasoningTokens : undefined,
|
||||||
...(tools.length && { tools })
|
toolTokens: savedToolTokens || undefined,
|
||||||
|
...(tools.length && { tools })
|
||||||
|
})
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -31,11 +31,20 @@ export const MessageLine = memo(function MessageLine({
|
||||||
const thinkingMode = sectionMode('thinking', detailsMode, sections)
|
const thinkingMode = sectionMode('thinking', detailsMode, sections)
|
||||||
const toolsMode = sectionMode('tools', detailsMode, sections)
|
const toolsMode = sectionMode('tools', detailsMode, sections)
|
||||||
const activityMode = sectionMode('activity', detailsMode, sections)
|
const activityMode = sectionMode('activity', detailsMode, sections)
|
||||||
|
const thinking = msg.thinking?.trim() ?? ''
|
||||||
|
|
||||||
if (msg.kind === 'trail' && msg.tools?.length) {
|
if (msg.kind === 'trail' && (msg.tools?.length || thinking)) {
|
||||||
return toolsMode !== 'hidden' || activityMode !== 'hidden' ? (
|
return thinkingMode !== 'hidden' || toolsMode !== 'hidden' || activityMode !== 'hidden' ? (
|
||||||
<Box flexDirection="column" marginTop={1}>
|
<Box flexDirection="column" marginTop={1}>
|
||||||
<ToolTrail detailsMode={detailsMode} sections={sections} t={t} trail={msg.tools} />
|
<ToolTrail
|
||||||
|
detailsMode={detailsMode}
|
||||||
|
reasoning={thinking}
|
||||||
|
reasoningTokens={msg.thinkingTokens}
|
||||||
|
sections={sections}
|
||||||
|
t={t}
|
||||||
|
toolTokens={msg.toolTokens}
|
||||||
|
trail={msg.tools ?? []}
|
||||||
|
/>
|
||||||
</Box>
|
</Box>
|
||||||
) : null
|
) : null
|
||||||
}
|
}
|
||||||
|
|
@ -61,7 +70,6 @@ export const MessageLine = memo(function MessageLine({
|
||||||
}
|
}
|
||||||
|
|
||||||
const { body, glyph, prefix } = ROLE[msg.role](t)
|
const { body, glyph, prefix } = ROLE[msg.role](t)
|
||||||
const thinking = msg.thinking?.trim() ?? ''
|
|
||||||
|
|
||||||
const showDetails =
|
const showDetails =
|
||||||
(toolsMode !== 'hidden' && Boolean(msg.tools?.length)) ||
|
(toolsMode !== 'hidden' && Boolean(msg.tools?.length)) ||
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue