From bcbbd9a4554b62290b3b4c8d57ee288bc8c5d273 Mon Sep 17 00:00:00 2001 From: anakin87 Date: Fri, 7 Aug 2026 16:38:34 +0200 Subject: [PATCH 01/13] toolset refactor POC --- docs-website/docs/tools/searchabletoolset.mdx | 2 +- docs-website/docs/tools/toolset.mdx | 22 +- haystack/components/agents/agent.py | 13 +- haystack/components/agents/utils.py | 76 ++-- haystack/tools/searchable_toolset.py | 44 +-- haystack/tools/skills/skill_toolset.py | 7 - haystack/tools/toolset.py | 325 ++++-------------- .../simplify-toolset-102a9effbe4f188f.yaml | 33 ++ test/components/agents/test_agent.py | 5 +- test/components/agents/test_utils.py | 39 ++- test/tools/skills/test_skill_toolset.py | 7 - test/tools/test_searchable_toolset.py | 73 ++-- test/tools/test_tools_utils.py | 4 +- test/tools/test_toolset.py | 207 ++--------- test/tools/test_toolset_wrapper.py | 253 -------------- 15 files changed, 274 insertions(+), 836 deletions(-) create mode 100644 releasenotes/notes/simplify-toolset-102a9effbe4f188f.yaml delete mode 100644 test/tools/test_toolset_wrapper.py diff --git a/docs-website/docs/tools/searchabletoolset.mdx b/docs-website/docs/tools/searchabletoolset.mdx index b4e1ef3465a..fd418e49e12 100644 --- a/docs-website/docs/tools/searchabletoolset.mdx +++ b/docs-website/docs/tools/searchabletoolset.mdx @@ -118,7 +118,7 @@ toolset = SearchableToolset( ### Reusing the toolset across multiple agent runs -You can safely reuse the same `SearchableToolset` instance across multiple agent runs, including concurrent ones. Each `Agent` run operates on an isolated, run-scoped copy of the toolset (created with [`spawn()`](toolset.mdx#run-scoped-copies-and-tool-selection)), so tools discovered in one run do not persist into, or collide with, other runs — every run starts fresh from the catalog: +You can safely reuse the same `SearchableToolset` instance across multiple agent runs, including concurrent ones. The `Agent` internally gives each run an isolated copy of the toolset, so tools discovered in one run do not persist into, or collide with, other runs — every run starts fresh from the catalog: ```python agent = Agent( diff --git a/docs-website/docs/tools/toolset.mdx b/docs-website/docs/tools/toolset.mdx index 33ce1a8f6cc..4cc1d8a3dbd 100644 --- a/docs-website/docs/tools/toolset.mdx +++ b/docs-website/docs/tools/toolset.mdx @@ -73,21 +73,25 @@ def multiply_numbers( math_toolset.add(multiply_numbers) - -# or, you can merge toolsets together -math_toolset.add(another_toolset) ``` -### Run-Scoped Copies and Tool Selection +### Combining Toolsets + +To use multiple Toolsets together, pass them as a list wherever tools are accepted. This keeps each Toolset intact, preserving its own lifecycle and serialization: + +```python +agent = Agent( + chat_generator=OpenAIChatGenerator(), tools=[math_toolset, another_toolset] +) +``` -A `Toolset` is never mutated in place during an [`Agent`](../pipeline-components/agents-1/agent.mdx) run. Each run operates on an isolated, run-scoped copy of the configured `Toolset`, created with the `spawn()` method. This makes concurrent runs that share the same `Toolset` instance safe: per-run state, such as an active tool-name selection or a [`SearchableToolset`](searchabletoolset.mdx)'s discovered tools, cannot leak or collide across runs. +To deliberately merge simple Toolsets into one, unpack them explicitly: `Toolset([*math_toolset, *another_toolset])`. Avoid this for Toolsets that manage resources or load tools lazily (such as an `MCPToolset`): pass them as a list instead. -You can also restrict an `Agent` to a subset of tools at runtime by passing tool names, for example `agent.run(tools=["tool_a", "tool_b"])`. When a `Toolset` is configured, the selection is applied to the live (run-scoped) `Toolset` rather than flattening it into a static list, so dynamic behavior like a `SearchableToolset`'s search and lazy loading keeps working over the selected subset. +### Tool Selection at Runtime -Two methods support this and can be overridden when subclassing: +A `Toolset` is never mutated in place during an [`Agent`](../pipeline-components/agents-1/agent.mdx) run, so concurrent runs sharing the same `Toolset` instance are safe. -- `get_selectable_tools()`: Returns every tool available for name-based selection, ignoring any active selection restriction. Override it if your subclass's iteration does not surface every selectable tool. -- `spawn()`: Returns an isolated, run-scoped copy of the `Toolset`. Override it if your subclass holds additional run-scoped state. +You can restrict an `Agent` to a subset of tools at runtime by passing tool names, for example `agent.run(tools=["tool_a", "tool_b"])`. This also works with a [`SearchableToolset`](searchabletoolset.mdx): the `Agent` internally gives each run an isolated copy carrying the selection, so dynamic behavior like search and lazy loading keeps working over the selected subset. ## Usage diff --git a/haystack/components/agents/agent.py b/haystack/components/agents/agent.py index f3f3147ca4e..10e280334bb 100644 --- a/haystack/components/agents/agent.py +++ b/haystack/components/agents/agent.py @@ -17,12 +17,12 @@ from haystack.components.agents.state.state_utils import merge_lists from haystack.components.agents.tool_calling import _run_tool, _run_tool_async from haystack.components.agents.utils import ( + _copy_tools_for_run, _record_context_tokens, _record_llm_usage, _record_tool_calls, _render_prompt_messages, _select_tools_by_name, - _spawn_tools, _template_for_role, _validate_prompt_message_blocks, ) @@ -791,10 +791,11 @@ def _select_tools(self, tools: ToolsType | list[str] | None = None) -> ToolsType or if any provided tool name is not valid. :raises TypeError: If tools is not a list of Tool objects, a Toolset, or a list of tool names (strings). """ - # Toolsets are spawned into per-run copies (see _spawn_tools / _select_tools_by_name) so concurrent runs - # sharing the same configured Toolset don't corrupt each other's run-scoped state. + # Toolsets with run-scoped state (those defining _copy_for_run(), e.g. SearchableToolset) are replaced + # by per-run copies (see _copy_tools_for_run / _select_tools_by_name) so concurrent runs sharing the + # same configured Toolset don't corrupt each other's run-scoped state. if tools is None: - return _spawn_tools(tools=self.tools) + return _copy_tools_for_run(tools=self.tools) if isinstance(tools, list) and all(isinstance(t, str) for t in tools): return _select_tools_by_name(self.tools, cast(list[str], tools)) @@ -803,14 +804,14 @@ def _select_tools(self, tools: ToolsType | list[str] | None = None) -> ToolsType # Per-run tools are not covered by the Agent's own warm_up(), so warm them up here. # warm_up() is expected to be idempotent, so re-warming on every run is cheap. warm_up_tools(tools=tools) - return _spawn_tools(tools=tools) + return _copy_tools_for_run(tools=tools) if isinstance(tools, list): selected = cast(list[Tool | Toolset], tools) # mypy can't narrow the Union type from isinstance check # Per-run tools are not covered by the Agent's own warm_up(), so warm them up here. # warm_up() is expected to be idempotent, so re-warming on every run is cheap. warm_up_tools(tools=selected) - return _spawn_tools(tools=selected) + return _copy_tools_for_run(tools=selected) raise TypeError( "tools must be a list of Tool and/or Toolset objects, a Toolset, or a list of tool names (strings)." diff --git a/haystack/components/agents/utils.py b/haystack/components/agents/utils.py index c34743a2cbc..310e649ef0e 100644 --- a/haystack/components/agents/utils.py +++ b/haystack/components/agents/utils.py @@ -90,18 +90,49 @@ def _record_tool_calls(state: State, tool_messages: list[ChatMessage]) -> None: # --------------------------- +def _copy_toolset_for_run(toolset: Toolset, selected_tool_names: set[str] | None = None) -> Toolset: + """ + Return a per-run copy of a Toolset with run-scoped state; plain Toolsets are returned unchanged. + + A Toolset signals run-scoped state by defining `_copy_for_run()` (e.g. SearchableToolset, whose + discovered tools and name selection are per-run). Plain Toolsets are read-only at run time and can be shared + across runs directly. Note: passing `selected_tool_names` to a plain Toolset is unsupported here; callers + filter plain Toolsets externally. + """ + copy_for_run = getattr(toolset, "_copy_for_run", None) + if callable(copy_for_run): + return copy_for_run(selected_tool_names=selected_tool_names) + return toolset + + +def _selectable_names(item: Tool | Toolset) -> set[str]: + """ + Resolve the tool names an item offers for name-based selection. + + A Toolset providing a `get_selectable_tools()` method (e.g. SearchableToolset, whose iteration does not + surface the full catalog) is asked through it; any other Toolset is warmed up and iterated. + """ + if not isinstance(item, Toolset): + return {item.name} + if hasattr(item, "get_selectable_tools"): + return {tool.name for tool in item.get_selectable_tools()} + item.warm_up() + return {tool.name for tool in item} + + def _select_tools_by_name(configured_tools: ToolsType, names: list[str]) -> list[Tool | Toolset]: """ Select configured tools by name for a single run. - Standalone Tools are kept when their name is requested. A Toolset that exposes a requested name is replaced by a - per-run `spawn()` (an isolated copy) with the requested names registered as its `_selected_tool_names`, so - dynamic toolsets such as SearchableToolset preserve their behavior (search/lazy-loading) over the selected subset - without mutating the shared, configured Toolset. + Standalone Tools are kept when their name is requested. A plain Toolset that exposes requested names is + warmed up and reduced to the matching Tools. A Toolset with run-scoped state (one defining + `_copy_for_run()`, such as SearchableToolset) is replaced by a per-run copy carrying the requested + names, so its dynamic behavior (search/lazy-loading) is preserved without mutating the shared, configured + Toolset. :param configured_tools: The tools configured on the Agent. :param names: The requested tool names. - :returns: The selected standalone Tools and/or spawned, selection-scoped Toolsets. + :returns: The selected Tools and/or selection-scoped Toolset copies. :raises ValueError: If no tools were configured, or if any requested name is not a valid tool name. """ if not configured_tools: @@ -112,14 +143,8 @@ def _select_tools_by_name(configured_tools: ToolsType, names: list[str]) -> list [configured_tools] if isinstance(configured_tools, Toolset) else list(configured_tools) ) - # Resolve selectable names per item. For Toolsets we use get_selectable_tools() so dynamic toolsets - # (e.g. SearchableToolset) offer their full catalog by name, not just the tools exposed by iteration. - selectable_per_item: list[tuple[Tool | Toolset, set[str]]] = [] - valid_tool_names: set[str] = set() - for item in items: - item_names = {tool.name for tool in item.get_selectable_tools()} if isinstance(item, Toolset) else {item.name} - selectable_per_item.append((item, item_names)) - valid_tool_names |= item_names + selectable_per_item = [(item, _selectable_names(item)) for item in items] + valid_tool_names = {name for _, item_names in selectable_per_item for name in item_names} invalid_tool_names = requested_names - valid_tool_names if invalid_tool_names: @@ -132,27 +157,28 @@ def _select_tools_by_name(configured_tools: ToolsType, names: list[str]) -> list matched = requested_names & item_names if not matched: continue - if isinstance(item, Toolset): - # Apply the selection to a per-run copy so the shared, configured Toolset is never mutated. - spawned = item.spawn() - spawned._selected_tool_names = matched - selected.append(spawned) + if isinstance(item, Toolset) and hasattr(item, "_copy_for_run"): + # The selection is applied to the per-run copy, so the shared, configured Toolset is never mutated. + selected.append(_copy_toolset_for_run(item, selected_tool_names=matched)) + elif isinstance(item, Toolset): + selected.extend(tool for tool in item if tool.name in matched) else: selected.append(item) return selected -def _spawn_tools(tools: ToolsType) -> ToolsType: +def _copy_tools_for_run(tools: ToolsType) -> ToolsType: """ - Return per-run copies of `tools`, replacing each Toolset with an isolated `spawn()` (Tools are passed through). + Return per-run copies of the Toolsets in `tools` that carry run-scoped state. - This isolates run-scoped Toolset state (e.g. a SearchableToolset's discovered tools and any active name - selection) so that concurrent runs sharing the same configured Toolset — such as parallel sub-agent tool calls - or concurrent requests against one Agent — don't corrupt each other. + A Toolset defining `_copy_for_run()` (e.g. SearchableToolset, whose discovered tools and name + selection are per-run) is replaced by an isolated copy, so concurrent runs sharing the same configured + Toolset — such as parallel sub-agent tool calls or concurrent requests against one Agent — don't corrupt + each other. Plain Toolsets and standalone Tools are read-only at run time and are passed through unchanged. """ if isinstance(tools, Toolset): - return tools.spawn() - return [item.spawn() if isinstance(item, Toolset) else item for item in tools] + return _copy_toolset_for_run(tools) + return [_copy_toolset_for_run(item) if isinstance(item, Toolset) else item for item in tools] # --------------------------- diff --git a/haystack/tools/searchable_toolset.py b/haystack/tools/searchable_toolset.py index 58809160c15..e1f7fb202b4 100644 --- a/haystack/tools/searchable_toolset.py +++ b/haystack/tools/searchable_toolset.py @@ -129,15 +129,14 @@ def __init__( self._bootstrap_tool: Tool | None = None self._document_store: InMemoryDocumentStore | None = None self._passthrough: bool | None = None - self._is_warmed_up = False + + # Optional per-run name filter, set on the copies returned by _copy_for_run(). When set, iteration only + # yields tools whose name is in this set, and search is scoped to it. None means no filtering. + self._selected_tool_names: set[str] | None = None # Initialize parent with empty tools list - we manage tools dynamically super().__init__(tools=[]) - def __add__(self, other: Tool | Toolset | list[Tool]) -> "Toolset": - """Concatenation is not supported for SearchableToolset.""" - raise NotImplementedError("SearchableToolset does not support concatenation.") - def add(self, tool: Tool | Toolset) -> None: """Adding new tools after initialization is not supported for SearchableToolset.""" raise NotImplementedError("SearchableToolset does not support adding new tools after initialization.") @@ -153,7 +152,7 @@ def warm_up(self) -> None: :raises ValueError: If the flattened catalog contains tools with duplicate names. """ - if self._is_warmed_up: + if self._passthrough is not None: return # Warm up the catalog first (triggers lazy connections like MCPToolset), then flatten — lazy toolsets will @@ -176,8 +175,6 @@ def warm_up(self) -> None: self._document_store.write_documents(documents, policy=DuplicatePolicy.OVERWRITE) self._bootstrap_tool = self._create_search_tool() - self._is_warmed_up = True - def get_selectable_tools(self) -> list[Tool]: """ Return the full catalog of tools that can be selected by name. @@ -187,8 +184,7 @@ def get_selectable_tools(self) -> list[Tool]: :returns: The flattened catalog of tools. """ - if not self._is_warmed_up: - self.warm_up() + self.warm_up() return list(self._catalog) def clear(self) -> None: @@ -200,21 +196,27 @@ def clear(self) -> None: """ self._discovered_tools.clear() - def spawn(self) -> "SearchableToolset": + def _copy_for_run(self, selected_tool_names: set[str] | None = None) -> "SearchableToolset": """ - Return an isolated copy for a single run. + Return a copy of this toolset to be used for a single Agent run. + + This is the internal method through which the Agent isolates run-scoped Toolset state: a Toolset that + defines it is replaced by a per-run copy at run start, so concurrent runs sharing the same configured + Toolset don't share discovered tools or collide on the active selection. - The copy shares the read-only catalog and BM25 index but gets fresh discovered tools and name selection, - plus a bootstrap search tool bound to the copy. This way concurrent runs sharing the same configured - SearchableToolset don't share discovered tools or collide on the active selection. + The copy shares the read-only configuration (catalog, BM25 index) but starts with fresh run state: + no discovered tools, a bootstrap search tool bound to the copy, and the given selection. The selection + is fixed for the copy's lifetime: iteration only yields selected tools (the bootstrap search tool stays + exposed) and search is scoped to them. + :param selected_tool_names: Optional catalog tool names this run is restricted to. None means no + restriction. :returns: A run-scoped copy of this SearchableToolset. """ - if not self._is_warmed_up: - self.warm_up() + self.warm_up() new = copy.copy(self) new._discovered_tools = {} - new._selected_tool_names = None + new._selected_tool_names = set(selected_tool_names) if selected_tool_names is not None else None # Rebuild the bootstrap tool so its closure is bound to the copy's discovered tools / selection # rather than the original's. The document store and catalog are read-only and stay shared. if not self._passthrough: @@ -307,12 +309,10 @@ def __iter__(self) -> Iterator[Tool]: set, but the bootstrap search tool is always exposed so search keeps working over the selected subset. Automatically calls warm_up() if needed to ensure the bootstrap tool is available. """ - # Unlike base Toolset/MCPToolset, which expose a placeholder tool before warm_up, this toolset materializes - # everything (flattened catalog, bootstrap tool, passthrough decision) in warm_up. + # This toolset materializes everything (flattened catalog, bootstrap tool, passthrough decision) in warm_up. # Without warming here, iterating before warm_up would yield nothing, so we warm up to make the toolset usable # at all. - if not self._is_warmed_up: - self.warm_up() + self.warm_up() if self._passthrough: yield from (tool for tool in self._catalog if self._is_selected(tool.name)) else: diff --git a/haystack/tools/skills/skill_toolset.py b/haystack/tools/skills/skill_toolset.py index c7ea90b6a7c..94b27a642e8 100644 --- a/haystack/tools/skills/skill_toolset.py +++ b/haystack/tools/skills/skill_toolset.py @@ -110,13 +110,6 @@ def add(self, tool: Tool | Toolset) -> None: "alongside them, e.g. tools=[skill_toolset, other_tool]." ) - def __add__(self, other: Tool | Toolset | list[Tool]) -> "Toolset": - """Concatenation is not supported for SearchableToolset.""" - raise NotImplementedError( - "SkillToolset does not support concatenation. To combine it with other tools, pass it to the Agent " - "alongside them, e.g. tools=[skill_toolset, other_tool]." - ) - def _load_skill_description(self) -> str: """ Build the `load_skill` tool description, including the catalog of discovered skills. diff --git a/haystack/tools/toolset.py b/haystack/tools/toolset.py index 845a24fa11c..256e0bb29ed 100644 --- a/haystack/tools/toolset.py +++ b/haystack/tools/toolset.py @@ -2,7 +2,6 @@ # # SPDX-License-Identifier: Apache-2.0 -import copy from collections.abc import Iterator from dataclasses import dataclass, field from typing import Any @@ -51,129 +50,68 @@ def subtract(a: Annotated[int, "first number"], b: Annotated[int, "second number By subclassing Toolset, you can create implementations that dynamically load tools from external sources like OpenAPI URLs, MCP servers, or other resources. + When implementing a custom Toolset subclass for dynamic tool loading: + - Load the tools in `warm_up()` and assign them to `self.tools`. Following the framework-wide `warm_up()` + convention, make it idempotent by guarding on your own state (e.g. `if self._client is not None: return`), + as it may be called before every run. + - Override `to_dict()` and `from_dict()` to serialize the endpoint descriptor (URL, server info) rather than + the dynamically loaded Tool instances. + Example: ```python - from typing import Annotated from haystack.core.serialization import generate_qualified_class_name - from haystack.tools import tool, Toolset - from haystack.components.agents import Agent - from haystack.components.generators.chat import OpenAIChatGenerator - - class CalculatorToolset(Toolset): - '''A toolset for calculator operations.''' + from haystack.tools import Toolset - def __init__(self) -> None: - super().__init__(self._create_tools()) + class RemoteServiceToolset(Toolset): + def __init__(self, endpoint: str) -> None: + self.endpoint = endpoint + self._client = None + super().__init__(tools=[]) # tools are loaded on warm_up() - def _create_tools(self): - # These tools are defined statically for illustration purposes only. - # In a real-world scenario, you would dynamically load tools from an external source here. - @tool - def add(a: Annotated[int, "first number"], b: Annotated[int, "second number"]) -> int: - '''Add two numbers.''' - return a + b - - @tool - def multiply(a: Annotated[int, "first number"], b: Annotated[int, "second number"]) -> int: - '''Multiply two numbers.''' - return a * b - - return [add, multiply] + def warm_up(self) -> None: + if self._client is not None: + return + self._client = connect(self.endpoint) + self.tools = self._client.fetch_tools() def to_dict(self): return { "type": generate_qualified_class_name(type(self)), - "data": {}, # no data to serialize as we define the tools dynamically + "data": {"endpoint": self.endpoint}, } @classmethod def from_dict(cls, data): - return cls() # Recreate the tools dynamically during deserialization - - # Create the dynamic toolset and use it with an Agent - calculator_toolset = CalculatorToolset() - agent = Agent(chat_generator=OpenAIChatGenerator(), tools=calculator_toolset) + return cls(endpoint=data["data"]["endpoint"]) ``` Toolset implements the collection interface (__iter__, __contains__, __len__, __getitem__), making it behave like a list of Tools. This makes it compatible with components that expect iterable tools, such as Agent or Haystack chat generators. - When implementing a custom Toolset subclass for dynamic tool loading: - - Perform the dynamic loading in the __init__ method - - Override to_dict() and from_dict() methods if your tools are defined dynamically - - Serialize endpoint descriptors rather than tool instances if your tools are loaded from external sources + To combine multiple Toolsets, pass them as a list wherever tools are accepted, e.g. + `Agent(tools=[toolset_a, toolset_b])`. This keeps each Toolset as a unit, preserving its lifecycle and + serialization. """ - # Use field() with default_factory to initialize the list tools: list[Tool] = field(default_factory=list) def __post_init__(self) -> None: """ - Validate and set up the toolset after initialization. - - This handles the case when tools are provided during initialization. + Validate the tools provided during initialization. """ - # If initialization was done a single Tool, raise an error if isinstance(self.tools, Tool): raise TypeError("A single Tool cannot be directly passed to Toolset. Please use a list: Toolset([tool])") - # Check for duplicate tool names in the initial set _check_duplicate_tool_names(self.tools) - # Tracks whether warm_up() has already run so subsequent calls become a no-op. - self._is_warmed_up = False - - # Optional per-run name filter. When set, iteration only yields tools whose name is in this set. - # None means no filtering. Set on a per-run spawn(), so it never leaks across runs. - self._selected_tool_names: set[str] | None = None - def __iter__(self) -> Iterator[Tool]: """ Return an iterator over the Tools in this Toolset. - This allows the Toolset to be used wherever a list of Tools is expected. If a name filter is active, - only the tools whose names are in it are yielded. - :returns: An iterator yielding Tool instances """ - for tool in self.tools: - if self._selected_tool_names is None or tool.name in self._selected_tool_names: - yield tool - - def get_selectable_tools(self) -> list[Tool]: - """ - Return the full set of tools that can be selected by name, ignoring any active name filter. - - This differs from iteration, which yields only the tools currently exposed (and respects the name filter). - Override this when a Toolset's iteration does not surface every selectable tool, so name-based selection - can still target the full set. - - Warms up the Toolset first if needed, so lazily loaded tools (those a Toolset fetches in `warm_up()`) - are available for selection. - - :returns: The list of tools available for name-based selection. - """ - if not self._is_warmed_up: - self.warm_up() - return list(self.tools) - - def spawn(self) -> "Toolset": - """ - Return an isolated copy of this Toolset for a single run. - - The copy shares this Toolset's read-only state (its tools and any warmed-up resources) but gets fresh - run-scoped state, so concurrent runs that share the same configured Toolset don't corrupt each other (for - example, one run's name selection leaking into another). Warms up first if needed so the copy shares the - warmed state. Subclasses with additional run-scoped state should override this. - - :returns: A run-scoped copy of this Toolset. - """ - if not self._is_warmed_up: - self.warm_up() - new = copy.copy(self) - new._selected_tool_names = None - return new + return iter(self.tools) def __contains__(self, item: str | Tool) -> bool: """ @@ -192,74 +130,57 @@ def __contains__(self, item: str | Tool) -> bool: return any(tool is item or tool == item for tool in self) return False - def warm_up(self) -> None: + def __len__(self) -> int: """ - Prepare the Toolset for use. + Return the number of Tools in this Toolset. + + :returns: Number of Tools + """ + return sum(1 for _ in self) - By default, this method iterates through and warms up all tools in the Toolset. - Subclasses can override this method to customize initialization behavior, such as: + def __getitem__(self, index: int) -> Tool: + """ + Get a Tool by index. - - Setting up shared resources (database connections, HTTP sessions) instead of - warming individual tools - - Implementing custom initialization logic for dynamically loaded tools - - Controlling when and how tools are initialized + :param index: Index of the Tool to get + :returns: The Tool at the specified index + """ + return list(self)[index] - For example, a Toolset that manages tools from an external service (like MCPToolset) - might override this to initialize a shared connection rather than warming up - individual tools: + def warm_up(self) -> None: + """ + Prepare the Toolset for use. - ```python - class MCPToolset(Toolset): - def warm_up(self) -> None: - # Only warm up the shared MCP connection, not individual tools - self.mcp_connection = establish_connection(self.server_url) - ``` + By default, this method warms up all tools in the Toolset. Subclasses that load tools dynamically + (e.g. from an MCP server or an OpenAPI spec) should override this method to fetch their tools and assign + them to `self.tools`. - This method is idempotent: it only warms up the tools the first time it is called. - Subclasses overriding it should preserve this contract (for example by guarding on - `self._is_warmed_up`). + Following the framework-wide convention, `warm_up()` may be called multiple times (e.g. before every run) + and implementations are responsible for making it idempotent. The default implementation delegates to the + tools' own idempotent `warm_up()`. Subclasses should guard on their own state, for example + `if self._client is not None: return`. """ - if self._is_warmed_up: - return for tool in self.tools: if hasattr(tool, "warm_up"): tool.warm_up() - self._is_warmed_up = True - def add(self, tool: "Tool | Toolset") -> None: + def add(self, tool: Tool) -> None: """ - Add a new Tool or merge another Toolset. - - If this Toolset has already been warmed up, the newly added Tool (or the tools of the - added Toolset) are warmed up immediately so they are ready to use without requiring a - second `warm_up()` call on the whole Toolset. + Add a new Tool to this Toolset. - Note: adding a Toolset flattens it into its individual tools, so this is only recommended - for Toolsets that don't manage shared resources in their `warm_up()` (or `__init__`). - For example, combining with an `MCPToolset`, which owns a shared connection, is not - recommended: the connection's lifecycle would no longer be managed by the original - Toolset. In those cases combine Toolsets with `+` (which preserves each Toolset as a - unit via `_ToolsetWrapper`) instead. + To combine whole Toolsets, pass them as a list wherever tools are accepted instead, e.g. + `Agent(tools=[toolset_a, toolset_b])`. This keeps each Toolset as a unit, preserving its lifecycle + and serialization. - :param tool: A Tool instance or another Toolset to add + :param tool: A Tool instance to add :raises ValueError: If adding the tool would result in duplicate tool names - :raises TypeError: If the provided object is not a Tool or Toolset + :raises TypeError: If the provided object is not a Tool """ - if not isinstance(tool, (Tool, Toolset)): - raise TypeError(f"Expected Tool or Toolset, got {type(tool).__name__}") + if not isinstance(tool, Tool): + raise TypeError(f"Expected Tool, got {type(tool).__name__}") - # Warm up the source before flattening so that lazily-loaded toolsets (e.g. MCPToolset) - # expose their tools, and so newly added tools are ready to use right away. - if self._is_warmed_up and hasattr(tool, "warm_up"): - tool.warm_up() - - new_tools = [tool] if isinstance(tool, Tool) else list(tool) - - # Check for duplicates before adding - combined_tools = self.tools + new_tools - _check_duplicate_tool_names(combined_tools) - - self.tools.extend(new_tools) + _check_duplicate_tool_names(self.tools + [tool]) + self.tools.append(tool) def to_dict(self) -> dict[str, Any]: """ @@ -302,131 +223,3 @@ def from_dict(cls, data: dict[str, Any]) -> "Toolset": tools.append(tool_class.from_dict(tool_data)) return cls(tools=tools) - - def __add__(self, other: "Tool | Toolset | list[Tool]") -> "Toolset": - """ - Concatenate this Toolset with another Tool, Toolset, or list of Tools. - - :param other: Another Tool, Toolset, or list of Tools to concatenate - :returns: A new Toolset containing all tools - :raises TypeError: If the other parameter is not a Tool, Toolset, or list of Tools - :raises ValueError: If the combination would result in duplicate tool names - """ - if isinstance(other, Tool): - return Toolset(tools=self.tools + [other]) - if isinstance(other, Toolset): - return _ToolsetWrapper([self, other]) - if isinstance(other, list) and all(isinstance(item, Tool) for item in other): - return Toolset(tools=self.tools + other) - raise TypeError(f"Cannot add {type(other).__name__} to Toolset") - - def __len__(self) -> int: - """ - Return the number of Tools in this Toolset (respecting any active name filter). - - :returns: Number of Tools - """ - return sum(1 for _ in self) - - def __getitem__(self, index: int) -> Tool: - """ - Get a Tool by index (respecting any active name filter). - - :param index: Index of the Tool to get - :returns: The Tool at the specified index - """ - return list(self)[index] - - -class _ToolsetWrapper(Toolset): - """ - A wrapper that holds multiple toolsets and provides a unified interface. - - This is used internally when combining different types of toolsets to preserve - their individual configurations while still being usable with Agent and Haystack chat generators. - """ - - def __init__(self, toolsets: list[Toolset]) -> None: - super().__init__([tool for toolset in toolsets for tool in toolset]) - self.toolsets = toolsets - # Tracks whether warm_up() has already run so subsequent calls become a no-op. - self._is_warmed_up = False - - def __iter__(self) -> Iterator[Tool]: - """Iterate over all tools from all toolsets, honoring any active name filter.""" - for toolset in self.toolsets: - for tool in toolset: - if self._selected_tool_names is None or tool.name in self._selected_tool_names: - yield tool - - def get_selectable_tools(self) -> list[Tool]: - """Return every selectable tool across all wrapped toolsets, ignoring any active filter.""" - return [tool for toolset in self.toolsets for tool in toolset.get_selectable_tools()] - - def spawn(self) -> "_ToolsetWrapper": - """Return an isolated copy with each wrapped toolset spawned.""" - return _ToolsetWrapper([toolset.spawn() for toolset in self.toolsets]) - - def __contains__(self, item: Any) -> bool: - """Check if a tool is in any of the toolsets.""" - return any(item in toolset for toolset in self.toolsets) - - def warm_up(self) -> None: - """ - Warm up all wrapped toolsets. - - This method is idempotent: it only warms up the wrapped toolsets the first time it is - called. The individual toolsets are themselves expected to have idempotent `warm_up()` - methods. - """ - if self._is_warmed_up: - return - for toolset in self.toolsets: - toolset.warm_up() - self._is_warmed_up = True - - def to_dict(self) -> dict[str, Any]: - """ - Serialize the wrapper to a dictionary. - - Each wrapped toolset is serialized via its own `to_dict()`, so any subclass that - overrides serialization (e.g. a toolset that serializes a connection/endpoint - descriptor) is preserved. - - :returns: A dictionary representation of the wrapper. - """ - return { - "type": generate_qualified_class_name(type(self)), - "data": {"toolsets": [toolset.to_dict() for toolset in self.toolsets]}, - } - - @classmethod - def from_dict(cls, data: dict[str, Any]) -> "_ToolsetWrapper": - """ - Deserialize a wrapper from a dictionary. - - :param data: Dictionary representation of the wrapper. - :returns: A new `_ToolsetWrapper` instance. - :raises TypeError: If any serialized entry is not a subclass of Toolset. - """ - inner_data = data["data"] - toolsets_data = inner_data.get("toolsets", []) - - toolsets = [] - for toolset_data in toolsets_data: - toolset_class = import_class_by_name(toolset_data["type"]) - if not issubclass(toolset_class, Toolset): - raise TypeError(f"Class '{toolset_class}' is not a subclass of Toolset") - toolsets.append(toolset_class.from_dict(toolset_data)) - - return cls(toolsets=toolsets) - - def __add__(self, other: Toolset | Tool | list[Tool]) -> "_ToolsetWrapper": - """Add another toolset or tool to this wrapper.""" - if isinstance(other, Toolset): - return _ToolsetWrapper(self.toolsets + [other]) - if isinstance(other, Tool): - return _ToolsetWrapper(self.toolsets + [Toolset([other])]) - if isinstance(other, list) and all(isinstance(item, Tool) for item in other): - return _ToolsetWrapper(self.toolsets + [Toolset(other)]) - raise TypeError(f"Cannot add {type(other).__name__} to _ToolsetWrapper") diff --git a/releasenotes/notes/simplify-toolset-102a9effbe4f188f.yaml b/releasenotes/notes/simplify-toolset-102a9effbe4f188f.yaml new file mode 100644 index 00000000000..1a79f8869d7 --- /dev/null +++ b/releasenotes/notes/simplify-toolset-102a9effbe4f188f.yaml @@ -0,0 +1,33 @@ +--- +upgrade: + - | + Toolset concatenation with ``+`` has been removed, along with the internal wrapper class it produced + (``_ToolsetWrapper``). To use multiple Toolsets together, pass them as a list wherever tools are accepted, + which keeps each Toolset intact with its own lifecycle and serialization: ``Agent(tools=[toolset_a, + toolset_b])``. To deliberately merge simple Toolsets into one, unpack them explicitly: + ``Toolset([*toolset_a, *toolset_b])``. Serialized pipelines containing the removed wrapper + (``haystack.tools.toolset._ToolsetWrapper``) can no longer be deserialized: recreate them by passing the + Toolsets as a list. + - | + ``Toolset.add()`` now accepts only ``Tool`` instances. Adding a whole Toolset used to flatten it, detaching + its tools from the Toolset that manages their lifecycle and serialization, and silently dropped the tools of + lazily-loading Toolsets that were not warmed up yet. To combine Toolsets, pass them as a list wherever tools + are accepted: ``Agent(tools=[toolset_a, toolset_b])``. + - | + ``Toolset.spawn()`` and ``Toolset.get_selectable_tools()`` have been removed from the base class, and the + per-run isolation machinery is now internal. Plain Toolsets hold no run-scoped state, so they are shared + across runs directly and reduced to their matching Tools on name selection. ``SearchableToolset`` (the one + Toolset with run-scoped state) is copied per run internally; its per-run behavior, including tool-name + selection via ``Agent.run(tools=[...])``, is unchanged. Third-party Toolset subclasses that overrode + ``spawn()`` no longer take part in per-run copying and behave as plain Toolsets. + - | + ``Toolset.warm_up()`` no longer tracks an internal "warmed up" flag. Following the framework-wide convention, + ``warm_up()`` can be called multiple times and implementations are responsible for their own idempotence: + the default implementation delegates to the tools' own idempotent ``warm_up()``, and Toolset subclasses that + load tools dynamically should guard on their own state (for example ``if self._client is not None: return``). + Subclasses that relied on the private ``Toolset._is_warmed_up`` attribute must define their own guard. +enhancements: + - | + Simplified ``Toolset`` to a plain collection of Tools: iteration, length, and membership checks now always + reflect the full tool list, with no hidden run-scoped filtering. The per-run machinery lives internally in + the Toolsets that need it (currently only ``SearchableToolset``). diff --git a/test/components/agents/test_agent.py b/test/components/agents/test_agent.py index 01616b3217e..691fc8105b0 100644 --- a/test/components/agents/test_agent.py +++ b/test/components/agents/test_agent.py @@ -2202,13 +2202,14 @@ def test_warm_up_toolset(self): agent.warm_up() assert toolset.was_warmed_up - def test_warm_up_mixed_toolsets(self): + def test_warm_up_list_of_toolsets(self): + # Toolsets are combined by passing them as a list; each keeps its own warm_up. tool1 = self._make_tracking_tool("tool1") toolset1 = self._make_tracking_toolset([tool1]) tool2 = self._make_tracking_tool("tool2") toolset2 = self._make_tracking_toolset([tool2]) - agent = Agent(chat_generator=MockChatGenerator("Hello"), tools=toolset1 + toolset2) + agent = Agent(chat_generator=MockChatGenerator("Hello"), tools=[toolset1, toolset2]) assert not toolset1.was_warmed_up assert not toolset2.was_warmed_up diff --git a/test/components/agents/test_utils.py b/test/components/agents/test_utils.py index 4bbf950014e..991e6cc660f 100644 --- a/test/components/agents/test_utils.py +++ b/test/components/agents/test_utils.py @@ -99,23 +99,40 @@ def test_raises_when_no_tools_configured(self, first_tool: Tool): with pytest.raises(ValueError, match="No tools were configured for the Agent at initialization."): _select_tools_by_name([], [first_tool.name]) - def test_spawns_toolsets_without_mutating_them(self, first_tool: Tool, second_tool: Tool): + def test_reduces_plain_toolsets_to_matching_tools(self, first_tool: Tool, second_tool: Tool): toolset = Toolset([first_tool, second_tool]) selected = _select_tools_by_name([toolset], [first_tool.name]) - spawned = selected[0] - assert isinstance(spawned, Toolset) - assert spawned is not toolset - assert spawned._selected_tool_names == {first_tool.name} - assert toolset._selected_tool_names is None + assert selected == [first_tool] + # The configured toolset is untouched. + assert list(toolset) == [first_tool, second_tool] def test_selects_standalone_tools_and_toolsets(self, first_tool: Tool, second_tool: Tool): toolset = Toolset([first_tool]) selected = _select_tools_by_name([second_tool, toolset], [first_tool.name, second_tool.name]) - assert second_tool in selected - spawned = next(item for item in selected if isinstance(item, Toolset)) - assert spawned is not toolset - assert spawned._selected_tool_names == {first_tool.name} - assert toolset._selected_tool_names is None + assert selected == [second_tool, first_tool] + + def test_copies_run_scoped_toolsets_for_run_with_the_selection(self, first_tool: Tool, second_tool: Tool): + class RunScopedToolset(Toolset): + """A Toolset defining _copy_for_run(), signaling run-scoped state.""" + + def __init__(self, tools): + super().__init__(tools) + self.selected: set[str] | None = None + + def _copy_for_run(self, selected_tool_names: set[str] | None = None) -> "RunScopedToolset": + new = RunScopedToolset(list(self.tools)) + new.selected = set(selected_tool_names) if selected_tool_names is not None else None + return new + + toolset = RunScopedToolset([first_tool, second_tool]) + selected = _select_tools_by_name([toolset], [first_tool.name]) + + run_copy = selected[0] + assert isinstance(run_copy, RunScopedToolset) + assert run_copy is not toolset + assert run_copy.selected == {first_tool.name} + # The configured toolset is untouched. + assert toolset.selected is None class TestContextTokensFromUsage: diff --git a/test/tools/skills/test_skill_toolset.py b/test/tools/skills/test_skill_toolset.py index e8ec3e1dec8..efd3eab876b 100644 --- a/test/tools/skills/test_skill_toolset.py +++ b/test/tools/skills/test_skill_toolset.py @@ -113,13 +113,6 @@ def test_add_is_not_supported(self, tmp_path): with pytest.raises(NotImplementedError, match="does not support adding tools"): toolset.add(extra) - def test_concat_is_not_supported(self, tmp_path): - _write_skill(tmp_path, "pdf-forms", description="Use to fill PDF forms.") - toolset = SkillToolset(FileSystemSkillStore(tmp_path)) - extra = Tool(name="extra", description="d", parameters={"type": "object", "properties": {}}, function=len) - with pytest.raises(NotImplementedError, match="does not support concatenation"): - _ = toolset + extra - def test_accepts_skill_store_instance(self, tmp_path): _write_skill(tmp_path, "pdf-forms", description="Use to fill PDF forms.") store = FileSystemSkillStore(tmp_path) diff --git a/test/tools/test_searchable_toolset.py b/test/tools/test_searchable_toolset.py index c83854c7a15..a7cfedff62f 100644 --- a/test/tools/test_searchable_toolset.py +++ b/test/tools/test_searchable_toolset.py @@ -127,12 +127,8 @@ def test_init_with_invalid_catalog(self): ) ) - def test_not_implemented_methods(self): + def test_add_is_not_supported(self): toolset = SearchableToolset(catalog=[]) - with pytest.raises(NotImplementedError): - toolset + Tool( - name="test", description="test", parameters={"type": "object", "properties": {}}, function=lambda: None - ) with pytest.raises(NotImplementedError): toolset.add( Tool( @@ -344,10 +340,10 @@ def test_iter_with_discovered_tools(self, large_catalog): def test_iter_automatically_warms_up(self, large_catalog): toolset = SearchableToolset(catalog=large_catalog) - assert not toolset._is_warmed_up + assert toolset._passthrough is None # not warmed up yet list(toolset) - assert toolset._is_warmed_up + assert toolset._passthrough is not None def test_contains_bootstrap_tool(self, large_catalog): """Test __contains__ for bootstrap tool.""" @@ -528,11 +524,11 @@ def test_not_warmed_up_after_agent_init(self, large_catalog, monkeypatch): """Initializing an Agent with a SearchableToolset must not warm it up (no premature flatten/connect).""" monkeypatch.setenv("OPENAI_API_KEY", "fake-key") toolset = SearchableToolset(catalog=large_catalog) - assert toolset._is_warmed_up is False + assert toolset._passthrough is None # not warmed up yet Agent(chat_generator=OpenAIChatGenerator(), tools=toolset) - assert toolset._is_warmed_up is False + assert toolset._passthrough is None # still not warmed up def test_warm_up_idempotent(self, large_catalog): """Test that warm_up can be called multiple times safely.""" @@ -829,53 +825,52 @@ def test_get_selectable_tools_exposes_full_catalog(self, large_catalog): # The catalog, however, is fully available for name-based selection. assert {tool.name for tool in toolset.get_selectable_tools()} == {tool.name for tool in large_catalog} - def test_runtime_tool_names_select_isolated_spawn_and_preserve_search(self, large_catalog, monkeypatch): - """Selecting catalog tool names returns an isolated spawn carrying the selection and keeping search active.""" + def test_runtime_tool_names_return_isolated_copy_and_preserve_search(self, large_catalog, monkeypatch): + """Selecting catalog tool names returns an isolated per-run copy carrying the selection, with search active.""" monkeypatch.setenv("OPENAI_API_KEY", "fake-key") toolset = SearchableToolset(catalog=large_catalog, search_threshold=3) # 8 tools -> search mode agent = Agent(chat_generator=OpenAIChatGenerator(), tools=toolset) selected = agent._select_tools(["get_weather", "add_numbers"]) - # An isolated spawn is returned with the selection; the configured toolset is not mutated. + # An isolated per-run copy is returned with the selection; the configured toolset is not mutated. assert len(selected) == 1 - spawned = selected[0] - assert isinstance(spawned, SearchableToolset) - assert spawned is not toolset - assert spawned._selected_tool_names == {"get_weather", "add_numbers"} + run_copy = selected[0] + assert isinstance(run_copy, SearchableToolset) + assert run_copy is not toolset + assert run_copy._selected_tool_names == {"get_weather", "add_numbers"} assert toolset._selected_tool_names is None - # Search is preserved on the spawn (not dismantled): only the bootstrap tool is exposed up front. - assert [tool.name for tool in spawned] == ["search_tools"] + # Search is preserved on the copy (not dismantled): only the bootstrap tool is exposed up front. + assert [tool.name for tool in run_copy] == ["search_tools"] # And search only discovers tools within the selected subset. - assert spawned._bootstrap_tool is not None - spawned._bootstrap_tool.invoke(tool_keywords="weather add stock multiply") - assert set(spawned._discovered_tools) <= {"get_weather", "add_numbers"} + assert run_copy._bootstrap_tool is not None + run_copy._bootstrap_tool.invoke(tool_keywords="weather add stock multiply") + assert set(run_copy._discovered_tools) <= {"get_weather", "add_numbers"} # The configured toolset's discovered tools are untouched. assert toolset._discovered_tools == {} - def test_spawns_have_independent_discovered_tools_and_selection(self, large_catalog): - """Two spawns of one SearchableToolset don't share discovered tools or collide on the active selection.""" + def test_run_copies_have_independent_discovered_tools_and_selection(self, large_catalog): + """Per-run copies of one SearchableToolset don't share discovered tools or collide on the selection.""" toolset = SearchableToolset(catalog=large_catalog, search_threshold=3) toolset.warm_up() - spawn_a = toolset.spawn() - spawn_b = toolset.spawn() + copy_a = toolset._copy_for_run(selected_tool_names={"get_weather"}) + copy_b = toolset._copy_for_run() - assert spawn_a is not spawn_b - assert spawn_a is not toolset - # Bootstrap tools are rebound per spawn (not shared with the original or each other). - assert spawn_a._bootstrap_tool is not None - assert spawn_a._bootstrap_tool is not spawn_b._bootstrap_tool + assert copy_a is not copy_b + assert copy_a is not toolset + # Bootstrap tools are rebound per copy (not shared with the original or each other). + assert copy_a._bootstrap_tool is not None + assert copy_a._bootstrap_tool is not copy_b._bootstrap_tool - spawn_a._selected_tool_names = {"get_weather"} - spawn_a._bootstrap_tool.invoke(tool_keywords="weather add stock multiply") + copy_a._bootstrap_tool.invoke(tool_keywords="weather add stock multiply") - # Discovery on spawn_a does not leak into spawn_b or the configured toolset. - assert set(spawn_a._discovered_tools) <= {"get_weather"} - assert spawn_b._discovered_tools == {} + # Discovery on copy_a does not leak into copy_b or the configured toolset. + assert set(copy_a._discovered_tools) <= {"get_weather"} + assert copy_b._discovered_tools == {} assert toolset._discovered_tools == {} # The selection is likewise isolated. - assert spawn_b._selected_tool_names is None + assert copy_b._selected_tool_names is None assert toolset._selected_tool_names is None def test_runtime_tool_names_passthrough_exposes_selected(self, large_catalog, monkeypatch): @@ -890,7 +885,7 @@ def test_runtime_tool_names_passthrough_exposes_selected(self, large_catalog, mo assert {tool.name for tool in flatten_tools_or_toolsets(selected)} == {"get_weather", "add_numbers"} def test_agent_run_with_runtime_tool_names(self, large_catalog): - """An Agent with a SearchableToolset runs with specific catalog tools selected by name on an isolated spawn.""" + """An Agent with a SearchableToolset runs with catalog tools selected by name on an isolated per-run copy.""" toolset = SearchableToolset(catalog=large_catalog, search_threshold=20) # passthrough exposes the selection @component @@ -916,7 +911,7 @@ def run(self, messages, tools=None, **kwargs): result = agent.run(messages=[ChatMessage.from_user("What's the weather in Berlin?")], tools=["get_weather"]) assert result["tool_call_counts"]["get_weather"] == 1 - # The Agent runs against an isolated spawn, so the configured toolset's selection never gets set. + # The Agent runs against an isolated per-run copy, so the configured toolset's selection never gets set. assert toolset._selected_tool_names is None def test_discovered_tool_call_counts_added_lazily(self, large_catalog): @@ -941,7 +936,7 @@ def test_discovered_tool_call_counts_added_lazily(self, large_catalog): # search_tools is seeded at init; get_weather is only counted after being discovered and called. assert counts["search_tools"] == 1 assert counts["get_weather"] == 1 - # The Agent discovers tools on an isolated spawn, so the configured toolset's discovered tools stay empty. + # Discovery happens on an isolated per-run copy: the configured toolset's discovered tools stay empty. assert toolset._discovered_tools == {} diff --git a/test/tools/test_tools_utils.py b/test/tools/test_tools_utils.py index d0d31fe870e..eddc3677e52 100644 --- a/test/tools/test_tools_utils.py +++ b/test/tools/test_tools_utils.py @@ -371,11 +371,13 @@ class WarmupCountingToolset(Toolset): def __init__(self, tools): super().__init__(tools) self.warm_up_count = 0 + self._loaded = False def warm_up(self): - if self._is_warmed_up: + if self._loaded: return self.warm_up_count += 1 + self._loaded = True super().warm_up() # Also warm up individual tools tool = WarmupCountingTool( diff --git a/test/tools/test_toolset.py b/test/tools/test_toolset.py index 139e13d2a5d..76930dc2574 100644 --- a/test/tools/test_toolset.py +++ b/test/tools/test_toolset.py @@ -121,20 +121,6 @@ def warm_up(self) -> None: self.warm_up_count += 1 -class WarmUpCountingToolset(Toolset): - """A Toolset that records how many times its own warm_up() did real work.""" - - def __init__(self, tools): - super().__init__(tools) - self.warm_up_count = 0 - - def warm_up(self) -> None: - if self._is_warmed_up: - return - self.warm_up_count += 1 - super().warm_up() - - class TestToolset: def test_toolset_with_multiple_tools(self, add_tool, multiply_tool): """Test that a Toolset with multiple tools works properly.""" @@ -181,42 +167,21 @@ def test_toolset_contains(self, add_tool, multiply_tool): assert "multiply" not in toolset assert "non_existent_tool" not in toolset - def test_toolset_addition(self, add_tool, multiply_tool, subtract_tool): - """Test that the __add__ method combines toolsets with various operand types.""" - base = Toolset([add_tool]) - - # Toolset + Tool - result = base + multiply_tool - assert isinstance(result, Toolset) - assert [t.name for t in result] == ["add", "multiply"] - - # Toolset + Toolset - result = base + Toolset([subtract_tool]) - assert isinstance(result, Toolset) - assert [t.name for t in result] == ["add", "subtract"] - - # Toolset + list[Tool] - result = base + [multiply_tool, subtract_tool] - assert isinstance(result, Toolset) - assert [t.name for t in result] == ["add", "multiply", "subtract"] - - # Unsupported operand types raise TypeError - with pytest.raises(TypeError): - base + "not_a_tool" # type: ignore[operator] - with pytest.raises(TypeError): - base + 123 # type: ignore[operator] + def test_combining_toolsets_via_unpacking(self, add_tool, multiply_tool, subtract_tool): + """Explicit unpacking is the supported way to build a merged Toolset.""" + combined = Toolset([*Toolset([add_tool, subtract_tool]), multiply_tool]) + assert [t.name for t in combined] == ["add", "subtract", "multiply"] # The combined tools remain invocable message = ChatMessage.from_assistant( tool_calls=[ ToolCall(tool_name="add", arguments={"a": 10, "b": 5}), ToolCall(tool_name="multiply", arguments={"a": 10, "b": 5}), - ToolCall(tool_name="subtract", arguments={"a": 10, "b": 5}), ] ) - tool_messages = _run_tool_messages(messages=[message], tools=result) + tool_messages = _run_tool_messages(messages=[message], tools=combined) tool_results = [tcr.result for message in tool_messages for tcr in message.tool_call_results] - assert tool_results == ["15", "50", "5"] + assert tool_results == ["15", "50"] def test_toolset_serialization(self, add_tool): """Test that a Toolset can be serialized and deserialized.""" @@ -244,9 +209,8 @@ def test_toolset_duplicate_tool_names(self, add_tool): with pytest.raises(ValueError, match="Duplicate tool names found"): toolset.add(add_tool) - toolset2 = Toolset([add_tool]) with pytest.raises(ValueError, match="Duplicate tool names found"): - _ = toolset + toolset2 + Toolset([*toolset, *Toolset([add_tool])]) class TestToolsetWithAgent: @@ -364,10 +328,6 @@ def test_pipeline_with_list_of_toolsets(self, add_tool, multiply_tool, monkeypat class TestToolsetWarmUp: """Stress tests for Toolset warm_up behavior.""" - def test_new_toolset_is_not_warmed_up(self): - toolset = Toolset([WarmUpCountingTool("a")]) - assert toolset._is_warmed_up is False - def test_warm_up_warms_all_tools(self): t1, t2 = WarmUpCountingTool("a"), WarmUpCountingTool("b") toolset = Toolset([t1, t2]) @@ -376,15 +336,16 @@ def test_warm_up_warms_all_tools(self): toolset.warm_up() assert t1.warm_up_count == 1 assert t2.warm_up_count == 1 - assert toolset._is_warmed_up is True - def test_warm_up_is_idempotent(self): + def test_warm_up_can_be_called_multiple_times(self): + # Per the framework-wide convention, warm_up() may be called before every run; the Toolset delegates to + # the tools' own warm_up(), which are responsible for their own idempotence. t1 = WarmUpCountingTool("a") toolset = Toolset([t1]) toolset.warm_up() toolset.warm_up() toolset.warm_up() - assert t1.warm_up_count == 1 + assert t1.warm_up_count == 3 def test_add_before_warm_up_does_not_warm_tools(self): existing = WarmUpCountingTool("a") @@ -398,148 +359,20 @@ def test_add_before_warm_up_does_not_warm_tools(self): assert existing.warm_up_count == 1 assert new_tool.warm_up_count == 1 - def test_add_tool_after_warm_up_warms_only_new_tool(self): + def test_add_tool_after_warm_up_warms_it_on_next_warm_up(self): existing = WarmUpCountingTool("a") toolset = Toolset([existing]) toolset.warm_up() - assert existing.warm_up_count == 1 new_tool = WarmUpCountingTool("b") toolset.add(new_tool) - # The new tool is warmed immediately, the already-warmed tool is not re-warmed. - assert new_tool.warm_up_count == 1 - assert existing.warm_up_count == 1 - - def test_add_toolset_after_warm_up_warms_added_toolset(self): - toolset = Toolset([WarmUpCountingTool("a")]) - toolset.warm_up() - added_tools = [WarmUpCountingTool("b"), WarmUpCountingTool("c")] - added = WarmUpCountingToolset(added_tools) - toolset.add(added) - # The added toolset's own warm_up() is invoked, warming its tools. - assert added.warm_up_count == 1 - assert all(tool.warm_up_count == 1 for tool in added_tools) - - def test_plus_returns_new_unwarmed_toolset(self): - ts1 = Toolset([WarmUpCountingTool("a")]) - ts1.warm_up() - assert ts1._is_warmed_up is True - new_tool = WarmUpCountingTool("b") - ts2 = ts1 + new_tool - # `+` returns a brand new Toolset object that has not been warmed up yet. - assert ts2 is not ts1 - assert ts2._is_warmed_up is False + # add() does not warm the new tool; the next warm_up() call does. assert new_tool.warm_up_count == 0 - ts2.warm_up() + toolset.warm_up() assert new_tool.warm_up_count == 1 - -class TestToolsetToolSelection: - """Tests for get_selectable_tools(), the name filter, and spawn().""" - - def test_no_filter_yields_all_tools(self, add_tool, multiply_tool): - toolset = Toolset([add_tool, multiply_tool]) - assert toolset._selected_tool_names is None - assert [tool.name for tool in toolset] == ["add", "multiply"] - assert len(toolset) == 2 - - def test_get_selectable_tools_returns_all_tools(self, add_tool, multiply_tool): - toolset = Toolset([add_tool, multiply_tool]) - assert toolset.get_selectable_tools() == [add_tool, multiply_tool] - - def test_get_selectable_tools_ignores_active_filter(self, add_tool, multiply_tool): - toolset = Toolset([add_tool, multiply_tool]) - toolset._selected_tool_names = {"add"} - # Iteration is filtered, but get_selectable_tools still returns the full set. - assert [tool.name for tool in toolset] == ["add"] - assert {tool.name for tool in toolset.get_selectable_tools()} == {"add", "multiply"} - - def test_get_selectable_tools_warms_up_lazy_toolset(self, add_tool, multiply_tool): - """get_selectable_tools() warms up a lazy toolset so its lazily loaded tools are available for selection.""" - - class LazyToolset(Toolset): - def __init__(self): - super().__init__([]) # no tools until warm_up - - def warm_up(self): - if self._is_warmed_up: - return - self.tools = [add_tool, multiply_tool] - self._is_warmed_up = True - - toolset = LazyToolset() - assert toolset._is_warmed_up is False - assert toolset.tools == [] # not loaded yet - - selectable = toolset.get_selectable_tools() - - assert toolset._is_warmed_up is True # get_selectable_tools triggered warm_up - assert [tool.name for tool in selectable] == ["add", "multiply"] - - def test_filter_restricts_iteration(self, add_tool, multiply_tool, subtract_tool): - toolset = Toolset([add_tool, multiply_tool, subtract_tool]) - toolset._selected_tool_names = {"add", "subtract"} - assert [tool.name for tool in toolset] == ["add", "subtract"] - - def test_filter_restricts_len(self, add_tool, multiply_tool, subtract_tool): - toolset = Toolset([add_tool, multiply_tool, subtract_tool]) - toolset._selected_tool_names = {"add"} - assert len(toolset) == 1 - - def test_filter_restricts_getitem(self, add_tool, multiply_tool, subtract_tool): - toolset = Toolset([add_tool, multiply_tool, subtract_tool]) - toolset._selected_tool_names = {"subtract"} - assert toolset[0].name == "subtract" - - def test_filter_restricts_contains(self, add_tool, multiply_tool): - toolset = Toolset([add_tool, multiply_tool]) - toolset._selected_tool_names = {"add"} - assert "add" in toolset - assert "multiply" not in toolset - assert add_tool in toolset - assert multiply_tool not in toolset - - def test_spawn_returns_isolated_copy(self, add_tool, multiply_tool): - toolset = Toolset([add_tool, multiply_tool]) - - spawned = toolset.spawn() - - assert spawned is not toolset - assert spawned._selected_tool_names is None - # The copy shares the same (read-only) tools. - assert list(spawned.tools) == list(toolset.tools) - - def test_spawn_selection_does_not_leak_to_original(self, add_tool, multiply_tool): - """A per-run selection set on a spawn must not affect the configured toolset or other spawns.""" - toolset = Toolset([add_tool, multiply_tool]) - - spawn_a = toolset.spawn() - spawn_b = toolset.spawn() - spawn_a._selected_tool_names = {"add"} - - # Each run sees only its own selection; the configured toolset stays unfiltered. - assert [tool.name for tool in spawn_a] == ["add"] - assert [tool.name for tool in spawn_b] == ["add", "multiply"] - assert [tool.name for tool in toolset] == ["add", "multiply"] - assert toolset._selected_tool_names is None - - def test_spawn_warms_up_lazy_toolset(self, add_tool, multiply_tool): - """spawn() warms up a lazy toolset so the copy shares the warmed state.""" - - class LazyToolset(Toolset): - def __init__(self): - super().__init__([]) - - def warm_up(self): - if self._is_warmed_up: - return - self.tools = [add_tool, multiply_tool] - self._is_warmed_up = True - - toolset = LazyToolset() - assert toolset._is_warmed_up is False - - spawned = toolset.spawn() - - assert toolset._is_warmed_up is True # spawn triggered warm_up - assert spawned._is_warmed_up is True - assert [tool.name for tool in spawned] == ["add", "multiply"] + def test_add_toolset_raises(self): + # add() accepts only Tools: to combine Toolsets, pass them as a list, e.g. Agent(tools=[ts_a, ts_b]). + toolset = Toolset([WarmUpCountingTool("a")]) + not_a_tool: Any = Toolset([WarmUpCountingTool("b")]) + with pytest.raises(TypeError, match="Expected Tool"): + toolset.add(not_a_tool) diff --git a/test/tools/test_toolset_wrapper.py b/test/tools/test_toolset_wrapper.py deleted file mode 100644 index cb4ac58b8b5..00000000000 --- a/test/tools/test_toolset_wrapper.py +++ /dev/null @@ -1,253 +0,0 @@ -# SPDX-FileCopyrightText: 2022-present deepset GmbH -# -# SPDX-License-Identifier: Apache-2.0 - -from typing import Annotated - -import pytest - -from haystack.components.agents import Agent -from haystack.components.generators.chat import OpenAIChatGenerator -from haystack.core.serialization import generate_qualified_class_name -from haystack.tools import Tool, Toolset, tool -from haystack.tools.toolset import _ToolsetWrapper - - -@tool -def add(a: Annotated[int, "first number"], b: Annotated[int, "second number"]) -> int: - """Add two numbers.""" - return a + b - - -@tool -def multiply(a: Annotated[int, "first number"], b: Annotated[int, "second number"]) -> int: - """Multiply two numbers.""" - return a * b - - -@tool -def subtract(a: Annotated[int, "first number"], b: Annotated[int, "second number"]) -> int: - """Subtract b from a.""" - return a - b - - -@tool -def rebuilt() -> str: - """A rebuilt tool.""" - return "rebuilt" - - -@pytest.fixture -def add_tool(): - return add - - -@pytest.fixture -def multiply_tool(): - return multiply - - -@pytest.fixture -def subtract_tool(): - return subtract - - -class WarmUpCountingTool(Tool): - """A Tool that records how many times warm_up() was called.""" - - def __init__(self, name: str): - super().__init__( - name=name, - description=f"{name} tool", - parameters={"type": "object", "properties": {}}, - function=lambda: None, - ) - self.warm_up_count = 0 - - def warm_up(self) -> None: - self.warm_up_count += 1 - - -class WarmUpCountingToolset(Toolset): - """A Toolset that records how many times its own warm_up() did real work.""" - - def __init__(self, tools): - super().__init__(tools) - self.warm_up_count = 0 - - def warm_up(self) -> None: - if self._is_warmed_up: - return - self.warm_up_count += 1 - super().warm_up() - - -class RebuildingToolset(Toolset): - """A toolset that rebuilds its tools on from_dict() instead of serializing them (like a dynamic toolset).""" - - def __init__(self): - super().__init__([rebuilt]) - - def to_dict(self): - return {"type": generate_qualified_class_name(type(self)), "data": {}} - - @classmethod - def from_dict(cls, data): - return cls() - - -class TestToolsetWrapper: - """Tests for the _ToolsetWrapper class""" - - def test_toolset_plus_toolset_creates_wrapper(self, add_tool, multiply_tool): - """Test that combining two Toolsets creates a _ToolsetWrapper and works correctly.""" - result = Toolset([add_tool]) + Toolset([multiply_tool]) - assert isinstance(result, _ToolsetWrapper) - assert len(result) == 2 - assert add_tool in result - assert multiply_tool in result - - def test_wrapper_getitem_supports_negative_indices(self, add_tool, multiply_tool): - """Test that _ToolsetWrapper.__getitem__ behaves like a list, including negative indices.""" - toolset1 = Toolset([add_tool]) - toolset2 = Toolset([multiply_tool]) - - result = toolset1 + toolset2 - - assert result[0] is add_tool - assert result[1] is multiply_tool - assert result[-1] is multiply_tool - assert result[-2] is add_tool - - with pytest.raises(IndexError): - _ = result[2] - - def test_wrapper_with_agent(self, add_tool, multiply_tool, monkeypatch): - """Test that _ToolsetWrapper works with Agent.""" - monkeypatch.setenv("OPENAI_API_KEY", "test") - wrapper = Toolset([add_tool]) + Toolset([multiply_tool]) - agent = Agent(chat_generator=OpenAIChatGenerator(), tools=wrapper) - agent.warm_up() - assert len(list(agent.tools)) == 2 - - def test_wrapper_chaining_and_duplicate_detection(self, add_tool, multiply_tool, subtract_tool): - """Test chaining operations and that duplicates are still detected.""" - # Chaining should work - result = Toolset([add_tool]) + Toolset([multiply_tool]) + Toolset([subtract_tool]) - assert len(result) == 3 - # Duplicates should be detected - toolset_with_dup = Toolset([add_tool]) - with pytest.raises(ValueError, match="Duplicate tool names found"): - _ = result + toolset_with_dup - - -class TestToolsetWrapperWarmUp: - """Tests for warm_up behavior of _ToolsetWrapper.""" - - def test_new_wrapper_is_not_warmed_up(self): - wrapper = Toolset([WarmUpCountingTool("a")]) + Toolset([WarmUpCountingTool("b")]) - assert wrapper._is_warmed_up is False - - def test_warm_up_delegates_to_each_toolset(self): - ts1 = WarmUpCountingToolset([WarmUpCountingTool("a")]) - ts2 = WarmUpCountingToolset([WarmUpCountingTool("b")]) - wrapper = ts1 + ts2 - wrapper.warm_up() - assert ts1.warm_up_count == 1 - assert ts2.warm_up_count == 1 - assert wrapper._is_warmed_up is True - - def test_warm_up_is_idempotent(self): - ts1 = WarmUpCountingToolset([WarmUpCountingTool("a")]) - ts2 = WarmUpCountingToolset([WarmUpCountingTool("b")]) - wrapper = ts1 + ts2 - wrapper.warm_up() - wrapper.warm_up() - wrapper.warm_up() - assert ts1.warm_up_count == 1 - assert ts2.warm_up_count == 1 - - -class TestToolsetWrapperSerialization: - """Tests for to_dict/from_dict of _ToolsetWrapper.""" - - def test_to_dict(self, add_tool, multiply_tool): - wrapper = Toolset([add_tool]) + Toolset([multiply_tool]) - data = wrapper.to_dict() - assert data["type"] == "haystack.tools.toolset._ToolsetWrapper" - assert len(data["data"]["toolsets"]) == 2 - assert all(ts["type"] == "haystack.tools.toolset.Toolset" for ts in data["data"]["toolsets"]) - - def test_from_dict_round_trip(self, add_tool, multiply_tool): - wrapper = Toolset([add_tool]) + Toolset([multiply_tool]) - restored = _ToolsetWrapper.from_dict(wrapper.to_dict()) - assert isinstance(restored, _ToolsetWrapper) - assert len(restored) == 2 - assert len(restored.toolsets) == 2 - assert "add" in restored - assert "multiply" in restored - - def test_to_dict_preserves_subclass_serialization(self, add_tool): - # RebuildingToolset has a custom to_dict that serializes no tools (they are rebuilt on from_dict). - wrapper = RebuildingToolset() + Toolset([add_tool]) - data = wrapper.to_dict() - - # Each wrapped toolset is serialized via its own to_dict, so the custom one is preserved. - assert data["data"]["toolsets"][0]["type"].endswith("RebuildingToolset") - assert data["data"]["toolsets"][0]["data"] == {} - - restored = _ToolsetWrapper.from_dict(data) - assert isinstance(restored.toolsets[0], RebuildingToolset) - assert "rebuilt" in restored - assert "add" in restored - - def test_from_dict_rejects_non_toolset(self, add_tool): - data = Toolset([add_tool]).to_dict() - data["data"] = {"toolsets": [{"type": "haystack.tools.tool.Tool", "data": {}}]} - - with pytest.raises(TypeError, match="is not a subclass of Toolset"): - _ToolsetWrapper.from_dict(data) - - -class TestToolsetWrapperToolSelection: - """Tests for get_selectable_tools(), the name filter, and spawn() on _ToolsetWrapper.""" - - def test_get_selectable_tools_aggregates_all_toolsets(self, add_tool, multiply_tool, subtract_tool): - wrapper = Toolset([add_tool]) + Toolset([multiply_tool, subtract_tool]) - assert {tool.name for tool in wrapper.get_selectable_tools()} == {"add", "multiply", "subtract"} - - def test_get_selectable_tools_ignores_active_filter(self, add_tool, multiply_tool): - wrapper = Toolset([add_tool]) + Toolset([multiply_tool]) - wrapper._selected_tool_names = {"add"} - # Iteration is filtered, but get_selectable_tools still returns the full set. - assert [tool.name for tool in wrapper] == ["add"] - assert {tool.name for tool in wrapper.get_selectable_tools()} == {"add", "multiply"} - - def test_filter_restricts_iteration_and_len(self, add_tool, multiply_tool, subtract_tool): - wrapper = Toolset([add_tool, multiply_tool]) + Toolset([subtract_tool]) - wrapper._selected_tool_names = {"add", "subtract"} - assert [tool.name for tool in wrapper] == ["add", "subtract"] - assert len(wrapper) == 2 - - def test_filter_restricts_getitem(self, add_tool, multiply_tool, subtract_tool): - wrapper = Toolset([add_tool, multiply_tool]) + Toolset([subtract_tool]) - wrapper._selected_tool_names = {"add", "subtract"} - assert wrapper[0].name == "add" - assert wrapper[-1].name == "subtract" - - def test_spawn_isolates_own_and_child_state(self, add_tool, multiply_tool): - ts1 = Toolset([add_tool]) - ts2 = Toolset([multiply_tool]) - wrapper = ts1 + ts2 - - spawned = wrapper.spawn() - - # The spawn and its wrapped toolsets are independent copies. - assert spawned is not wrapper - spawned._selected_tool_names = {"add"} - assert {tool.name for tool in spawned} == {"add"} - # The configured wrapper and its children are untouched. - assert wrapper._selected_tool_names is None - assert ts1._selected_tool_names is None - assert ts2._selected_tool_names is None - assert {tool.name for tool in wrapper} == {"add", "multiply"} From 0987b78c8a77b3821675eaf7cbb0b57d9c28bd70 Mon Sep 17 00:00:00 2001 From: anakin87 Date: Fri, 7 Aug 2026 17:45:37 +0200 Subject: [PATCH 02/13] refactor --- docs-website/docs/tools/toolset.mdx | 4 ++-- haystack/components/agents/agent.py | 11 ++--------- haystack/components/agents/utils.py | 25 +++++++++++++------------ haystack/tools/toolset.py | 4 ++++ test/components/agents/test_agent.py | 19 +++++++++++++++++++ test/components/agents/test_utils.py | 19 +++++++++++++++++++ test/tools/test_searchable_toolset.py | 2 +- test/tools/test_toolset.py | 19 +++++++++++++++++++ 8 files changed, 79 insertions(+), 24 deletions(-) diff --git a/docs-website/docs/tools/toolset.mdx b/docs-website/docs/tools/toolset.mdx index 4cc1d8a3dbd..1489b96eeb5 100644 --- a/docs-website/docs/tools/toolset.mdx +++ b/docs-website/docs/tools/toolset.mdx @@ -89,9 +89,9 @@ To deliberately merge simple Toolsets into one, unpack them explicitly: `Toolset ### Tool Selection at Runtime -A `Toolset` is never mutated in place during an [`Agent`](../pipeline-components/agents-1/agent.mdx) run, so concurrent runs sharing the same `Toolset` instance are safe. +You can restrict an [`Agent`](../pipeline-components/agents-1/agent.mdx) to a subset of tools at runtime by passing tool names, for example `agent.run(tools=["tool_a", "tool_b"])`. This also works with a [`SearchableToolset`](searchabletoolset.mdx): search and lazy loading keep working over the selected subset. -You can restrict an `Agent` to a subset of tools at runtime by passing tool names, for example `agent.run(tools=["tool_a", "tool_b"])`. This also works with a [`SearchableToolset`](searchabletoolset.mdx): the `Agent` internally gives each run an isolated copy carrying the selection, so dynamic behavior like search and lazy loading keeps working over the selected subset. +Runtime selection never modifies your configured `Toolset`, and neither do a `SearchableToolset`'s discoveries. You can safely share the same `Toolset` instance across runs, including concurrent ones; just avoid adding or removing tools while runs are in progress. ## Usage diff --git a/haystack/components/agents/agent.py b/haystack/components/agents/agent.py index 10e280334bb..bd567ecd2bb 100644 --- a/haystack/components/agents/agent.py +++ b/haystack/components/agents/agent.py @@ -51,7 +51,6 @@ warm_up_hooks_async, ) from haystack.tools import ( - Tool, Toolset, ToolsType, _check_duplicate_tool_names, @@ -800,14 +799,8 @@ def _select_tools(self, tools: ToolsType | list[str] | None = None) -> ToolsType if isinstance(tools, list) and all(isinstance(t, str) for t in tools): return _select_tools_by_name(self.tools, cast(list[str], tools)) - if isinstance(tools, Toolset): - # Per-run tools are not covered by the Agent's own warm_up(), so warm them up here. - # warm_up() is expected to be idempotent, so re-warming on every run is cheap. - warm_up_tools(tools=tools) - return _copy_tools_for_run(tools=tools) - - if isinstance(tools, list): - selected = cast(list[Tool | Toolset], tools) # mypy can't narrow the Union type from isinstance check + if isinstance(tools, (Toolset, list)): + selected = cast(ToolsType, tools) # mypy can't narrow the Union type from the isinstance checks # Per-run tools are not covered by the Agent's own warm_up(), so warm them up here. # warm_up() is expected to be idempotent, so re-warming on every run is cheap. warm_up_tools(tools=selected) diff --git a/haystack/components/agents/utils.py b/haystack/components/agents/utils.py index 310e649ef0e..4359883e3a7 100644 --- a/haystack/components/agents/utils.py +++ b/haystack/components/agents/utils.py @@ -92,12 +92,12 @@ def _record_tool_calls(state: State, tool_messages: list[ChatMessage]) -> None: def _copy_toolset_for_run(toolset: Toolset, selected_tool_names: set[str] | None = None) -> Toolset: """ - Return a per-run copy of a Toolset with run-scoped state; plain Toolsets are returned unchanged. + Return a per-run copy of a Toolset with run-scoped state; a plain Toolset is returned as is. - A Toolset signals run-scoped state by defining `_copy_for_run()` (e.g. SearchableToolset, whose - discovered tools and name selection are per-run). Plain Toolsets are read-only at run time and can be shared - across runs directly. Note: passing `selected_tool_names` to a plain Toolset is unsupported here; callers - filter plain Toolsets externally. + A Toolset signals run-scoped state by defining `_copy_for_run()` (e.g. SearchableToolset, whose discovered + tools and name selection are per-run); the copy carries the given name selection. Plain Toolsets are + read-only at run time and can be shared across runs directly, so no copy is made (and any selection is + applied externally by the caller). """ copy_for_run = getattr(toolset, "_copy_for_run", None) if callable(copy_for_run): @@ -110,7 +110,8 @@ def _selectable_names(item: Tool | Toolset) -> set[str]: Resolve the tool names an item offers for name-based selection. A Toolset providing a `get_selectable_tools()` method (e.g. SearchableToolset, whose iteration does not - surface the full catalog) is asked through it; any other Toolset is warmed up and iterated. + surface the full catalog) is asked through it; any other Toolset is warmed up first, so lazily loaded + tools are selectable too. """ if not isinstance(item, Toolset): return {item.name} @@ -157,13 +158,13 @@ def _select_tools_by_name(configured_tools: ToolsType, names: list[str]) -> list matched = requested_names & item_names if not matched: continue - if isinstance(item, Toolset) and hasattr(item, "_copy_for_run"): - # The selection is applied to the per-run copy, so the shared, configured Toolset is never mutated. - selected.append(_copy_toolset_for_run(item, selected_tool_names=matched)) - elif isinstance(item, Toolset): - selected.extend(tool for tool in item if tool.name in matched) - else: + if not isinstance(item, Toolset): selected.append(item) + elif (run_copy := _copy_toolset_for_run(item, selected_tool_names=matched)) is not item: + # The selection is carried by the per-run copy, so the shared, configured Toolset is never mutated. + selected.append(run_copy) + else: + selected.extend(tool for tool in item if tool.name in matched) return selected diff --git a/haystack/tools/toolset.py b/haystack/tools/toolset.py index 256e0bb29ed..96a8874bf42 100644 --- a/haystack/tools/toolset.py +++ b/haystack/tools/toolset.py @@ -94,15 +94,18 @@ def from_dict(cls, data): serialization. """ + # Use field() with default_factory to initialize the list tools: list[Tool] = field(default_factory=list) def __post_init__(self) -> None: """ Validate the tools provided during initialization. """ + # If initialization was done a single Tool, raise an error if isinstance(self.tools, Tool): raise TypeError("A single Tool cannot be directly passed to Toolset. Please use a list: Toolset([tool])") + # Check for duplicate tool names in the initial set _check_duplicate_tool_names(self.tools) def __iter__(self) -> Iterator[Tool]: @@ -179,6 +182,7 @@ def add(self, tool: Tool) -> None: if not isinstance(tool, Tool): raise TypeError(f"Expected Tool, got {type(tool).__name__}") + # Check for duplicates before adding _check_duplicate_tool_names(self.tools + [tool]) self.tools.append(tool) diff --git a/test/components/agents/test_agent.py b/test/components/agents/test_agent.py index 691fc8105b0..6108087d2dc 100644 --- a/test/components/agents/test_agent.py +++ b/test/components/agents/test_agent.py @@ -1772,6 +1772,25 @@ def test_agent_span_has_parent_when_in_pipeline(self, spying_tracer, weather_too class TestAgentToolSelection: + def test_run_raises_on_duplicate_tool_names_across_toolsets(self): + # Duplicate names across combined sources are caught by the per-step validation, since no check at + # composition time can see the tools of lazily-loading Toolsets. + def make_tool(description: str) -> Tool: + return Tool( + name="same_name", + description=description, + parameters={"type": "object", "properties": {}}, + function=lambda: None, + ) + + agent = Agent( + chat_generator=MockChatGenerator("Hello"), + tools=[Toolset([make_tool("first")]), Toolset([make_tool("second")])], + ) + agent.warm_up() + with pytest.raises(ValueError, match="Duplicate tool names"): + agent.run(messages=[ChatMessage.from_user("hi")]) + def test_tool_selection_new_tool(self, weather_tool: Tool, component_tool: Tool): chat_generator = MockChatGenerator("Hello") agent = Agent(chat_generator=chat_generator, tools=[weather_tool], system_prompt="This is a system prompt.") diff --git a/test/components/agents/test_utils.py b/test/components/agents/test_utils.py index 991e6cc660f..d78cf2b6304 100644 --- a/test/components/agents/test_utils.py +++ b/test/components/agents/test_utils.py @@ -111,6 +111,25 @@ def test_selects_standalone_tools_and_toolsets(self, first_tool: Tool, second_to selected = _select_tools_by_name([second_tool, toolset], [first_tool.name, second_tool.name]) assert selected == [second_tool, first_tool] + def test_warms_up_lazy_toolsets_to_resolve_names(self, first_tool: Tool, second_tool: Tool): + class LazyToolset(Toolset): + """A Toolset that loads its tools on warm_up(), like toolsets backed by external services.""" + + def __init__(self, tools_to_load): + self._tools_to_load = tools_to_load + super().__init__([]) # no tools until warm_up + + def warm_up(self): + if not self.tools: + self.tools = list(self._tools_to_load) + + toolset = LazyToolset([first_tool, second_tool]) + assert list(toolset) == [] # not loaded yet + + # Name resolution warms the toolset first, so lazily loaded tools are selectable. + selected = _select_tools_by_name([toolset], [first_tool.name]) + assert selected == [first_tool] + def test_copies_run_scoped_toolsets_for_run_with_the_selection(self, first_tool: Tool, second_tool: Tool): class RunScopedToolset(Toolset): """A Toolset defining _copy_for_run(), signaling run-scoped state.""" diff --git a/test/tools/test_searchable_toolset.py b/test/tools/test_searchable_toolset.py index a7cfedff62f..a677690008c 100644 --- a/test/tools/test_searchable_toolset.py +++ b/test/tools/test_searchable_toolset.py @@ -127,7 +127,7 @@ def test_init_with_invalid_catalog(self): ) ) - def test_add_is_not_supported(self): + def test_not_implemented_methods(self): toolset = SearchableToolset(catalog=[]) with pytest.raises(NotImplementedError): toolset.add( diff --git a/test/tools/test_toolset.py b/test/tools/test_toolset.py index 76930dc2574..b1dac8c5c03 100644 --- a/test/tools/test_toolset.py +++ b/test/tools/test_toolset.py @@ -276,6 +276,25 @@ def test_agent_serde_with_list_of_toolsets(self, weather_tool, add_tool, monkeyp assert len(deserialized_agent.tools) == 2 assert all(isinstance(ts, Toolset) for ts in deserialized_agent.tools) + def test_agent_serde_with_list_containing_custom_toolset(self, multiply_tool, monkeypatch): + """A custom-serde Toolset inside a list roundtrips through its own to_dict/from_dict.""" + monkeypatch.setenv("OPENAI_API_KEY", "test") + agent = Agent(chat_generator=OpenAIChatGenerator(), tools=[DynamicToolset(), Toolset([multiply_tool])]) + data = agent.to_dict() + + tools_data = data["init_parameters"]["tools"] + assert tools_data[0]["type"] == "test_toolset.DynamicToolset" + assert tools_data[0]["data"] == {} + assert tools_data[1]["type"] == "haystack.tools.toolset.Toolset" + + deserialized_agent = Agent.from_dict(data) + custom_toolset, plain_toolset = deserialized_agent.tools[0], deserialized_agent.tools[1] + assert isinstance(custom_toolset, DynamicToolset) + assert isinstance(plain_toolset, Toolset) + # The custom toolset rebuilt its tools via its own from_dict. + assert [tool.name for tool in custom_toolset] == ["add"] + assert [tool.name for tool in plain_toolset] == ["multiply"] + def test_list_of_toolsets_runtime_override(self, weather_tool, add_tool, multiply_tool): """Test that list of Toolsets can be passed as runtime override to Agent.run().""" toolset2 = Toolset([add_tool]) From 725b405d734b88d43b1e784777532b0fb8caba90 Mon Sep 17 00:00:00 2001 From: anakin87 Date: Fri, 7 Aug 2026 18:33:20 +0200 Subject: [PATCH 03/13] clean uo --- docs-website/docs/tools/toolset.mdx | 2 - haystack/components/agents/agent.py | 2 +- haystack/components/agents/utils.py | 30 +++--------- haystack/tools/searchable_toolset.py | 14 ++---- haystack/tools/toolset.py | 47 ++++++++++++++----- .../simplify-toolset-102a9effbe4f188f.yaml | 37 ++++----------- test/components/agents/test_agent.py | 45 ++++++++++++++++-- test/tools/test_toolset.py | 31 +++++++++++- 8 files changed, 129 insertions(+), 79 deletions(-) diff --git a/docs-website/docs/tools/toolset.mdx b/docs-website/docs/tools/toolset.mdx index 1489b96eeb5..a14194656d9 100644 --- a/docs-website/docs/tools/toolset.mdx +++ b/docs-website/docs/tools/toolset.mdx @@ -85,8 +85,6 @@ agent = Agent( ) ``` -To deliberately merge simple Toolsets into one, unpack them explicitly: `Toolset([*math_toolset, *another_toolset])`. Avoid this for Toolsets that manage resources or load tools lazily (such as an `MCPToolset`): pass them as a list instead. - ### Tool Selection at Runtime You can restrict an [`Agent`](../pipeline-components/agents-1/agent.mdx) to a subset of tools at runtime by passing tool names, for example `agent.run(tools=["tool_a", "tool_b"])`. This also works with a [`SearchableToolset`](searchabletoolset.mdx): search and lazy loading keep working over the selected subset. diff --git a/haystack/components/agents/agent.py b/haystack/components/agents/agent.py index bd567ecd2bb..39160a9df1d 100644 --- a/haystack/components/agents/agent.py +++ b/haystack/components/agents/agent.py @@ -571,7 +571,7 @@ def _register_prompt_variables(self) -> None: def _warm_up_tools(self) -> None: """Warm up the configured tools once.""" if not self._tools_warmed_up: - if self.tools: + if self.tools is not None: warm_up_tools(tools=self.tools) self._tools_warmed_up = True diff --git a/haystack/components/agents/utils.py b/haystack/components/agents/utils.py index 4359883e3a7..c644d1d443c 100644 --- a/haystack/components/agents/utils.py +++ b/haystack/components/agents/utils.py @@ -92,12 +92,9 @@ def _record_tool_calls(state: State, tool_messages: list[ChatMessage]) -> None: def _copy_toolset_for_run(toolset: Toolset, selected_tool_names: set[str] | None = None) -> Toolset: """ - Return a per-run copy of a Toolset with run-scoped state; a plain Toolset is returned as is. + Return a per-run copy of a Toolset that defines `_copy_for_run()` (e.g. SearchableToolset). - A Toolset signals run-scoped state by defining `_copy_for_run()` (e.g. SearchableToolset, whose discovered - tools and name selection are per-run); the copy carries the given name selection. Plain Toolsets are - read-only at run time and can be shared across runs directly, so no copy is made (and any selection is - applied externally by the caller). + The copy carries the given name selection. A plain Toolset has no run-scoped state and is returned as is. """ copy_for_run = getattr(toolset, "_copy_for_run", None) if callable(copy_for_run): @@ -105,22 +102,6 @@ def _copy_toolset_for_run(toolset: Toolset, selected_tool_names: set[str] | None return toolset -def _selectable_names(item: Tool | Toolset) -> set[str]: - """ - Resolve the tool names an item offers for name-based selection. - - A Toolset providing a `get_selectable_tools()` method (e.g. SearchableToolset, whose iteration does not - surface the full catalog) is asked through it; any other Toolset is warmed up first, so lazily loaded - tools are selectable too. - """ - if not isinstance(item, Toolset): - return {item.name} - if hasattr(item, "get_selectable_tools"): - return {tool.name for tool in item.get_selectable_tools()} - item.warm_up() - return {tool.name for tool in item} - - def _select_tools_by_name(configured_tools: ToolsType, names: list[str]) -> list[Tool | Toolset]: """ Select configured tools by name for a single run. @@ -136,7 +117,7 @@ def _select_tools_by_name(configured_tools: ToolsType, names: list[str]) -> list :returns: The selected Tools and/or selection-scoped Toolset copies. :raises ValueError: If no tools were configured, or if any requested name is not a valid tool name. """ - if not configured_tools: + if configured_tools is None or (not isinstance(configured_tools, Toolset) and not configured_tools): raise ValueError("No tools were configured for the Agent at initialization.") requested_names = set(names) @@ -144,7 +125,10 @@ def _select_tools_by_name(configured_tools: ToolsType, names: list[str]) -> list [configured_tools] if isinstance(configured_tools, Toolset) else list(configured_tools) ) - selectable_per_item = [(item, _selectable_names(item)) for item in items] + selectable_per_item = [ + (item, {tool.name for tool in item.get_selectable_tools()} if isinstance(item, Toolset) else {item.name}) + for item in items + ] valid_tool_names = {name for _, item_names in selectable_per_item for name in item_names} invalid_tool_names = requested_names - valid_tool_names diff --git a/haystack/tools/searchable_toolset.py b/haystack/tools/searchable_toolset.py index e1f7fb202b4..7cff662633a 100644 --- a/haystack/tools/searchable_toolset.py +++ b/haystack/tools/searchable_toolset.py @@ -198,16 +198,12 @@ def clear(self) -> None: def _copy_for_run(self, selected_tool_names: set[str] | None = None) -> "SearchableToolset": """ - Return a copy of this toolset to be used for a single Agent run. + Return an isolated copy for a single Agent run, carrying the given name selection. - This is the internal method through which the Agent isolates run-scoped Toolset state: a Toolset that - defines it is replaced by a per-run copy at run start, so concurrent runs sharing the same configured - Toolset don't share discovered tools or collide on the active selection. - - The copy shares the read-only configuration (catalog, BM25 index) but starts with fresh run state: - no discovered tools, a bootstrap search tool bound to the copy, and the given selection. The selection - is fixed for the copy's lifetime: iteration only yields selected tools (the bootstrap search tool stays - exposed) and search is scoped to them. + The copy shares the read-only catalog and BM25 index but gets fresh discovered tools and a bootstrap + search tool bound to the copy; the selection is fixed for the copy's lifetime and scopes both iteration + and search. This way concurrent runs sharing the same configured SearchableToolset don't share + discovered tools or collide on the active selection. :param selected_tool_names: Optional catalog tool names this run is restricted to. None means no restriction. diff --git a/haystack/tools/toolset.py b/haystack/tools/toolset.py index 96a8874bf42..36fcf340cc9 100644 --- a/haystack/tools/toolset.py +++ b/haystack/tools/toolset.py @@ -154,27 +154,50 @@ def warm_up(self) -> None: """ Prepare the Toolset for use. - By default, this method warms up all tools in the Toolset. Subclasses that load tools dynamically - (e.g. from an MCP server or an OpenAPI spec) should override this method to fetch their tools and assign - them to `self.tools`. - - Following the framework-wide convention, `warm_up()` may be called multiple times (e.g. before every run) - and implementations are responsible for making it idempotent. The default implementation delegates to the - tools' own idempotent `warm_up()`. Subclasses should guard on their own state, for example - `if self._client is not None: return`. + By default, this method iterates through and warms up all tools in the Toolset. + Subclasses can override this method to customize initialization behavior, such as: + + - Setting up shared resources (database connections, HTTP sessions) instead of + warming individual tools + - Loading tools dynamically from an external source and assigning them to `self.tools` + - Controlling when and how tools are initialized + + For example, a Toolset that manages tools from an external service (like MCPToolset) + might override this to initialize a shared connection and load the tools through it: + + ```python + class MCPToolset(Toolset): + def warm_up(self) -> None: + if self.mcp_connection is not None: + return + self.mcp_connection = establish_connection(self.server_url) + self.tools = self.mcp_connection.fetch_tools() + ``` + + Following the framework-wide convention, this method may be called multiple times (e.g. before every + run): implementations are responsible for their own idempotence, guarding on their own state as in the + example above. The default implementation delegates to the tools' own idempotent `warm_up()`. """ for tool in self.tools: if hasattr(tool, "warm_up"): tool.warm_up() + def get_selectable_tools(self) -> list[Tool]: + """ + Return the tools available for name-based selection (e.g. via `Agent.run(tools=["tool_name"])`). + + Warms up the Toolset first, so lazily loaded tools are selectable too. Subclasses whose iteration does + not surface every selectable tool (e.g. SearchableToolset) override this to return the full set. + + :returns: The list of tools available for name-based selection. + """ + self.warm_up() + return list(self.tools) + def add(self, tool: Tool) -> None: """ Add a new Tool to this Toolset. - To combine whole Toolsets, pass them as a list wherever tools are accepted instead, e.g. - `Agent(tools=[toolset_a, toolset_b])`. This keeps each Toolset as a unit, preserving its lifecycle - and serialization. - :param tool: A Tool instance to add :raises ValueError: If adding the tool would result in duplicate tool names :raises TypeError: If the provided object is not a Tool diff --git a/releasenotes/notes/simplify-toolset-102a9effbe4f188f.yaml b/releasenotes/notes/simplify-toolset-102a9effbe4f188f.yaml index 1a79f8869d7..ce519c2fa15 100644 --- a/releasenotes/notes/simplify-toolset-102a9effbe4f188f.yaml +++ b/releasenotes/notes/simplify-toolset-102a9effbe4f188f.yaml @@ -1,33 +1,14 @@ --- upgrade: - | - Toolset concatenation with ``+`` has been removed, along with the internal wrapper class it produced - (``_ToolsetWrapper``). To use multiple Toolsets together, pass them as a list wherever tools are accepted, - which keeps each Toolset intact with its own lifecycle and serialization: ``Agent(tools=[toolset_a, - toolset_b])``. To deliberately merge simple Toolsets into one, unpack them explicitly: - ``Toolset([*toolset_a, *toolset_b])``. Serialized pipelines containing the removed wrapper - (``haystack.tools.toolset._ToolsetWrapper``) can no longer be deserialized: recreate them by passing the - Toolsets as a list. + Combining Toolsets with ``+`` or ``Toolset.add(toolset)`` is no longer supported. Pass Toolsets as a list + wherever tools are accepted instead: ``Agent(tools=[toolset_a, toolset_b])``. - | - ``Toolset.add()`` now accepts only ``Tool`` instances. Adding a whole Toolset used to flatten it, detaching - its tools from the Toolset that manages their lifecycle and serialization, and silently dropped the tools of - lazily-loading Toolsets that were not warmed up yet. To combine Toolsets, pass them as a list wherever tools - are accepted: ``Agent(tools=[toolset_a, toolset_b])``. + ``Toolset.spawn()`` has been removed. It was only needed internally: plain Toolsets are shared across runs, + and ``SearchableToolset`` handles its own per-run isolation. - | - ``Toolset.spawn()`` and ``Toolset.get_selectable_tools()`` have been removed from the base class, and the - per-run isolation machinery is now internal. Plain Toolsets hold no run-scoped state, so they are shared - across runs directly and reduced to their matching Tools on name selection. ``SearchableToolset`` (the one - Toolset with run-scoped state) is copied per run internally; its per-run behavior, including tool-name - selection via ``Agent.run(tools=[...])``, is unchanged. Third-party Toolset subclasses that overrode - ``spawn()`` no longer take part in per-run copying and behave as plain Toolsets. - - | - ``Toolset.warm_up()`` no longer tracks an internal "warmed up" flag. Following the framework-wide convention, - ``warm_up()`` can be called multiple times and implementations are responsible for their own idempotence: - the default implementation delegates to the tools' own idempotent ``warm_up()``, and Toolset subclasses that - load tools dynamically should guard on their own state (for example ``if self._client is not None: return``). - Subclasses that relied on the private ``Toolset._is_warmed_up`` attribute must define their own guard. -enhancements: - - | - Simplified ``Toolset`` to a plain collection of Tools: iteration, length, and membership checks now always - reflect the full tool list, with no hidden run-scoped filtering. The per-run machinery lives internally in - the Toolsets that need it (currently only ``SearchableToolset``). + Haystack can call ``warm_up()`` on Tools and Toolsets more than once, for example before every run. + Previously ``Toolset`` silently absorbed the repeated calls with an internal flag; now every call reaches + your ``warm_up()``. If your custom Tool or Toolset does expensive work there (connecting to a server, + loading a model), make sure a second call does nothing: check whether the work is already done and return + early, for example ``if self._client is not None: return``. diff --git a/test/components/agents/test_agent.py b/test/components/agents/test_agent.py index 6108087d2dc..89db68b7ee7 100644 --- a/test/components/agents/test_agent.py +++ b/test/components/agents/test_agent.py @@ -1772,9 +1772,8 @@ def test_agent_span_has_parent_when_in_pipeline(self, spying_tracer, weather_too class TestAgentToolSelection: - def test_run_raises_on_duplicate_tool_names_across_toolsets(self): - # Duplicate names across combined sources are caught by the per-step validation, since no check at - # composition time can see the tools of lazily-loading Toolsets. + @staticmethod + def _agent_with_duplicate_tool_names() -> Agent: def make_tool(description: str) -> Tool: return Tool( name="same_name", @@ -1788,9 +1787,22 @@ def make_tool(description: str) -> Tool: tools=[Toolset([make_tool("first")]), Toolset([make_tool("second")])], ) agent.warm_up() + return agent + + def test_run_raises_on_duplicate_tool_names_across_toolsets(self): + # Duplicate names across combined sources are caught by the per-step validation, since no check at + # composition time can see the tools of lazily-loading Toolsets. + agent = self._agent_with_duplicate_tool_names() with pytest.raises(ValueError, match="Duplicate tool names"): agent.run(messages=[ChatMessage.from_user("hi")]) + @pytest.mark.asyncio + async def test_run_async_raises_on_duplicate_tool_names_across_toolsets(self): + # The async step loop performs its own per-step validation, on a separate code path from the sync one. + agent = self._agent_with_duplicate_tool_names() + with pytest.raises(ValueError, match="Duplicate tool names"): + await agent.run_async(messages=[ChatMessage.from_user("hi")]) + def test_tool_selection_new_tool(self, weather_tool: Tool, component_tool: Tool): chat_generator = MockChatGenerator("Hello") agent = Agent(chat_generator=chat_generator, tools=[weather_tool], system_prompt="This is a system prompt.") @@ -2221,6 +2233,33 @@ def test_warm_up_toolset(self): agent.warm_up() assert toolset.was_warmed_up + def test_warm_up_covers_empty_lazy_toolset(self): + # A Toolset that only loads its tools on warm_up() has len 0 before warming: the Agent must warm it + # anyway (a truthiness check on the toolset would skip it and permanently mark tools as warmed). + class LazyToolset(Toolset): + def __init__(self): + self.loaded = False + super().__init__(tools=[]) + + def warm_up(self): + if self.loaded: + return + self.loaded = True + self.tools = [ + Tool( + name="lazy_tool", + description="d", + parameters={"type": "object", "properties": {}}, + function=lambda: None, + ) + ] + + toolset = LazyToolset() + agent = Agent(chat_generator=MockChatGenerator("Hello"), tools=toolset) + agent.warm_up() + assert toolset.loaded + assert [tool.name for tool in toolset] == ["lazy_tool"] + def test_warm_up_list_of_toolsets(self): # Toolsets are combined by passing them as a list; each keeps its own warm_up. tool1 = self._make_tracking_tool("tool1") diff --git a/test/tools/test_toolset.py b/test/tools/test_toolset.py index b1dac8c5c03..f333aae2216 100644 --- a/test/tools/test_toolset.py +++ b/test/tools/test_toolset.py @@ -168,7 +168,7 @@ def test_toolset_contains(self, add_tool, multiply_tool): assert "non_existent_tool" not in toolset def test_combining_toolsets_via_unpacking(self, add_tool, multiply_tool, subtract_tool): - """Explicit unpacking is the supported way to build a merged Toolset.""" + """A Toolset can be built from the tools of existing ones; the combined tools remain invocable.""" combined = Toolset([*Toolset([add_tool, subtract_tool]), multiply_tool]) assert [t.name for t in combined] == ["add", "subtract", "multiply"] @@ -395,3 +395,32 @@ def test_add_toolset_raises(self): not_a_tool: Any = Toolset([WarmUpCountingTool("b")]) with pytest.raises(TypeError, match="Expected Tool"): toolset.add(not_a_tool) + + +class TestToolsetToolSelection: + """Tests for get_selectable_tools().""" + + def test_get_selectable_tools_returns_all_tools(self, add_tool, multiply_tool): + toolset = Toolset([add_tool, multiply_tool]) + assert toolset.get_selectable_tools() == [add_tool, multiply_tool] + + def test_get_selectable_tools_warms_up_lazy_toolset(self, add_tool, multiply_tool): + """get_selectable_tools() warms up a lazy toolset so its lazily loaded tools are available for selection.""" + + class LazyToolset(Toolset): + def __init__(self): + self._loaded = False + super().__init__([]) # no tools until warm_up + + def warm_up(self): + if self._loaded: + return + self._loaded = True + self.tools = [add_tool, multiply_tool] + + toolset = LazyToolset() + assert toolset.tools == [] # not loaded yet + + selectable = toolset.get_selectable_tools() + + assert [tool.name for tool in selectable] == ["add", "multiply"] From 5a37f397c7a6118ac6f110239f5ac535d543bff1 Mon Sep 17 00:00:00 2001 From: anakin87 Date: Fri, 7 Aug 2026 19:56:28 +0200 Subject: [PATCH 04/13] less changes --- haystack/components/agents/agent.py | 23 +++++----- haystack/components/agents/utils.py | 58 +++++++++++--------------- haystack/tools/searchable_toolset.py | 31 ++++---------- haystack/tools/skills/skill_toolset.py | 2 +- haystack/tools/toolset.py | 16 +++++++ test/components/agents/test_agent.py | 6 +-- test/components/agents/test_utils.py | 29 +++++++++++-- test/tools/test_searchable_toolset.py | 12 +++--- test/tools/test_toolset.py | 15 +++++++ 9 files changed, 112 insertions(+), 80 deletions(-) diff --git a/haystack/components/agents/agent.py b/haystack/components/agents/agent.py index 39160a9df1d..707ae11d7f0 100644 --- a/haystack/components/agents/agent.py +++ b/haystack/components/agents/agent.py @@ -17,12 +17,12 @@ from haystack.components.agents.state.state_utils import merge_lists from haystack.components.agents.tool_calling import _run_tool, _run_tool_async from haystack.components.agents.utils import ( - _copy_tools_for_run, _record_context_tokens, _record_llm_usage, _record_tool_calls, _render_prompt_messages, _select_tools_by_name, + _spawn_tools, _template_for_role, _validate_prompt_message_blocks, ) @@ -477,7 +477,6 @@ def __init__( # noqa: PLR0913 self.tool_concurrency_limit = tool_concurrency_limit self.tool_streaming_callback_passthrough = tool_streaming_callback_passthrough self.hooks = hooks - self._tools_warmed_up = False self._hooks_warmed_up = False # --- State schema --- @@ -569,11 +568,13 @@ def _register_prompt_variables(self) -> None: component.set_input_type(self, name=var_name, type=Any, default=None) def _warm_up_tools(self) -> None: - """Warm up the configured tools once.""" - if not self._tools_warmed_up: - if self.tools is not None: - warm_up_tools(tools=self.tools) - self._tools_warmed_up = True + """ + Warm up the configured tools. + + Called on every warm_up() (and therefore every run), so tools added to a Toolset after the first run are + warmed too. Tools' warm_up() is expected to be idempotent, making repeated warming cheap. + """ + warm_up_tools(tools=self.tools) def _warm_up_hooks(self) -> None: """Warm up the configured hooks once.""" @@ -790,11 +791,11 @@ def _select_tools(self, tools: ToolsType | list[str] | None = None) -> ToolsType or if any provided tool name is not valid. :raises TypeError: If tools is not a list of Tool objects, a Toolset, or a list of tool names (strings). """ - # Toolsets with run-scoped state (those defining _copy_for_run(), e.g. SearchableToolset) are replaced - # by per-run copies (see _copy_tools_for_run / _select_tools_by_name) so concurrent runs sharing the + # Toolsets with run-scoped state (those overriding spawn(), e.g. SearchableToolset) are replaced + # by per-run copies (see _spawn_tools / _select_tools_by_name) so concurrent runs sharing the # same configured Toolset don't corrupt each other's run-scoped state. if tools is None: - return _copy_tools_for_run(tools=self.tools) + return _spawn_tools(tools=self.tools) if isinstance(tools, list) and all(isinstance(t, str) for t in tools): return _select_tools_by_name(self.tools, cast(list[str], tools)) @@ -804,7 +805,7 @@ def _select_tools(self, tools: ToolsType | list[str] | None = None) -> ToolsType # Per-run tools are not covered by the Agent's own warm_up(), so warm them up here. # warm_up() is expected to be idempotent, so re-warming on every run is cheap. warm_up_tools(tools=selected) - return _copy_tools_for_run(tools=selected) + return _spawn_tools(tools=selected) raise TypeError( "tools must be a list of Tool and/or Toolset objects, a Toolset, or a list of tool names (strings)." diff --git a/haystack/components/agents/utils.py b/haystack/components/agents/utils.py index c644d1d443c..1c2816b138a 100644 --- a/haystack/components/agents/utils.py +++ b/haystack/components/agents/utils.py @@ -90,34 +90,21 @@ def _record_tool_calls(state: State, tool_messages: list[ChatMessage]) -> None: # --------------------------- -def _copy_toolset_for_run(toolset: Toolset, selected_tool_names: set[str] | None = None) -> Toolset: - """ - Return a per-run copy of a Toolset that defines `_copy_for_run()` (e.g. SearchableToolset). - - The copy carries the given name selection. A plain Toolset has no run-scoped state and is returned as is. - """ - copy_for_run = getattr(toolset, "_copy_for_run", None) - if callable(copy_for_run): - return copy_for_run(selected_tool_names=selected_tool_names) - return toolset - - def _select_tools_by_name(configured_tools: ToolsType, names: list[str]) -> list[Tool | Toolset]: """ Select configured tools by name for a single run. - Standalone Tools are kept when their name is requested. A plain Toolset that exposes requested names is - warmed up and reduced to the matching Tools. A Toolset with run-scoped state (one defining - `_copy_for_run()`, such as SearchableToolset) is replaced by a per-run copy carrying the requested - names, so its dynamic behavior (search/lazy-loading) is preserved without mutating the shared, configured - Toolset. + Standalone Tools are kept when their name is requested. A Toolset with run-scoped state (one overriding + `spawn()`, such as SearchableToolset) is replaced by a per-run copy carrying the requested names, so its + dynamic behavior (search/lazy-loading) is preserved without mutating the shared, configured Toolset. Any + other Toolset is warmed up and reduced to the matching Tools. :param configured_tools: The tools configured on the Agent. :param names: The requested tool names. :returns: The selected Tools and/or selection-scoped Toolset copies. :raises ValueError: If no tools were configured, or if any requested name is not a valid tool name. """ - if configured_tools is None or (not isinstance(configured_tools, Toolset) and not configured_tools): + if configured_tools is None: raise ValueError("No tools were configured for the Agent at initialization.") requested_names = set(names) @@ -126,10 +113,13 @@ def _select_tools_by_name(configured_tools: ToolsType, names: list[str]) -> list ) selectable_per_item = [ - (item, {tool.name for tool in item.get_selectable_tools()} if isinstance(item, Toolset) else {item.name}) - for item in items + (item, item.get_selectable_tools() if isinstance(item, Toolset) else [item]) for item in items ] - valid_tool_names = {name for _, item_names in selectable_per_item for name in item_names} + valid_tool_names = {tool.name for _, selectable in selectable_per_item for tool in selectable} + # Emptiness is only detectable here: a dynamic Toolset may look empty before get_selectable_tools() has + # resolved its real catalog. + if not valid_tool_names: + raise ValueError("No tools were configured for the Agent at initialization.") invalid_tool_names = requested_names - valid_tool_names if invalid_tool_names: @@ -138,32 +128,34 @@ def _select_tools_by_name(configured_tools: ToolsType, names: list[str]) -> list ) selected: list[Tool | Toolset] = [] - for item, item_names in selectable_per_item: - matched = requested_names & item_names + for item, selectable in selectable_per_item: + matched = requested_names & {tool.name for tool in selectable} if not matched: continue if not isinstance(item, Toolset): selected.append(item) - elif (run_copy := _copy_toolset_for_run(item, selected_tool_names=matched)) is not item: + elif (run_copy := item.spawn(selected_tool_names=matched)) is not item: # The selection is carried by the per-run copy, so the shared, configured Toolset is never mutated. selected.append(run_copy) else: - selected.extend(tool for tool in item if tool.name in matched) + # Select from the same list validation used (get_selectable_tools), not from iteration: a dynamic + # toolset that doesn't override spawn() may not surface every selectable tool via __iter__. + selected.extend(tool for tool in selectable if tool.name in matched) return selected -def _copy_tools_for_run(tools: ToolsType) -> ToolsType: +def _spawn_tools(tools: ToolsType) -> ToolsType: """ - Return per-run copies of the Toolsets in `tools` that carry run-scoped state. + Return per-run copies of `tools`, replacing each Toolset with its `spawn()` (Tools are passed through). - A Toolset defining `_copy_for_run()` (e.g. SearchableToolset, whose discovered tools and name - selection are per-run) is replaced by an isolated copy, so concurrent runs sharing the same configured - Toolset — such as parallel sub-agent tool calls or concurrent requests against one Agent — don't corrupt - each other. Plain Toolsets and standalone Tools are read-only at run time and are passed through unchanged. + This isolates run-scoped Toolset state (e.g. a SearchableToolset's discovered tools and any active name + selection) so that concurrent runs sharing the same configured Toolset — such as parallel sub-agent tool calls + or concurrent requests against one Agent — don't corrupt each other. A plain Toolset has no run-scoped state + and its `spawn()` returns itself unchanged. """ if isinstance(tools, Toolset): - return _copy_toolset_for_run(tools) - return [_copy_toolset_for_run(item) if isinstance(item, Toolset) else item for item in tools] + return tools.spawn() + return [item.spawn() if isinstance(item, Toolset) else item for item in tools] # --------------------------- diff --git a/haystack/tools/searchable_toolset.py b/haystack/tools/searchable_toolset.py index 7cff662633a..e1388667413 100644 --- a/haystack/tools/searchable_toolset.py +++ b/haystack/tools/searchable_toolset.py @@ -130,14 +130,14 @@ def __init__( self._document_store: InMemoryDocumentStore | None = None self._passthrough: bool | None = None - # Optional per-run name filter, set on the copies returned by _copy_for_run(). When set, iteration only + # Optional per-run name filter, set on the copies returned by spawn(). When set, iteration only # yields tools whose name is in this set, and search is scoped to it. None means no filtering. self._selected_tool_names: set[str] | None = None # Initialize parent with empty tools list - we manage tools dynamically super().__init__(tools=[]) - def add(self, tool: Tool | Toolset) -> None: + def add(self, tool: Tool) -> None: """Adding new tools after initialization is not supported for SearchableToolset.""" raise NotImplementedError("SearchableToolset does not support adding new tools after initialization.") @@ -196,14 +196,14 @@ def clear(self) -> None: """ self._discovered_tools.clear() - def _copy_for_run(self, selected_tool_names: set[str] | None = None) -> "SearchableToolset": + def spawn(self, selected_tool_names: set[str] | None = None) -> "SearchableToolset": """ - Return an isolated copy for a single Agent run, carrying the given name selection. + Return an isolated copy for a single run, carrying the given name selection. - The copy shares the read-only catalog and BM25 index but gets fresh discovered tools and a bootstrap - search tool bound to the copy; the selection is fixed for the copy's lifetime and scopes both iteration - and search. This way concurrent runs sharing the same configured SearchableToolset don't share - discovered tools or collide on the active selection. + The copy shares the read-only catalog and BM25 index but gets fresh discovered tools and name selection, + plus a bootstrap search tool bound to the copy; the selection is fixed for the copy's lifetime and scopes + both iteration and search. This way concurrent runs sharing the same configured SearchableToolset don't + share discovered tools or collide on the active selection. :param selected_tool_names: Optional catalog tool names this run is restricted to. None means no restriction. @@ -316,11 +316,6 @@ def __iter__(self) -> Iterator[Tool]: yield self._bootstrap_tool yield from (tool for tool in self._discovered_tools.values() if self._is_selected(tool.name)) - def __len__(self) -> int: - """Return the number of currently available tools.""" - # the number of tools is computed by invoking __iter__ on the toolset - return sum(1 for _ in self) - def __contains__(self, item: str | Tool) -> bool: """ Check if a tool is available by Tool instance or tool name string. @@ -334,16 +329,6 @@ def __contains__(self, item: str | Tool) -> bool: return any(tool == item for tool in self) raise TypeError(f"Invalid item type: {type(item)}. Must be Tool or str.") - def __getitem__(self, index: int) -> Tool: - """ - Get a tool by index. - - :param index: Index of the tool to retrieve. - :returns: The tool at the given index. - :raises IndexError: If the index is out of range. - """ - return list(self)[index] - def to_dict(self) -> dict[str, Any]: """ Serialize the toolset to a dictionary. diff --git a/haystack/tools/skills/skill_toolset.py b/haystack/tools/skills/skill_toolset.py index 94b27a642e8..d3b46d4187b 100644 --- a/haystack/tools/skills/skill_toolset.py +++ b/haystack/tools/skills/skill_toolset.py @@ -103,7 +103,7 @@ def warm_up(self) -> None: self._load_skill_tool.description = self._load_skill_description() self._is_warmed_up = True - def add(self, tool: Tool | Toolset) -> None: + def add(self, tool: Tool) -> None: """Adding tools is not supported: a SkillToolset's tools are fixed and defined by its store.""" raise NotImplementedError( "SkillToolset does not support adding tools. To combine it with other tools, pass it to the Agent " diff --git a/haystack/tools/toolset.py b/haystack/tools/toolset.py index 36fcf340cc9..7075c241a6e 100644 --- a/haystack/tools/toolset.py +++ b/haystack/tools/toolset.py @@ -194,6 +194,22 @@ def get_selectable_tools(self) -> list[Tool]: self.warm_up() return list(self.tools) + def spawn(self, selected_tool_names: set[str] | None = None) -> "Toolset": # noqa: ARG002 + """ + Return an isolated instance of this Toolset for a single run. + + A plain Toolset has no run-scoped state, so the default implementation returns `self` unchanged and + ignores `selected_tool_names` (the Agent materializes name selections itself in that case). Subclasses + with additional run-scoped state (e.g. SearchableToolset) should override this to return a copy that + shares the read-only state (its tools and any warmed-up resources) but gets fresh run-scoped state and + carries the selection, so concurrent runs that share the same configured Toolset don't corrupt each + other (for example, one run's discovered tools leaking into another). + + :param selected_tool_names: Optional tool names this run is restricted to. None means no restriction. + :returns: This Toolset, or a run-scoped copy of it. + """ + return self + def add(self, tool: Tool) -> None: """ Add a new Tool to this Toolset. diff --git a/test/components/agents/test_agent.py b/test/components/agents/test_agent.py index 89db68b7ee7..67ecf45f84a 100644 --- a/test/components/agents/test_agent.py +++ b/test/components/agents/test_agent.py @@ -2295,7 +2295,8 @@ def test_warm_up_mixed_list_of_tools_and_toolsets(self): assert toolset1.was_warmed_up assert toolset2.was_warmed_up - def test_warm_up_is_idempotent(self): + def test_warm_up_rewarms_tools_on_every_call(self): + """Tools are warmed on every warm_up() so tools added to a Toolset after the first run get warmed too.""" call_count = {"n": 0} tool = Tool( name="counting_tool", @@ -2316,7 +2317,7 @@ def counting_warm_up(): agent.warm_up() agent.warm_up() - assert call_count["n"] == 1 + assert call_count["n"] == 3 def test_warm_up_refreshes_toolset(self): """Agent.warm_up() must warm up lazy toolsets (e.g. MCPToolset) so the actual tools are available at runtime.""" @@ -2459,7 +2460,6 @@ def test_warm_up_delegates_to_chat_generator(self, weather_tool): chat_generator.warm_up.reset_mock() agent.run([ChatMessage.from_user("What is the weather in Berlin?")]) - assert agent._tools_warmed_up is True # warm_up runs twice here: the Agent delegates to the generator, and the generator's own run() self-warms assert chat_generator.warm_up.call_count == 2 diff --git a/test/components/agents/test_utils.py b/test/components/agents/test_utils.py index d78cf2b6304..1e9eff8141e 100644 --- a/test/components/agents/test_utils.py +++ b/test/components/agents/test_utils.py @@ -99,6 +99,10 @@ def test_raises_when_no_tools_configured(self, first_tool: Tool): with pytest.raises(ValueError, match="No tools were configured for the Agent at initialization."): _select_tools_by_name([], [first_tool.name]) + def test_raises_when_configured_toolset_is_empty(self, first_tool: Tool): + with pytest.raises(ValueError, match="No tools were configured for the Agent at initialization."): + _select_tools_by_name(Toolset([]), [first_tool.name]) + def test_reduces_plain_toolsets_to_matching_tools(self, first_tool: Tool, second_tool: Tool): toolset = Toolset([first_tool, second_tool]) selected = _select_tools_by_name([toolset], [first_tool.name]) @@ -130,15 +134,15 @@ def warm_up(self): selected = _select_tools_by_name([toolset], [first_tool.name]) assert selected == [first_tool] - def test_copies_run_scoped_toolsets_for_run_with_the_selection(self, first_tool: Tool, second_tool: Tool): + def test_spawns_toolsets_without_mutating_them(self, first_tool: Tool, second_tool: Tool): class RunScopedToolset(Toolset): - """A Toolset defining _copy_for_run(), signaling run-scoped state.""" + """A Toolset overriding spawn(), signaling run-scoped state.""" def __init__(self, tools): super().__init__(tools) self.selected: set[str] | None = None - def _copy_for_run(self, selected_tool_names: set[str] | None = None) -> "RunScopedToolset": + def spawn(self, selected_tool_names: set[str] | None = None) -> "RunScopedToolset": new = RunScopedToolset(list(self.tools)) new.selected = set(selected_tool_names) if selected_tool_names is not None else None return new @@ -153,6 +157,25 @@ def _copy_for_run(self, selected_tool_names: set[str] | None = None) -> "RunScop # The configured toolset is untouched. assert toolset.selected is None + def test_selects_tools_not_surfaced_by_iteration(self, first_tool: Tool, second_tool: Tool): + class DiscoveryToolset(Toolset): + """A dynamic Toolset without a spawn() override: iteration yields less than get_selectable_tools().""" + + def __init__(self, catalog): + self._catalog = catalog + super().__init__([]) + + def __iter__(self): + yield from [] # nothing discovered yet + + def get_selectable_tools(self) -> list[Tool]: + return list(self._catalog) + + toolset = DiscoveryToolset([first_tool, second_tool]) + # The requested tool must be selected even though iteration does not surface it. + selected = _select_tools_by_name([toolset], [first_tool.name]) + assert selected == [first_tool] + class TestContextTokensFromUsage: """`_context_tokens_from_usage` normalizes real provider `meta["usage"]` shapes to input + output tokens.""" diff --git a/test/tools/test_searchable_toolset.py b/test/tools/test_searchable_toolset.py index a677690008c..0a3007af0f7 100644 --- a/test/tools/test_searchable_toolset.py +++ b/test/tools/test_searchable_toolset.py @@ -825,8 +825,8 @@ def test_get_selectable_tools_exposes_full_catalog(self, large_catalog): # The catalog, however, is fully available for name-based selection. assert {tool.name for tool in toolset.get_selectable_tools()} == {tool.name for tool in large_catalog} - def test_runtime_tool_names_return_isolated_copy_and_preserve_search(self, large_catalog, monkeypatch): - """Selecting catalog tool names returns an isolated per-run copy carrying the selection, with search active.""" + def test_runtime_tool_names_select_isolated_spawn_and_preserve_search(self, large_catalog, monkeypatch): + """Selecting catalog tool names returns an isolated spawn carrying the selection, with search active.""" monkeypatch.setenv("OPENAI_API_KEY", "fake-key") toolset = SearchableToolset(catalog=large_catalog, search_threshold=3) # 8 tools -> search mode agent = Agent(chat_generator=OpenAIChatGenerator(), tools=toolset) @@ -849,13 +849,13 @@ def test_runtime_tool_names_return_isolated_copy_and_preserve_search(self, large # The configured toolset's discovered tools are untouched. assert toolset._discovered_tools == {} - def test_run_copies_have_independent_discovered_tools_and_selection(self, large_catalog): - """Per-run copies of one SearchableToolset don't share discovered tools or collide on the selection.""" + def test_spawns_have_independent_discovered_tools_and_selection(self, large_catalog): + """Spawns of one SearchableToolset don't share discovered tools or collide on the selection.""" toolset = SearchableToolset(catalog=large_catalog, search_threshold=3) toolset.warm_up() - copy_a = toolset._copy_for_run(selected_tool_names={"get_weather"}) - copy_b = toolset._copy_for_run() + copy_a = toolset.spawn(selected_tool_names={"get_weather"}) + copy_b = toolset.spawn() assert copy_a is not copy_b assert copy_a is not toolset diff --git a/test/tools/test_toolset.py b/test/tools/test_toolset.py index f333aae2216..ef911eb8ae2 100644 --- a/test/tools/test_toolset.py +++ b/test/tools/test_toolset.py @@ -397,6 +397,21 @@ def test_add_toolset_raises(self): toolset.add(not_a_tool) +class TestToolsetSpawn: + """Tests for spawn(), the run-scoping hook.""" + + def test_spawn_returns_self_for_plain_toolset(self, add_tool, multiply_tool): + """A plain Toolset has no run-scoped state, so spawn() returns the same instance.""" + toolset = Toolset([add_tool, multiply_tool]) + assert toolset.spawn() is toolset + + def test_spawn_ignores_selection_for_plain_toolset(self, add_tool, multiply_tool): + """The base spawn() ignores the selection (the Agent materializes it) and does not mutate the toolset.""" + toolset = Toolset([add_tool, multiply_tool]) + assert toolset.spawn(selected_tool_names={"add"}) is toolset + assert [tool.name for tool in toolset] == ["add", "multiply"] + + class TestToolsetToolSelection: """Tests for get_selectable_tools().""" From 48c31143a09e6541dfa1f8c09fbfd7448d1163d3 Mon Sep 17 00:00:00 2001 From: anakin87 Date: Fri, 7 Aug 2026 20:07:55 +0200 Subject: [PATCH 05/13] align --- haystack/components/agents/agent.py | 5 +-- haystack/tools/toolset.py | 58 ++++++++++++++-------------- test/components/agents/test_utils.py | 9 ++--- test/tools/test_toolset.py | 23 ++--------- 4 files changed, 39 insertions(+), 56 deletions(-) diff --git a/haystack/components/agents/agent.py b/haystack/components/agents/agent.py index 707ae11d7f0..fc0967a375c 100644 --- a/haystack/components/agents/agent.py +++ b/haystack/components/agents/agent.py @@ -791,9 +791,8 @@ def _select_tools(self, tools: ToolsType | list[str] | None = None) -> ToolsType or if any provided tool name is not valid. :raises TypeError: If tools is not a list of Tool objects, a Toolset, or a list of tool names (strings). """ - # Toolsets with run-scoped state (those overriding spawn(), e.g. SearchableToolset) are replaced - # by per-run copies (see _spawn_tools / _select_tools_by_name) so concurrent runs sharing the - # same configured Toolset don't corrupt each other's run-scoped state. + # Toolsets are spawned per run (see _spawn_tools / _select_tools_by_name) so concurrent runs + # sharing the same configured Toolset don't corrupt each other's run-scoped state. if tools is None: return _spawn_tools(tools=self.tools) diff --git a/haystack/tools/toolset.py b/haystack/tools/toolset.py index 7075c241a6e..9839e1a643f 100644 --- a/haystack/tools/toolset.py +++ b/haystack/tools/toolset.py @@ -112,10 +112,40 @@ def __iter__(self) -> Iterator[Tool]: """ Return an iterator over the Tools in this Toolset. + This allows the Toolset to be used wherever a list of Tools is expected. + :returns: An iterator yielding Tool instances """ return iter(self.tools) + def get_selectable_tools(self) -> list[Tool]: + """ + Return the tools available for name-based selection (e.g. via `Agent.run(tools=["tool_name"])`). + + Warms up the Toolset first, so lazily loaded tools are selectable too. Subclasses whose iteration does + not surface every selectable tool (e.g. SearchableToolset) override this to return the full set. + + :returns: The list of tools available for name-based selection. + """ + self.warm_up() + return list(self.tools) + + def spawn(self, selected_tool_names: set[str] | None = None) -> "Toolset": # noqa: ARG002 + """ + Return an isolated instance of this Toolset for a single run. + + A plain Toolset has no run-scoped state, so the default implementation returns `self` unchanged and + ignores `selected_tool_names` (the Agent materializes name selections itself in that case). Subclasses + with additional run-scoped state (e.g. SearchableToolset) should override this to return a copy that + shares the read-only state (its tools and any warmed-up resources) but gets fresh run-scoped state and + carries the selection, so concurrent runs that share the same configured Toolset don't corrupt each + other (for example, one run's discovered tools leaking into another). + + :param selected_tool_names: Optional tool names this run is restricted to. None means no restriction. + :returns: This Toolset, or a run-scoped copy of it. + """ + return self + def __contains__(self, item: str | Tool) -> bool: """ Check if a tool is in this Toolset. @@ -182,34 +212,6 @@ def warm_up(self) -> None: if hasattr(tool, "warm_up"): tool.warm_up() - def get_selectable_tools(self) -> list[Tool]: - """ - Return the tools available for name-based selection (e.g. via `Agent.run(tools=["tool_name"])`). - - Warms up the Toolset first, so lazily loaded tools are selectable too. Subclasses whose iteration does - not surface every selectable tool (e.g. SearchableToolset) override this to return the full set. - - :returns: The list of tools available for name-based selection. - """ - self.warm_up() - return list(self.tools) - - def spawn(self, selected_tool_names: set[str] | None = None) -> "Toolset": # noqa: ARG002 - """ - Return an isolated instance of this Toolset for a single run. - - A plain Toolset has no run-scoped state, so the default implementation returns `self` unchanged and - ignores `selected_tool_names` (the Agent materializes name selections itself in that case). Subclasses - with additional run-scoped state (e.g. SearchableToolset) should override this to return a copy that - shares the read-only state (its tools and any warmed-up resources) but gets fresh run-scoped state and - carries the selection, so concurrent runs that share the same configured Toolset don't corrupt each - other (for example, one run's discovered tools leaking into another). - - :param selected_tool_names: Optional tool names this run is restricted to. None means no restriction. - :returns: This Toolset, or a run-scoped copy of it. - """ - return self - def add(self, tool: Tool) -> None: """ Add a new Tool to this Toolset. diff --git a/test/components/agents/test_utils.py b/test/components/agents/test_utils.py index 1e9eff8141e..cde495b64c3 100644 --- a/test/components/agents/test_utils.py +++ b/test/components/agents/test_utils.py @@ -95,13 +95,10 @@ def test_raises_for_invalid_name(self, first_tool: Tool): with pytest.raises(ValueError, match="The following tool names are not valid"): _select_tools_by_name([first_tool], ["unknown"]) - def test_raises_when_no_tools_configured(self, first_tool: Tool): + @pytest.mark.parametrize("configured_tools", [[], Toolset([])], ids=["empty_list", "empty_toolset"]) + def test_raises_when_no_tools_configured(self, configured_tools, first_tool: Tool): with pytest.raises(ValueError, match="No tools were configured for the Agent at initialization."): - _select_tools_by_name([], [first_tool.name]) - - def test_raises_when_configured_toolset_is_empty(self, first_tool: Tool): - with pytest.raises(ValueError, match="No tools were configured for the Agent at initialization."): - _select_tools_by_name(Toolset([]), [first_tool.name]) + _select_tools_by_name(configured_tools, [first_tool.name]) def test_reduces_plain_toolsets_to_matching_tools(self, first_tool: Tool, second_tool: Tool): toolset = Toolset([first_tool, second_tool]) diff --git a/test/tools/test_toolset.py b/test/tools/test_toolset.py index ef911eb8ae2..f15e874daad 100644 --- a/test/tools/test_toolset.py +++ b/test/tools/test_toolset.py @@ -366,25 +366,13 @@ def test_warm_up_can_be_called_multiple_times(self): toolset.warm_up() assert t1.warm_up_count == 3 - def test_add_before_warm_up_does_not_warm_tools(self): - existing = WarmUpCountingTool("a") - toolset = Toolset([existing]) - new_tool = WarmUpCountingTool("b") - toolset.add(new_tool) - # Nothing is warmed until warm_up() is called explicitly. - assert existing.warm_up_count == 0 - assert new_tool.warm_up_count == 0 - toolset.warm_up() - assert existing.warm_up_count == 1 - assert new_tool.warm_up_count == 1 - - def test_add_tool_after_warm_up_warms_it_on_next_warm_up(self): + def test_add_never_warms_the_new_tool(self): + # add() is stateless: it never warms the added tool; the next warm_up() call warms whatever is present. existing = WarmUpCountingTool("a") toolset = Toolset([existing]) toolset.warm_up() new_tool = WarmUpCountingTool("b") toolset.add(new_tool) - # add() does not warm the new tool; the next warm_up() call does. assert new_tool.warm_up_count == 0 toolset.warm_up() assert new_tool.warm_up_count == 1 @@ -401,13 +389,10 @@ class TestToolsetSpawn: """Tests for spawn(), the run-scoping hook.""" def test_spawn_returns_self_for_plain_toolset(self, add_tool, multiply_tool): - """A plain Toolset has no run-scoped state, so spawn() returns the same instance.""" + """A plain Toolset has no run-scoped state: spawn() returns the same instance, with or without a + selection (which it ignores, since the Agent materializes it), and never mutates the toolset.""" toolset = Toolset([add_tool, multiply_tool]) assert toolset.spawn() is toolset - - def test_spawn_ignores_selection_for_plain_toolset(self, add_tool, multiply_tool): - """The base spawn() ignores the selection (the Agent materializes it) and does not mutate the toolset.""" - toolset = Toolset([add_tool, multiply_tool]) assert toolset.spawn(selected_tool_names={"add"}) is toolset assert [tool.name for tool in toolset] == ["add", "multiply"] From a3057fa570a7b14a9af4b13f01f62156cfc9650b Mon Sep 17 00:00:00 2001 From: anakin87 Date: Fri, 7 Aug 2026 20:17:35 +0200 Subject: [PATCH 06/13] more --- docs-website/docs/tools/searchabletoolset.mdx | 2 +- docs-website/docs/tools/toolset.mdx | 11 ++++++++--- haystack/components/agents/agent.py | 7 +------ haystack/components/agents/utils.py | 6 ++---- haystack/tools/searchable_toolset.py | 6 +++--- haystack/tools/toolset.py | 14 ++++++-------- test/tools/test_toolset.py | 2 -- 7 files changed, 21 insertions(+), 27 deletions(-) diff --git a/docs-website/docs/tools/searchabletoolset.mdx b/docs-website/docs/tools/searchabletoolset.mdx index fd418e49e12..b4e1ef3465a 100644 --- a/docs-website/docs/tools/searchabletoolset.mdx +++ b/docs-website/docs/tools/searchabletoolset.mdx @@ -118,7 +118,7 @@ toolset = SearchableToolset( ### Reusing the toolset across multiple agent runs -You can safely reuse the same `SearchableToolset` instance across multiple agent runs, including concurrent ones. The `Agent` internally gives each run an isolated copy of the toolset, so tools discovered in one run do not persist into, or collide with, other runs — every run starts fresh from the catalog: +You can safely reuse the same `SearchableToolset` instance across multiple agent runs, including concurrent ones. Each `Agent` run operates on an isolated, run-scoped copy of the toolset (created with [`spawn()`](toolset.mdx#run-scoped-copies-and-tool-selection)), so tools discovered in one run do not persist into, or collide with, other runs — every run starts fresh from the catalog: ```python agent = Agent( diff --git a/docs-website/docs/tools/toolset.mdx b/docs-website/docs/tools/toolset.mdx index a14194656d9..55667129325 100644 --- a/docs-website/docs/tools/toolset.mdx +++ b/docs-website/docs/tools/toolset.mdx @@ -85,11 +85,16 @@ agent = Agent( ) ``` -### Tool Selection at Runtime +### Run-Scoped Copies and Tool Selection -You can restrict an [`Agent`](../pipeline-components/agents-1/agent.mdx) to a subset of tools at runtime by passing tool names, for example `agent.run(tools=["tool_a", "tool_b"])`. This also works with a [`SearchableToolset`](searchabletoolset.mdx): search and lazy loading keep working over the selected subset. +A `Toolset` is never mutated in place during an [`Agent`](../pipeline-components/agents-1/agent.mdx) run. A `Toolset` with run-scoped state, such as a [`SearchableToolset`](searchabletoolset.mdx), is replaced in each run by an isolated, run-scoped copy created with the `spawn()` method. This makes concurrent runs that share the same `Toolset` instance safe: per-run state, such as a `SearchableToolset`'s discovered tools or active tool-name selection, cannot leak or collide across runs. A plain `Toolset` has no run-scoped state and is used as is; just avoid adding or removing tools while runs are in progress. -Runtime selection never modifies your configured `Toolset`, and neither do a `SearchableToolset`'s discoveries. You can safely share the same `Toolset` instance across runs, including concurrent ones; just avoid adding or removing tools while runs are in progress. +You can also restrict an `Agent` to a subset of tools at runtime by passing tool names, for example `agent.run(tools=["tool_a", "tool_b"])`. When the configured `Toolset` overrides `spawn()`, the selection is applied to the live (run-scoped) copy rather than flattening it into a static list, so dynamic behavior like a `SearchableToolset`'s search and lazy loading keeps working over the selected subset. + +Two methods support this and can be overridden when subclassing: + +- `get_selectable_tools()`: Returns every tool available for name-based selection. Override it if your subclass's iteration does not surface every selectable tool. +- `spawn()`: Returns the `Toolset` itself, which has no run-scoped state to isolate. Override it to return an isolated, run-scoped copy if your subclass holds run-scoped state. ## Usage diff --git a/haystack/components/agents/agent.py b/haystack/components/agents/agent.py index fc0967a375c..cd64b09a12d 100644 --- a/haystack/components/agents/agent.py +++ b/haystack/components/agents/agent.py @@ -568,12 +568,7 @@ def _register_prompt_variables(self) -> None: component.set_input_type(self, name=var_name, type=Any, default=None) def _warm_up_tools(self) -> None: - """ - Warm up the configured tools. - - Called on every warm_up() (and therefore every run), so tools added to a Toolset after the first run are - warmed too. Tools' warm_up() is expected to be idempotent, making repeated warming cheap. - """ + """Warm up the configured tools. Called on every warm_up(), so late-added tools are warmed too.""" warm_up_tools(tools=self.tools) def _warm_up_hooks(self) -> None: diff --git a/haystack/components/agents/utils.py b/haystack/components/agents/utils.py index 1c2816b138a..c9d991b3384 100644 --- a/haystack/components/agents/utils.py +++ b/haystack/components/agents/utils.py @@ -116,8 +116,7 @@ def _select_tools_by_name(configured_tools: ToolsType, names: list[str]) -> list (item, item.get_selectable_tools() if isinstance(item, Toolset) else [item]) for item in items ] valid_tool_names = {tool.name for _, selectable in selectable_per_item for tool in selectable} - # Emptiness is only detectable here: a dynamic Toolset may look empty before get_selectable_tools() has - # resolved its real catalog. + # A dynamic Toolset may look empty before its catalog is resolved, so emptiness is checked here. if not valid_tool_names: raise ValueError("No tools were configured for the Agent at initialization.") @@ -138,8 +137,7 @@ def _select_tools_by_name(configured_tools: ToolsType, names: list[str]) -> list # The selection is carried by the per-run copy, so the shared, configured Toolset is never mutated. selected.append(run_copy) else: - # Select from the same list validation used (get_selectable_tools), not from iteration: a dynamic - # toolset that doesn't override spawn() may not surface every selectable tool via __iter__. + # Select from the same list validation used: iteration may not surface every selectable tool. selected.extend(tool for tool in selectable if tool.name in matched) return selected diff --git a/haystack/tools/searchable_toolset.py b/haystack/tools/searchable_toolset.py index e1388667413..50681632336 100644 --- a/haystack/tools/searchable_toolset.py +++ b/haystack/tools/searchable_toolset.py @@ -201,9 +201,9 @@ def spawn(self, selected_tool_names: set[str] | None = None) -> "SearchableTools Return an isolated copy for a single run, carrying the given name selection. The copy shares the read-only catalog and BM25 index but gets fresh discovered tools and name selection, - plus a bootstrap search tool bound to the copy; the selection is fixed for the copy's lifetime and scopes - both iteration and search. This way concurrent runs sharing the same configured SearchableToolset don't - share discovered tools or collide on the active selection. + plus a bootstrap search tool bound to the copy; the selection scopes both iteration and search. This way + concurrent runs sharing the same configured SearchableToolset don't share discovered tools or collide on + the active selection. :param selected_tool_names: Optional catalog tool names this run is restricted to. None means no restriction. diff --git a/haystack/tools/toolset.py b/haystack/tools/toolset.py index 9839e1a643f..df3be268066 100644 --- a/haystack/tools/toolset.py +++ b/haystack/tools/toolset.py @@ -132,14 +132,12 @@ def get_selectable_tools(self) -> list[Tool]: def spawn(self, selected_tool_names: set[str] | None = None) -> "Toolset": # noqa: ARG002 """ - Return an isolated instance of this Toolset for a single run. - - A plain Toolset has no run-scoped state, so the default implementation returns `self` unchanged and - ignores `selected_tool_names` (the Agent materializes name selections itself in that case). Subclasses - with additional run-scoped state (e.g. SearchableToolset) should override this to return a copy that - shares the read-only state (its tools and any warmed-up resources) but gets fresh run-scoped state and - carries the selection, so concurrent runs that share the same configured Toolset don't corrupt each - other (for example, one run's discovered tools leaking into another). + Return this Toolset, or an isolated copy of it, for a single run. + + A plain Toolset has no run-scoped state, so the default implementation returns `self` and ignores the + selection (the Agent materializes it). Subclasses with run-scoped state (e.g. SearchableToolset) override + this to return a copy carrying the selection, so concurrent runs sharing the same configured Toolset + don't corrupt each other. :param selected_tool_names: Optional tool names this run is restricted to. None means no restriction. :returns: This Toolset, or a run-scoped copy of it. diff --git a/test/tools/test_toolset.py b/test/tools/test_toolset.py index f15e874daad..8c11578ab8a 100644 --- a/test/tools/test_toolset.py +++ b/test/tools/test_toolset.py @@ -389,8 +389,6 @@ class TestToolsetSpawn: """Tests for spawn(), the run-scoping hook.""" def test_spawn_returns_self_for_plain_toolset(self, add_tool, multiply_tool): - """A plain Toolset has no run-scoped state: spawn() returns the same instance, with or without a - selection (which it ignores, since the Agent materializes it), and never mutates the toolset.""" toolset = Toolset([add_tool, multiply_tool]) assert toolset.spawn() is toolset assert toolset.spawn(selected_tool_names={"add"}) is toolset From 907b9ea5f1ea5cf9558407f5dceb72327f22224d Mon Sep 17 00:00:00 2001 From: anakin87 Date: Fri, 7 Aug 2026 20:24:37 +0200 Subject: [PATCH 07/13] improve --- docs-website/docs/tools/toolset.mdx | 6 +++--- .../notes/simplify-toolset-102a9effbe4f188f.yaml | 15 +++++++-------- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/docs-website/docs/tools/toolset.mdx b/docs-website/docs/tools/toolset.mdx index 55667129325..9c5792dbaa0 100644 --- a/docs-website/docs/tools/toolset.mdx +++ b/docs-website/docs/tools/toolset.mdx @@ -77,7 +77,7 @@ math_toolset.add(multiply_numbers) ### Combining Toolsets -To use multiple Toolsets together, pass them as a list wherever tools are accepted. This keeps each Toolset intact, preserving its own lifecycle and serialization: +To use multiple Toolsets together, pass them as a list wherever tools are accepted: ```python agent = Agent( @@ -87,9 +87,9 @@ agent = Agent( ### Run-Scoped Copies and Tool Selection -A `Toolset` is never mutated in place during an [`Agent`](../pipeline-components/agents-1/agent.mdx) run. A `Toolset` with run-scoped state, such as a [`SearchableToolset`](searchabletoolset.mdx), is replaced in each run by an isolated, run-scoped copy created with the `spawn()` method. This makes concurrent runs that share the same `Toolset` instance safe: per-run state, such as a `SearchableToolset`'s discovered tools or active tool-name selection, cannot leak or collide across runs. A plain `Toolset` has no run-scoped state and is used as is; just avoid adding or removing tools while runs are in progress. +An [`Agent`](../pipeline-components/agents-1/agent.mdx) run never modifies your configured `Toolset`. A `Toolset` with per-run state, such as a [`SearchableToolset`](searchabletoolset.mdx), is copied for each run through its `spawn()` method, so concurrent runs cannot leak state (like discovered tools) into each other. A plain `Toolset` has no per-run state and is shared as is; just avoid adding or removing tools while runs are in progress. -You can also restrict an `Agent` to a subset of tools at runtime by passing tool names, for example `agent.run(tools=["tool_a", "tool_b"])`. When the configured `Toolset` overrides `spawn()`, the selection is applied to the live (run-scoped) copy rather than flattening it into a static list, so dynamic behavior like a `SearchableToolset`'s search and lazy loading keeps working over the selected subset. +You can also restrict an `Agent` to a subset of tools at runtime by passing tool names, for example `agent.run(tools=["tool_a", "tool_b"])`. The selection applies only to that run, and dynamic behavior like a `SearchableToolset`'s search keeps working over the selected subset. Two methods support this and can be overridden when subclassing: diff --git a/releasenotes/notes/simplify-toolset-102a9effbe4f188f.yaml b/releasenotes/notes/simplify-toolset-102a9effbe4f188f.yaml index ce519c2fa15..a5428d500d9 100644 --- a/releasenotes/notes/simplify-toolset-102a9effbe4f188f.yaml +++ b/releasenotes/notes/simplify-toolset-102a9effbe4f188f.yaml @@ -2,13 +2,12 @@ upgrade: - | Combining Toolsets with ``+`` or ``Toolset.add(toolset)`` is no longer supported. Pass Toolsets as a list - wherever tools are accepted instead: ``Agent(tools=[toolset_a, toolset_b])``. - - | - ``Toolset.spawn()`` has been removed. It was only needed internally: plain Toolsets are shared across runs, - and ``SearchableToolset`` handles its own per-run isolation. + wherever tools are accepted instead: ``Agent(tools=[toolset_a, toolset_b])``. Pipelines serialized with a + ``+``-combined Toolset cannot be loaded anymore (their YAML references the removed internal + ``_ToolsetWrapper`` class): recreate them with the list form and serialize them again. - | Haystack can call ``warm_up()`` on Tools and Toolsets more than once, for example before every run. - Previously ``Toolset`` silently absorbed the repeated calls with an internal flag; now every call reaches - your ``warm_up()``. If your custom Tool or Toolset does expensive work there (connecting to a server, - loading a model), make sure a second call does nothing: check whether the work is already done and return - early, for example ``if self._client is not None: return``. + Previously ``Toolset`` absorbed repeated calls with an internal ``_is_warmed_up`` flag; that flag is gone + and every call now reaches your ``warm_up()``. If your custom Tool or Toolset does expensive work there + (connecting to a server, loading a model), or relied on the ``_is_warmed_up`` attribute, guard with your + own state and return early, for example ``if self._client is not None: return``. From cb6094ec0f62213d44c5c513cc4a708e43392f34 Mon Sep 17 00:00:00 2001 From: anakin87 Date: Mon, 10 Aug 2026 10:46:32 +0200 Subject: [PATCH 08/13] simplify --- haystack/components/agents/utils.py | 21 ++++++++++++--------- haystack/tools/toolset.py | 4 ---- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/haystack/components/agents/utils.py b/haystack/components/agents/utils.py index c9d991b3384..92f7532800b 100644 --- a/haystack/components/agents/utils.py +++ b/haystack/components/agents/utils.py @@ -112,9 +112,12 @@ def _select_tools_by_name(configured_tools: ToolsType, names: list[str]) -> list [configured_tools] if isinstance(configured_tools, Toolset) else list(configured_tools) ) - selectable_per_item = [ - (item, item.get_selectable_tools() if isinstance(item, Toolset) else [item]) for item in items - ] + # Resolve the tools each item offers for selection + selectable_per_item: list[tuple[Tool | Toolset, list[Tool]]] = [] + for item in items: + selectable = item.get_selectable_tools() if isinstance(item, Toolset) else [item] + selectable_per_item.append((item, selectable)) + valid_tool_names = {tool.name for _, selectable in selectable_per_item for tool in selectable} # A dynamic Toolset may look empty before its catalog is resolved, so emptiness is checked here. if not valid_tool_names: @@ -131,13 +134,13 @@ def _select_tools_by_name(configured_tools: ToolsType, names: list[str]) -> list matched = requested_names & {tool.name for tool in selectable} if not matched: continue - if not isinstance(item, Toolset): - selected.append(item) - elif (run_copy := item.spawn(selected_tool_names=matched)) is not item: - # The selection is carried by the per-run copy, so the shared, configured Toolset is never mutated. - selected.append(run_copy) + spawned = item.spawn(selected_tool_names=matched) if isinstance(item, Toolset) else item + if spawned is not item: + # spawn() returned a per-run copy that already restricts itself to the selected tools. + selected.append(spawned) else: - # Select from the same list validation used: iteration may not surface every selectable tool. + # No per-run copy: extract the selected tools from `selectable`, the same list the names were + # validated against. Iterating a dynamic Toolset instead could silently miss some of them. selected.extend(tool for tool in selectable if tool.name in matched) return selected diff --git a/haystack/tools/toolset.py b/haystack/tools/toolset.py index df3be268066..6914e0243e9 100644 --- a/haystack/tools/toolset.py +++ b/haystack/tools/toolset.py @@ -88,10 +88,6 @@ def from_dict(cls, data): Toolset implements the collection interface (__iter__, __contains__, __len__, __getitem__), making it behave like a list of Tools. This makes it compatible with components that expect iterable tools, such as Agent or Haystack chat generators. - - To combine multiple Toolsets, pass them as a list wherever tools are accepted, e.g. - `Agent(tools=[toolset_a, toolset_b])`. This keeps each Toolset as a unit, preserving its lifecycle and - serialization. """ # Use field() with default_factory to initialize the list From 18c78e659f36faf6d566d92209a0d977de0b3ebc Mon Sep 17 00:00:00 2001 From: anakin87 Date: Mon, 10 Aug 2026 11:50:11 +0200 Subject: [PATCH 09/13] reduce diff --- haystack/tools/toolset.py | 11 ++--- test/components/agents/test_agent.py | 71 ++++++++++----------------- test/tools/test_searchable_toolset.py | 54 ++++++++++---------- 3 files changed, 58 insertions(+), 78 deletions(-) diff --git a/haystack/tools/toolset.py b/haystack/tools/toolset.py index 6914e0243e9..6bb61f705db 100644 --- a/haystack/tools/toolset.py +++ b/haystack/tools/toolset.py @@ -51,9 +51,8 @@ def subtract(a: Annotated[int, "first number"], b: Annotated[int, "second number OpenAPI URLs, MCP servers, or other resources. When implementing a custom Toolset subclass for dynamic tool loading: - - Load the tools in `warm_up()` and assign them to `self.tools`. Following the framework-wide `warm_up()` - convention, make it idempotent by guarding on your own state (e.g. `if self._client is not None: return`), - as it may be called before every run. + - Load the tools in `warm_up()` and assign them to `self.tools`. Since `warm_up()` may be called before + every run, make it idempotent by guarding on your own state (e.g. `if self._client is not None: return`). - Override `to_dict()` and `from_dict()` to serialize the endpoint descriptor (URL, server info) rather than the dynamically loaded Tool instances. @@ -198,9 +197,9 @@ def warm_up(self) -> None: self.tools = self.mcp_connection.fetch_tools() ``` - Following the framework-wide convention, this method may be called multiple times (e.g. before every - run): implementations are responsible for their own idempotence, guarding on their own state as in the - example above. The default implementation delegates to the tools' own idempotent `warm_up()`. + This method may be called multiple times (e.g. before every run): implementations are responsible for + their own idempotence, guarding on their own state as in the example above. The default implementation delegates + to the tools' own idempotent `warm_up()`. """ for tool in self.tools: if hasattr(tool, "warm_up"): diff --git a/test/components/agents/test_agent.py b/test/components/agents/test_agent.py index 67ecf45f84a..d5d119b5532 100644 --- a/test/components/agents/test_agent.py +++ b/test/components/agents/test_agent.py @@ -1790,15 +1790,12 @@ def make_tool(description: str) -> Tool: return agent def test_run_raises_on_duplicate_tool_names_across_toolsets(self): - # Duplicate names across combined sources are caught by the per-step validation, since no check at - # composition time can see the tools of lazily-loading Toolsets. agent = self._agent_with_duplicate_tool_names() with pytest.raises(ValueError, match="Duplicate tool names"): agent.run(messages=[ChatMessage.from_user("hi")]) @pytest.mark.asyncio async def test_run_async_raises_on_duplicate_tool_names_across_toolsets(self): - # The async step loop performs its own per-step validation, on a separate code path from the sync one. agent = self._agent_with_duplicate_tool_names() with pytest.raises(ValueError, match="Duplicate tool names"): await agent.run_async(messages=[ChatMessage.from_user("hi")]) @@ -2233,33 +2230,6 @@ def test_warm_up_toolset(self): agent.warm_up() assert toolset.was_warmed_up - def test_warm_up_covers_empty_lazy_toolset(self): - # A Toolset that only loads its tools on warm_up() has len 0 before warming: the Agent must warm it - # anyway (a truthiness check on the toolset would skip it and permanently mark tools as warmed). - class LazyToolset(Toolset): - def __init__(self): - self.loaded = False - super().__init__(tools=[]) - - def warm_up(self): - if self.loaded: - return - self.loaded = True - self.tools = [ - Tool( - name="lazy_tool", - description="d", - parameters={"type": "object", "properties": {}}, - function=lambda: None, - ) - ] - - toolset = LazyToolset() - agent = Agent(chat_generator=MockChatGenerator("Hello"), tools=toolset) - agent.warm_up() - assert toolset.loaded - assert [tool.name for tool in toolset] == ["lazy_tool"] - def test_warm_up_list_of_toolsets(self): # Toolsets are combined by passing them as a list; each keeps its own warm_up. tool1 = self._make_tracking_tool("tool1") @@ -2296,7 +2266,6 @@ def test_warm_up_mixed_list_of_tools_and_toolsets(self): assert toolset2.was_warmed_up def test_warm_up_rewarms_tools_on_every_call(self): - """Tools are warmed on every warm_up() so tools added to a Toolset after the first run get warmed too.""" call_count = {"n": 0} tool = Tool( name="counting_tool", @@ -2319,14 +2288,26 @@ def counting_warm_up(): assert call_count["n"] == 3 - def test_warm_up_refreshes_toolset(self): - """Agent.warm_up() must warm up lazy toolsets (e.g. MCPToolset) so the actual tools are available at runtime.""" - placeholder_tool = Tool( - name="mcp_not_connected_placeholder_123", - description="Placeholder tool before connection", - parameters={"type": "object", "properties": {}}, - function=lambda: "placeholder", - ) + @pytest.mark.parametrize( + "initial_tools", + [ + pytest.param([], id="empty"), + pytest.param( + [ + Tool( + name="mcp_not_connected_placeholder_123", + description="Placeholder tool before connection", + parameters={"type": "object", "properties": {}}, + function=lambda: "placeholder", + ) + ], + id="placeholder", + ), + ], + ) + def test_warm_up_loads_lazy_toolset(self, initial_tools): + # A lazy toolset (e.g. MCPToolset) loads its real tools on warm_up(). Before that it may be empty + # (a truthiness check would skip it) or expose a placeholder: warm_up must load the real tools either way. actual_tool = Tool( name="get_time", description="Get the current time in ISO format", @@ -2334,21 +2315,21 @@ def test_warm_up_refreshes_toolset(self): function=lambda: "2024-12-01T12:00:00Z", ) - class MockMCPToolset(Toolset): + class LazyToolset(Toolset): def __init__(self): - super().__init__([placeholder_tool]) self._connected = False + super().__init__(list(initial_tools)) def warm_up(self): if not self._connected: self.tools = [actual_tool] self._connected = True - mcp_toolset = MockMCPToolset() - agent = Agent(chat_generator=MockChatGenerator("Hello"), tools=mcp_toolset) - assert mcp_toolset.tools == [placeholder_tool] + toolset = LazyToolset() + agent = Agent(chat_generator=MockChatGenerator("Hello"), tools=toolset) + assert toolset.tools == initial_tools agent.warm_up() - assert mcp_toolset.tools == [actual_tool] + assert toolset.tools == [actual_tool] def test_run_warms_lazy_toolset_before_tool_selection(self): """ diff --git a/test/tools/test_searchable_toolset.py b/test/tools/test_searchable_toolset.py index 0a3007af0f7..fd9a9a2ead9 100644 --- a/test/tools/test_searchable_toolset.py +++ b/test/tools/test_searchable_toolset.py @@ -826,51 +826,51 @@ def test_get_selectable_tools_exposes_full_catalog(self, large_catalog): assert {tool.name for tool in toolset.get_selectable_tools()} == {tool.name for tool in large_catalog} def test_runtime_tool_names_select_isolated_spawn_and_preserve_search(self, large_catalog, monkeypatch): - """Selecting catalog tool names returns an isolated spawn carrying the selection, with search active.""" + """Selecting catalog tool names returns an isolated spawn carrying the selection and keeping search active.""" monkeypatch.setenv("OPENAI_API_KEY", "fake-key") toolset = SearchableToolset(catalog=large_catalog, search_threshold=3) # 8 tools -> search mode agent = Agent(chat_generator=OpenAIChatGenerator(), tools=toolset) selected = agent._select_tools(["get_weather", "add_numbers"]) - # An isolated per-run copy is returned with the selection; the configured toolset is not mutated. + # An isolated spawn is returned with the selection; the configured toolset is not mutated. assert len(selected) == 1 - run_copy = selected[0] - assert isinstance(run_copy, SearchableToolset) - assert run_copy is not toolset - assert run_copy._selected_tool_names == {"get_weather", "add_numbers"} + spawned = selected[0] + assert isinstance(spawned, SearchableToolset) + assert spawned is not toolset + assert spawned._selected_tool_names == {"get_weather", "add_numbers"} assert toolset._selected_tool_names is None - # Search is preserved on the copy (not dismantled): only the bootstrap tool is exposed up front. - assert [tool.name for tool in run_copy] == ["search_tools"] + # Search is preserved on the spawn (not dismantled): only the bootstrap tool is exposed up front. + assert [tool.name for tool in spawned] == ["search_tools"] # And search only discovers tools within the selected subset. - assert run_copy._bootstrap_tool is not None - run_copy._bootstrap_tool.invoke(tool_keywords="weather add stock multiply") - assert set(run_copy._discovered_tools) <= {"get_weather", "add_numbers"} + assert spawned._bootstrap_tool is not None + spawned._bootstrap_tool.invoke(tool_keywords="weather add stock multiply") + assert set(spawned._discovered_tools) <= {"get_weather", "add_numbers"} # The configured toolset's discovered tools are untouched. assert toolset._discovered_tools == {} def test_spawns_have_independent_discovered_tools_and_selection(self, large_catalog): - """Spawns of one SearchableToolset don't share discovered tools or collide on the selection.""" + """Two spawns of one SearchableToolset don't share discovered tools or collide on the active selection.""" toolset = SearchableToolset(catalog=large_catalog, search_threshold=3) toolset.warm_up() - copy_a = toolset.spawn(selected_tool_names={"get_weather"}) - copy_b = toolset.spawn() + spawn_a = toolset.spawn(selected_tool_names={"get_weather"}) + spawn_b = toolset.spawn() - assert copy_a is not copy_b - assert copy_a is not toolset - # Bootstrap tools are rebound per copy (not shared with the original or each other). - assert copy_a._bootstrap_tool is not None - assert copy_a._bootstrap_tool is not copy_b._bootstrap_tool + assert spawn_a is not spawn_b + assert spawn_a is not toolset + # Bootstrap tools are rebound per spawn (not shared with the original or each other). + assert spawn_a._bootstrap_tool is not None + assert spawn_a._bootstrap_tool is not spawn_b._bootstrap_tool - copy_a._bootstrap_tool.invoke(tool_keywords="weather add stock multiply") + spawn_a._bootstrap_tool.invoke(tool_keywords="weather add stock multiply") - # Discovery on copy_a does not leak into copy_b or the configured toolset. - assert set(copy_a._discovered_tools) <= {"get_weather"} - assert copy_b._discovered_tools == {} + # Discovery on spawn_a does not leak into spawn_b or the configured toolset. + assert set(spawn_a._discovered_tools) <= {"get_weather"} + assert spawn_b._discovered_tools == {} assert toolset._discovered_tools == {} # The selection is likewise isolated. - assert copy_b._selected_tool_names is None + assert spawn_b._selected_tool_names is None assert toolset._selected_tool_names is None def test_runtime_tool_names_passthrough_exposes_selected(self, large_catalog, monkeypatch): @@ -885,7 +885,7 @@ def test_runtime_tool_names_passthrough_exposes_selected(self, large_catalog, mo assert {tool.name for tool in flatten_tools_or_toolsets(selected)} == {"get_weather", "add_numbers"} def test_agent_run_with_runtime_tool_names(self, large_catalog): - """An Agent with a SearchableToolset runs with catalog tools selected by name on an isolated per-run copy.""" + """An Agent with a SearchableToolset runs with specific catalog tools selected by name on an isolated spawn.""" toolset = SearchableToolset(catalog=large_catalog, search_threshold=20) # passthrough exposes the selection @component @@ -911,7 +911,7 @@ def run(self, messages, tools=None, **kwargs): result = agent.run(messages=[ChatMessage.from_user("What's the weather in Berlin?")], tools=["get_weather"]) assert result["tool_call_counts"]["get_weather"] == 1 - # The Agent runs against an isolated per-run copy, so the configured toolset's selection never gets set. + # The Agent runs against an isolated spawn, so the configured toolset's selection never gets set. assert toolset._selected_tool_names is None def test_discovered_tool_call_counts_added_lazily(self, large_catalog): @@ -936,7 +936,7 @@ def test_discovered_tool_call_counts_added_lazily(self, large_catalog): # search_tools is seeded at init; get_weather is only counted after being discovered and called. assert counts["search_tools"] == 1 assert counts["get_weather"] == 1 - # Discovery happens on an isolated per-run copy: the configured toolset's discovered tools stay empty. + # The Agent discovers tools on an isolated spawn, so the configured toolset's discovered tools stay empty. assert toolset._discovered_tools == {} From baf99d06ebe48575a5fa79af63152c9e7e377f5f Mon Sep 17 00:00:00 2001 From: anakin87 Date: Mon, 10 Aug 2026 12:12:41 +0200 Subject: [PATCH 10/13] rm some comments --- test/tools/test_toolset.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/test/tools/test_toolset.py b/test/tools/test_toolset.py index 8c11578ab8a..937752ba7f1 100644 --- a/test/tools/test_toolset.py +++ b/test/tools/test_toolset.py @@ -168,7 +168,6 @@ def test_toolset_contains(self, add_tool, multiply_tool): assert "non_existent_tool" not in toolset def test_combining_toolsets_via_unpacking(self, add_tool, multiply_tool, subtract_tool): - """A Toolset can be built from the tools of existing ones; the combined tools remain invocable.""" combined = Toolset([*Toolset([add_tool, subtract_tool]), multiply_tool]) assert [t.name for t in combined] == ["add", "subtract", "multiply"] @@ -357,8 +356,6 @@ def test_warm_up_warms_all_tools(self): assert t2.warm_up_count == 1 def test_warm_up_can_be_called_multiple_times(self): - # Per the framework-wide convention, warm_up() may be called before every run; the Toolset delegates to - # the tools' own warm_up(), which are responsible for their own idempotence. t1 = WarmUpCountingTool("a") toolset = Toolset([t1]) toolset.warm_up() @@ -367,7 +364,6 @@ def test_warm_up_can_be_called_multiple_times(self): assert t1.warm_up_count == 3 def test_add_never_warms_the_new_tool(self): - # add() is stateless: it never warms the added tool; the next warm_up() call warms whatever is present. existing = WarmUpCountingTool("a") toolset = Toolset([existing]) toolset.warm_up() @@ -378,7 +374,6 @@ def test_add_never_warms_the_new_tool(self): assert new_tool.warm_up_count == 1 def test_add_toolset_raises(self): - # add() accepts only Tools: to combine Toolsets, pass them as a list, e.g. Agent(tools=[ts_a, ts_b]). toolset = Toolset([WarmUpCountingTool("a")]) not_a_tool: Any = Toolset([WarmUpCountingTool("b")]) with pytest.raises(TypeError, match="Expected Tool"): From 93a188891d767186fa2119ad4ccbd5a93a32a7c9 Mon Sep 17 00:00:00 2001 From: anakin87 Date: Mon, 10 Aug 2026 12:18:04 +0200 Subject: [PATCH 11/13] improve release note --- .../notes/simplify-toolset-102a9effbe4f188f.yaml | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/releasenotes/notes/simplify-toolset-102a9effbe4f188f.yaml b/releasenotes/notes/simplify-toolset-102a9effbe4f188f.yaml index a5428d500d9..25f520eb569 100644 --- a/releasenotes/notes/simplify-toolset-102a9effbe4f188f.yaml +++ b/releasenotes/notes/simplify-toolset-102a9effbe4f188f.yaml @@ -1,10 +1,13 @@ --- upgrade: - | - Combining Toolsets with ``+`` or ``Toolset.add(toolset)`` is no longer supported. Pass Toolsets as a list - wherever tools are accepted instead: ``Agent(tools=[toolset_a, toolset_b])``. Pipelines serialized with a - ``+``-combined Toolset cannot be loaded anymore (their YAML references the removed internal - ``_ToolsetWrapper`` class): recreate them with the list form and serialize them again. + The ``+`` operator for combining Toolsets has been removed. Pass Toolsets as a list wherever tools are + accepted instead: ``Agent(tools=[toolset_a, toolset_b])``. Pipelines serialized with a ``+``-combined + Toolset cannot be loaded anymore (their YAML references the removed internal ``_ToolsetWrapper`` class): + recreate them with the list form and serialize them again. + - | + ``Toolset.add()`` now only accepts a single Tool, not another Toolset. To combine Toolsets, pass them as + a list: ``Agent(tools=[toolset_a, toolset_b])``. - | Haystack can call ``warm_up()`` on Tools and Toolsets more than once, for example before every run. Previously ``Toolset`` absorbed repeated calls with an internal ``_is_warmed_up`` flag; that flag is gone From 4eedc4c09d769cadd0e4640d49647444bba42bb8 Mon Sep 17 00:00:00 2001 From: anakin87 Date: Mon, 10 Aug 2026 14:59:35 +0200 Subject: [PATCH 12/13] rm unnecessary method --- haystack/components/agents/agent.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/haystack/components/agents/agent.py b/haystack/components/agents/agent.py index cd64b09a12d..f883f7953ca 100644 --- a/haystack/components/agents/agent.py +++ b/haystack/components/agents/agent.py @@ -567,10 +567,6 @@ def _register_prompt_variables(self) -> None: else: component.set_input_type(self, name=var_name, type=Any, default=None) - def _warm_up_tools(self) -> None: - """Warm up the configured tools. Called on every warm_up(), so late-added tools are warmed too.""" - warm_up_tools(tools=self.tools) - def _warm_up_hooks(self) -> None: """Warm up the configured hooks once.""" if not self._hooks_warmed_up: @@ -585,14 +581,14 @@ async def _warm_up_hooks_async(self) -> None: def warm_up(self) -> None: """Warm up the tools, hooks, and the underlying chat generator.""" - self._warm_up_tools() + warm_up_tools(tools=self.tools) self._warm_up_hooks() if hasattr(self.chat_generator, "warm_up"): self.chat_generator.warm_up() async def warm_up_async(self) -> None: """Warm up the tools, hooks, and the underlying chat generator on the serving event loop.""" - self._warm_up_tools() + warm_up_tools(tools=self.tools) await self._warm_up_hooks_async() if hasattr(self.chat_generator, "warm_up_async"): await self.chat_generator.warm_up_async() From 3bc0b51fa63c6bbb3f7ff93431e8f0ee7d5710b5 Mon Sep 17 00:00:00 2001 From: anakin87 Date: Tue, 11 Aug 2026 10:18:20 +0200 Subject: [PATCH 13/13] clarifu --- haystack/components/agents/utils.py | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/haystack/components/agents/utils.py b/haystack/components/agents/utils.py index 92f7532800b..dba07b5de73 100644 --- a/haystack/components/agents/utils.py +++ b/haystack/components/agents/utils.py @@ -90,6 +90,24 @@ def _record_tool_calls(state: State, tool_messages: list[ChatMessage]) -> None: # --------------------------- +def _spawn_selection_copy(item: Tool | Toolset, selected_tool_names: set[str]) -> Toolset | None: + """ + Return the per-run copy carrying the selection, or None if the item does not provide one. + + A Toolset with run-scoped state (e.g. SearchableToolset) overrides `spawn()` to return a copy that + applies `selected_tool_names` itself. A plain Toolset returns itself from `spawn()` (it has nothing + to isolate), and a standalone Tool has no `spawn()`: in both cases the caller applies the selection. + + :param item: A configured Tool or Toolset. + :param selected_tool_names: The tool names selected for this run. + :returns: The selection-carrying per-run copy, or None. + """ + if not isinstance(item, Toolset): + return None + spawned = item.spawn(selected_tool_names=selected_tool_names) + return spawned if spawned is not item else None + + def _select_tools_by_name(configured_tools: ToolsType, names: list[str]) -> list[Tool | Toolset]: """ Select configured tools by name for a single run. @@ -134,13 +152,12 @@ def _select_tools_by_name(configured_tools: ToolsType, names: list[str]) -> list matched = requested_names & {tool.name for tool in selectable} if not matched: continue - spawned = item.spawn(selected_tool_names=matched) if isinstance(item, Toolset) else item - if spawned is not item: - # spawn() returned a per-run copy that already restricts itself to the selected tools. - selected.append(spawned) + run_copy = _spawn_selection_copy(item, matched) + if run_copy is not None: + selected.append(run_copy) else: - # No per-run copy: extract the selected tools from `selectable`, the same list the names were - # validated against. Iterating a dynamic Toolset instead could silently miss some of them. + # Select from `selectable`, the list the names were validated against: iterating a dynamic + # Toolset could silently miss tools. selected.extend(tool for tool in selectable if tool.name in matched) return selected