mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-29 06:31:32 +00:00
fix(tui): delineate assistant responses from details (#31087)
* fix(tui): delineate assistant responses from details Add a muted Response marker before assistant text when thinking/tool details are visible so reasoning and final output do not visually run together. * fix(tui): account for response separator height Keep virtual transcript estimates aligned with the new response separator and avoid allocating trimmed copies of long assistant text. * fix(tui): gate response separator estimate on details Only add response-separator height when assistant details actually render, and use a non-allocating body-text check. * fix(tui): skip empty detail height estimates Do not add virtual transcript height for assistant details when no thinking or tool detail UI will render. * fix(tui): estimate details by section visibility Pass resolved thinking/tool visibility into virtual height estimates so hidden detail sections do not reserve response-separator rows.
This commit is contained in:
parent
0ec0cafdd0
commit
50aaf0c4ad
5 changed files with 106 additions and 4 deletions
19
ui-tui/src/__tests__/messageLine.test.ts
Normal file
19
ui-tui/src/__tests__/messageLine.test.ts
Normal file
|
|
@ -0,0 +1,19 @@
|
|||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
import { shouldShowResponseSeparator } from '../components/messageLine.js'
|
||||
|
||||
describe('shouldShowResponseSeparator', () => {
|
||||
it('separates assistant response text from visible details', () => {
|
||||
expect(shouldShowResponseSeparator({ role: 'assistant', text: 'final', thinking: 'plan' }, true)).toBe(true)
|
||||
})
|
||||
|
||||
it('does not add a response separator without details or body text', () => {
|
||||
expect(shouldShowResponseSeparator({ role: 'assistant', text: 'final' }, false)).toBe(false)
|
||||
expect(shouldShowResponseSeparator({ role: 'assistant', text: ' ', thinking: 'plan' }, true)).toBe(false)
|
||||
})
|
||||
|
||||
it('does not add response separators to non-assistant transcript rows', () => {
|
||||
expect(shouldShowResponseSeparator({ role: 'user', text: 'prompt' }, true)).toBe(false)
|
||||
expect(shouldShowResponseSeparator({ role: 'system', text: 'note' }, true)).toBe(false)
|
||||
})
|
||||
})
|
||||
|
|
@ -32,6 +32,45 @@ describe('virtual height estimates', () => {
|
|||
)
|
||||
})
|
||||
|
||||
it('accounts for the response separator when assistant details are visible', () => {
|
||||
const msg: Msg = { role: 'assistant', text: 'ok', thinking: 'plan' }
|
||||
|
||||
expect(estimatedMsgHeight(msg, 80, { compact: false, details: true })).toBe(
|
||||
estimatedMsgHeight(msg, 80, { compact: false, details: false }) + 3
|
||||
)
|
||||
})
|
||||
|
||||
it('does not account for a response separator without visible details', () => {
|
||||
const msg: Msg = { role: 'assistant', text: 'ok' }
|
||||
|
||||
expect(estimatedMsgHeight(msg, 80, { compact: false, details: true })).toBe(
|
||||
estimatedMsgHeight(msg, 80, { compact: false, details: false })
|
||||
)
|
||||
})
|
||||
|
||||
it('honors per-section visibility when estimating response separators', () => {
|
||||
const thinkingOnly: Msg = { role: 'assistant', text: 'ok', thinking: 'plan' }
|
||||
const toolsOnly: Msg = { role: 'assistant', text: 'ok', tools: ['Tool A'] }
|
||||
|
||||
expect(
|
||||
estimatedMsgHeight(thinkingOnly, 80, {
|
||||
compact: false,
|
||||
details: true,
|
||||
thinkingVisible: false,
|
||||
toolsVisible: true
|
||||
})
|
||||
).toBe(estimatedMsgHeight(thinkingOnly, 80, { compact: false, details: false }))
|
||||
|
||||
expect(
|
||||
estimatedMsgHeight(toolsOnly, 80, {
|
||||
compact: false,
|
||||
details: true,
|
||||
thinkingVisible: true,
|
||||
toolsVisible: false
|
||||
})
|
||||
).toBe(estimatedMsgHeight(toolsOnly, 80, { compact: false, details: false }))
|
||||
})
|
||||
|
||||
it('reserves two extra rows for the inter-turn separator on non-first user messages', () => {
|
||||
const msg: Msg = { role: 'user', text: 'follow-up question' }
|
||||
const base = estimatedMsgHeight(msg, 80, { compact: false, details: false })
|
||||
|
|
|
|||
|
|
@ -252,7 +252,10 @@ export function useMainApp(gw: GatewayClient) {
|
|||
return `${thinking}:${tools}`
|
||||
}, [ui.detailsMode, ui.detailsModeCommandOverride, ui.sections])
|
||||
|
||||
const detailsVisible = detailsLayoutKey !== 'hidden:hidden'
|
||||
const [thinkingDetailsMode, toolsDetailsMode] = detailsLayoutKey.split(':')
|
||||
const thinkingDetailsVisible = thinkingDetailsMode !== 'hidden'
|
||||
const toolsDetailsVisible = toolsDetailsMode !== 'hidden'
|
||||
const detailsVisible = thinkingDetailsVisible || toolsDetailsVisible
|
||||
const userPromptWidth = composerPromptWidth(ui.theme.brand.prompt)
|
||||
const heightCacheKey = `${ui.sid ?? 'draft'}:${cols}:${userPromptWidth}:${ui.compact ? '1' : '0'}:${detailsLayoutKey}`
|
||||
|
||||
|
|
@ -281,10 +284,21 @@ export function useMainApp(gw: GatewayClient) {
|
|||
estimatedMsgHeight(virtualRows[index]!.msg, cols, {
|
||||
compact: ui.compact,
|
||||
details: detailsVisible,
|
||||
thinkingVisible: thinkingDetailsVisible,
|
||||
toolsVisible: toolsDetailsVisible,
|
||||
userPrompt: ui.theme.brand.prompt,
|
||||
withSeparator: virtualRows[index]!.msg.role === 'user' && firstUserIdx >= 0 && index > firstUserIdx
|
||||
}),
|
||||
[cols, detailsVisible, firstUserIdx, ui.compact, ui.theme.brand.prompt, virtualRows]
|
||||
[
|
||||
cols,
|
||||
detailsVisible,
|
||||
firstUserIdx,
|
||||
thinkingDetailsVisible,
|
||||
toolsDetailsVisible,
|
||||
ui.compact,
|
||||
ui.theme.brand.prompt,
|
||||
virtualRows
|
||||
]
|
||||
)
|
||||
|
||||
const syncHeightCache = useCallback(
|
||||
|
|
|
|||
|
|
@ -109,6 +109,8 @@ export const MessageLine = memo(function MessageLine({
|
|||
const showDetails =
|
||||
(toolsMode !== 'hidden' && Boolean(msg.tools?.length)) || (thinkingMode !== 'hidden' && Boolean(thinking))
|
||||
|
||||
const showResponseSeparator = shouldShowResponseSeparator(msg, showDetails)
|
||||
|
||||
const content = (() => {
|
||||
if (msg.kind === 'slash') {
|
||||
return <Text color={t.color.muted}>{msg.text}</Text>
|
||||
|
|
@ -195,6 +197,17 @@ export const MessageLine = memo(function MessageLine({
|
|||
</Box>
|
||||
)}
|
||||
|
||||
{showResponseSeparator && (
|
||||
<Box marginBottom={1}>
|
||||
<NoSelect flexShrink={0} fromLeftEdge width={gutterWidth}>
|
||||
<Text color={t.color.border}>└─ </Text>
|
||||
</NoSelect>
|
||||
<Text color={t.color.muted} dim>
|
||||
Response
|
||||
</Text>
|
||||
</Box>
|
||||
)}
|
||||
|
||||
<Box>
|
||||
<NoSelect flexShrink={0} fromLeftEdge width={gutterWidth}>
|
||||
<Text bold={msg.role === 'user'} color={prefix}>
|
||||
|
|
@ -208,6 +221,9 @@ export const MessageLine = memo(function MessageLine({
|
|||
)
|
||||
})
|
||||
|
||||
export const shouldShowResponseSeparator = (msg: Msg, showDetails: boolean): boolean =>
|
||||
msg.role === 'assistant' && showDetails && /\S/.test(msg.text)
|
||||
|
||||
interface MessageLineProps {
|
||||
cols: number
|
||||
compact?: boolean
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
import { TERMUX_TUI_MODE } from '../config/env.js'
|
||||
import type { Msg } from '../types.js'
|
||||
|
||||
import { TERMUX_TUI_MODE } from '../config/env.js'
|
||||
import { transcriptBodyWidth } from './inputMetrics.js'
|
||||
|
||||
const hashText = (text: string) => {
|
||||
|
|
@ -72,11 +72,15 @@ export const estimatedMsgHeight = (
|
|||
{
|
||||
compact,
|
||||
details,
|
||||
thinkingVisible = details,
|
||||
toolsVisible = details,
|
||||
userPrompt = '',
|
||||
withSeparator = false
|
||||
}: {
|
||||
compact: boolean
|
||||
details: boolean
|
||||
thinkingVisible?: boolean
|
||||
toolsVisible?: boolean
|
||||
userPrompt?: string
|
||||
withSeparator?: boolean
|
||||
}
|
||||
|
|
@ -111,7 +115,17 @@ export const estimatedMsgHeight = (
|
|||
}
|
||||
|
||||
if (details) {
|
||||
h += (msg.tools?.length ?? 0) + wrappedLines(msg.thinking ?? '', bodyWidth)
|
||||
const hasVisibleTools = toolsVisible && Boolean(msg.tools?.length)
|
||||
const hasVisibleThinking = thinkingVisible && /\S/.test(msg.thinking ?? '')
|
||||
const hasVisibleDetails = hasVisibleTools || hasVisibleThinking
|
||||
|
||||
if (hasVisibleDetails) {
|
||||
h += (hasVisibleTools ? (msg.tools?.length ?? 0) : 0) + (hasVisibleThinking ? wrappedLines(msg.thinking ?? '', bodyWidth) : 0)
|
||||
|
||||
if (msg.role === 'assistant' && /\S/.test(msg.text)) {
|
||||
h += 2
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (msg.role === 'user' || msg.kind === 'diff') {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue