From 6604e94c75ac4b4967e17c5456411267c7ecdb89 Mon Sep 17 00:00:00 2001
From: Brooklyn Nicholson <brooklyn.bb.nicholson@gmail.com>
Date: Fri, 24 Apr 2026 03:01:06 -0500
Subject: [PATCH] fix(tui): gate messageLine on content-bearing sections, not
 all sections
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Round-2 Copilot review on #14968 caught two leftover spots that didn't
fully respect per-section overrides:

- messageLine.tsx (trail branch): the previous fix gated on
  `SECTION_NAMES.some(...)`, which stayed true whenever any section was
  visible.  With `thinking: 'expanded'` as the new built-in default,
  that meant `display.sections.tools: hidden` left an empty wrapper Box
  alive for trail messages.  Now gates on the actual content-bearing
  sections for a trail message — `tools` OR `activity` — so a
  tools-hidden config drops the wrapper cleanly.

- messageLine.tsx (showDetails): still keyed off the global
  `detailsMode !== 'hidden'`, so per-section overrides like
  `sections.thinking: expanded` couldn't escape global hidden for
  assistant messages with reasoning + tool metadata.  Recomputed via
  resolved per-section modes (`thinkingMode`/`toolsMode`).

- types.ts: rewrote the SectionVisibility doc comment to reflect the
  actual resolution order (explicit override → SECTION_DEFAULTS →
  global), so the docstring stops claiming "missing keys fall back to
  the global mode" when SECTION_DEFAULTS now layers in between.

All three lookups (thinking/tools/activity) are computed once at the
top of MessageLine and shared by every branch.
---
 ui-tui/src/components/messageLine.tsx | 25 +++++++++++++++++--------
 ui-tui/src/types.ts                   | 10 +++++-----
 2 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/ui-tui/src/components/messageLine.tsx b/ui-tui/src/components/messageLine.tsx
index f2c0241f..a73a4f01 100644
--- a/ui-tui/src/components/messageLine.tsx
+++ b/ui-tui/src/components/messageLine.tsx
@@ -1,7 +1,7 @@
 import { Ansi, Box, NoSelect, Text } from '@hermes/ink'
 import { memo } from 'react'
 
-import { SECTION_NAMES, sectionMode } from '../domain/details.js'
+import { sectionMode } from '../domain/details.js'
 import { LONG_MSG } from '../config/limits.js'
 import { userDisplay } from '../domain/messages.js'
 import { ROLE } from '../domain/roles.js'
@@ -21,13 +21,19 @@ export const MessageLine = memo(function MessageLine({
   sections,
   t
 }: MessageLineProps) {
-  if (msg.kind === 'trail' && msg.tools?.length) {
-    // 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')
+  // Per-section overrides win over the global mode, so resolve each section
+  // we might consume here once and gate visibility on the *content-bearing*
+  // sections only — never on the global mode.  A `trail` message feeds Tool
+  // calls + Activity; an assistant message with thinking/tools metadata
+  // feeds Thinking + Tool calls.  Gating on every section would let
+  // `thinking` (expanded by default) keep an empty wrapper alive when only
+  // `tools` is hidden — exactly the empty-Box bug Copilot caught.
+  const thinkingMode = sectionMode('thinking', detailsMode, sections)
+  const toolsMode = sectionMode('tools', detailsMode, sections)
+  const activityMode = sectionMode('activity', detailsMode, sections)
 
-    return anyVisible ? (
+  if (msg.kind === 'trail' && msg.tools?.length) {
+    return toolsMode !== 'hidden' || activityMode !== 'hidden' ? (
       <Box flexDirection="column" marginTop={1}>
         <ToolTrail detailsMode={detailsMode} sections={sections} t={t} trail={msg.tools} />
       </Box>
@@ -56,7 +62,10 @@ export const MessageLine = memo(function MessageLine({
 
   const { body, glyph, prefix } = ROLE[msg.role](t)
   const thinking = msg.thinking?.trim() ?? ''
-  const showDetails = detailsMode !== 'hidden' && (Boolean(msg.tools?.length) || Boolean(thinking))
+
+  const showDetails =
+    (toolsMode !== 'hidden' && Boolean(msg.tools?.length)) ||
+    (thinkingMode !== 'hidden' && Boolean(thinking))
 
   const content = (() => {
     if (msg.kind === 'slash') {
diff --git a/ui-tui/src/types.ts b/ui-tui/src/types.ts
index eb805361..3fdb39b8 100644
--- a/ui-tui/src/types.ts
+++ b/ui-tui/src/types.ts
@@ -116,11 +116,11 @@ export type Role = 'assistant' | 'system' | 'tool' | 'user'
 export type DetailsMode = 'hidden' | 'collapsed' | 'expanded'
 export type ThinkingMode = 'collapsed' | 'truncated' | 'full'
 
-// Per-section overrides on top of the global DetailsMode.  Each missing key
-// falls back to the global mode; an explicit value overrides for that one
-// section only — so users can keep the accordion collapsed by default while
-// auto-expanding tools, or hide the activity panel entirely without touching
-// thinking/tools/subagents.
+// Per-section overrides for the agent details accordion.  Resolution order
+// at lookup time is: explicit `display.sections.<name>` → built-in
+// SECTION_DEFAULTS → global `details_mode`.  Today the built-in defaults
+// expand `thinking`/`tools` and hide `activity`; `subagents` falls through
+// to the global mode.  Any explicit value still wins for that one section.
 export type SectionName = 'thinking' | 'tools' | 'subagents' | 'activity'
 export type SectionVisibility = Partial<Record<SectionName, DetailsMode>>