mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-04-25 00:51:20 +00:00
Merge pull request #15321 from NousResearch/bb/tui-inline-diff-tooltrail-order
fix(tui): render tool trail before anchored inline diffs
This commit is contained in:
commit
93ddff53e3
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