mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-04-25 00:51:20 +00:00
fix(tui): give inline_diff segments blank-line breathing room
Visual polish on top of the segment-anchor change: diff blocks were
butting up against the narration around them. Tag diff-only segments
with `kind: 'diff'` (extended on Msg) and give them `marginTop={1}` +
`marginBottom={1}` in MessageLine, matching the spacing we already
use for user messages. Also swaps the regex-based `diffSegmentBody`
check for an explicit `kind === 'diff'` guard so the dedupe path is
clearer.
This commit is contained in:
parent
11b2942f16
commit
2258a181f0
4 changed files with 25 additions and 19 deletions
|
|
@ -165,19 +165,19 @@ describe('createGatewayEventHandler', () => {
|
||||||
// Diff is already committed to segmentMessages as its own segment —
|
// Diff is already committed to segmentMessages as its own segment —
|
||||||
// nothing is "pending" anymore. The pre-tool narration is also flushed.
|
// nothing is "pending" anymore. The pre-tool narration is also flushed.
|
||||||
expect(appended).toHaveLength(0)
|
expect(appended).toHaveLength(0)
|
||||||
expect(turnController.segmentMessages.map(m => m.text)).toEqual([
|
expect(turnController.segmentMessages).toEqual([
|
||||||
'Editing the file',
|
{ role: 'assistant', text: 'Editing the file' },
|
||||||
`\`\`\`diff\n${cleaned}\n\`\`\``
|
{ kind: 'diff', role: 'assistant', text: `\`\`\`diff\n${cleaned}\n\`\`\`` }
|
||||||
])
|
])
|
||||||
|
|
||||||
onEvent({ payload: { text: 'patch applied' }, type: 'message.complete' } as any)
|
onEvent({ payload: { text: 'patch applied' }, type: 'message.complete' } as any)
|
||||||
|
|
||||||
// Three messages in the transcript, in order: pre-tool narration →
|
// Three messages in the transcript, in order: pre-tool narration →
|
||||||
// diff → post-tool narration. The final message does NOT contain
|
// diff (kind='diff' so MessageLine gives it blank-line breathing room)
|
||||||
// `diff` content.
|
// → post-tool narration. The final message does NOT contain a diff.
|
||||||
expect(appended).toHaveLength(3)
|
expect(appended).toHaveLength(3)
|
||||||
expect(appended[0]?.text).toBe('Editing the file')
|
expect(appended[0]?.text).toBe('Editing the file')
|
||||||
expect(appended[1]?.text).toBe(`\`\`\`diff\n${cleaned}\n\`\`\``)
|
expect(appended[1]).toMatchObject({ kind: 'diff', text: `\`\`\`diff\n${cleaned}\n\`\`\`` })
|
||||||
expect(appended[2]?.text).toBe('patch applied')
|
expect(appended[2]?.text).toBe('patch applied')
|
||||||
expect(appended[2]?.text).not.toContain('```diff')
|
expect(appended[2]?.text).not.toContain('```diff')
|
||||||
})
|
})
|
||||||
|
|
@ -214,6 +214,7 @@ describe('createGatewayEventHandler', () => {
|
||||||
|
|
||||||
// diff segment first, final narration second
|
// diff segment first, final narration second
|
||||||
expect(appended).toHaveLength(2)
|
expect(appended).toHaveLength(2)
|
||||||
|
expect(appended[0]?.kind).toBe('diff')
|
||||||
expect(appended[0]?.text).not.toContain('┊ review diff')
|
expect(appended[0]?.text).not.toContain('┊ review diff')
|
||||||
expect(appended[0]?.text).toContain('--- a/foo.ts')
|
expect(appended[0]?.text).toContain('--- a/foo.ts')
|
||||||
expect(appended[1]?.text).toBe('done')
|
expect(appended[1]?.text).toBe('done')
|
||||||
|
|
@ -247,9 +248,10 @@ 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: diff block (no tool row), final narration (tool row
|
// Two segments: diff block (kind='diff', no tool row), final narration
|
||||||
// belongs here since pendingSegmentTools carries across the flush).
|
// (tool row belongs here since pendingSegmentTools carries across the flush).
|
||||||
expect(appended).toHaveLength(2)
|
expect(appended).toHaveLength(2)
|
||||||
|
expect(appended[0]?.kind).toBe('diff')
|
||||||
expect(appended[0]?.text).toContain('```diff')
|
expect(appended[0]?.text).toContain('```diff')
|
||||||
expect(appended[0]?.tools ?? []).toEqual([])
|
expect(appended[0]?.tools ?? []).toEqual([])
|
||||||
expect(appended[1]?.text).toBe('done')
|
expect(appended[1]?.text).toBe('done')
|
||||||
|
|
|
||||||
|
|
@ -19,17 +19,16 @@ const INTERRUPT_COOLDOWN_MS = 1500
|
||||||
const ACTIVITY_LIMIT = 8
|
const ACTIVITY_LIMIT = 8
|
||||||
const TRAIL_LIMIT = 8
|
const TRAIL_LIMIT = 8
|
||||||
|
|
||||||
// Matches segments produced by pushInlineDiffSegment — a bare ```diff fence
|
// Extracts the raw patch from a diff-only segment produced by
|
||||||
// wrapping the raw patch, no surrounding prose. Used at message.complete to
|
// pushInlineDiffSegment. Used at message.complete to dedupe against final
|
||||||
// dedupe against final assistant text that narrates the same patch.
|
// assistant text that narrates the same patch. Returns null for anything
|
||||||
const DIFF_SEGMENT_RE = /^```diff\n([\s\S]*?)\n```$/
|
// else so real assistant narration never gets touched.
|
||||||
|
|
||||||
const diffSegmentBody = (msg: Msg): null | string => {
|
const diffSegmentBody = (msg: Msg): null | string => {
|
||||||
if (msg.role !== 'assistant' || msg.tools?.length) {
|
if (msg.kind !== 'diff') {
|
||||||
return null
|
return null
|
||||||
}
|
}
|
||||||
|
|
||||||
const m = msg.text.match(DIFF_SEGMENT_RE)
|
const m = msg.text.match(/^```diff\n([\s\S]*?)\n```$/)
|
||||||
|
|
||||||
return m ? m[1]! : null
|
return m ? m[1]! : null
|
||||||
}
|
}
|
||||||
|
|
@ -226,7 +225,7 @@ class TurnController {
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
this.segmentMessages = [...this.segmentMessages, { role: 'assistant', text: block }]
|
this.segmentMessages = [...this.segmentMessages, { kind: 'diff', role: 'assistant', text: block }]
|
||||||
patchTurnState({ streamSegments: this.segmentMessages })
|
patchTurnState({ streamSegments: this.segmentMessages })
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -81,11 +81,16 @@ export const MessageLine = memo(function MessageLine({
|
||||||
return <Text {...(body ? { color: body } : {})}>{msg.text}</Text>
|
return <Text {...(body ? { color: body } : {})}>{msg.text}</Text>
|
||||||
})()
|
})()
|
||||||
|
|
||||||
|
// Diff segments (emitted by pushInlineDiffSegment between narration
|
||||||
|
// segments) need a blank line on both sides so the patch doesn't butt up
|
||||||
|
// against the prose around it.
|
||||||
|
const isDiffSegment = msg.kind === 'diff'
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<Box
|
<Box
|
||||||
flexDirection="column"
|
flexDirection="column"
|
||||||
marginBottom={msg.role === 'user' ? 1 : 0}
|
marginBottom={msg.role === 'user' || isDiffSegment ? 1 : 0}
|
||||||
marginTop={msg.role === 'user' || msg.kind === 'slash' ? 1 : 0}
|
marginTop={msg.role === 'user' || msg.kind === 'slash' || isDiffSegment ? 1 : 0}
|
||||||
>
|
>
|
||||||
{showDetails && (
|
{showDetails && (
|
||||||
<Box flexDirection="column" marginBottom={1}>
|
<Box flexDirection="column" marginBottom={1}>
|
||||||
|
|
|
||||||
|
|
@ -102,7 +102,7 @@ export interface ClarifyReq {
|
||||||
|
|
||||||
export interface Msg {
|
export interface Msg {
|
||||||
info?: SessionInfo
|
info?: SessionInfo
|
||||||
kind?: 'intro' | 'panel' | 'slash' | 'trail'
|
kind?: 'diff' | 'intro' | 'panel' | 'slash' | 'trail'
|
||||||
panelData?: PanelData
|
panelData?: PanelData
|
||||||
role: Role
|
role: Role
|
||||||
text: string
|
text: string
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue