Skip to content

Commit f8599d0

Browse files
SiddharthMantriSiddharthMantri
committed
Address pr comments, reuse components, extract constant
1 parent 89d30f7 commit f8599d0

3 files changed

Lines changed: 151 additions & 45 deletions

File tree

‎x-pack/platform/plugins/shared/agent_builder/public/application/components/agents/tools/agent_tools.tsx‎

Lines changed: 81 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,76 @@ import { useAgentBuilderServices } from '../../../hooks/use_agent_builder_servic
3030
import { useToasts } from '../../../hooks/use_toasts';
3131
import { queryKeys } from '../../../query_keys';
3232
import { useFlyoutState } from '../../../hooks/use_flyout_state';
33-
import { isToolSelected, toggleToolSelection } from '../../../utils/tool_selection_utils';
33+
import {
34+
getActiveTools,
35+
isToolSelected,
36+
toggleToolSelection,
37+
} from '../../../utils/tool_selection_utils';
3438
import { ActiveItemRow } from '../common/active_item_row';
3539
import { ToolLibraryPanel } from './tool_library_panel';
3640
import { ToolDetailPanel } from './tool_detail_panel';
3741

3842
const FLEX_ITEM_WIDTH = '280px';
3943

44+
const ActiveToolsList: React.FC<{
45+
filteredActiveTools: ToolDefinition[];
46+
searchQuery: string;
47+
selectedToolId: string | null;
48+
enableElasticCapabilities: boolean;
49+
defaultToolIdSet: Set<string>;
50+
isRemoving: boolean;
51+
onSelect: (id: string) => void;
52+
onRemove: (tool: ToolDefinition) => void;
53+
}> = ({
54+
filteredActiveTools,
55+
searchQuery,
56+
selectedToolId,
57+
enableElasticCapabilities,
58+
defaultToolIdSet,
59+
isRemoving,
60+
onSelect,
61+
onRemove,
62+
}) => {
63+
if (filteredActiveTools.length === 0) {
64+
return (
65+
<EuiText size="s" color="subdued" textAlign="center">
66+
<p>
67+
{searchQuery.trim()
68+
? labels.agentTools.noActiveToolsMatchMessage
69+
: labels.agentTools.noActiveToolsMessage}
70+
</p>
71+
</EuiText>
72+
);
73+
}
74+
75+
return (
76+
<>
77+
{filteredActiveTools.map((tool) => {
78+
const isBuiltinManaged = enableElasticCapabilities && defaultToolIdSet.has(tool.id);
79+
return (
80+
<ActiveItemRow
81+
key={tool.id}
82+
id={tool.id}
83+
name={tool.id}
84+
isSelected={selectedToolId === tool.id}
85+
onSelect={() => onSelect(tool.id)}
86+
onRemove={() => onRemove(tool)}
87+
isRemoving={isRemoving}
88+
removeAriaLabel={labels.agentTools.removeToolAriaLabel}
89+
readOnlyContent={
90+
isBuiltinManaged ? (
91+
<EuiBadge color="hollow">
92+
{labels.agentTools.elasticCapabilitiesReadOnlyBadge}
93+
</EuiBadge>
94+
) : undefined
95+
}
96+
/>
97+
);
98+
})}
99+
</>
100+
);
101+
};
102+
40103
export const AgentTools: React.FC = () => {
41104
const { agentId } = useParams<{ agentId: string }>();
42105
const { euiTheme } = useEuiTheme();
@@ -65,18 +128,13 @@ export const AgentTools: React.FC = () => {
65128

66129
const defaultToolIdSet = useMemo(() => new Set<string>(defaultAgentToolIds), []);
67130

68-
const activeTools = useMemo(() => {
69-
if (!agent) return [];
70-
const explicitTools = allTools.filter((t) => isToolSelected(t, agentToolSelections));
71-
if (enableElasticCapabilities) {
72-
const explicitIdSet = new Set(explicitTools.map((t) => t.id));
73-
const defaultToolsNotExplicit = allTools.filter(
74-
(t) => defaultToolIdSet.has(t.id) && !explicitIdSet.has(t.id)
75-
);
76-
return [...explicitTools, ...defaultToolsNotExplicit];
77-
}
78-
return explicitTools;
79-
}, [allTools, agentToolSelections, agent, enableElasticCapabilities, defaultToolIdSet]);
131+
const activeTools = useMemo(
132+
() =>
133+
agent
134+
? getActiveTools(allTools, agentToolSelections, enableElasticCapabilities, defaultToolIdSet)
135+
: [],
136+
[allTools, agentToolSelections, agent, enableElasticCapabilities, defaultToolIdSet]
137+
);
80138

81139
const activeToolIdSet = useMemo(() => new Set(activeTools.map((t) => t.id)), [activeTools]);
82140

@@ -259,38 +317,16 @@ export const AgentTools: React.FC = () => {
259317
padding: 0px ${euiTheme.size.m} ${euiTheme.size.s} 0px;
260318
`}
261319
>
262-
{filteredActiveTools.length === 0 ? (
263-
<EuiText size="s" color="subdued" textAlign="center">
264-
<p>
265-
{searchQuery.trim()
266-
? labels.agentTools.noActiveToolsMatchMessage
267-
: labels.agentTools.noActiveToolsMessage}
268-
</p>
269-
</EuiText>
270-
) : (
271-
filteredActiveTools.map((tool) => {
272-
const isBuiltinManaged = enableElasticCapabilities && defaultToolIdSet.has(tool.id);
273-
return (
274-
<ActiveItemRow
275-
key={tool.id}
276-
id={tool.id}
277-
name={tool.id}
278-
isSelected={selectedToolId === tool.id}
279-
onSelect={() => setSelectedToolId(tool.id)}
280-
onRemove={() => handleRemoveTool(tool)}
281-
isRemoving={updateToolsMutation.isLoading}
282-
removeAriaLabel={labels.agentTools.removeToolAriaLabel}
283-
readOnlyContent={
284-
isBuiltinManaged ? (
285-
<EuiBadge color="hollow">
286-
{labels.agentTools.elasticCapabilitiesReadOnlyBadge}
287-
</EuiBadge>
288-
) : undefined
289-
}
290-
/>
291-
);
292-
})
293-
)}
320+
<ActiveToolsList
321+
filteredActiveTools={filteredActiveTools}
322+
searchQuery={searchQuery}
323+
selectedToolId={selectedToolId}
324+
enableElasticCapabilities={enableElasticCapabilities}
325+
defaultToolIdSet={defaultToolIdSet}
326+
isRemoving={updateToolsMutation.isLoading}
327+
onSelect={setSelectedToolId}
328+
onRemove={handleRemoveTool}
329+
/>
294330
</div>
295331
</EuiFlexItem>
296332

‎x-pack/platform/plugins/shared/agent_builder/public/application/utils/tool_selection_utils.test.ts‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import { ToolType, allToolsSelectionWildcard } from '@kbn/agent-builder-common';
1414
import {
1515
toggleToolSelection,
1616
isToolSelected,
17+
getActiveTools,
1718
cleanInvalidToolReferences,
1819
} from './tool_selection_utils';
1920
import type { AgentEditState } from '../hooks/agents/use_agent_edit';
@@ -79,6 +80,53 @@ describe('tool_selection_utils', () => {
7980
});
8081
});
8182

83+
describe('getActiveTools', () => {
84+
const defaultToolIds = new Set(['tool1', 'tool3']);
85+
86+
it('should return only explicitly selected tools when elastic capabilities are disabled', () => {
87+
const selections: ToolSelection[] = [{ tool_ids: ['tool2'] }];
88+
const result = getActiveTools(mockTools, selections, false, defaultToolIds);
89+
90+
expect(result.map((t) => t.id)).toEqual(['tool2']);
91+
});
92+
93+
it('should include default tools when elastic capabilities are enabled', () => {
94+
const selections: ToolSelection[] = [{ tool_ids: ['tool2'] }];
95+
const result = getActiveTools(mockTools, selections, true, defaultToolIds);
96+
97+
expect(result.map((t) => t.id)).toEqual(['tool2', 'tool1', 'tool3']);
98+
});
99+
100+
it('should not duplicate tools that are both explicit and default', () => {
101+
const selections: ToolSelection[] = [{ tool_ids: ['tool1', 'tool2'] }];
102+
const result = getActiveTools(mockTools, selections, true, defaultToolIds);
103+
104+
expect(result.map((t) => t.id)).toEqual(['tool1', 'tool2', 'tool3']);
105+
});
106+
107+
it('should return empty array when no tools match selections', () => {
108+
const selections: ToolSelection[] = [{ tool_ids: [] }];
109+
const result = getActiveTools(mockTools, selections, false, defaultToolIds);
110+
111+
expect(result).toEqual([]);
112+
});
113+
114+
it('should return only default tools when no explicit selections and elastic capabilities are enabled', () => {
115+
const selections: ToolSelection[] = [{ tool_ids: [] }];
116+
const result = getActiveTools(mockTools, selections, true, defaultToolIds);
117+
118+
expect(result.map((t) => t.id)).toEqual(['tool1', 'tool3']);
119+
});
120+
121+
it('should handle wildcard selections with elastic capabilities', () => {
122+
const selections: ToolSelection[] = [{ tool_ids: [allToolsSelectionWildcard] }];
123+
const result = getActiveTools(mockTools, selections, true, defaultToolIds);
124+
125+
// All tools already selected via wildcard, no defaults to append
126+
expect(result.map((t) => t.id)).toEqual(['tool1', 'tool2', 'tool3']);
127+
});
128+
});
129+
82130
describe('cleanInvalidToolReferences', () => {
83131
const mockToolDefinitions: ToolDefinition[] = [
84132
{

‎x-pack/platform/plugins/shared/agent_builder/public/application/utils/tool_selection_utils.ts‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,28 @@ export const toggleToolSelection = (
9393
}
9494
};
9595

96+
/**
97+
* Returns the list of active tools for an agent, combining explicitly selected tools
98+
* with default tools when elastic capabilities are enabled. Default tools that are
99+
* already explicitly selected are not duplicated.
100+
*/
101+
export const getActiveTools = <T extends ToolSelectionRelevantFields>(
102+
allTools: T[],
103+
agentToolSelections: ToolSelection[],
104+
enableElasticCapabilities: boolean,
105+
defaultToolIds: Set<string>
106+
): T[] => {
107+
const explicitTools = allTools.filter((t) => isToolSelected(t, agentToolSelections));
108+
if (enableElasticCapabilities) {
109+
const explicitIdSet = new Set(explicitTools.map((t) => t.id));
110+
const defaultToolsNotExplicit = allTools.filter(
111+
(t) => defaultToolIds.has(t.id) && !explicitIdSet.has(t.id)
112+
);
113+
return [...explicitTools, ...defaultToolsNotExplicit];
114+
}
115+
return explicitTools;
116+
};
117+
96118
/**
97119
* Removes invalid tool references from the agent configuration.
98120
* Filters out tool IDs that don't exist in the available tools list,

0 commit comments

Comments
 (0)