Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion apps/desktop/src/app/session/hooks/use-hermes-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ import {
setCurrentPersonality,
setCurrentReasoningEffort,
setCurrentServiceTier,
setIntroPersonality
setIntroPersonality,
setKeepInterimAssistantMessages
} from '@/store/session'
import { applyAutoSpeakFromConfig } from '@/store/voice-prefs'

Expand Down Expand Up @@ -87,6 +88,9 @@ export function useHermesConfig({ activeSessionIdRef, refreshProjectBranch }: He

setVoiceMaxRecordingSeconds(recordingLimit(config.voice?.max_recording_seconds))
setSttEnabled(config.stt?.enabled !== false)
// Default true — only an explicit `false` opts into the lean (collapse
// interim narration) view, matching the backend display-config default.
setKeepInterimAssistantMessages(config.display?.interim_assistant_messages !== false)
applyAutoSpeakFromConfig(config)
} catch {
// Config is nice-to-have; chat still works without it.
Expand Down
33 changes: 5 additions & 28 deletions apps/desktop/src/app/session/hooks/use-message-stream/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,15 @@ import {
type ChatMessagePart,
chatMessageText,
type GatewayEventPayload,
mergeFinalAssistantText,
reasoningPart,
renderMediaTags,
upsertToolPart
} from '@/lib/chat-messages'
import {
dedupeGeneratedImageEchoesInParts,
generatedImageEchoSources,
stripGeneratedImageEchoes
} from '@/lib/generated-images'
import { dedupeGeneratedImageEchoesInParts } from '@/lib/generated-images'
import { parseTodos } from '@/lib/todos'
import { dispatchNativeNotification } from '@/store/native-notifications'
import { $keepInterimAssistantMessages } from '@/store/session'
import { broadcastSessionsChanged } from '@/store/session-sync'
import { upsertSubagent } from '@/store/subagents'
import { setSessionTodos } from '@/store/todos'
Expand Down Expand Up @@ -354,28 +352,7 @@ export function useMessageStream({
const streamId = state.streamId
const finalText = renderMediaTags(text).trim()
const completionError = completionErrorText(finalText)
const normalize = (value: string) => value.replace(/\s+/g, ' ').trim()

const replaceTextPart = (parts: ChatMessagePart[]) => {
const visibleFinalText = stripGeneratedImageEchoes(finalText, generatedImageEchoSources(parts)).trim()
const dedupeReference = normalize(visibleFinalText)

const kept = parts.filter(part => {
if (part.type === 'text') {
return false
}

if (part.type !== 'reasoning' || !dedupeReference) {
return true
}

const r = normalize(part.text)

return !(r && (dedupeReference.startsWith(r) || r.startsWith(dedupeReference)))
})

return visibleFinalText ? [...kept, assistantTextPart(visibleFinalText)] : kept
}
const keepInterim = $keepInterimAssistantMessages.get()

const completeMessage = (message: ChatMessage): ChatMessage =>
completionError
Expand All @@ -387,7 +364,7 @@ export function useMessageStream({
}
: {
...message,
parts: replaceTextPart(message.parts),
parts: mergeFinalAssistantText(message.parts, finalText, keepInterim),
pending: false
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
import { QueryClient } from '@tanstack/react-query'
import { act, cleanup, render, waitFor } from '@testing-library/react'
import { useEffect, useRef } from 'react'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'

import type { ClientSessionState } from '@/app/types'
import { createClientSessionState } from '@/lib/chat-runtime'
import { $keepInterimAssistantMessages, setKeepInterimAssistantMessages } from '@/store/session'
import type { RpcEvent } from '@/types/hermes'

import { useMessageStream } from './index'

// Integration test for the interim-narration fix: drive the REAL hook through a
// multi-step turn (delta → tool → delta → complete) via handleGatewayEvent and
// assert the mid-turn narration survives on the finalized message parts. This
// exercises the wiring (the $keepInterimAssistantMessages atom + its read in
// completeAssistantMessage) that the pure mergeFinalAssistantText unit tests in
// lib/chat-messages.test.ts cannot cover.

const SID = 'session-1'

let handleEvent: ((event: RpcEvent) => void) | null = null
let readState: (() => ClientSessionState | undefined) | null = null

function Harness() {
const activeSessionIdRef = useRef<string | null>(SID)
const sessionStateByRuntimeIdRef = useRef(new Map<string, ClientSessionState>())
const queryClientRef = useRef(new QueryClient())

const stream = useMessageStream({
activeSessionIdRef,
hydrateFromStoredSession: vi.fn(async () => undefined),
queryClient: queryClientRef.current,
refreshHermesConfig: vi.fn(async () => undefined),
refreshSessions: vi.fn(async () => undefined),
sessionStateByRuntimeIdRef,
updateSessionState: (sessionId, updater) => {
const current = sessionStateByRuntimeIdRef.current.get(sessionId) ?? createClientSessionState()
const next = updater(current)
sessionStateByRuntimeIdRef.current.set(sessionId, next)

return next
}
})

useEffect(() => {
handleEvent = stream.handleGatewayEvent
readState = () => sessionStateByRuntimeIdRef.current.get(SID)
}, [stream.handleGatewayEvent])

return null
}

async function mountStream() {
render(<Harness />)
await waitFor(() => expect(handleEvent).not.toBeNull())
}

function fire(event: RpcEvent) {
act(() => handleEvent!(event))
}

// Deltas are queued but flushed synchronously at tool.start and message.complete
// boundaries (flushQueuedDeltas is called in those handlers), so a multi-step
// turn resolves deterministically without advancing any timer.
function runMultiStepTurn() {
fire({ payload: {}, session_id: SID, type: 'message.start' })
fire({ payload: { text: 'Let me check the repo.' }, session_id: SID, type: 'message.delta' })
fire({ payload: { name: 'terminal', tool_id: 'tc-1' }, session_id: SID, type: 'tool.start' })
fire({
payload: { name: 'terminal', tool_id: 'tc-1', result: { output: 'no ci', exit_code: 0 } },
session_id: SID,
type: 'tool.complete'
})
fire({ payload: { text: 'No CI here.' }, session_id: SID, type: 'message.delta' })
fire({ payload: { text: 'No CI here.' }, session_id: SID, type: 'message.complete' })
}

function textPartTexts(state: ClientSessionState | undefined): string[] {
const message = state?.messages.at(-1)

return (message?.parts ?? [])
.filter((p): p is { type: 'text'; text: string } => p.type === 'text')
.map(p => p.text)
}

function partTypes(state: ClientSessionState | undefined): string[] {
return (state?.messages.at(-1)?.parts ?? []).map(p => p.type)
}

describe('useMessageStream interim narration on completion', () => {
const initialKeepInterim = $keepInterimAssistantMessages.get()

beforeEach(() => {
handleEvent = null
readState = null
})

afterEach(() => {
cleanup()
setKeepInterimAssistantMessages(initialKeepInterim)
vi.restoreAllMocks()
})

it('keeps mid-turn narration when interim_assistant_messages is on (default)', async () => {
setKeepInterimAssistantMessages(true)
await mountStream()

runMultiStepTurn()

expect(partTypes(readState!())).toEqual(['text', 'tool-call', 'text'])
expect(textPartTexts(readState!())).toEqual(['Let me check the repo.', 'No CI here.'])
})

it('collapses to the final message when interim_assistant_messages is off', async () => {
setKeepInterimAssistantMessages(false)
await mountStream()

runMultiStepTurn()

// Lean mode: the interim "Let me check the repo." narration is dropped; only
// the tool part and the final text survive.
expect(partTypes(readState!())).toEqual(['tool-call', 'text'])
expect(textPartTexts(readState!())).toEqual(['No CI here.'])
})
})
78 changes: 78 additions & 0 deletions apps/desktop/src/lib/chat-messages.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
appendAssistantTextPart,
appendReasoningPart,
chatMessageText,
mergeFinalAssistantText,
preserveLocalAssistantErrors,
renderMediaTags,
toChatMessages,
Expand Down Expand Up @@ -785,3 +786,80 @@ describe('upsertToolPart', () => {
})
})
})

describe('mergeFinalAssistantText', () => {
const textParts = (parts: ChatMessagePart[]) =>
parts.filter((p): p is Extract<ChatMessagePart, { type: 'text' }> => p.type === 'text').map(p => p.text)

describe('keepInterim = true (default / comprehensive)', () => {
it('keeps interim narration and upgrades the trailing segment in place', () => {
// text → tool → text, final text restates only the LAST segment.
let parts: ChatMessagePart[] = appendAssistantTextPart([], 'Let me check the repo.')
parts = upsertToolPart(parts, { name: 'terminal', tool_id: 'tc-1' }, 'running')
parts = appendAssistantTextPart(parts, 'No CI here.')

const merged = mergeFinalAssistantText(parts, 'No CI here.', true)

expect(merged.map(p => p.type)).toEqual(['text', 'tool-call', 'text'])
expect(textParts(merged)).toEqual(['Let me check the repo.', 'No CI here.'])
})

it('appends the final text as a new part when it does not restate the last streamed segment', () => {
let parts: ChatMessagePart[] = appendAssistantTextPart([], 'Working on it.')
parts = upsertToolPart(parts, { name: 'terminal', tool_id: 'tc-1' }, 'running')

// No trailing text part after the tool; final text is genuinely new.
const merged = mergeFinalAssistantText(parts, 'All done.', true)

expect(merged.map(p => p.type)).toEqual(['text', 'tool-call', 'text'])
expect(textParts(merged)).toEqual(['Working on it.', 'All done.'])
})

it('drops reasoning the final text restates but keeps interim narration', () => {
let parts: ChatMessagePart[] = appendReasoningPart([], 'The answer is 42.')
parts = appendAssistantTextPart(parts, 'Intermediate note.')
parts = upsertToolPart(parts, { name: 'terminal', tool_id: 'tc-1' }, 'running')

const merged = mergeFinalAssistantText(parts, 'The answer is 42.', true)

// reasoning that the final text restates is dropped; interim text stays;
// the new final text is appended.
expect(merged.some(p => p.type === 'reasoning')).toBe(false)
expect(textParts(merged)).toEqual(['Intermediate note.', 'The answer is 42.'])
})

it('returns kept parts unchanged when the final text is empty', () => {
let parts: ChatMessagePart[] = appendAssistantTextPart([], 'Only narration.')
parts = upsertToolPart(parts, { name: 'terminal', tool_id: 'tc-1' }, 'running')

const merged = mergeFinalAssistantText(parts, '', true)

expect(textParts(merged)).toEqual(['Only narration.'])
})
})

describe('keepInterim = false (lean / legacy collapse)', () => {
it('drops every interim text part and re-appends only the final text', () => {
let parts: ChatMessagePart[] = appendAssistantTextPart([], 'Let me check the repo.')
parts = upsertToolPart(parts, { name: 'terminal', tool_id: 'tc-1' }, 'running')
parts = appendAssistantTextPart(parts, 'No CI here.')

const merged = mergeFinalAssistantText(parts, 'No CI here.', false)

// Historical behavior: tool survives, all text collapses to one trailing part.
expect(merged.map(p => p.type)).toEqual(['tool-call', 'text'])
expect(textParts(merged)).toEqual(['No CI here.'])
})

it('keeps tool/reasoning parts the final text does not restate', () => {
let parts: ChatMessagePart[] = appendReasoningPart([], 'unrelated thought')
parts = appendAssistantTextPart(parts, 'chatter')
parts = upsertToolPart(parts, { name: 'terminal', tool_id: 'tc-1' }, 'running')

const merged = mergeFinalAssistantText(parts, 'Final answer.', false)

expect(merged.map(p => p.type)).toEqual(['reasoning', 'tool-call', 'text'])
expect(textParts(merged)).toEqual(['Final answer.'])
})
})
})
Loading
Loading