mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-04-25 00:51:20 +00:00
fix(tui): per-section overrides escape global details_mode: hidden
Copilot review on #14968 caught that the early returns gated on the global `detailsMode === 'hidden'` short-circuited every render path before sectionMode() got a chance to apply per-section overrides — so `details_mode: hidden` + `sections.tools: expanded` was silently a no-op. Three call sites had the same bug shape; all now key off the resolved section modes: - ToolTrail: replace the `detailsMode === 'hidden'` early return with an `allHidden = every section resolved to hidden` check. When that's true, fall back to the floating-alert backstop (errors/warnings) so quiet-mode users aren't blind to ambient failures, and update the comment block to match the actual condition. - messageLine.tsx: drop the same `detailsMode === 'hidden'` pre-check on `msg.kind === 'trail'`; only skip rendering the wrapper when every section resolves to hidden (`SECTION_NAMES.some(...) !== 'hidden'`). - useMainApp.ts: rebuild `showProgressArea` around `anyPanelVisible` instead of branching on the global mode. This also fixes the suppressed Copilot concern about an empty wrapper Box rendering above the streaming area when ToolTrail returns null. Regression test in details.test.ts pins the override-escapes-hidden behaviour for tools/thinking/activity. 271/271 vitest, lints clean.
This commit is contained in:
parent
005cc29e98
commit
70925363b6
6 changed files with 57 additions and 32 deletions
|
|
@ -90,4 +90,13 @@ describe('sectionMode', () => {
|
||||||
expect(sectionMode('activity', 'expanded', { activity: 'collapsed' })).toBe('collapsed')
|
expect(sectionMode('activity', 'expanded', { activity: 'collapsed' })).toBe('collapsed')
|
||||||
expect(sectionMode('tools', 'collapsed', { tools: 'expanded' })).toBe('expanded')
|
expect(sectionMode('tools', 'collapsed', { tools: 'expanded' })).toBe('expanded')
|
||||||
})
|
})
|
||||||
|
|
||||||
|
it('lets per-section overrides escape the global hidden mode', () => {
|
||||||
|
// Regression for the case where global details_mode: hidden used to
|
||||||
|
// short-circuit the entire accordion and prevent overrides from
|
||||||
|
// surfacing — `sections.tools: expanded` must still resolve to expanded.
|
||||||
|
expect(sectionMode('tools', 'hidden', { tools: 'expanded' })).toBe('expanded')
|
||||||
|
expect(sectionMode('thinking', 'hidden', { thinking: 'collapsed' })).toBe('collapsed')
|
||||||
|
expect(sectionMode('activity', 'hidden', { activity: 'expanded' })).toBe('expanded')
|
||||||
|
})
|
||||||
})
|
})
|
||||||
|
|
|
||||||
|
|
@ -1,7 +1,7 @@
|
||||||
import { NO_CONFIRM_DESTRUCTIVE } from '../../../config/env.js'
|
import { NO_CONFIRM_DESTRUCTIVE } from '../../../config/env.js'
|
||||||
import { dailyFortune, randomFortune } from '../../../content/fortunes.js'
|
import { dailyFortune, randomFortune } from '../../../content/fortunes.js'
|
||||||
import { HOTKEYS } from '../../../content/hotkeys.js'
|
import { HOTKEYS } from '../../../content/hotkeys.js'
|
||||||
import { isSectionName, nextDetailsMode, parseDetailsMode, SECTION_NAMES } from '../../../domain/details.js'
|
import { SECTION_NAMES, isSectionName, nextDetailsMode, parseDetailsMode } from '../../../domain/details.js'
|
||||||
import type {
|
import type {
|
||||||
ConfigGetValueResponse,
|
ConfigGetValueResponse,
|
||||||
ConfigSetResponse,
|
ConfigSetResponse,
|
||||||
|
|
@ -62,7 +62,10 @@ export const coreCommands: SlashCommand[] = [
|
||||||
{
|
{
|
||||||
rows: [
|
rows: [
|
||||||
['/details [hidden|collapsed|expanded|cycle]', 'set global agent detail visibility mode'],
|
['/details [hidden|collapsed|expanded|cycle]', 'set global agent detail visibility mode'],
|
||||||
['/details <section> [hidden|collapsed|expanded|reset]', 'override one section (thinking/tools/subagents/activity)'],
|
[
|
||||||
|
'/details <section> [hidden|collapsed|expanded|reset]',
|
||||||
|
'override one section (thinking/tools/subagents/activity)'
|
||||||
|
],
|
||||||
['/fortune [random|daily]', 'show a random or daily local fortune']
|
['/fortune [random|daily]', 'show a random or daily local fortune']
|
||||||
],
|
],
|
||||||
title: 'TUI'
|
title: 'TUI'
|
||||||
|
|
@ -159,8 +162,7 @@ export const coreCommands: SlashCommand[] = [
|
||||||
const mode = parseDetailsMode(r?.value) ?? ui.detailsMode
|
const mode = parseDetailsMode(r?.value) ?? ui.detailsMode
|
||||||
patchUiState({ detailsMode: mode })
|
patchUiState({ detailsMode: mode })
|
||||||
|
|
||||||
const overrides = SECTION_NAMES
|
const overrides = SECTION_NAMES.filter(s => ui.sections[s])
|
||||||
.filter(s => ui.sections[s])
|
|
||||||
.map(s => `${s}=${ui.sections[s]}`)
|
.map(s => `${s}=${ui.sections[s]}`)
|
||||||
.join(' ')
|
.join(' ')
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -4,6 +4,7 @@ import { useCallback, useEffect, useMemo, useRef, useState } from 'react'
|
||||||
|
|
||||||
import { STARTUP_RESUME_ID } from '../config/env.js'
|
import { STARTUP_RESUME_ID } from '../config/env.js'
|
||||||
import { MAX_HISTORY, WHEEL_SCROLL_STEP } from '../config/limits.js'
|
import { MAX_HISTORY, WHEEL_SCROLL_STEP } from '../config/limits.js'
|
||||||
|
import { SECTION_NAMES, sectionMode } from '../domain/details.js'
|
||||||
import { attachedImageNotice, imageTokenMeta } from '../domain/messages.js'
|
import { attachedImageNotice, imageTokenMeta } from '../domain/messages.js'
|
||||||
import { fmtCwdBranch, shortCwd } from '../domain/paths.js'
|
import { fmtCwdBranch, shortCwd } from '../domain/paths.js'
|
||||||
import { type GatewayClient } from '../gatewayClient.js'
|
import { type GatewayClient } from '../gatewayClient.js'
|
||||||
|
|
@ -630,11 +631,15 @@ export function useMainApp(gw: GatewayClient) {
|
||||||
|
|
||||||
const hasReasoning = Boolean(turn.reasoning.trim())
|
const hasReasoning = Boolean(turn.reasoning.trim())
|
||||||
|
|
||||||
const showProgressArea =
|
// Per-section overrides win over the global mode — when every section is
|
||||||
ui.detailsMode === 'hidden'
|
// resolved to hidden, the only thing ToolTrail will surface is the
|
||||||
? turn.activity.some(item => item.tone !== 'info')
|
// floating-alert backstop (errors/warnings). Mirror that so we don't
|
||||||
: Boolean(
|
// render an empty wrapper Box above the streaming area in quiet mode.
|
||||||
ui.busy ||
|
const anyPanelVisible = SECTION_NAMES.some(s => sectionMode(s, ui.detailsMode, ui.sections) !== 'hidden')
|
||||||
|
|
||||||
|
const showProgressArea = anyPanelVisible
|
||||||
|
? Boolean(
|
||||||
|
ui.busy ||
|
||||||
turn.outcome ||
|
turn.outcome ||
|
||||||
turn.streamPendingTools.length ||
|
turn.streamPendingTools.length ||
|
||||||
turn.streamSegments.length ||
|
turn.streamSegments.length ||
|
||||||
|
|
@ -643,7 +648,8 @@ export function useMainApp(gw: GatewayClient) {
|
||||||
turn.turnTrail.length ||
|
turn.turnTrail.length ||
|
||||||
hasReasoning ||
|
hasReasoning ||
|
||||||
turn.activity.length
|
turn.activity.length
|
||||||
)
|
)
|
||||||
|
: turn.activity.some(item => item.tone !== 'info')
|
||||||
|
|
||||||
const appActions = useMemo(
|
const appActions = useMemo(
|
||||||
() => ({
|
() => ({
|
||||||
|
|
|
||||||
|
|
@ -1,6 +1,7 @@
|
||||||
import { Ansi, Box, NoSelect, Text } from '@hermes/ink'
|
import { Ansi, Box, NoSelect, Text } from '@hermes/ink'
|
||||||
import { memo } from 'react'
|
import { memo } from 'react'
|
||||||
|
|
||||||
|
import { SECTION_NAMES, sectionMode } from '../domain/details.js'
|
||||||
import { LONG_MSG } from '../config/limits.js'
|
import { LONG_MSG } from '../config/limits.js'
|
||||||
import { userDisplay } from '../domain/messages.js'
|
import { userDisplay } from '../domain/messages.js'
|
||||||
import { ROLE } from '../domain/roles.js'
|
import { ROLE } from '../domain/roles.js'
|
||||||
|
|
@ -21,11 +22,16 @@ export const MessageLine = memo(function MessageLine({
|
||||||
t
|
t
|
||||||
}: MessageLineProps) {
|
}: MessageLineProps) {
|
||||||
if (msg.kind === 'trail' && msg.tools?.length) {
|
if (msg.kind === 'trail' && msg.tools?.length) {
|
||||||
return detailsMode === 'hidden' ? null : (
|
// Per-section overrides win over the global mode, so don't pre-empt on
|
||||||
|
// `detailsMode === 'hidden'` — only skip when EVERY section is hidden,
|
||||||
|
// matching ToolTrail's own internal short-circuit.
|
||||||
|
const anyVisible = SECTION_NAMES.some(s => sectionMode(s, detailsMode, sections) !== 'hidden')
|
||||||
|
|
||||||
|
return anyVisible ? (
|
||||||
<Box flexDirection="column" marginTop={1}>
|
<Box flexDirection="column" marginTop={1}>
|
||||||
<ToolTrail detailsMode={detailsMode} sections={sections} t={t} trail={msg.tools} />
|
<ToolTrail detailsMode={detailsMode} sections={sections} t={t} trail={msg.tools} />
|
||||||
</Box>
|
</Box>
|
||||||
)
|
) : null
|
||||||
}
|
}
|
||||||
|
|
||||||
if (msg.role === 'tool') {
|
if (msg.role === 'tool') {
|
||||||
|
|
|
||||||
|
|
@ -1,5 +1,5 @@
|
||||||
import { Box, NoSelect, Text } from '@hermes/ink'
|
import { Box, NoSelect, Text } from '@hermes/ink'
|
||||||
import { memo, type ReactNode, useEffect, useMemo, useState } from 'react'
|
import { memo, useEffect, useMemo, useState, type ReactNode } from 'react'
|
||||||
import spinners, { type BrailleSpinnerName } from 'unicode-animations'
|
import spinners, { type BrailleSpinnerName } from 'unicode-animations'
|
||||||
|
|
||||||
import { THINKING_COT_MAX } from '../config/limits.js'
|
import { THINKING_COT_MAX } from '../config/limits.js'
|
||||||
|
|
@ -874,18 +874,22 @@ export const ToolTrail = memo(function ToolTrail({
|
||||||
const delegateGroups = groups.filter(g => g.label.startsWith('Delegate Task'))
|
const delegateGroups = groups.filter(g => g.label.startsWith('Delegate Task'))
|
||||||
const inlineDelegateKey = hasSubagents && delegateGroups.length === 1 ? delegateGroups[0]!.key : null
|
const inlineDelegateKey = hasSubagents && delegateGroups.length === 1 ? delegateGroups[0]!.key : null
|
||||||
|
|
||||||
// ── Hidden: errors/warnings only ──────────────────────────────
|
// ── Backstop: floating alerts when every panel is hidden ─────────
|
||||||
//
|
//
|
||||||
// When the global details_mode is 'hidden' (or all sections are individually
|
// Per-section overrides win over the global details_mode (they're computed
|
||||||
// hidden), the accordion collapses entirely. Errors/warnings still float
|
// by sectionMode), so we only collapse to nothing when EVERY section is
|
||||||
// as inline alerts UNLESS the activity section is explicitly hidden — that
|
// resolved to hidden — that way `details_mode: hidden` + `sections.tools:
|
||||||
// override means "I don't want to see meta at all", so respect it.
|
// expanded` still renders the tools panel. When all panels are hidden
|
||||||
|
// AND ambient errors/warnings exist, surface them as a compact inline
|
||||||
|
// backstop so quiet-mode users aren't blind to failures.
|
||||||
|
|
||||||
if (detailsMode === 'hidden') {
|
const allHidden =
|
||||||
if (visible.activity === 'hidden') {
|
visible.thinking === 'hidden' &&
|
||||||
return null
|
visible.tools === 'hidden' &&
|
||||||
}
|
visible.subagents === 'hidden' &&
|
||||||
|
visible.activity === 'hidden'
|
||||||
|
|
||||||
|
if (allHidden) {
|
||||||
const alerts = activity.filter(i => i.tone !== 'info').slice(-2)
|
const alerts = activity.filter(i => i.tone !== 'info').slice(-2)
|
||||||
|
|
||||||
return alerts.length ? (
|
return alerts.length ? (
|
||||||
|
|
|
||||||
|
|
@ -17,10 +17,12 @@ const THINKING_FALLBACK: Record<string, DetailsMode> = {
|
||||||
truncated: 'collapsed'
|
truncated: 'collapsed'
|
||||||
}
|
}
|
||||||
|
|
||||||
const norm = (v: unknown) => String(v ?? '').trim().toLowerCase()
|
const norm = (v: unknown) =>
|
||||||
|
String(v ?? '')
|
||||||
|
.trim()
|
||||||
|
.toLowerCase()
|
||||||
|
|
||||||
export const parseDetailsMode = (v: unknown): DetailsMode | null =>
|
export const parseDetailsMode = (v: unknown): DetailsMode | null => MODES.find(m => m === norm(v)) ?? null
|
||||||
MODES.find(m => m === norm(v)) ?? null
|
|
||||||
|
|
||||||
export const isSectionName = (v: unknown): v is SectionName =>
|
export const isSectionName = (v: unknown): v is SectionName =>
|
||||||
typeof v === 'string' && (SECTION_NAMES as readonly string[]).includes(v)
|
typeof v === 'string' && (SECTION_NAMES as readonly string[]).includes(v)
|
||||||
|
|
@ -42,11 +44,7 @@ export const resolveSections = (raw: unknown): SectionVisibility =>
|
||||||
|
|
||||||
// Effective mode for one section: explicit override → SECTION_DEFAULTS → global.
|
// Effective mode for one section: explicit override → SECTION_DEFAULTS → global.
|
||||||
// Single source of truth for "is this section open by default / rendered at all".
|
// Single source of truth for "is this section open by default / rendered at all".
|
||||||
export const sectionMode = (
|
export const sectionMode = (name: SectionName, global: DetailsMode, sections?: SectionVisibility): DetailsMode =>
|
||||||
name: SectionName,
|
sections?.[name] ?? SECTION_DEFAULTS[name] ?? global
|
||||||
global: DetailsMode,
|
|
||||||
sections?: SectionVisibility
|
|
||||||
): DetailsMode => sections?.[name] ?? SECTION_DEFAULTS[name] ?? global
|
|
||||||
|
|
||||||
export const nextDetailsMode = (m: DetailsMode): DetailsMode =>
|
export const nextDetailsMode = (m: DetailsMode): DetailsMode => MODES[(MODES.indexOf(m) + 1) % MODES.length]!
|
||||||
MODES[(MODES.indexOf(m) + 1) % MODES.length]!
|
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue