mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-04-25 00:51:20 +00:00
fix(tui): restore voice handlers + address copilot review
Rebase-artefact cleanup on this branch: - Restore `voice.status` and `voice.transcript` cases in createGatewayEventHandler plus the `voice` / `submission` / `composer.setInput` ctx destructuring. They were added to main in the 58-commit gap that this branch was originally cut behind; dropping them was unintentional. - Rebase the test ctx shape to match main (voice.* fakes, submission.submitRef, composer.setInput) and apply the same segment-anchor test rewrites on top. - Drop the `#14XXX` placeholder from the tool.complete comment; replace with a plain-English rationale. - Rewrite the broken mid-word "pushInlineDiff- Segment" in turnController's dedupe comment to refer to pushInlineDiffSegment and `kind: 'diff'` plainly. - Collapse the filter predicate in recordMessageComplete from a 4-line if/return into one boolean expression — same semantics, reads left-to-right as a single predicate. Copilot review threads resolved: #3134668789, #3134668805, #3134668822.
This commit is contained in:
Brooklyn Nicholson
parent
2258a181f0
commit
4ae5b58cb1
3 changed files with 88 additions and 44 deletions
|
|
@ -15,7 +15,8 @@ const buildCtx = (appended: Msg[]) =>
|
|||
composer: {
|
||||
dequeue: () => undefined,
|
||||
queueEditRef: ref<null | number>(null),
|
||||
sendQueued: vi.fn()
|
||||
sendQueued: vi.fn(),
|
||||
setInput: vi.fn()
|
||||
},
|
||||
gateway: {
|
||||
gw: { request: vi.fn() },
|
||||
|
|
@ -29,6 +30,9 @@ const buildCtx = (appended: Msg[]) =>
|
|||
resumeById: vi.fn(),
|
||||
setCatalog: vi.fn()
|
||||
},
|
||||
submission: {
|
||||
submitRef: { current: vi.fn() }
|
||||
},
|
||||
system: {
|
||||
bellOnComplete: false,
|
||||
sys: vi.fn()
|
||||
|
|
@ -38,6 +42,11 @@ const buildCtx = (appended: Msg[]) =>
|
|||
panel: (title: string, sections: any[]) =>
|
||||
appended.push({ kind: 'panel', panelData: { sections, title }, role: 'system', text: '' }),
|
||||
setHistoryItems: vi.fn()
|
||||
},
|
||||
voice: {
|
||||
setProcessing: vi.fn(),
|
||||
setRecording: vi.fn(),
|
||||
setVoiceEnabled: vi.fn()
|
||||
}
|
||||
}) as any
|
||||
|
||||
|
|
@ -148,36 +157,30 @@ describe('createGatewayEventHandler', () => {
|
|||
const onEvent = createGatewayEventHandler(buildCtx(appended))
|
||||
const diff = '\u001b[31m--- a/foo.ts\u001b[0m\n\u001b[32m+++ b/foo.ts\u001b[0m\n@@\n-old\n+new'
|
||||
const cleaned = '--- a/foo.ts\n+++ b/foo.ts\n@@\n-old\n+new'
|
||||
const block = `\`\`\`diff\n${cleaned}\n\`\`\``
|
||||
|
||||
// Narration → tool → tool-complete → more narration → message-complete.
|
||||
// The diff MUST land between the two narration segments, not tacked
|
||||
// onto the final one.
|
||||
onEvent({ payload: { text: 'Editing the file' }, type: 'message.delta' } as any)
|
||||
onEvent({
|
||||
payload: { context: 'foo.ts', name: 'patch', tool_id: 'tool-1' },
|
||||
type: 'tool.start'
|
||||
} as any)
|
||||
onEvent({
|
||||
payload: { inline_diff: diff, summary: 'patched', tool_id: 'tool-1' },
|
||||
type: 'tool.complete'
|
||||
} as any)
|
||||
onEvent({ payload: { context: 'foo.ts', name: 'patch', tool_id: 'tool-1' }, type: 'tool.start' } as any)
|
||||
onEvent({ payload: { inline_diff: diff, summary: 'patched', tool_id: 'tool-1' }, type: 'tool.complete' } as any)
|
||||
|
||||
// Diff is already committed to segmentMessages as its own segment —
|
||||
// nothing is "pending" anymore. The pre-tool narration is also flushed.
|
||||
// Diff is already committed to segmentMessages as its own segment.
|
||||
expect(appended).toHaveLength(0)
|
||||
expect(turnController.segmentMessages).toEqual([
|
||||
{ role: 'assistant', text: 'Editing the file' },
|
||||
{ kind: 'diff', role: 'assistant', text: `\`\`\`diff\n${cleaned}\n\`\`\`` }
|
||||
{ kind: 'diff', role: 'assistant', text: block }
|
||||
])
|
||||
|
||||
onEvent({ payload: { text: 'patch applied' }, type: 'message.complete' } as any)
|
||||
|
||||
// Three messages in the transcript, in order: pre-tool narration →
|
||||
// diff (kind='diff' so MessageLine gives it blank-line breathing room)
|
||||
// → post-tool narration. The final message does NOT contain a diff.
|
||||
// Three transcript messages: pre-tool narration → diff (kind='diff',
|
||||
// so MessageLine gives it blank-line breathing room) → post-tool
|
||||
// narration. The final message does NOT contain a diff.
|
||||
expect(appended).toHaveLength(3)
|
||||
expect(appended[0]?.text).toBe('Editing the file')
|
||||
expect(appended[1]).toMatchObject({ kind: 'diff', text: `\`\`\`diff\n${cleaned}\n\`\`\`` })
|
||||
expect(appended[1]).toMatchObject({ kind: 'diff', text: block })
|
||||
expect(appended[2]?.text).toBe('patch applied')
|
||||
expect(appended[2]?.text).not.toContain('```diff')
|
||||
})
|
||||
|
|
@ -188,10 +191,7 @@ describe('createGatewayEventHandler', () => {
|
|||
const cleaned = '--- a/foo.ts\n+++ b/foo.ts\n@@\n-old\n+new'
|
||||
const assistantText = `Done. Here's the inline diff:\n\n\`\`\`diff\n${cleaned}\n\`\`\``
|
||||
|
||||
onEvent({
|
||||
payload: { inline_diff: cleaned, summary: 'patched', tool_id: 'tool-1' },
|
||||
type: 'tool.complete'
|
||||
} as any)
|
||||
onEvent({ payload: { inline_diff: cleaned, summary: 'patched', tool_id: 'tool-1' }, type: 'tool.complete' } as any)
|
||||
onEvent({ payload: { text: assistantText }, type: 'message.complete' } as any)
|
||||
|
||||
// Only the final message — diff-only segment dropped so we don't
|
||||
|
|
@ -206,13 +206,10 @@ describe('createGatewayEventHandler', () => {
|
|||
const onEvent = createGatewayEventHandler(buildCtx(appended))
|
||||
const raw = ' \u001b[33m┊ review diff\u001b[0m\n--- a/foo.ts\n+++ b/foo.ts\n@@\n-old\n+new'
|
||||
|
||||
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)
|
||||
|
||||
// diff segment first, final narration second
|
||||
// diff segment first (kind='diff'), final narration second
|
||||
expect(appended).toHaveLength(2)
|
||||
expect(appended[0]?.kind).toBe('diff')
|
||||
expect(appended[0]?.text).not.toContain('┊ review diff')
|
||||
|
|
@ -226,10 +223,7 @@ describe('createGatewayEventHandler', () => {
|
|||
const inlineDiff = '--- a/foo.ts\n+++ b/foo.ts\n@@\n-old\n+new'
|
||||
const assistantText = 'Done. Clean swap:\n\n```diff\n-old\n+new\n```'
|
||||
|
||||
onEvent({
|
||||
payload: { inline_diff: inlineDiff, summary: 'patched', tool_id: 'tool-1' },
|
||||
type: 'tool.complete'
|
||||
} as any)
|
||||
onEvent({ payload: { inline_diff: inlineDiff, summary: 'patched', tool_id: 'tool-1' }, type: 'tool.complete' } as any)
|
||||
onEvent({ payload: { text: assistantText }, type: 'message.complete' } as any)
|
||||
|
||||
expect(appended).toHaveLength(1)
|
||||
|
|
@ -248,8 +242,9 @@ describe('createGatewayEventHandler', () => {
|
|||
} as any)
|
||||
onEvent({ payload: { text: 'done' }, type: 'message.complete' } as any)
|
||||
|
||||
// Two segments: diff block (kind='diff', no tool row), final narration
|
||||
// (tool row belongs here since pendingSegmentTools carries across the flush).
|
||||
// Two segments: the diff block (kind='diff', no tool row) and the final
|
||||
// narration (tool row belongs here since pendingSegmentTools carries
|
||||
// across the flushStreamingSegment call).
|
||||
expect(appended).toHaveLength(2)
|
||||
expect(appended[0]?.kind).toBe('diff')
|
||||
expect(appended[0]?.text).toContain('```diff')
|
||||
|
|
|
|||
|
|
@ -51,6 +51,9 @@ export function createGatewayEventHandler(ctx: GatewayEventHandlerContext): (ev:
|
|||
const { STARTUP_RESUME_ID, newSession, resumeById, setCatalog } = ctx.session
|
||||
const { bellOnComplete, stdout, sys } = ctx.system
|
||||
const { appendMessage, panel, setHistoryItems } = ctx.transcript
|
||||
const { setInput } = ctx.composer
|
||||
const { submitRef } = ctx.submission
|
||||
const { setProcessing: setVoiceProcessing, setRecording: setVoiceRecording, setVoiceEnabled } = ctx.voice
|
||||
|
||||
let pendingThinkingStatus = ''
|
||||
let thinkingStatusTimer: null | ReturnType<typeof setTimeout> = null
|
||||
|
|
@ -261,6 +264,57 @@ export function createGatewayEventHandler(ctx: GatewayEventHandlerContext): (ev:
|
|||
return
|
||||
}
|
||||
|
||||
case 'voice.status': {
|
||||
// Continuous VAD loop reports its internal state so the status bar
|
||||
// can show listening / transcribing / idle without polling.
|
||||
const state = String(ev.payload?.state ?? '')
|
||||
|
||||
if (state === 'listening') {
|
||||
setVoiceRecording(true)
|
||||
setVoiceProcessing(false)
|
||||
} else if (state === 'transcribing') {
|
||||
setVoiceRecording(false)
|
||||
setVoiceProcessing(true)
|
||||
} else {
|
||||
setVoiceRecording(false)
|
||||
setVoiceProcessing(false)
|
||||
}
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
case 'voice.transcript': {
|
||||
// CLI parity: the 3-strikes silence detector flipped off automatically.
|
||||
// Mirror that on the UI side and tell the user why the mode is off.
|
||||
if (ev.payload?.no_speech_limit) {
|
||||
setVoiceEnabled(false)
|
||||
setVoiceRecording(false)
|
||||
setVoiceProcessing(false)
|
||||
sys('voice: no speech detected 3 times, continuous mode stopped')
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
const text = String(ev.payload?.text ?? '').trim()
|
||||
|
||||
if (!text) {
|
||||
return
|
||||
}
|
||||
|
||||
// CLI parity: _pending_input.put(transcript) unconditionally feeds
|
||||
// the transcript to the agent as its next turn — draft handling
|
||||
// doesn't apply because voice-mode users are speaking, not typing.
|
||||
//
|
||||
// We can't branch on composer input from inside a setInput updater
|
||||
// (React strict mode double-invokes it, duplicating the submit).
|
||||
// Just clear + defer submit so the cleared input is committed before
|
||||
// submit reads it.
|
||||
setInput('')
|
||||
setTimeout(() => submitRef.current(text), 0)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
case 'gateway.start_timeout': {
|
||||
const { cwd, python } = ev.payload ?? {}
|
||||
const trace = python || cwd ? ` · ${String(python || '')} ${String(cwd || '')}`.trim() : ''
|
||||
|
|
@ -331,12 +385,11 @@ export function createGatewayEventHandler(ctx: GatewayEventHandlerContext): (ev:
|
|||
return
|
||||
}
|
||||
|
||||
// Anchor the diff to the segment where the edit actually happened
|
||||
// (between the narration that preceded the tool call and whatever
|
||||
// the agent streams afterwards). The previous end-merge put the
|
||||
// diff at the bottom of the final message even when the edit fired
|
||||
// mid-turn, which read as "the agent wrote this after saying
|
||||
// that" — misleading, and dropped for #14XXX.
|
||||
// Anchor the diff to where the edit happened in the turn — between
|
||||
// the narration that preceded the tool call and whatever the agent
|
||||
// streams afterwards. The previous end-merge put the diff at the
|
||||
// bottom of the final message even when the edit fired mid-turn,
|
||||
// which read as "the agent wrote this after saying that".
|
||||
turnController.pushInlineDiffSegment(inlineDiffText)
|
||||
|
||||
return
|
||||
|
|
|
|||
|
|
@ -282,18 +282,14 @@ class TurnController {
|
|||
// Drop diff-only segments the agent is about to narrate in the final
|
||||
// reply. Without this, a closing "here's the diff …" message would
|
||||
// render two stacked copies of the same patch. Only touches segments
|
||||
// whose entire body is a ```diff``` fence emitted by pushInlineDiff-
|
||||
// Segment — real assistant narration stays put.
|
||||
// with `kind: 'diff'` emitted by pushInlineDiffSegment — real
|
||||
// assistant narration stays put.
|
||||
const finalHasOwnDiffFence = /```(?:diff|patch)\b/i.test(finalText)
|
||||
|
||||
const segments = this.segmentMessages.filter(msg => {
|
||||
const body = diffSegmentBody(msg)
|
||||
|
||||
if (body === null) {
|
||||
return true
|
||||
}
|
||||
|
||||
return !finalHasOwnDiffFence && !finalText.includes(body)
|
||||
return body === null || (!finalHasOwnDiffFence && !finalText.includes(body))
|
||||
})
|
||||
|
||||
const finalMessages = [...segments]
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue