Skip to content

feat(tools): implement GroupedToolSelector for enhanced tool selection#15

Open
Zarqu0n wants to merge 1 commit intoownpilot:mainfrom
Zarqu0n:add/grouped_tool_selector
Open

feat(tools): implement GroupedToolSelector for enhanced tool selection#15
Zarqu0n wants to merge 1 commit intoownpilot:mainfrom
Zarqu0n:add/grouped_tool_selector

Conversation

@Zarqu0n
Copy link
Copy Markdown

@Zarqu0n Zarqu0n commented Mar 9, 2026

This pull request introduces a new GroupedToolSelector component to improve the user experience when selecting tools for agents. The change refactors both the agent creation and editing modals to use this new component, resulting in a more organized and scalable UI for tool selection. Additionally, there are minor dependency updates in the lockfile.

UI/UX Improvements:

  • Added the new GroupedToolSelector component, which organizes tools by category, supports group-level select/deselect, searching, and improved selection feedback. (packages/ui/src/components/GroupedToolSelector.tsx)
  • Refactored CreateAgentModal and EditAgentModal to use GroupedToolSelector instead of fetching and rendering tools directly, removing old tool-fetching and selection logic. (packages/ui/src/components/CreateAgentModal.tsx, packages/ui/src/components/EditAgentModal.tsx) [1] [2] [3] [4] [5] [6] [7] [8] [9] [10]

Dependency Updates:

  • Updated @types/node version in pnpm-lock.yaml to 22.19.13 and adjusted related package references. [1] [2] [3] [4]
  • Updated the grammy package specifier to use a caret version range in pnpm-lock.yaml.
Ekran Resmi 2026-03-09 19 37 27

Copy link
Copy Markdown
Collaborator

@ersinkoc ersinkoc left a comment

Choose a reason for hiding this comment

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

Review: GroupedToolSelector

The component extraction is well-structured and the UI is a clear improvement. A few things to address before merge:

Must Fix

  1. Stale lockfile — PR branch is behind main. The @types/node and grammy versions in pnpm-lock.yaml are regressed. Please rebase onto current main and drop unrelated lockfile changes.

Should Fix

  1. Missing unmount cleanup — The useEffect fetch in GroupedToolSelector.tsx lacks a cancellation guard. Other components in the codebase (ToolPalette.tsx, ToolsPage.tsx) use a let cancelled = false pattern with cleanup return. Please add the same to prevent stale state updates on rapid step navigation.

  2. selectAll/deselectAll should operate on filtered results — When searching, clicking "Select All" selects ALL tools (unfiltered), not just the visible ones. This is a UX surprise. Use filteredCategories instead of categories in selectAll(), and merge with existing selections rather than replacing them.

Nice to Have

  1. Import CategoryData type from tools.ts instead of redefining it locally — avoids drift risk.
  2. Use Set<string> for selectedTools lookups.includes() is O(n) per tool on every render. A useMemo(() => new Set(selectedTools), [selectedTools]) would match the pattern in ToolPicker.tsx.
  3. aria-label on the emoji span uses the raw category key (e.g. "filesystem"). Consider using category.info.description for better screen reader output.

Overall the component is solid — just needs the lockfile fixed and the two behavioral issues addressed. Nice work on the grouped UI!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants