fix(web): consolidate tool card display settings - #1599
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Findings
- [Major] Combined mode changes the visible chronology of standalone cards — the response-wide scan collects every eligible tool, then inserts the whole group at the first tool. For
[read, permission, edit], the rendered order becomes[group(read, edit), permission], so permission, user-input, plan, lifecycle, and assistant-text cards can appear after work that originally followed them. Evidenceweb/src/chat/toolGroups.ts:393andweb/src/chat/toolGroups.ts:402.
Suggested fix:// Remove the response-wide grouping branch and use the contiguous scan below. const groupingFamily = getGroupingFamily(block, groupingMode) if (!groupingFamily) { visibleBlocks.push(block) continue } const tools: ToolCallBlock[] = [block] let cursor = index + 1 while (cursor < blocks.length) { const candidate = blocks[cursor] if (candidate.kind !== 'tool-call' || getGroupingFamily(candidate, groupingMode) !== groupingFamily) break tools.push(candidate) cursor += 1 }
Summary
- Review mode: initial
- One Major finding. Static full-diff review of 24 changed web files. Residual risk: visual/responsive behavior was not exercised locally.
Testing
- Not run (automation; PR content treated as untrusted).
- GitHub
integration: passed. - GitHub
test: failed at unchangedweb/src/components/assistant-ui/markdown-a.test.tsx:64(duplicate object property); not attributable to this diff.
HAPI Bot
| const firstTool = tools[0] | ||
| for (const candidate of responseBlocks) { | ||
| if (candidate === firstTool) { | ||
| appendToolGroup(visibleBlocks, tools, 'default', options, previousGroups) |
There was a problem hiding this comment.
[MAJOR] Combined mode reorders standalone cards
tools contains every eligible tool in the entire assistant-role run, and this call inserts all of them at the first tool. For [read, permission, edit], the output becomes [group(read, edit), permission], so the UI shows the edit before the permission that originally preceded it. The same reordering affects user-input, plan, lifecycle, and assistant-text cards, making the audit sequence misleading.
Suggested fix:
// Remove the response-wide grouping branch and let the existing contiguous
// grouping scan below stop at every standalone/non-tool boundary.
const groupingFamily = getGroupingFamily(block, groupingMode)
if (!groupingFamily) {
visibleBlocks.push(block)
continue
}
const tools: ToolCallBlock[] = [block]
let cursor = index + 1
while (cursor < blocks.length) {
const candidate = blocks[cursor]
if (candidate.kind !== 'tool-call'
|| getGroupingFamily(candidate, groupingMode) !== groupingFamily) break
tools.push(candidate)
cursor += 1
}
There was a problem hiding this comment.
Findings
- [Major] Combined mode hides permission denial/cancellation reasons —
groupedbypasses the Codex-only classification guard, whileisInteractiveToolBlockexcludes only pending permissions. A denied or canceled Bash/Edit adjacent to another eligible tool is therefore folded into a group. The grouped detail path rendersToolDetailDialogContent, which does not render thePermissionFooterthat surfacespermission.reason, so the operator loses the recorded explanation. Evidenceweb/src/chat/toolGroups.ts:308,web/src/components/ToolCard/ToolGroupCard.tsx:434,web/src/components/ToolCard/ToolCard.tsx:628.
Suggested fix:const permission = block.tool.permission const hasTerminalPermissionReason = ( permission?.status === 'denied' || permission?.status === 'canceled' ) && Boolean(permission.reason) return INTERACTIVE_TOOL_NAMES.has(normalizeToolIdentifier(block.tool.name)) || permission?.status === 'pending' || hasTerminalPermissionReason || isAskUserQuestionToolName(block.tool.name) || isRequestUserInputToolName(block.tool.name)
- [Minor] Mode changes can still reuse the same tool-group id — clearing
previousGroupsdoes not change the deterministic fallbacktool-group:${firstToolId}. When a Combined group beginning with Codex exploration tools becomes a Compact/Detailed exploration group, both groups retain the same id and usually the samedefaultOpen=false;ToolGroupCardtherefore keeps local open/hydration state instead of resetting for the selected mode. Evidenceweb/src/chat/toolGroups.ts:380andweb/src/components/ToolCard/ToolGroupCard.tsx:210.
Suggested fix:function createToolGroupId( tools: ToolCallBlock[], needsOlderHistory: boolean, previousGroups: ToolGroupBlock[], groupingFamily: 'default' | 'codex-exploration' ): string { // Keep the existing previous-group reuse above. const boundaryId = needsOlderHistory ? lastToolId : firstToolId return `tool-group:${groupingFamily}:${boundaryId}` }
Summary
- Review mode: follow-up after new commits
- One Major and one Minor finding. The prior chronology finding is resolved by the latest commit. Residual risk: visual/responsive behavior was not exercised locally.
Testing
- Not run (automation; PR content treated as untrusted).
- GitHub
integration: passed. - GitHub
test: failed during typecheck at unchangedweb/src/components/assistant-ui/markdown-a.test.tsx:64(duplicate object property); not attributable to this diff. - Add coverage proving denial reasons remain visible in Combined mode and that group ids differ across Combined ↔ Compact/Detailed transitions.
HAPI Bot
| if (MILESTONE_TOOL_NAMES.has(normalizedName)) return false | ||
| if (isInteractiveToolBlock(block)) return false | ||
| if (block.tool.name === 'CodexBash' && getCodexCommandActions(block).length > 0) { | ||
| if (groupingMode === 'classified' && block.tool.name === 'CodexBash' && getCodexCommandActions(block).length > 0) { |
There was a problem hiding this comment.
[MAJOR] Combined mode hides permission denial/cancellation reasons
In grouped mode this Codex-only guard is skipped, while isInteractiveToolBlock excludes only pending permissions. A denied/canceled Bash or Edit next to another eligible tool is therefore folded into ToolGroupCard. Its dialog uses ToolDetailDialogContent (ToolGroupCard.tsx:434), which never renders the PermissionFooter used by standalone cards (ToolCard.tsx:628) to show permission.reason. The default mode can thus remove the operator-visible explanation for why an action did not run.
Suggested fix:
const permission = block.tool.permission
const hasTerminalPermissionReason = (
permission?.status === 'denied' || permission?.status === 'canceled'
) && Boolean(permission.reason)
return INTERACTIVE_TOOL_NAMES.has(normalizeToolIdentifier(block.tool.name))
|| permission?.status === 'pending'
|| hasTerminalPermissionReason
|| isAskUserQuestionToolName(block.tool.name)
|| isRequestUserInputToolName(block.tool.name)| const visibleBlocks: VisibleChatBlock[] = [] | ||
| const previousGroups = options.previousGroups ?? [] | ||
| const groupingMode = options.groupingMode ?? 'classified' | ||
| const previousGroups = options.previousGroupingMode == null || options.previousGroupingMode === groupingMode |
There was a problem hiding this comment.
[MINOR] Mode switching still reuses the same group id
Discarding previousGroups is insufficient because createToolGroupId still falls back to tool-group:${firstToolId}. For [Codex read, Codex search, command], Combined creates a default group beginning at the first read; Compact creates a Codex-exploration group beginning at that same read. Both receive the same id and normally the same defaultOpen=false, so ToolGroupCard does not run its reset effect and carries open/hydration state into the new display mode.
Suggested fix:
function createToolGroupId(
tools: ToolCallBlock[],
needsOlderHistory: boolean,
previousGroups: ToolGroupBlock[],
groupingFamily: 'default' | 'codex-exploration'
): string {
// Keep the existing previous-group reuse above.
const boundaryId = needsOlderHistory ? lastToolId : firstToolId
return `tool-group:${groupingFamily}:${boundaryId}`
}Also assert that the classified group's id differs from the grouped id; the current test only checks uniqueness between the two classified groups.
There was a problem hiding this comment.
Findings
- None.
Summary
- Review mode: follow-up after new commits
- No new high-confidence issues found in the latest full diff. The two findings from the prior HAPI Bot pass are addressed by the new commit with targeted regression coverage.
- Residual risk: the current test workflow stops during web typecheck at unchanged web/src/components/assistant-ui/markdown-a.test.tsx:64 (duplicate object property), so the web unit suite does not run on this head. This failure is not attributable to the PR diff.
Testing
- Not run locally (automation; PR content treated as untrusted).
- GitHub integration: passed.
- GitHub test: failed before tests during the unrelated typecheck error above.
- Static diff check: passed.
HAPI Bot
There was a problem hiding this comment.
Findings
- [Major] Detailed mode skips ACP and non-canonical terminal cards — the new classifier recognizes aliases and nativeKind: execute as commands, but terminal-card rendering still checks only four exact tool names. ACP calls whose name is a title, plus aliases such as Shell, bash, and run_command, therefore keep the generic minimal card and never show the inline result preview in Detailed mode. Evidence web/src/chat/toolGroups.ts:192, web/src/chat/toolGroups.ts:195; related context web/src/components/ToolCard/ToolCard.tsx:30.
Suggested fix:import { getToolGroupActionKind } from '@/chat/toolGroups' const isTerminalTool = getToolGroupActionKind(props.block) === 'command' const useCompactTerminalCard = isTerminalTool && props.terminalToolDisplayMode === 'compact' const showInline = isSubagentToolName(toolName) ? false : isTerminalTool ? props.terminalToolDisplayMode === 'detailed' : !presentation.minimal
Summary
- Review mode: follow-up after new commits
- One Major finding. The latest full 25-file diff was reviewed; prior chronology, permission-reason, and group-ID findings remain resolved.
Testing
- Not run locally (automation; PR content treated as untrusted).
- GitHub integration: passed.
- GitHub test: passed.
- Add ToolCard coverage for lowercase/snake-case command aliases and ACP nativeKind: execute in Compact and Detailed modes.
HAPI Bot
| if (COMMAND_TOOL_NAMES.has(name)) return 'command' | ||
| if (MUTATION_TOOL_NAMES.has(name)) return 'mutation' | ||
|
|
||
| const nativeKind = normalizeToolIdentifier(block.tool.nativeKind) |
There was a problem hiding this comment.
[MAJOR] Detailed mode skips ACP and non-canonical terminal cards
This classifier now recognizes command aliases and nativeKind: execute, but ToolCard still selects Compact/Detailed behavior from the exact-name set Bash | CodexBash | shell_command | run_shell_command. For ACP title-as-name calls and aliases such as Shell, bash, or run_command, the generic presentation remains minimal: true, so Detailed mode never renders its inline input/output preview.
Suggested fix:
import { getToolGroupActionKind } from '@/chat/toolGroups'
const isTerminalTool = getToolGroupActionKind(props.block) === 'command'
const useCompactTerminalCard =
isTerminalTool && props.terminalToolDisplayMode === 'compact'
const showInline = isSubagentToolName(toolName)
? false
: isTerminalTool
? props.terminalToolDisplayMode === 'detailed'
: !presentation.minimal
There was a problem hiding this comment.
Findings
- [Major] Compact mode still expands user-shell Codex read/search commands -
getToolGroupActionKindclassifies structured Codex actions by semantic kind, whileisCodexExplorationToolintentionally excludescommand_source: userShell. A standaloneCodexBashread/search command therefore getsisTerminalTool = false; when it has output, Compact mode renders the inline result instead of command-only. Evidenceweb/src/components/ToolCard/ToolCard.tsx:452; related contextweb/src/chat/toolGroups.ts:181,web/src/chat/codexCommandPresentation.ts:59.
Suggested fix:const isTerminalTool = toolName === 'CodexBash' || getToolGroupActionKind(props.block) === 'command'
- [Minor] Switching display modes can mark already-seen tools as new - a history-mode watermark created in Combined mode stores only the group id. Switching to Compact/Detailed replaces that group with member tool ids, but the watermark is retained and cannot recognize the reverse group-to-members transition, so the "new messages" badge can increment without new content. Evidence
web/src/components/SessionChat.tsx:1318; related contextweb/src/chat/unseenBlocks.ts:35,web/src/chat/unseenBlocks.ts:53.
Suggested fix:if (isToolGroupBlock(block)) { for (const tool of block.tools) ids.add(tool.id) }
- [Minor] Storage errors undo the selected mode in the current tab - the setter optimistically updates state, but the same-tab change event immediately re-reads storage. Because the safe storage helpers swallow write failures, browsers where localStorage is readable but not writable reset the selection back to the stored/default value. Evidence
web/src/hooks/useToolCardDisplayMode.ts:122.
Suggested fix:const onChange = (event: Event) => { const detail = (event as CustomEvent<unknown>).detail const mode = parseToolCardDisplayMode(typeof detail === 'string' ? detail : null) setToolCardDisplayModeState(mode ?? getInitialToolCardDisplayMode()) } window.dispatchEvent(new CustomEvent(CHANGE_EVENT, { detail: mode }))
Summary
Review mode: follow-up after new commits
One Major and two Minor findings. The prior ACP/non-canonical terminal-card finding is partially resolved, but semantic Codex action classification still bypasses Compact behavior for user-shell reads/searches.
Testing
- Not run locally (automation; PR content treated as untrusted).
- GitHub integration: passed.
- GitHub test: passed.
- GitHub pr-review: pending at submission.
- Add regressions for a
CodexBashuserShellread with stdout in Compact/Detailed modes, Combined-to-Compact watermark preservation, and localStoragesetItemfailure.
HAPI Bot
| const isCodexAgentCard = toolName === 'CodexAgent' | ||
| const useCompactTerminalCard = shouldUseCompactTerminalToolCard(toolName, props.terminalToolDisplayMode) | ||
| const showInline = shouldShowInlineToolCardBody(toolName, presentation.minimal, props.terminalToolDisplayMode) | ||
| const isTerminalTool = getToolGroupActionKind(props.block) === 'command' |
There was a problem hiding this comment.
[MAJOR] Compact mode still expands user-shell Codex read/search commands
getToolGroupActionKind is a semantic classifier: structured Codex reads/searches return read/search. But command_source: userShell deliberately keeps those CodexBash calls out of exploration groups, so they render as standalone terminal cards. Here they get isTerminalTool = false; with stdout, Compact mode renders the inline result instead of command-only.
Suggested fix:
const isTerminalTool =
toolName === 'CodexBash'
|| getToolGroupActionKind(props.block) === 'command'Add a regression using a CodexBash userShell read action with non-empty stdout.
| hasMoreMessages: props.hasMoreMessages, | ||
| previousGroups: visibleGroupsRef.current, | ||
| previousGroupingMode: visibleGroupsModeRef.current, | ||
| groupingMode: toolGroupingMode, |
There was a problem hiding this comment.
[MINOR] Display-mode switches can mark already-seen tools as new
In history mode, a watermark captured in Combined mode contains the tool-group id. Switching to Compact/Detailed replaces that group with its member tool ids, while useUnseenBlockCount retains the old watermark. The reverse group-to-members transition is not recognized, so the "new messages" badge can increment without any new content.
Suggested fix in createUnseenWatermark:
if (isToolGroupBlock(block)) {
for (const tool of block.tools) ids.add(tool.id)
}This complements the existing classified-to-grouped member matching.
| } else { | ||
| safeSetItem(STORAGE_KEY, mode) | ||
| } | ||
| if (isBrowser()) window.dispatchEvent(new Event(CHANGE_EVENT)) |
There was a problem hiding this comment.
[MINOR] Storage failures undo the selected mode
The setter updates local state, then this same-tab event synchronously re-reads storage. Since safeSetItem/safeRemoveItem swallow failures, a browser where localStorage is readable but not writable immediately resets the UI to the stored/default mode.
Suggested fix: carry the selected mode in the custom event and use storage only as fallback.
const onChange = (event: Event) => {
const detail = (event as CustomEvent<unknown>).detail
const mode = parseToolCardDisplayMode(typeof detail === 'string' ? detail : null)
setToolCardDisplayModeState(mode ?? getInitialToolCardDisplayMode())
}
window.dispatchEvent(new CustomEvent(CHANGE_EVENT, { detail: mode }))
There was a problem hiding this comment.
Findings
- None.
Summary
Review mode: follow-up after new commits
No reportable issues found in the latest full diff. The prior Major and two Minor findings are addressed at the current head. Residual risk is limited to browser-level visual/responsive behavior and cross-tab storage synchronization, which are not directly exercised by the added unit tests.
Testing
- Not run locally (automation; PR content treated as untrusted).
- GitHub integration: passed.
- GitHub test: passed.
- GitHub pr-review: in progress at submission.
HAPI Bot
There was a problem hiding this comment.
Findings
- [Major] Required web test suite fails after the implicit grouping default changed.
- [Minor] ACP
nativeKindcan be overridden by a title-derived tool name, producing the wrong display style.
Summary
Review mode: follow-up after new commits
Two reportable issues found in the latest full diff. The previous HAPI Bot findings remain addressed. Residual risk remains around browser-level visual/responsive behavior and cross-tab storage synchronization.
Testing
- Not run locally (automation; PR content treated as untrusted).
- GitHub
drift-gate: passed. - GitHub
integration: passed. - GitHub
test: failed inweb/src/lib/assistant-runtime.test.ts:842because the new default no longer creates the expected tool group.
HAPI Bot
| ): VisibleChatBlock[] { | ||
| const visibleBlocks: VisibleChatBlock[] = [] | ||
| const previousGroups = options.previousGroups ?? [] | ||
| const groupingMode = options.groupingMode ?? 'classified' |
There was a problem hiding this comment.
[MAJOR] The new implicit classified default leaves the required web suite red. The current-head CI run fails at web/src/lib/assistant-runtime.test.ts:842: its Read + Grep case calls this helper without groupingMode, so it no longer produces the tool group needed to preserve roundSummary. This is directly triggered by this fallback and blocks the required test check.
Suggested fix:
const visible = buildVisibleChatBlocks([
userText('u1'),
firstTool,
toolCall('grep', { localId: 'L1' })
], {
hasMoreMessages: false,
groupingMode: 'grouped'
})| } | ||
| if (name === 'WebFetch' || name === 'WebSearch') return 'web' | ||
|
|
||
| const name = normalizeToolIdentifier(block.tool.name) |
There was a problem hiding this comment.
[MINOR] Prefer ACP nativeKind before the title-derived tool name. ACP stores the arbitrary presentation title as tool.name while preserving its semantic kind separately (cli/src/agent/utils.ts:24-26, web/src/chat/normalizeAgent.ts:1204-1208). Because the name sets run first here, an execute call such as { name: 'ls', nativeKind: 'execute' } is classified as search; isTerminalToolBlock then returns false, so Detailed mode omits the terminal output preview and Combined mode labels it as search.
Suggested fix:
const nativeKind = normalizeToolIdentifier(block.tool.nativeKind)
if (WEB_NATIVE_KINDS.has(nativeKind)) return 'web'
if (READ_NATIVE_KINDS.has(nativeKind)) return 'read'
if (SEARCH_NATIVE_KINDS.has(nativeKind)) return 'search'
if (COMMAND_NATIVE_KINDS.has(nativeKind)) return 'command'
if (MUTATION_NATIVE_KINDS.has(nativeKind)) return 'mutation'
const name = normalizeToolIdentifier(block.tool.name)
// existing name fallbacks...
There was a problem hiding this comment.
Findings
- None.
Summary
Review mode: follow-up after new commits
No reportable issues found in the latest full diff. The previous HAPI Bot findings are addressed at this head. Residual risk remains around browser-level visual/responsive behavior and real cross-tab storage synchronization, which the unit suite does not exercise directly.
Testing
- Not run locally (automation; PR content treated as untrusted).
- GitHub
drift-gate: passed. - GitHub
integration: passed. - GitHub
test: passed. - Static full-diff review and
git diff --check: passed.
HAPI Bot
1 participant
Summary
This follow-up fixes inconsistent tool-card presentation across agents by consolidating the existing grouping and terminal-card controls into one consistent display-style setting.
Combined,Compact, andDetailedstyles.Combinedmode.Actions N/操作 Nsummary on mobile.Display modes
Compatibility
hapi-tool-grouping-modeandhapi-terminal-tool-display-modevalues remain readable.CompactandDetailedmodes.Related work
Test plan
bun run test:webbun run build:web