fix(tui): keep MoA reference blocks visible when the thinking section is hidden - #64701
fix(tui): keep MoA reference blocks visible when the thinking section is hidden#64701wesleysimplicio wants to merge 3 commits into
Conversation
… is hidden Every moa.reference gateway event stores its labelled reference-model output in a Msg's generic `thinking` field (turnController's recordMoaReference), which messageLine.tsx and the ToolTrail component gate on `display.sections.thinking`'s resolved mode. When that mode resolves to `hidden`, MoA reference blocks were suppressed along with ordinary model reasoning — even though (per NousResearch#53855) references are the mixture-of-agents process the user explicitly opted into, not private reasoning, and should stay visible regardless of the thinking-section setting. Adds Msg.isMoaReference (set by recordMoaReference), a shouldShowThinkingTrail helper mirroring the existing shouldShowResponseSeparator pattern, and a reasoningAlwaysVisible prop threaded into ToolTrail to bypass the two suppression gates (the trail-wrapper return-null check and the allHidden/panel-push checks) plus the panel's initial open state and the shift-click expand-all gesture, so a MoA reference panel is not just present in the tree but actually visible and openable on first paint. Fixes NousResearch#64657
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Approved
Overview
TUI fix: keeps MoA (Mixture of Agents) reference blocks visible when the thinking section is hidden, rather than collapsing them out of view.
Assessment
- Correctness: Preserving reference blocks during thinking collapse is the right UX behavior.
- Security: No security changes.
- Debug artifacts: None.
Summary
Clean UX fix. LGTM.
Reviewed by Hermes Agent
teknium1
left a comment
There was a problem hiding this comment.
Thanks for tracing the generic-thinking visibility path; the premise is confirmed on current main (ui-tui/src/app/turnController.ts:715-720, ui-tui/src/components/messageLine.tsx:77-92).
Problems
ui-tui/src/components/thinking.tsx:738seedsopenThinkingfromreasoningAlwaysVisible, but the mount effect at:754-759immediately assignsvisible.thinking === 'expanded'. Forthinking: hidden, the newly visible MoA panel therefore collapses immediately.- The live path is still blocked before
MessageLineis reached. On the PR head,ui-tui/src/app/useMainApp.ts:1012-1042falls back to activity-only when all panels are hidden; it does not recognize an MoA segment instreamSegments.StreamingAssistantthen returns early atui-tui/src/components/streamingAssistant.tsx:40.
Suggested changes
- Preserve the MoA initial-open state through the initial visibility sync without restoring the prior manual-chevron lock; cover it with a mounted-component test.
- Include
isMoaReferencein the all-hidden live-progress predicate and test both live arrival and the settled transcript withsections.thinking: hidden.
Automated hermes-sweeper review.
…ce panels 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 NousResearch#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.
|
Both issues fixed:
Added Fail-before verified: reverting only the Full suite: 1115 passed, 8 failed — all 8 pre-existing/unrelated (terminalSetup, terminalParity, editor-resolution env-path tests), confirmed by running them in isolation with the same result regardless of this diff. |
What does this PR do?
The Ink TUI shows Mixture-of-Agents (MoA) reference-model output as labelled blocks in the
"Thinking" trail (introduced in #53855: each reference model's output is shown before the
aggregator's final answer, since it's the mixture-of-agents process the user explicitly opted
into by selecting a MoA preset — not private model reasoning). However,
moa.referenceevents arestored via
recordMoaReferenceinto aMsg's genericthinkingfield — the same field ordinarymodel reasoning uses — and both
messageLine.tsxand theToolTrailcomponent (thinking.tsx)gate visibility of that field purely on
display.sections.thinking's resolved mode. When a usersets
sections.thinking: hidden(to suppress ordinary reasoning noise), the MoA reference blockswere suppressed right along with it, making
/moaappear to skip its reference model entirely —the classic CLI shows the same run's reference block correctly, confirming this is TUI-specific.
The fix tags
Msgsegments produced byrecordMoaReferencewithisMoaReference: trueandthreads a bypass through every gate that currently keys purely off
thinkingmode: themessageLine.tsxwrapper (whether to render the trail block at all),ToolTrail'sallHiddenbackstop and panel-push condition (whether the reasoning panel exists in the tree), and — found
during review — the panel's initial open/collapsed state and the shift-click "expand all" gesture
(so the panel isn't just present but technically collapsed-and-unreachable). The initial-mount
useStategets the bypass but the reactive re-syncuseEffectdeliberately does not, peran existing code comment warning that OR-ing a force-open flag into that effect "locks the panel
open and silently breaks manual chevron clicks" (regression history: #14968) — so a user can still
manually collapse an MoA panel after it opens.
How it works
flowchart TD A["moa.reference event"] --> B["recordMoaReference pushes Msg\nwith isMoaReference: true"] B --> C{"display.sections.thinking === hidden?"} C -->|"before this PR"| D["messageLine.tsx / ToolTrail gate\non thinkingMode alone\n→ block fully suppressed"] C -->|"after this PR"| E["shouldShowThinkingTrail() / reasoningAlwaysVisible\nbypass the thinkingMode gate\n→ block renders, opens on mount,\nremains reachable via expand-all"]Related Issue
Fixes #64657
Type of Change
Changes Made
ui-tui/src/types.ts:Msggets a new optionalisMoaReference?: booleanfield.ui-tui/src/app/turnController.ts:recordMoaReferencesetsisMoaReference: trueon the segment it pushes.ui-tui/src/components/messageLine.tsx: extractsshouldShowThinkingTrail(a pure helper, following the file's existingshouldShowResponseSeparatorconvention) that bypasses the hidden-mode gate whenmsg.isMoaReference; passesreasoningAlwaysVisible={msg.isMoaReference}toToolTrail.ui-tui/src/components/thinking.tsx(ToolTrail): accepts the newreasoningAlwaysVisibleprop and uses it inallHidden, the panel-push condition, the panel's initialopenThinkingmount state, andexpandAll()'s thinking branch.ui-tui/src/__tests__/messageLine.test.ts: newdescribe('shouldShowThinkingTrail', ...)block — hidden-with-no-flag, visible-if-any-section-visible, and the MoA-reference-bypass case from the issue.Step-by-step
messageLine.tsx'sthinkingMode !== 'hidden' || toolsMode !== 'hidden' || activityMode !== 'hidden'gate andToolTrail'sallHidden/panel-push checks inthinking.tsx, none of which distinguish MoA content from ordinary reasoning.Msg.isMoaReference, set it inrecordMoaReference, and extractedshouldShowThinkingTrailas a directly-unit-testable pure function (this file already tests pure helpers this way, no component-render harness needed).shouldShowThinkingTrail is not a functionbefore the export existed / logic missing after), passes after./simplicio-review). It confirmed the core plumbing was correct but found the panel would still mount collapsed (initialuseState(visible.thinking === 'expanded')has no bypass) and be unreachable via the shift-click expand-all gesture — a partial defeat of the "always visible" intent even with the main gates fixed. Fixed both, being careful to only bypass the one-time mountuseStateand the user-initiatedexpandAll()action — not the reactive re-syncuseEffect, which an existing comment explicitly warns against forcing open (would silently break manual collapse, the exact regression from feat(tui): per-section visibility for the details accordion #14968).Acceptance Criteria
display.sections.thinking: hiddenand an MoA preset with at least one reference model, when amoa.referenceevent arrives, then its labelled block renders, opens by default, and is not collapsed-and-hidden.thinking: hidden; existingshouldShowResponseSeparatortests untouched.How to Test
main, setdisplay.sections.thinking: hidden, configure an MoA preset with ≥1 reference model, run/moa <prompt>in the Ink TUI — no reference block appears.Tests Performed
shouldShowThinkingTrailtestsnpx vitest run src/__tests__/messageLine.test.ts(fromui-tui)6 passednpx vitest run(fromui-tui)main(Windows-path-vs-POSIX-path and$PATH-format assumptions ineditor.test.ts/terminalSetup.test.ts/terminalParity.test.ts, unrelated to this change, pre-existing in this dev environment)npx tsc -b . --noEmit(fromui-tui)npx eslint src/components/messageLine.tsx src/components/thinking.tsx src/app/turnController.ts src/types.ts src/__tests__/messageLine.test.tsnpx prettier --checkon all touched filesTypeError: shouldShowThinkingTrail is not a function(3 of 6 fail); all 6 pass with the fix restoredChecklist
Code
Documentation & Housekeeping
cli-config.yaml.exampleupdated if config keys changed — N/ACONTRIBUTING.md/AGENTS.mdupdated if architecture/workflow changed — N/AScreenshots / Logs