From d7fbd13997c7ceb73fa31934abc76a0529f6da98 Mon Sep 17 00:00:00 2001 From: "Simplicio, Wesley (ext)" Date: Thu, 16 Jul 2026 01:12:44 -0300 Subject: [PATCH] fix(tui): fix mount-collapse and live-progress gating for MoA reference panels MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Maintainer review (hermes-sweeper) on this PR found the fix was incomplete: two paths still hid the MoA reference panel under thinking: hidden. 1. thinking.tsx: the mount useState correctly seeds openThinking from (visible.thinking === 'expanded' || reasoningAlwaysVisible), but the re-sync effect on [visible] fires after the FIRST render too, not just later updates, and lacks the reasoningAlwaysVisible OR — so it immediately collapsed a just-opened MoA panel right after mount. Skip only the effect's very first run (a ref flag); every later visible change still re-syncs without the override, preserving the documented no-OR-at-effect-time contract (manual collapse sticks). 2. useMainApp.ts: showProgressArea's streamSegments predicate gated thinking content on thinkingPanelVisible alone, so an MoA reference segment (segment.isMoaReference, same flag messageLine.tsx's shouldShowThinkingTrail already honors per #64657) never kept the live progress area up when thinking was hidden — StreamingAssistant then returned early before MessageLine was ever reached. Added the same override. Added tests/thinkingMoaReferenceVisibility.test.tsx: mounts ToolTrail with reasoningAlwaysVisible + sections.thinking: hidden, awaits queued effects, and asserts the chevron is still open (▾, not ▸) once they settle. Validation: npx vitest run src/__tests__/thinkingMoaReferenceVisibility.test.tsx -> 1 passed Fail-before: reverting only the thinking.tsx ref-guard reproduces the exact regression -- the same test's frame capture shows the panel open on first paint then collapsing to ▸ once the effect fires, and the 'not.toContain(▸)' assertion fails as expected. npx vitest run (full ui-tui suite): 1115 passed, 8 failed -- all 8 pre-existing and unrelated (terminalSetup/terminalParity/editor resolution env-path tests), confirmed by running them in isolation with the same result regardless of this diff. npx tsc --noEmit: clean. npx eslint src/components/thinking.tsx src/app/useMainApp.ts: clean. --- .../thinkingMoaReferenceVisibility.test.tsx | 61 +++++++++++++++++++ ui-tui/src/app/useMainApp.ts | 13 ++-- ui-tui/src/components/thinking.tsx | 16 ++++- 3 files changed, 85 insertions(+), 5 deletions(-) create mode 100644 ui-tui/src/__tests__/thinkingMoaReferenceVisibility.test.tsx diff --git a/ui-tui/src/__tests__/thinkingMoaReferenceVisibility.test.tsx b/ui-tui/src/__tests__/thinkingMoaReferenceVisibility.test.tsx new file mode 100644 index 000000000000..91b332c00075 --- /dev/null +++ b/ui-tui/src/__tests__/thinkingMoaReferenceVisibility.test.tsx @@ -0,0 +1,61 @@ +import { PassThrough } from 'stream' + +import { renderSync } from '@hermes/ink' +import React from 'react' +import { describe, expect, it } from 'vitest' + +import { ToolTrail } from '../components/thinking.js' +import { stripAnsi } from '../lib/text.js' +import { DEFAULT_THEME } from '../theme.js' + +describe('ToolTrail — MoA reference panel visibility (#64701)', () => { + it('stays expanded after mount effects settle when reasoningAlwaysVisible is set, even under sections.thinking: hidden', async () => { + const stdout = new PassThrough() + const stdin = new PassThrough() + const stderr = new PassThrough() + let output = '' + + Object.assign(stdout, { columns: 60, isTTY: false, rows: 20 }) + Object.assign(stdin, { isTTY: false }) + Object.assign(stderr, { isTTY: false }) + stdout.on('data', chunk => { + output += chunk.toString() + }) + + const instance = renderSync( + , + { + patchConsole: false, + stderr: stderr as NodeJS.WriteStream, + stdin: stdin as NodeJS.ReadStream, + stdout: stdout as NodeJS.WriteStream + } + ) + + // Let queued passive effects (and any re-render they trigger) flush + // before reading the frame — the #64701 bug is specifically that the + // re-sync effect fires AFTER the first paint and clobbers the + // reasoningAlwaysVisible mount value, so asserting on the pre-effect + // frame alone would miss the regression entirely. + await new Promise(resolve => setImmediate(resolve)) + await new Promise(resolve => setImmediate(resolve)) + + const frame = stripAnsi(output) + + instance.unmount() + instance.cleanup() + + // Open chevron (▾) means the panel is still expanded once effects have + // settled, as the reasoningAlwaysVisible-seeded useState value intends. + // A collapsed (▸) render here means the re-sync effect fired on mount + // and clobbered it — the exact #64701 regression. + expect(frame).toContain('▾ ') + expect(frame).toContain('Thinking') + expect(frame).not.toContain('▸ ') + }) +}) diff --git a/ui-tui/src/app/useMainApp.ts b/ui-tui/src/app/useMainApp.ts index 586d30716e42..5149850c69a0 100644 --- a/ui-tui/src/app/useMainApp.ts +++ b/ui-tui/src/app/useMainApp.ts @@ -1057,16 +1057,21 @@ export function useMainApp(gw: GatewayClient) { state.streamSegments.some(segment => { const hasThinking = Boolean(segment.thinking?.trim()) const hasTrailTools = Boolean(segment.tools?.length) + // A MoA reference segment (segment.isMoaReference) is the + // user-facing mixture-of-agents process the user opted into, not + // private model reasoning — it must keep the live progress area + // (and therefore StreamingAssistant) up even when the thinking + // panel is hidden, matching shouldShowThinkingTrail's settled- + // transcript override in messageLine.tsx (#64657/#64701). + const thinkingVisible = thinkingPanelVisible || Boolean(segment.isMoaReference) if (segment.kind === 'trail' && !segment.text) { - return ( - (thinkingPanelVisible && hasThinking) || ((toolsPanelVisible || activityPanelVisible) && hasTrailTools) - ) + return (thinkingVisible && hasThinking) || ((toolsPanelVisible || activityPanelVisible) && hasTrailTools) } return ( Boolean(segment.text?.trim()) || - (thinkingPanelVisible && hasThinking) || + (thinkingVisible && hasThinking) || ((toolsPanelVisible || activityPanelVisible) && hasTrailTools) ) }) || diff --git a/ui-tui/src/components/thinking.tsx b/ui-tui/src/components/thinking.tsx index 090473648a97..47c8d667d8ac 100644 --- a/ui-tui/src/components/thinking.tsx +++ b/ui-tui/src/components/thinking.tsx @@ -1,5 +1,5 @@ import { Box, NoSelect, Text } from '@hermes/ink' -import { memo, type ReactNode, useEffect, useMemo, useState } from 'react' +import { memo, type ReactNode, useEffect, useMemo, useRef, useState } from 'react' import spinners, { type BrailleSpinnerName } from 'unicode-animations' import { THINKING_COT_MAX } from '../config/limits.js' @@ -751,7 +751,21 @@ export const ToolTrail = memo(function ToolTrail({ return () => clearInterval(id) }, [openTools, tools.length, visible.tools]) + // Effects run after the FIRST render too, not just on later updates — so + // this re-sync was clobbering the reasoningAlwaysVisible mount value above + // right after mount, collapsing a just-opened MoA reference panel under + // `thinking: hidden` before the user ever saw it (#64701). Skip only the + // very first run; every subsequent `visible` change (the case this effect + // exists for) still re-syncs without the override, so a manual collapse + // still sticks per the no-OR-at-effect-time rule above. + const skippedInitialSync = useRef(false) useEffect(() => { + if (!skippedInitialSync.current) { + skippedInitialSync.current = true + + return + } + setOpenThinking(visible.thinking === 'expanded') setOpenTools(visible.tools === 'expanded') setOpenSubagents(visible.subagents === 'expanded')