From c4e0ae55348d4e997b648fd7e9b7601c8601b767 Mon Sep 17 00:00:00 2001 From: HamidOna13 Date: Sat, 1 Nov 2025 02:37:13 +0000 Subject: [PATCH 1/3] fix: warm up individual tools inside Toolsets in warm_up_tools() Related Issues: * Follows up on PR #9942 (feat: Add warm_up() method to ChatGenerators) * Addresses bug discovered during implementation of PR #9942 for issue #9907 Proposed Changes: The warm_up_tools() utility function was only calling warm_up() on Toolset objects themselves, but not on the individual Tool instances contained within them. This meant tools inside a Toolset were not properly initialized before use. This PR modifies warm_up_tools() to iterate through Toolsets and call warm_up() on each individual tool, in addition to calling warm_up() on the Toolset itself. Changes: - Modified warm_up_tools() in haystack/tools/utils.py to iterate through Toolsets when encountered (both as single argument and within lists) - Added iteration to call warm_up() on each individual Tool inside Toolsets - Added comprehensive test class TestWarmUpTools with 7 test cases How did you test it: - Added 7 comprehensive unit tests in test/tools/test_tools_utils.py: * test_warm_up_tools_with_none - handles None input * test_warm_up_tools_with_single_tool - single tool in list * test_warm_up_tools_with_single_toolset - KEY TEST: verifies both Toolset and individual tools are warmed * test_warm_up_tools_with_list_containing_toolset - toolset within list * test_warm_up_tools_with_multiple_toolsets - multiple toolsets * test_warm_up_tools_with_mixed_tools_and_toolsets - mixed scenarios * test_warm_up_tools_idempotency - safe to call multiple times Notes for the reviewer: I discovered this bug while implementing PR #9942 (for issue #9907). When a Toolset object is passed to a component's tools parameter, the warm_up_tools() function only calls Toolset.warm_up(), which is a no-op. It doesn't iterate through the individual tools inside the Toolset to warm them up. acknowledged by @vblagoje and @sjrl This implementation: - Modified warm_up_tools() to iterate through Toolsets and call warm_up() on each individual tool - Added comprehensive tests for Toolset warming behavior - Verified both the Toolset and its contained tools are warmed up Checklist: I have read the contributors guidelines and the code of conduct I have updated the related issue with new insights and changes I added unit tests and updated the docstrings I've used one of the conventional commit types for my PR title: fix: I documented my code I ran pre-commit hooks and fixed any issue --- haystack/tools/utils.py | 14 ++- test/tools/test_tools_utils.py | 221 ++++++++++++++++++++++++++++++++- 2 files changed, 233 insertions(+), 2 deletions(-) diff --git a/haystack/tools/utils.py b/haystack/tools/utils.py index c2dd4d82a9..7108a7f320 100644 --- a/haystack/tools/utils.py +++ b/haystack/tools/utils.py @@ -24,12 +24,24 @@ def warm_up_tools(tools: "Optional[ToolsType]" = None) -> None: if isinstance(tools, Toolset): if hasattr(tools, "warm_up"): tools.warm_up() + # Also warm up individual tools inside the toolset + for tool in tools: + if hasattr(tool, "warm_up"): + tool.warm_up() return # If tools is a list, warm up each item (Tool or Toolset) if isinstance(tools, list): for item in tools: - if isinstance(item, (Toolset, Tool)) and hasattr(item, "warm_up"): + if isinstance(item, Toolset): + # Warm up the toolset itself + if hasattr(item, "warm_up"): + item.warm_up() + # Also warm up individual tools inside the toolset + for tool in item: + if hasattr(tool, "warm_up"): + tool.warm_up() + elif isinstance(item, Tool) and hasattr(item, "warm_up"): item.warm_up() diff --git a/test/tools/test_tools_utils.py b/test/tools/test_tools_utils.py index 477719b72a..7aa7eaa00f 100644 --- a/test/tools/test_tools_utils.py +++ b/test/tools/test_tools_utils.py @@ -4,7 +4,7 @@ import pytest -from haystack.tools import Tool, Toolset, flatten_tools_or_toolsets +from haystack.tools import Tool, Toolset, flatten_tools_or_toolsets, warm_up_tools def add_numbers(a: int, b: int) -> int: @@ -171,3 +171,222 @@ def test_flatten_multiple_toolsets(self, add_tool, multiply_tool, subtract_tool) assert result[0].name == "add" assert result[1].name == "multiply" assert result[2].name == "subtract" + + +class WarmupTrackingTool(Tool): + """A tool that tracks whether warm_up was called.""" + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.was_warmed_up = False + + def warm_up(self): + self.was_warmed_up = True + + +class WarmupTrackingToolset(Toolset): + """A toolset that tracks whether warm_up was called.""" + + def __init__(self, tools): + super().__init__(tools) + self.was_warmed_up = False + + def warm_up(self): + self.was_warmed_up = True + + +class TestWarmUpTools: + """Tests for the warm_up_tools() function""" + + def test_warm_up_tools_with_none(self): + """Test that warm_up_tools with None does nothing.""" + # Should not raise any errors + warm_up_tools(None) + + def test_warm_up_tools_with_single_tool(self): + """Test that warm_up_tools works with a single tool in a list.""" + tool = WarmupTrackingTool( + name="test_tool", + description="A test tool", + parameters={"type": "object", "properties": {}}, + function=lambda: "test", + ) + + assert not tool.was_warmed_up + warm_up_tools([tool]) + assert tool.was_warmed_up + + def test_warm_up_tools_with_single_toolset(self): + """ + Test that when passing a single Toolset, both the Toolset.warm_up() + and each individual tool's warm_up() are called. + """ + tool1 = WarmupTrackingTool( + name="tool1", + description="First tool", + parameters={"type": "object", "properties": {}}, + function=lambda: "tool1", + ) + tool2 = WarmupTrackingTool( + name="tool2", + description="Second tool", + parameters={"type": "object", "properties": {}}, + function=lambda: "tool2", + ) + + toolset = WarmupTrackingToolset([tool1, tool2]) + + assert not toolset.was_warmed_up + assert not tool1.was_warmed_up + assert not tool2.was_warmed_up + + warm_up_tools(toolset) + + # Both the toolset itself and individual tools should be warmed up + assert toolset.was_warmed_up + assert tool1.was_warmed_up + assert tool2.was_warmed_up + + def test_warm_up_tools_with_list_containing_toolset(self): + """Test that when a Toolset is in a list, individual tools inside get warmed up.""" + tool1 = WarmupTrackingTool( + name="tool1", + description="First tool", + parameters={"type": "object", "properties": {}}, + function=lambda: "tool1", + ) + tool2 = WarmupTrackingTool( + name="tool2", + description="Second tool", + parameters={"type": "object", "properties": {}}, + function=lambda: "tool2", + ) + + toolset = WarmupTrackingToolset([tool1, tool2]) + + assert not toolset.was_warmed_up + assert not tool1.was_warmed_up + assert not tool2.was_warmed_up + + warm_up_tools([toolset]) + + # Both the toolset itself and individual tools should be warmed up + assert toolset.was_warmed_up + assert tool1.was_warmed_up + assert tool2.was_warmed_up + + def test_warm_up_tools_with_multiple_toolsets(self): + """Test multiple Toolsets in a list.""" + tool1 = WarmupTrackingTool( + name="tool1", + description="First tool", + parameters={"type": "object", "properties": {}}, + function=lambda: "tool1", + ) + tool2 = WarmupTrackingTool( + name="tool2", + description="Second tool", + parameters={"type": "object", "properties": {}}, + function=lambda: "tool2", + ) + tool3 = WarmupTrackingTool( + name="tool3", + description="Third tool", + parameters={"type": "object", "properties": {}}, + function=lambda: "tool3", + ) + + toolset1 = WarmupTrackingToolset([tool1]) + toolset2 = WarmupTrackingToolset([tool2, tool3]) + + assert not toolset1.was_warmed_up + assert not toolset2.was_warmed_up + assert not tool1.was_warmed_up + assert not tool2.was_warmed_up + assert not tool3.was_warmed_up + + warm_up_tools([toolset1, toolset2]) + + # Both toolsets and all individual tools should be warmed up + assert toolset1.was_warmed_up + assert toolset2.was_warmed_up + assert tool1.was_warmed_up + assert tool2.was_warmed_up + assert tool3.was_warmed_up + + def test_warm_up_tools_with_mixed_tools_and_toolsets(self): + """Test list with both Tool objects and Toolsets.""" + standalone_tool = WarmupTrackingTool( + name="standalone", + description="Standalone tool", + parameters={"type": "object", "properties": {}}, + function=lambda: "standalone", + ) + toolset_tool1 = WarmupTrackingTool( + name="toolset_tool1", + description="Tool in toolset", + parameters={"type": "object", "properties": {}}, + function=lambda: "toolset_tool1", + ) + toolset_tool2 = WarmupTrackingTool( + name="toolset_tool2", + description="Another tool in toolset", + parameters={"type": "object", "properties": {}}, + function=lambda: "toolset_tool2", + ) + + toolset = WarmupTrackingToolset([toolset_tool1, toolset_tool2]) + + assert not standalone_tool.was_warmed_up + assert not toolset.was_warmed_up + assert not toolset_tool1.was_warmed_up + assert not toolset_tool2.was_warmed_up + + warm_up_tools([standalone_tool, toolset]) + + # All tools and the toolset should be warmed up + assert standalone_tool.was_warmed_up + assert toolset.was_warmed_up + assert toolset_tool1.was_warmed_up + assert toolset_tool2.was_warmed_up + + def test_warm_up_tools_idempotency(self): + """Test that calling warm_up_tools() multiple times is safe.""" + + class WarmupCountingTool(Tool): + """A tool that counts how many times warm_up was called.""" + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.warm_up_count = 0 + + def warm_up(self): + self.warm_up_count += 1 + + class WarmupCountingToolset(Toolset): + """A toolset that counts how many times warm_up was called.""" + + def __init__(self, tools): + super().__init__(tools) + self.warm_up_count = 0 + + def warm_up(self): + self.warm_up_count += 1 + + tool = WarmupCountingTool( + name="counting_tool", + description="A counting tool", + parameters={"type": "object", "properties": {}}, + function=lambda: "test", + ) + toolset = WarmupCountingToolset([tool]) + + # Call warm_up_tools multiple times + warm_up_tools(toolset) + warm_up_tools(toolset) + warm_up_tools(toolset) + + # warm_up_tools itself doesn't prevent multiple calls, + # but verify the calls actually happen multiple times + assert toolset.warm_up_count == 3 + assert tool.warm_up_count == 3 From afca1f2839104755ad2fb7c18129f94fdd0445bd Mon Sep 17 00:00:00 2001 From: HamidOna13 Date: Sat, 1 Nov 2025 03:00:58 +0000 Subject: [PATCH 2/3] added release note --- .../notes/fix-toolset-warm-up-3e985e6a5e95a55d.yaml | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 releasenotes/notes/fix-toolset-warm-up-3e985e6a5e95a55d.yaml diff --git a/releasenotes/notes/fix-toolset-warm-up-3e985e6a5e95a55d.yaml b/releasenotes/notes/fix-toolset-warm-up-3e985e6a5e95a55d.yaml new file mode 100644 index 0000000000..198c83a265 --- /dev/null +++ b/releasenotes/notes/fix-toolset-warm-up-3e985e6a5e95a55d.yaml @@ -0,0 +1,7 @@ +fixes: + - | + Fixed warm_up_tools() utility function to properly warm up individual + tools inside Toolset objects. Previously, only the Toolset's warm_up() + method was called (which is a no-op), leaving individual tools + uninitialized. Now both the Toolset and its contained tools are + properly warmed up before pipeline execution." From b52d4848bcabb0af17558d43878d3fa8f6e2c3cf Mon Sep 17 00:00:00 2001 From: HamidOna13 Date: Tue, 4 Nov 2025 19:28:15 +0000 Subject: [PATCH 3/3] refactor: move tool warm-up iteration to Toolset.warm_up() Addresses architectural feedback - moved iteration logic from warm_up_tools() to base Toolset.warm_up() for better encapsulation. Subclasses can now override warm_up() to customize initialization without breaking the contract. - Toolset.warm_up() now iterates and warms tools by default - warm_up_tools() simplified to delegate to warm_up() - Updated tests and release notes --- haystack/tools/toolset.py | 24 +++++++++++++++++-- haystack/tools/utils.py | 22 ++++++----------- .../fix-toolset-warm-up-3e985e6a5e95a55d.yaml | 12 +++++----- test/components/tools/test_tool_invoker.py | 2 ++ test/tools/test_tools_utils.py | 3 +++ 5 files changed, 40 insertions(+), 23 deletions(-) diff --git a/haystack/tools/toolset.py b/haystack/tools/toolset.py index d9f7617e64..cab65aaff2 100644 --- a/haystack/tools/toolset.py +++ b/haystack/tools/toolset.py @@ -189,10 +189,30 @@ def warm_up(self) -> None: """ Prepare the Toolset for use. - Override this method to set up shared resources like database connections or HTTP sessions. + 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 + - Implementing custom initialization logic for dynamically loaded 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 rather than warming up + individual tools: + + ```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) + ``` + This method should be idempotent, as it may be called multiple times. """ - pass + for tool in self.tools: + if hasattr(tool, "warm_up"): + tool.warm_up() def add(self, tool: Union[Tool, "Toolset"]) -> None: """ diff --git a/haystack/tools/utils.py b/haystack/tools/utils.py index 7108a7f320..d708f45735 100644 --- a/haystack/tools/utils.py +++ b/haystack/tools/utils.py @@ -15,33 +15,25 @@ def warm_up_tools(tools: "Optional[ToolsType]" = None) -> None: """ Warm up tools from various formats (Tools, Toolsets, or mixed lists). + For Toolset objects, this delegates to Toolset.warm_up(), which by default + warms up all tools in the Toolset. Toolset subclasses can override warm_up() + to customize initialization behavior (e.g., setting up shared resources). + :param tools: A list of Tool and/or Toolset objects, a single Toolset, or None. """ if tools is None: return - # If tools is a single Toolset, warm up the toolset itself - if isinstance(tools, Toolset): + # If tools is a single Toolset or Tool, warm it up + if isinstance(tools, (Toolset, Tool)): if hasattr(tools, "warm_up"): tools.warm_up() - # Also warm up individual tools inside the toolset - for tool in tools: - if hasattr(tool, "warm_up"): - tool.warm_up() return # If tools is a list, warm up each item (Tool or Toolset) if isinstance(tools, list): for item in tools: - if isinstance(item, Toolset): - # Warm up the toolset itself - if hasattr(item, "warm_up"): - item.warm_up() - # Also warm up individual tools inside the toolset - for tool in item: - if hasattr(tool, "warm_up"): - tool.warm_up() - elif isinstance(item, Tool) and hasattr(item, "warm_up"): + if hasattr(item, "warm_up"): item.warm_up() diff --git a/releasenotes/notes/fix-toolset-warm-up-3e985e6a5e95a55d.yaml b/releasenotes/notes/fix-toolset-warm-up-3e985e6a5e95a55d.yaml index 198c83a265..1b58492d2e 100644 --- a/releasenotes/notes/fix-toolset-warm-up-3e985e6a5e95a55d.yaml +++ b/releasenotes/notes/fix-toolset-warm-up-3e985e6a5e95a55d.yaml @@ -1,7 +1,7 @@ -fixes: +enhancements: - | - Fixed warm_up_tools() utility function to properly warm up individual - tools inside Toolset objects. Previously, only the Toolset's warm_up() - method was called (which is a no-op), leaving individual tools - uninitialized. Now both the Toolset and its contained tools are - properly warmed up before pipeline execution." + Improved Toolset warm-up architecture for better encapsulation. The base + Toolset.warm_up() method now warms up all tools by default, while subclasses + can override it to customize initialization (e.g., setting up shared resources + instead of warming individual tools). The warm_up_tools() utility function has + been simplified to delegate to Toolset.warm_up(). diff --git a/test/components/tools/test_tool_invoker.py b/test/components/tools/test_tool_invoker.py index 5bd917211c..c3c1c329e8 100644 --- a/test/components/tools/test_tool_invoker.py +++ b/test/components/tools/test_tool_invoker.py @@ -136,6 +136,8 @@ def __init__(self, tools): def warm_up(self): self.was_warmed_up = True + # Call parent to warm up individual tools + super().warm_up() class TestToolInvokerCore: diff --git a/test/tools/test_tools_utils.py b/test/tools/test_tools_utils.py index 7aa7eaa00f..4a016e8e4d 100644 --- a/test/tools/test_tools_utils.py +++ b/test/tools/test_tools_utils.py @@ -193,6 +193,8 @@ def __init__(self, tools): def warm_up(self): self.was_warmed_up = True + # Call parent to warm up individual tools + super().warm_up() class TestWarmUpTools: @@ -372,6 +374,7 @@ def __init__(self, tools): def warm_up(self): self.warm_up_count += 1 + super().warm_up() # Also warm up individual tools tool = WarmupCountingTool( name="counting_tool",