Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 66 additions & 0 deletions web/src/chat/toolGroups.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,72 @@ describe('buildVisibleChatBlocks', () => {
expect(isToolGroupBlock(visible[2]) && visible[2].presentationMode).toBe('codex-exploration')
})

it('groups structured general Codex commands when grouped mode is selected', () => {
const first = makeToolBlock('codex-test-1', 'CodexBash', {
command: 'bun test',
command_actions: [{ type: 'unknown', command: 'bun test' }]
})
const second = makeToolBlock('codex-test-2', 'CodexBash', {
command: 'bun run typecheck',
command_actions: [{ type: 'unknown', command: 'bun run typecheck' }]
})

const classified = buildVisibleChatBlocks([first, second], {
hasMoreMessages: false,
groupingMode: 'classified'
})
const grouped = buildVisibleChatBlocks([first, second], {
hasMoreMessages: false,
groupingMode: 'grouped'
})

expect(classified).toEqual([first, second])
expect(grouped).toHaveLength(1)
expect(isToolGroupBlock(grouped[0]) && grouped[0].presentationMode).toBe('default')
expect(isToolGroupBlock(grouped[0]) && grouped[0].defaultOpen).toBe(false)
})

it('does not reuse grouped ids when switching to classified mode', () => {
const firstRead = makeToolBlock('codex-read-1', 'CodexBash', {
command: 'cat package.json',
command_actions: [{
type: 'read',
command: 'cat package.json',
name: 'package.json',
path: '/repo/package.json'
}]
})
const command = makeToolBlock('codex-test', 'CodexBash', {
command: 'bun test',
command_actions: [{ type: 'unknown', command: 'bun test' }]
})
const secondRead = makeToolBlock('codex-read-2', 'CodexBash', {
command: 'cat README.md',
command_actions: [{
type: 'read',
command: 'cat README.md',
name: 'README.md',
path: '/repo/README.md'
}]
})
const blocks = [firstRead, command, secondRead]
const grouped = buildVisibleChatBlocks(blocks, {
hasMoreMessages: false,
groupingMode: 'grouped'
})
const classified = buildVisibleChatBlocks(blocks, {
hasMoreMessages: false,
previousGroups: grouped.filter(isToolGroupBlock),
previousGroupingMode: 'grouped',
groupingMode: 'classified'
})
const classifiedGroups = classified.filter(isToolGroupBlock)

expect(grouped.filter(isToolGroupBlock)).toHaveLength(1)
expect(classifiedGroups).toHaveLength(2)
expect(new Set(classifiedGroups.map((group) => group.id)).size).toBe(2)
})

it('groups contiguous eligible root tool cards', () => {
const visible = buildVisibleChatBlocks([
makeToolBlock('read-1', 'Read', { file_path: 'src/a.ts' }),
Expand Down
22 changes: 14 additions & 8 deletions web/src/chat/toolGroups.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { isRequestUserInputToolName } from '@/components/ToolCard/requestUserInp
import { getInputStringAny } from '@/lib/toolInputUtils'

export type ToolGroupActionKind = 'read' | 'search' | 'command' | 'mutation' | 'web' | 'other'
export type ToolGroupingMode = 'grouped' | 'classified'

export type ToolGroupSummary = {
totalTools: number
Expand Down Expand Up @@ -55,6 +56,8 @@ export function visibleBlockRole(block: VisibleChatBlock): VisibleChatBlockRole
type ToolGroupingOptions = {
hasMoreMessages: boolean
previousGroups?: ToolGroupBlock[]
previousGroupingMode?: ToolGroupingMode
groupingMode?: ToolGroupingMode
codexExplorationCollapsed?: boolean
}

Expand Down Expand Up @@ -221,20 +224,20 @@ function isInteractiveToolBlock(block: ToolCallBlock): boolean {
|| isRequestUserInputToolName(block.tool.name)
}

export function isEligibleForToolGrouping(block: ToolCallBlock): boolean {
export function isEligibleForToolGrouping(block: ToolCallBlock, groupingMode: ToolGroupingMode = 'classified'): boolean {
if (isSubagentToolName(block.tool.name)) return false
if (PLAN_TOOL_NAMES.has(block.tool.name)) return false
if (MILESTONE_TOOL_NAMES.has(block.tool.name)) 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) {
return isCodexExplorationTool(block)
}
return true
}

function getGroupingFamily(block: ToolCallBlock): 'default' | 'codex-exploration' | null {
if (!isEligibleForToolGrouping(block)) return null
return isCodexExplorationTool(block) ? 'codex-exploration' : 'default'
function getGroupingFamily(block: ToolCallBlock, groupingMode: ToolGroupingMode): 'default' | 'codex-exploration' | null {
if (!isEligibleForToolGrouping(block, groupingMode)) return null
return groupingMode === 'classified' && isCodexExplorationTool(block) ? 'codex-exploration' : 'default'
}

function createToolGroupId(
Expand Down Expand Up @@ -264,15 +267,18 @@ export function buildVisibleChatBlocks(
options: ToolGroupingOptions
): VisibleChatBlock[] {
const visibleBlocks: VisibleChatBlock[] = []
const previousGroups = options.previousGroups ?? []
const groupingMode = options.groupingMode ?? 'classified'
const previousGroups = options.previousGroupingMode == null || options.previousGroupingMode === groupingMode
? (options.previousGroups ?? [])
: []

for (let index = 0; index < blocks.length; index += 1) {
const block = blocks[index]
if (block.kind !== 'tool-call') {
visibleBlocks.push(block)
continue
}
const groupingFamily = getGroupingFamily(block)
const groupingFamily = getGroupingFamily(block, groupingMode)
if (!groupingFamily) {
visibleBlocks.push(block)
continue
Expand All @@ -282,7 +288,7 @@ export function buildVisibleChatBlocks(
let cursor = index + 1
while (cursor < blocks.length) {
const candidate = blocks[cursor]
if (candidate.kind !== 'tool-call' || getGroupingFamily(candidate) !== groupingFamily) {
if (candidate.kind !== 'tool-call' || getGroupingFamily(candidate, groupingMode) !== groupingFamily) {
break
}
tools.push(candidate)
Expand Down
10 changes: 8 additions & 2 deletions web/src/components/SessionChat.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ import { TeamPanel } from '@/components/TeamPanel'
import { SessionStatusPanel } from '@/components/SessionStatusPanel'
import { buildSessionStatusData } from '@/chat/sessionStatus'
import { usePlatform } from '@/hooks/usePlatform'
import { useToolGroupingMode } from '@/hooks/useToolGroupingMode'
import { useSessionActions } from '@/hooks/mutations/useSessionActions'
import { useCodexModels } from '@/hooks/queries/useCodexModels'
import { useCursorModels } from '@/hooks/queries/useCursorModels'
Expand Down Expand Up @@ -555,6 +556,7 @@ function SessionChatInner(props: SessionChatProps) {
const { t } = useTranslation()
const { codexExplorationCollapsed } = useCodexExplorationCollapse()
const navigate = useNavigate()
const { toolGroupingMode } = useToolGroupingMode()
const [historyActionPending, setHistoryActionPending] = useState(false)

const onForkConversation = useCallback(async (messageLocalId?: string) => {
Expand Down Expand Up @@ -593,6 +595,7 @@ function SessionChatInner(props: SessionChatProps) {
const normalizedCacheRef = useRef<Map<string, { source: DecryptedMessage; normalized: NormalizedMessage | null }>>(new Map())
const blocksByIdRef = useRef<Map<string, ChatBlock>>(new Map())
const visibleGroupsRef = useRef<ToolGroupBlock[]>([])
const visibleGroupsModeRef = useRef(toolGroupingMode)
const [forceScrollToken, setForceScrollToken] = useState(0)
const [outlineOpen, setOutlineOpen] = useState(props.initialOutlineOpen ?? false)
const [terminalVisible, setTerminalVisible] = useState(false)
Expand Down Expand Up @@ -1284,9 +1287,11 @@ function SessionChatInner(props: SessionChatProps) {
() => buildVisibleChatBlocks(reconciled.blocks, {
hasMoreMessages: props.hasMoreMessages,
previousGroups: visibleGroupsRef.current,
previousGroupingMode: visibleGroupsModeRef.current,
groupingMode: toolGroupingMode,
codexExplorationCollapsed
}),
[reconciled.blocks, props.hasMoreMessages, codexExplorationCollapsed]
[reconciled.blocks, props.hasMoreMessages, toolGroupingMode, codexExplorationCollapsed]
)

// Fork-current must compare against assistant-ui message ids (`kind:id`),
Expand Down Expand Up @@ -1314,8 +1319,9 @@ function SessionChatInner(props: SessionChatProps) {
}, [latestCompletedBoundaryId])

useEffect(() => {
visibleGroupsModeRef.current = toolGroupingMode
visibleGroupsRef.current = visibleBlocks.filter(isToolGroupBlock)
}, [visibleBlocks])
}, [toolGroupingMode, visibleBlocks])

// "N new messages" counts rendered blocks, not raw messages: a subagent run
// is dozens of sidechain messages but a single Task card, and a tool_use +
Expand Down
47 changes: 47 additions & 0 deletions web/src/hooks/useToolGroupingMode.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import { act, renderHook } from '@testing-library/react'
import { beforeEach, describe, expect, it } from 'vitest'
import {
DEFAULT_TOOL_GROUPING_MODE,
getInitialToolGroupingMode,
getToolGroupingModeOptions,
useToolGroupingMode,
} from './useToolGroupingMode'

describe('useToolGroupingMode helpers', () => {
beforeEach(() => {
window.localStorage.clear()
})

it('returns grouped and classified options', () => {
expect(getToolGroupingModeOptions()).toEqual([
{ value: 'grouped', labelKey: 'settings.chat.toolGrouping.grouped' },
{ value: 'classified', labelKey: 'settings.chat.toolGrouping.classified' },
])
})

it('defaults to grouped for missing or invalid values', () => {
expect(getInitialToolGroupingMode()).toBe(DEFAULT_TOOL_GROUPING_MODE)
window.localStorage.setItem('hapi-tool-grouping-mode', 'invalid')
expect(getInitialToolGroupingMode()).toBe(DEFAULT_TOOL_GROUPING_MODE)
})

it('reads the classified preference', () => {
window.localStorage.setItem('hapi-tool-grouping-mode', 'classified')
expect(getInitialToolGroupingMode()).toBe('classified')
})

it('syncs changes between hook consumers in the same window', () => {
const first = renderHook(() => useToolGroupingMode())
const second = renderHook(() => useToolGroupingMode())

act(() => first.result.current.setToolGroupingMode('classified'))

expect(first.result.current.toolGroupingMode).toBe('classified')
expect(second.result.current.toolGroupingMode).toBe('classified')

act(() => second.result.current.setToolGroupingMode('grouped'))

expect(first.result.current.toolGroupingMode).toBe('grouped')
expect(second.result.current.toolGroupingMode).toBe('grouped')
})
})
74 changes: 74 additions & 0 deletions web/src/hooks/useToolGroupingMode.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
import { useCallback, useEffect, useState } from 'react'
import type { ToolGroupingMode } from '@/chat/toolGroups'

export const DEFAULT_TOOL_GROUPING_MODE: ToolGroupingMode = 'grouped'

export function getToolGroupingModeOptions(): ReadonlyArray<{ value: ToolGroupingMode; labelKey: string }> {
return [
{ value: 'grouped', labelKey: 'settings.chat.toolGrouping.grouped' },
{ value: 'classified', labelKey: 'settings.chat.toolGrouping.classified' },
]
}

const STORAGE_KEY = 'hapi-tool-grouping-mode'
const CHANGE_EVENT = 'hapi-tool-grouping-mode-change'

function isBrowser(): boolean {
return typeof window !== 'undefined' && typeof document !== 'undefined'
}

function parseToolGroupingMode(raw: string | null): ToolGroupingMode {
return raw === 'grouped' || raw === 'classified'
? raw
: DEFAULT_TOOL_GROUPING_MODE
}

export function getInitialToolGroupingMode(): ToolGroupingMode {
if (!isBrowser()) return DEFAULT_TOOL_GROUPING_MODE
try {
return parseToolGroupingMode(localStorage.getItem(STORAGE_KEY))
} catch {
return DEFAULT_TOOL_GROUPING_MODE
}
}

export function useToolGroupingMode(): {
toolGroupingMode: ToolGroupingMode
setToolGroupingMode: (mode: ToolGroupingMode) => void
} {
const [toolGroupingMode, setToolGroupingModeState] = useState<ToolGroupingMode>(getInitialToolGroupingMode)

useEffect(() => {
if (!isBrowser()) return
const onStorage = (event: StorageEvent) => {
if (event.key === STORAGE_KEY) {
setToolGroupingModeState(parseToolGroupingMode(event.newValue))
}
}
const onChange = () => {
setToolGroupingModeState(getInitialToolGroupingMode())
}
window.addEventListener('storage', onStorage)
window.addEventListener(CHANGE_EVENT, onChange)
return () => {
window.removeEventListener('storage', onStorage)
window.removeEventListener(CHANGE_EVENT, onChange)
}
}, [])

const setToolGroupingMode = useCallback((mode: ToolGroupingMode) => {
setToolGroupingModeState(mode)
try {
if (mode === DEFAULT_TOOL_GROUPING_MODE) {
localStorage.removeItem(STORAGE_KEY)
} else {
localStorage.setItem(STORAGE_KEY, mode)
}
window.dispatchEvent(new Event(CHANGE_EVENT))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MINOR] Keep the custom notification independent of localStorage success

The event is dispatched inside the try block. If localStorage.setItem/removeItem throws (blocked storage or quota failure), this hook updates its own state at line 60, but the already-mounted SessionChat hook never receives the new mode. The Settings radio can therefore show Classified while the chat stays Grouped.

Suggested fix:

const onChange = (event: Event) => {
    const detail = (event as CustomEvent<ToolGroupingMode>).detail
    setToolGroupingModeState(parseToolGroupingMode(detail ?? null))
}

try {
    if (mode === DEFAULT_TOOL_GROUPING_MODE) {
        localStorage.removeItem(STORAGE_KEY)
    } else {
        localStorage.setItem(STORAGE_KEY, mode)
    }
} catch {
    // Ignore persistence errors.
}
window.dispatchEvent(new CustomEvent<ToolGroupingMode>(CHANGE_EVENT, { detail: mode }))

Please add a two-consumer regression test with localStorage.setItem throwing.

} catch {
// Ignore storage errors
}
}, [])

return { toolGroupingMode, setToolGroupingMode }
}
3 changes: 3 additions & 0 deletions web/src/lib/locales/en.ts
Original file line number Diff line number Diff line change
Expand Up @@ -799,6 +799,9 @@ export default {
'settings.chat.terminalToolDisplay': 'Terminal Tool Cards',
'settings.chat.terminalToolDisplay.compact': 'Compact (command only)',
'settings.chat.terminalToolDisplay.detailed': 'Detailed (show output preview)',
'settings.chat.toolGrouping': 'Group Tools',
'settings.chat.toolGrouping.grouped': 'Grouped',
'settings.chat.toolGrouping.classified': 'Classified',
'settings.chat.codexExplorationCollapsed': 'Collapse explored tool groups by default',
'settings.chat.codexExplorationCollapsed.desc': 'Keep Codex read and search activity collapsed until you open it.',
'settings.chat.reasoningCollapsed': 'Collapse reasoning by default',
Expand Down
3 changes: 3 additions & 0 deletions web/src/lib/locales/zh-CN.ts
Original file line number Diff line number Diff line change
Expand Up @@ -798,6 +798,9 @@ export default {
'settings.chat.terminalToolDisplay': '终端工具卡片',
'settings.chat.terminalToolDisplay.compact': '简洁(仅命令)',
'settings.chat.terminalToolDisplay.detailed': '详细(显示输出预览)',
'settings.chat.toolGrouping': '是否聚合',
'settings.chat.toolGrouping.grouped': '聚合',
'settings.chat.toolGrouping.classified': '细分',
'settings.chat.codexExplorationCollapsed': '已探索工具组默认收起',
'settings.chat.codexExplorationCollapsed.desc': 'Codex 的读取和搜索活动默认收起,点击后查看详情。',
'settings.chat.reasoningCollapsed': '思考默认折叠',
Expand Down
8 changes: 8 additions & 0 deletions web/src/routes/settings/chat.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { useTranslation } from '@/lib/use-translation'
import { getComposerEnterBehaviorOptions, useComposerEnterBehavior } from '@/hooks/useComposerEnterBehavior'
import { getTerminalToolDisplayModeOptions, useTerminalToolDisplayMode } from '@/hooks/useTerminalToolDisplayMode'
import { getToolGroupingModeOptions, useToolGroupingMode } from '@/hooks/useToolGroupingMode'
import { useCodexExplorationCollapse } from '@/hooks/useCodexExplorationCollapse'
import { useReasoningCollapse } from '@/hooks/useReasoningCollapse'
import {
Expand Down Expand Up @@ -50,6 +51,7 @@ export default function SettingsChatPage() {
const { t } = useTranslation()
const { composerEnterBehavior, setComposerEnterBehavior } = useComposerEnterBehavior()
const { terminalToolDisplayMode, setTerminalToolDisplayMode } = useTerminalToolDisplayMode()
const { toolGroupingMode, setToolGroupingMode } = useToolGroupingMode()
const { codexExplorationCollapsed, setCodexExplorationCollapsed } = useCodexExplorationCollapse()
const { reasoningCollapsed, setReasoningCollapsed } = useReasoningCollapse()
const { toolGroupBackground, userMessageBackground, setToolGroupBackground, setUserMessageBackground } = useChatSurfaceColors()
Expand All @@ -71,6 +73,12 @@ export default function SettingsChatPage() {
options={getTerminalToolDisplayModeOptions().map((option) => ({ value: option.value, label: t(option.labelKey) }))}
onChange={setTerminalToolDisplayMode}
/>
<SettingsChoiceGroup
label={t('settings.chat.toolGrouping')}
value={toolGroupingMode}
options={getToolGroupingModeOptions().map((option) => ({ value: option.value, label: t(option.labelKey) }))}
onChange={setToolGroupingMode}
/>
<SettingsSwitch
label={t('settings.chat.codexExplorationCollapsed')}
description={t('settings.chat.codexExplorationCollapsed.desc')}
Expand Down
10 changes: 10 additions & 0 deletions web/src/routes/settings/index.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,14 @@ vi.mock('@/hooks/useCodexExplorationCollapse', () => ({
useCodexExplorationCollapse: () => ({ codexExplorationCollapsed: true, setCodexExplorationCollapsed }),
}))

vi.mock('@/hooks/useToolGroupingMode', () => ({
useToolGroupingMode: () => ({ toolGroupingMode: 'grouped', setToolGroupingMode: vi.fn() }),
getToolGroupingModeOptions: () => [
{ value: 'grouped', labelKey: 'settings.chat.toolGrouping.grouped' },
{ value: 'classified', labelKey: 'settings.chat.toolGrouping.classified' },
],
}))

vi.mock('@/hooks/useChatSurfaceColors', () => ({
useChatSurfaceColors: () => ({
toolGroupBackground: 'default',
Expand Down Expand Up @@ -279,6 +287,8 @@ describe('responsive settings pages', () => {
renderPage(<SettingsChatPage />)
fireEvent.click(screen.getByRole('radio', { name: 'Insert newline' }))
expect(setComposerEnterBehavior).toHaveBeenCalledWith('newline')
expect(screen.getByRole('radio', { name: 'Grouped' })).toBeInTheDocument()
expect(screen.getByRole('radio', { name: 'Grouped' })).toBeChecked()
expect(screen.getByText('Grouped Tool Use Background')).toBeInTheDocument()
})

Expand Down
Loading