Skip to content

Commit 6cda068

Browse files
committed
perf: optimize agent tool picker
1 parent 86c17d2 commit 6cda068

5 files changed

Lines changed: 202 additions & 46 deletions

File tree

apps/cli/src/commands/agent/agents/ui/EditAgent.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ export function EditAgent(props: {
139139
</Box>
140140
) : null}
141141
</Panel>
142-
<Instructions instructions="Press Enter to toggle selection - Up/Down Navigate - Esc to go back" />
142+
<Instructions instructions="Enter activate - c continue - a all/none - Up/Down or j/k navigate - Esc back" />
143143
</>
144144
)
145145
}
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
import { describe, expect, test } from 'bun:test'
2+
import {
3+
__getFocusableToolPickerIndexForTests,
4+
__getToolPickerMaxVisibleItemsForTests,
5+
} from './ToolPicker'
6+
7+
describe('ToolPicker viewport sizing', () => {
8+
test('keeps a useful minimum in small terminals', () => {
9+
expect(__getToolPickerMaxVisibleItemsForTests(10)).toBe(5)
10+
})
11+
12+
test('caps rendered rows in large terminals', () => {
13+
expect(__getToolPickerMaxVisibleItemsForTests(80)).toBe(14)
14+
})
15+
16+
test('uses available space between the min and max', () => {
17+
expect(__getToolPickerMaxVisibleItemsForTests(22)).toBe(12)
18+
})
19+
})
20+
21+
describe('ToolPicker focus movement', () => {
22+
const items = [{}, { isHeader: true }, {}, {}, { isHeader: true }, {}]
23+
24+
test('skips headers while moving down', () => {
25+
expect(__getFocusableToolPickerIndexForTests(items, 1, 1)).toBe(2)
26+
})
27+
28+
test('skips headers while moving up', () => {
29+
expect(__getFocusableToolPickerIndexForTests(items, 4, -1)).toBe(3)
30+
})
31+
32+
test('falls back when the target edge is a header', () => {
33+
expect(
34+
__getFocusableToolPickerIndexForTests([{ isHeader: true }, {}], 0, -1),
35+
).toBe(1)
36+
})
37+
})

apps/cli/src/commands/agent/agents/ui/wizard/ToolPicker.tsx

Lines changed: 162 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1,39 +1,100 @@
1-
import React, { useMemo, useState } from 'react'
1+
import React, { useCallback, useEffect, useMemo, useState } from 'react'
22
import { Box, Text } from 'ink'
33
import figures from 'figures'
44
import type { Tool } from '../../tooling'
55
import { themeColor } from '../colors'
66
import { parseMcpToolName } from '../utils'
77
import { useKeypress } from '#ui-ink/hooks/useKeypress'
8+
import { useTerminalSize } from '#ui-ink/hooks/useTerminalSize'
9+
import { getWindowedList } from '#ui-ink/primitives/list/windowedList'
10+
11+
const MIN_VISIBLE_TOOL_ITEMS = 5
12+
const MAX_VISIBLE_TOOL_ITEMS = 14
13+
const TOOL_PICKER_RESERVED_ROWS = 10
14+
15+
type ToolPickerItem = {
16+
id: string
17+
label: string
18+
isHeader?: boolean
19+
isToggle?: boolean
20+
action: () => void
21+
}
22+
23+
function clamp(value: number, min: number, max: number): number {
24+
return Math.min(max, Math.max(min, value))
25+
}
26+
27+
function getToolPickerMaxVisibleItems(rows: number): number {
28+
return clamp(
29+
rows - TOOL_PICKER_RESERVED_ROWS,
30+
MIN_VISIBLE_TOOL_ITEMS,
31+
MAX_VISIBLE_TOOL_ITEMS,
32+
)
33+
}
34+
35+
function getFocusableToolPickerIndex(
36+
items: ReadonlyArray<{ isHeader?: boolean }>,
37+
targetIndex: number,
38+
direction: -1 | 1,
39+
): number {
40+
if (items.length === 0) return 0
41+
42+
const clampedTarget = clamp(targetIndex, 0, items.length - 1)
43+
if (!items[clampedTarget]?.isHeader) return clampedTarget
44+
45+
for (
46+
let index = clampedTarget + direction;
47+
index >= 0 && index < items.length;
48+
index += direction
49+
) {
50+
if (!items[index]?.isHeader) return index
51+
}
52+
53+
for (
54+
let index = clampedTarget - direction;
55+
index >= 0 && index < items.length;
56+
index -= direction
57+
) {
58+
if (!items[index]?.isHeader) return index
59+
}
60+
61+
return 0
62+
}
63+
64+
export const __getToolPickerMaxVisibleItemsForTests =
65+
getToolPickerMaxVisibleItems
66+
export const __getFocusableToolPickerIndexForTests = getFocusableToolPickerIndex
867

968
export function ToolPicker(props: {
1069
tools: Tool[]
1170
initialTools: string[] | undefined
1271
onComplete: (tools: string[] | undefined) => void
1372
onCancel: () => void
1473
}) {
74+
const { tools, initialTools, onComplete, onCancel } = props
75+
const terminalSize = useTerminalSize()
1576
const normalizedTools = useMemo(() => {
1677
const unique = new Map<string, Tool>()
17-
for (const tool of props.tools) {
78+
for (const tool of tools) {
1879
if (!tool?.name) continue
1980
unique.set(tool.name, tool)
2081
}
2182
return Array.from(unique.values()).sort((a, b) =>
2283
a.name.localeCompare(b.name),
2384
)
24-
}, [props.tools])
85+
}, [tools])
2586

2687
const allToolNames = useMemo(
2788
() => normalizedTools.map(t => t.name),
2889
[normalizedTools],
2990
)
3091

3192
const initialSelectedNames = useMemo(() => {
32-
if (!props.initialTools) return allToolNames
33-
if (props.initialTools.includes('*')) return allToolNames
93+
if (!initialTools) return allToolNames
94+
if (initialTools.includes('*')) return allToolNames
3495
const available = new Set(allToolNames)
35-
return props.initialTools.filter(t => available.has(t))
36-
}, [props.initialTools, allToolNames])
96+
return initialTools.filter(t => available.has(t))
97+
}, [initialTools, allToolNames])
3798

3899
const [selected, setSelected] = useState<string[]>(initialSelectedNames)
39100
const [cursorIndex, setCursorIndex] = useState(0)
@@ -43,30 +104,30 @@ export function ToolPicker(props: {
43104
const isAllSelected =
44105
selected.length === allToolNames.length && allToolNames.length > 0
45106

46-
const toggleOne = (name: string) => {
107+
const toggleOne = useCallback((name: string) => {
47108
setSelected(prev =>
48109
prev.includes(name) ? prev.filter(x => x !== name) : [...prev, name],
49110
)
50-
}
111+
}, [])
51112

52-
const toggleMany = (names: string[], enable: boolean) => {
113+
const toggleMany = useCallback((names: string[], enable: boolean) => {
53114
setSelected(prev => {
54115
if (enable) {
55116
const missing = names.filter(n => !prev.includes(n))
56117
return [...prev, ...missing]
57118
}
58119
return prev.filter(n => !names.includes(n))
59120
})
60-
}
121+
}, [])
61122

62-
const complete = () => {
123+
const complete = useCallback(() => {
63124
const next =
64125
selected.length === allToolNames.length &&
65126
allToolNames.every(n => selected.includes(n))
66127
? undefined
67128
: selected
68-
props.onComplete(next)
69-
}
129+
onComplete(next)
130+
}, [allToolNames, onComplete, selected])
70131

71132
const categorized = useMemo(() => {
72133
const readOnly = new Set(['Read', 'LS', 'Glob', 'Grep'])
@@ -104,16 +165,8 @@ export function ToolPicker(props: {
104165
.sort((a, b) => a.serverName.localeCompare(b.serverName))
105166
}, [categorized.mcp])
106167

107-
type Item = {
108-
id: string
109-
label: string
110-
isHeader?: boolean
111-
isToggle?: boolean
112-
action: () => void
113-
}
114-
115-
const items: Item[] = useMemo(() => {
116-
const out: Item[] = []
168+
const items: ToolPickerItem[] = useMemo(() => {
169+
const out: ToolPickerItem[] = []
117170

118171
out.push({ id: 'continue', label: '[ Continue ]', action: complete })
119172
out.push({
@@ -204,11 +257,47 @@ export function ToolPicker(props: {
204257
mcpServers,
205258
selectedSet,
206259
showAdvanced,
260+
toggleMany,
261+
toggleOne,
207262
])
208263

209-
useKeypress((_input, key) => {
264+
useEffect(() => {
265+
setCursorIndex(prev =>
266+
getFocusableToolPickerIndex(items, Math.min(prev, items.length - 1), -1),
267+
)
268+
}, [items])
269+
270+
const maxVisibleItems = getToolPickerMaxVisibleItems(terminalSize.rows)
271+
const window = useMemo(
272+
() =>
273+
getWindowedList({
274+
itemCount: items.length,
275+
focusIndex: cursorIndex,
276+
maxVisible: maxVisibleItems,
277+
indicatorRows: 2,
278+
}),
279+
[cursorIndex, items.length, maxVisibleItems],
280+
)
281+
const visibleItems = useMemo(
282+
() => items.slice(window.start, window.end),
283+
[items, window.end, window.start],
284+
)
285+
286+
useKeypress((input, key) => {
287+
const inputChar = input.length === 1 ? input : ''
288+
210289
if (key.escape) {
211-
props.onCancel()
290+
onCancel()
291+
return true
292+
}
293+
294+
if (inputChar === 'c') {
295+
complete()
296+
return true
297+
}
298+
299+
if (inputChar === 'a') {
300+
toggleMany(allToolNames, !isAllSelected)
212301
return true
213302
}
214303

@@ -218,32 +307,57 @@ export function ToolPicker(props: {
218307
return true
219308
}
220309

221-
if (key.upArrow) {
222-
let next = cursorIndex - 1
223-
while (next > 0 && items[next]?.isHeader) next--
224-
setCursorIndex(Math.max(0, next))
310+
if (key.upArrow || inputChar === 'k') {
311+
setCursorIndex(prev => getFocusableToolPickerIndex(items, prev - 1, -1))
312+
return true
313+
}
314+
315+
if (key.downArrow || inputChar === 'j') {
316+
setCursorIndex(prev => getFocusableToolPickerIndex(items, prev + 1, 1))
225317
return true
226318
}
227319

228-
if (key.downArrow) {
229-
let next = cursorIndex + 1
230-
while (next < items.length - 1 && items[next]?.isHeader) next++
231-
setCursorIndex(Math.min(items.length - 1, next))
320+
if (key.pageUp) {
321+
setCursorIndex(prev =>
322+
getFocusableToolPickerIndex(items, prev - window.visibleCount, -1),
323+
)
324+
return true
325+
}
326+
327+
if (key.pageDown) {
328+
setCursorIndex(prev =>
329+
getFocusableToolPickerIndex(items, prev + window.visibleCount, 1),
330+
)
331+
return true
332+
}
333+
334+
if (key.home || inputChar === 'g') {
335+
setCursorIndex(getFocusableToolPickerIndex(items, 0, 1))
336+
return true
337+
}
338+
339+
if (key.end || inputChar === 'G') {
340+
setCursorIndex(getFocusableToolPickerIndex(items, items.length - 1, -1))
232341
return true
233342
}
234343
})
235344

345+
const topIndicator = window.showUpIndicator
346+
? `More above (${window.start})`
347+
: ' '
348+
const bottomIndicator = window.showDownIndicator
349+
? `More below (${items.length - window.end})`
350+
: ' '
351+
const rangeSummary =
352+
items.length > maxVisibleItems
353+
? `Showing ${window.start + 1}-${window.end} of ${items.length}`
354+
: null
355+
236356
return (
237357
<Box flexDirection="column" marginTop={1}>
238-
<Text
239-
color={cursorIndex === 0 ? themeColor('suggestion') : undefined}
240-
bold={cursorIndex === 0}
241-
>
242-
{cursorIndex === 0 ? `${figures.pointer} ` : ' '}[ Continue ]
243-
</Text>
244-
<Text dimColor>{'-'.repeat(40)}</Text>
245-
{items.slice(1).map((item, idx) => {
246-
const index = idx + 1
358+
<Text dimColor>{topIndicator}</Text>
359+
{visibleItems.map((item, idx) => {
360+
const index = window.start + idx
247361
const focused = index === cursorIndex
248362
const prefix = item.isHeader
249363
? ''
@@ -267,12 +381,17 @@ export function ToolPicker(props: {
267381
</React.Fragment>
268382
)
269383
})}
384+
<Text dimColor>{bottomIndicator}</Text>
270385
<Box marginTop={1} flexDirection="column">
271386
<Text dimColor>
272387
{isAllSelected
273388
? 'All tools selected'
274389
: `${selectedSet.size} of ${allToolNames.length} tools selected`}
275390
</Text>
391+
{rangeSummary ? <Text dimColor>{rangeSummary}</Text> : null}
392+
<Text dimColor>
393+
c continue - a all/none - j/k or arrows - PgUp/PgDn - Home/End
394+
</Text>
276395
</Box>
277396
</Box>
278397
)

apps/cli/src/commands/agent/agents/ui/wizard/steps/StepSelectTools.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ export function StepSelectTools(props: {
3333
/>
3434
</Box>
3535
</Panel>
36-
<Instructions instructions="Press Enter to activate - Up/Down to navigate - Esc to go back" />
36+
<Instructions instructions="Enter activate - c continue - a all/none - Up/Down or j/k navigate - Esc back" />
3737
</>
3838
)
3939
}

apps/cli/src/ui/components/CustomSelect/use-select-state.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ export const useSelectState = ({
202202
onFocus,
203203
focusValue,
204204
}: UseSelectStateProps) => {
205-
const flatOptions = flattenOptions(options)
205+
const flatOptions = useMemo(() => flattenOptions(options), [options])
206206

207207
const [state, dispatch] = useReducer(
208208
reducer,

0 commit comments

Comments
 (0)