From d7a852b6cb4a15031162c824c60d6572f8cfa02b Mon Sep 17 00:00:00 2001 From: Twisha Bansal Date: Thu, 27 Mar 2025 18:41:48 +0530 Subject: [PATCH 01/13] initial bound params code --- .../toolbox-core/src/toolbox_core/tool.py | 217 ++++++++++++++++-- 1 file changed, 199 insertions(+), 18 deletions(-) diff --git a/packages/toolbox-core/src/toolbox_core/tool.py b/packages/toolbox-core/src/toolbox_core/tool.py index 48e4626c..c5d87010 100644 --- a/packages/toolbox-core/src/toolbox_core/tool.py +++ b/packages/toolbox-core/src/toolbox_core/tool.py @@ -13,11 +13,14 @@ # limitations under the License. +import warnings from inspect import Parameter, Signature -from typing import Any +from typing import Any, Callable, TypeVar, Union from aiohttp import ClientSession +T = TypeVar("T", bound="ToolboxTool") + class ToolboxTool: """ @@ -25,7 +28,10 @@ class ToolboxTool: Instances of this class behave like asynchronous functions. When called, they send a request to the corresponding tool's endpoint on the Toolbox server with - the provided arguments. + the provided arguments, including any bound parameters. + + Methods like `bind_param` return *new* instances + with the added state, ensuring immutability of the original tool object. It utilizes Python's introspection features (`__name__`, `__doc__`, `__signature__`, `__annotations__`) so that standard tools like `help()` @@ -43,6 +49,7 @@ def __init__( name: str, desc: str, params: list[Parameter], + bound_params: dict[str, Union[Any, Callable[[], Any]]] | None = None, ): """ Initializes a callable that will trigger the tool invocation through the Toolbox server. @@ -54,43 +61,217 @@ def __init__( desc: The description of the remote tool (used as its docstring). params: A list of `inspect.Parameter` objects defining the tool's arguments and their types/defaults. + bound_params: Pre-existing bound parameters. """ + self.__base_url = base_url # used to invoke the toolbox API self.__session = session self.__url = f"{base_url}/api/tool/{name}/invoke" + self.__original_params = params + + # Store bound params + self.__bound_params = bound_params or {} + + # Filter out bound parameters from the signature exposed to the user + visible_params = [p for p in params if p.name not in self.__bound_params] # the following properties are set to help anyone that might inspect it determine self.__name__ = name self.__doc__ = desc - self.__signature__ = Signature(parameters=params, return_annotation=str) - self.__annotations__ = {p.name: p.annotation for p in params} + # The signature only shows non-bound parameters + self.__signature__ = Signature(parameters=visible_params, return_annotation=str) + self.__annotations__ = {p.name: p.annotation for p in visible_params} # TODO: self.__qualname__ ?? async def __call__(self, *args: Any, **kwargs: Any) -> str: """ - Asynchronously calls the remote tool with the provided arguments. + Asynchronously calls the remote tool with the provided arguments and bound parameters. - Validates arguments against the tool's signature, then sends them - as a JSON payload in a POST request to the tool's invoke URL. + Validates arguments against the tool's signature (excluding bound parameters), + then sends bound parameters and call arguments as a JSON payload in a POST request to the tool's invoke URL. Args: - *args: Positional arguments for the tool. - **kwargs: Keyword arguments for the tool. + *args: Positional arguments for the tool (for non-bound parameters). + **kwargs: Keyword arguments for the tool (for non-bound parameters). Returns: The string result returned by the remote tool execution. + + Raises: + TypeError: If a bound parameter conflicts with a parameter provided at call time. + Exception: If the remote tool call results in an error. """ - all_args = self.__signature__.bind(*args, **kwargs) - all_args.apply_defaults() # Include default values if not provided - payload = all_args.arguments + # Resolve bound parameters by evaluating callables + resolved_bound_params: dict[str, Any] = {} + for name, value_or_callable in self.__bound_params.items(): + try: + resolved_bound_params[name] = ( + value_or_callable() + if callable(value_or_callable) + else value_or_callable + ) + except Exception as e: + raise RuntimeError( + f"Error evaluating bound parameter '{name}' for tool '{self.__name__}': {e}" + ) from e + + # Check for conflicts between resolved bound params and kwargs + conflicts = resolved_bound_params.keys() & kwargs.keys() + if conflicts: + raise TypeError( + f"Tool '{self.__name__}': Cannot provide value during call for already bound argument(s): {', '.join(conflicts)}" + ) + merged_kwargs = {**resolved_bound_params, **kwargs} + # Bind *args and merged_kwargs using the *original* full signature + # This ensures all parameters (bound and call-time) are accounted for. + full_signature = Signature( + parameters=self.__original_params, return_annotation=str + ) + try: + # We use merged_kwargs here; args fill positional slots first. + # Bound parameters passed positionally via *args is complex and less intuitive, + # so we primarily expect bound params to be treated like pre-filled keywords. + # If a user *really* wanted to bind a purely positional param, they could, + # but providing it again via *args at call time would be an error caught by bind(). + all_args = full_signature.bind(*args, **merged_kwargs) + except TypeError as e: + raise TypeError( + f"Argument binding error for tool '{self.__name__}' (check bound params and call arguments): {e}" + ) from e + + all_args.apply_defaults() + + # Make the API call async with self.__session.post( self.__url, - json=payload, + payload=all_args.arguments, ) as resp: - ret = await resp.json() - if "error" in ret: - # TODO: better error - raise Exception(ret["error"]) - return ret.get("result", ret) + try: + ret = await resp.json() + except Exception as e: + raise Exception( + f"Failed to decode JSON response from tool '{self.__name__}': {e}. Status: {resp.status}, Body: {await resp.text()}" + ) from e + + if resp.status >= 400 or "error" in ret: + error_detail = ret.get("error", ret) if isinstance(ret, dict) else ret + raise Exception( + f"Tool '{self.__name__}' invocation failed with status {resp.status}: {error_detail}" + ) + + # Handle cases where 'result' might be missing but no explicit error given + return ret.get( + "result", str(ret) + ) # Return string representation if 'result' key missing + + # # --- Methods for adding state (return new instances) --- + # def _copy_with_updates( + # self: T, + # *, + # add_bound_params: dict[str, Union[Any, Callable[[], Any]]] | None = None, + # ) -> T: + # """Creates a new instance with updated bound params.""" + # new_bound_params = self.__bound_params.copy() + # if add_bound_params: + # new_bound_params.update(add_bound_params) + # + # return self.__class__( + # session=self.__session, + # base_url=self.__base_url, + # name=self.__name__, + # desc=self.__doc__ or "", + # params=self.__original_params, + # _bound_params=new_bound_params, + # ) + # + # def bind_params( + # self: T, + # params_to_bind: dict[str, Union[Any, Callable[[], Any]]], + # strict: bool = True, + # ) -> T: + # """ + # Returns a *new* tool instance with the provided parameters bound. + # + # Bound parameters are pre-filled values or callables that resolve to values + # when the tool is called. They are not part of the signature of the + # returned tool instance. + # + # Args: + # params_to_bind: A dictionary mapping parameter names to their + # values or callables that return the value. + # strict: If True (default), raises ValueError if attempting to bind + # a parameter that doesn't exist in the original tool signature + # or is already bound in this instance. If False, issues a warning. + # + # Returns: + # A new ToolboxTool instance with the specified parameters bound. + # + # Raises: + # ValueError: If strict is True and a parameter name is invalid or + # already bound. + # """ + # invalid_params: list[str] = [] + # duplicate_params: list[str] = [] + # original_param_names = {p.name for p in self.__original_params} + # + # for name in params_to_bind: + # if name not in original_param_names: + # invalid_params.append(name) + # elif name in self.__bound_params: + # duplicate_params.append(name) + # + # messages: list[str] = [] + # if invalid_params: + # messages.append( + # f"Parameter(s) {', '.join(invalid_params)} do not exist in the signature for tool '{self.__name__}'." + # ) + # if duplicate_params: + # messages.append( + # f"Parameter(s) {', '.join(duplicate_params)} are already bound in this instance of tool '{self.__name__}'." + # ) + # + # if messages: + # message = "\n".join(messages) + # if strict: + # raise ValueError(message) + # else: + # warnings.warn(message) + # # Filter out problematic params if not strict + # params_to_bind = { + # k: v + # for k, v in params_to_bind.items() + # if k not in invalid_params and k not in duplicate_params + # } + # + # if not params_to_bind: + # return self + # + # return self._copy_with_updates(add_bound_params=params_to_bind) + # + # def bind_param( + # self: T, + # param_name: str, + # param_value: Union[Any, Callable[[], Any]], + # strict: bool = True, + # ) -> T: + # """ + # Returns a *new* tool instance with the provided parameter bound. + # + # Convenience method for binding a single parameter. + # + # Args: + # param_name: The name of the parameter to bind. + # param_value: The value or callable for the parameter. + # strict: If True (default), raises ValueError if the parameter name + # is invalid or already bound. If False, issues a warning. + # + # Returns: + # A new ToolboxTool instance with the specified parameter bound. + # + # Raises: + # ValueError: If strict is True and the parameter name is invalid or + # already bound. + # """ + # return self.bind_params({param_name: param_value}, strict=strict) \ No newline at end of file From 8e96af03f58527c892e3bdf9da45b1753f9452b6 Mon Sep 17 00:00:00 2001 From: Twisha Bansal Date: Thu, 27 Mar 2025 18:58:08 +0530 Subject: [PATCH 02/13] still in progress --- .../toolbox-core/src/toolbox_core/tool.py | 89 ++-- packages/toolbox-core/tests/test_tool.py | 410 ++++++++++++++++++ 2 files changed, 470 insertions(+), 29 deletions(-) create mode 100644 packages/toolbox-core/tests/test_tool.py diff --git a/packages/toolbox-core/src/toolbox_core/tool.py b/packages/toolbox-core/src/toolbox_core/tool.py index c5d87010..8f675717 100644 --- a/packages/toolbox-core/src/toolbox_core/tool.py +++ b/packages/toolbox-core/src/toolbox_core/tool.py @@ -84,69 +84,100 @@ def __init__( self.__annotations__ = {p.name: p.annotation for p in visible_params} # TODO: self.__qualname__ ?? - async def __call__(self, *args: Any, **kwargs: Any) -> str: + def _resolve_callable_args( + self, callable_args: dict[str, Union[Any, Callable[[], Any]]] + ) -> dict[str, Any]: """ - Asynchronously calls the remote tool with the provided arguments and bound parameters. - - Validates arguments against the tool's signature (excluding bound parameters), - then sends bound parameters and call arguments as a JSON payload in a POST request to the tool's invoke URL. - - Args: - *args: Positional arguments for the tool (for non-bound parameters). - **kwargs: Keyword arguments for the tool (for non-bound parameters). + Evaluates any callable arguments. Returns: - The string result returned by the remote tool execution. + A dictionary containing all args with callables resolved. Raises: - TypeError: If a bound parameter conflicts with a parameter provided at call time. - Exception: If the remote tool call results in an error. + RuntimeError: If evaluating a callable argument fails. """ - # Resolve bound parameters by evaluating callables - resolved_bound_params: dict[str, Any] = {} - for name, value_or_callable in self.__bound_params.items(): + resolved_params: dict[str, Any] = {} + for name, value_or_callable in callable_args.items(): try: - resolved_bound_params[name] = ( + resolved_params[name] = ( value_or_callable() if callable(value_or_callable) else value_or_callable ) except Exception as e: raise RuntimeError( - f"Error evaluating bound parameter '{name}' for tool '{self.__name__}': {e}" + f"Error evaluating argument '{name}' for tool '{self.__name__}': {e}" ) from e + return resolved_params + + def _prepare_arguments(self, *args: Any, **kwargs: Any) -> dict[str, Any]: + """ + Resolves bound parameters, merges with call arguments, and binds them + to the tool's full signature. + + Args: + *args: Positional arguments provided at call time. + **kwargs: Keyword arguments provided at call time. + + Returns: + A dictionary of all arguments ready to be sent to the API. + + Raises: + TypeError: If call-time arguments conflict with bound parameters, + or if arguments don't match the tool's signature. + RuntimeError: If evaluating a bound parameter fails. + """ + resolved_bound_params = self._resolve_callable_args(self.__bound_params) - # Check for conflicts between resolved bound params and kwargs + # Check for conflicts between resolved bound params and keyword arguments conflicts = resolved_bound_params.keys() & kwargs.keys() if conflicts: raise TypeError( f"Tool '{self.__name__}': Cannot provide value during call for already bound argument(s): {', '.join(conflicts)}" ) + + # Merge resolved bound parameters with provided keyword arguments merged_kwargs = {**resolved_bound_params, **kwargs} # Bind *args and merged_kwargs using the *original* full signature - # This ensures all parameters (bound and call-time) are accounted for. full_signature = Signature( parameters=self.__original_params, return_annotation=str ) try: - # We use merged_kwargs here; args fill positional slots first. - # Bound parameters passed positionally via *args is complex and less intuitive, - # so we primarily expect bound params to be treated like pre-filled keywords. - # If a user *really* wanted to bind a purely positional param, they could, - # but providing it again via *args at call time would be an error caught by bind(). - all_args = full_signature.bind(*args, **merged_kwargs) + bound_args = full_signature.bind(*args, **merged_kwargs) except TypeError as e: raise TypeError( - f"Argument binding error for tool '{self.__name__}' (check bound params and call arguments): {e}" + f"Argument binding error for tool '{self.__name__}' (check arguments against signature {full_signature} and bound params {list(self.__bound_params.keys())}): {e}" ) from e - all_args.apply_defaults() + # Apply default values for any missing arguments + bound_args.apply_defaults() + return bound_args.arguments + + async def __call__(self, *args: Any, **kwargs: Any) -> str: + """ + Asynchronously calls the remote tool with the provided arguments and bound parameters. + + Validates arguments against the tool's signature (excluding bound parameters), + then sends bound parameters and call arguments as a JSON payload in a POST request to the tool's invoke URL. + + Args: + *args: Positional arguments for the tool (for non-bound parameters). + **kwargs: Keyword arguments for the tool (for non-bound parameters). + + Returns: + The string result returned by the remote tool execution. + + Raises: + TypeError: If a bound parameter conflicts with a parameter provided at call time. + Exception: If the remote tool call results in an error. + """ + arguments_payload = self._prepare_arguments(*args, **kwargs) # Make the API call async with self.__session.post( self.__url, - payload=all_args.arguments, + payload=arguments_payload, ) as resp: try: ret = await resp.json() @@ -274,4 +305,4 @@ async def __call__(self, *args: Any, **kwargs: Any) -> str: # ValueError: If strict is True and the parameter name is invalid or # already bound. # """ - # return self.bind_params({param_name: param_value}, strict=strict) \ No newline at end of file + # return self.bind_params({param_name: param_value}, strict=strict) diff --git a/packages/toolbox-core/tests/test_tool.py b/packages/toolbox-core/tests/test_tool.py new file mode 100644 index 00000000..29799e63 --- /dev/null +++ b/packages/toolbox-core/tests/test_tool.py @@ -0,0 +1,410 @@ +# Copyright 2025 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from inspect import Signature +from typing import Any, Callable, Optional +from unittest.mock import AsyncMock, MagicMock + +import pytest + +from toolbox_core.protocol import ParameterSchema +from toolbox_core.tool import ToolboxTool + + +class TestToolboxTool: + @pytest.fixture + def mock_session(self) -> MagicMock: # Added self + session = MagicMock() + session.post = MagicMock() + return session + + @pytest.fixture + def tool_details(self) -> dict: + base_url = "http://fake-toolbox.com" + tool_name = "test_tool" + params = [ + ParameterSchema("arg1", Parameter.POSITIONAL_OR_KEYWORD, annotation=str), + ParameterSchema( + "opt_arg", + Parameter.POSITIONAL_OR_KEYWORD, + default=123, + annotation=Optional[int], + ), + ] + return { + "base_url": base_url, + "name": tool_name, + "desc": "A tool for testing.", + "params": params, + "signature": Signature(parameters=params, return_annotation=str), + "expected_url": f"{base_url}/api/tool/{tool_name}/invoke", + "annotations": {"arg1": str, "opt_arg": Optional[int]}, + } + + @pytest.fixture + def tool(self, mock_session: MagicMock, tool_details: dict) -> ToolboxTool: + return ToolboxTool( + session=mock_session, + base_url=tool_details["base_url"], + name=tool_details["name"], + desc=tool_details["desc"], + params=tool_details["params"], + ) + + @pytest.fixture + def configure_mock_response(self, mock_session: MagicMock) -> Callable: + def _configure(json_data: Any, status: int = 200): + mock_resp = MagicMock() + mock_resp.status = status + mock_resp.json = AsyncMock(return_value=json_data) + mock_resp.__aenter__.return_value = mock_resp + mock_resp.__aexit__.return_value = None + mock_session.post.return_value = mock_resp + + return _configure + + @pytest.mark.asyncio + async def test_initialization_and_introspection( + self, tool: ToolboxTool, tool_details: dict + ): + """Verify attributes are set correctly during initialization.""" + assert tool.__name__ == tool_details["name"] + assert tool.__doc__ == tool_details["desc"] + assert tool._ToolboxTool__url == tool_details["expected_url"] + assert tool._ToolboxTool__session is tool._ToolboxTool__session + assert tool.__signature__ == tool_details["signature"] + assert tool.__annotations__ == tool_details["annotations"] + # assert hasattr(tool, "__qualname__") + + @pytest.mark.asyncio + async def test_call_success( + self, + tool: ToolboxTool, + mock_session: MagicMock, + tool_details: dict, + configure_mock_response: Callable, + ): + expected_result = "Operation successful!" + configure_mock_response({"result": expected_result}) + + arg1_val = "test_value" + opt_arg_val = 456 + result = await tool(arg1_val, opt_arg=opt_arg_val) + + assert result == expected_result + mock_session.post.assert_called_once_with( + tool_details["expected_url"], + json={"arg1": arg1_val, "opt_arg": opt_arg_val}, + ) + mock_session.post.return_value.__aenter__.return_value.json.assert_awaited_once() + + @pytest.mark.asyncio + async def test_call_success_with_defaults( + self, + tool: ToolboxTool, + mock_session: MagicMock, + tool_details: dict, + configure_mock_response: Callable, + ): + expected_result = "Default success!" + configure_mock_response({"result": expected_result}) + + arg1_val = "another_test" + default_opt_val = tool_details["params"][1].default + result = await tool(arg1_val) + + assert result == expected_result + mock_session.post.assert_called_once_with( + tool_details["expected_url"], + json={"arg1": arg1_val, "opt_arg": default_opt_val}, + ) + mock_session.post.return_value.__aenter__.return_value.json.assert_awaited_once() + + @pytest.mark.asyncio + async def test_call_api_error( + self, + tool: ToolboxTool, + mock_session: MagicMock, + tool_details: dict, + configure_mock_response: Callable, + ): + error_message = "Tool execution failed on server" + configure_mock_response({"error": error_message}) + default_opt_val = tool_details["params"][1].default + + with pytest.raises(Exception) as exc_info: + await tool("some_arg") + + assert str(exc_info.value) == error_message + mock_session.post.assert_called_once_with( + tool_details["expected_url"], + json={"arg1": "some_arg", "opt_arg": default_opt_val}, + ) + mock_session.post.return_value.__aenter__.return_value.json.assert_awaited_once() + + @pytest.mark.asyncio + async def test_call_missing_result_key( + self, + tool: ToolboxTool, + mock_session: MagicMock, + tool_details: dict, + configure_mock_response: Callable, + ): + fallback_response = {"status": "completed", "details": "some info"} + configure_mock_response(fallback_response) + default_opt_val = tool_details["params"][1].default + + result = await tool("value_for_arg1") + + assert result == fallback_response + mock_session.post.assert_called_once_with( + tool_details["expected_url"], + json={"arg1": "value_for_arg1", "opt_arg": default_opt_val}, + ) + mock_session.post.return_value.__aenter__.return_value.json.assert_awaited_once() + + @pytest.mark.asyncio + async def test_call_invalid_arguments_type_error( + self, tool: ToolboxTool, mock_session: MagicMock + ): + with pytest.raises(TypeError): + await tool("val1", 2, 3) + + with pytest.raises(TypeError): + await tool("val1", non_existent_arg="bad") + + with pytest.raises(TypeError): + await tool(opt_arg=500) + + mock_session.post.assert_not_called() + + @pytest.fixture + def bound_arg1_value(self) -> str: + return "statically_bound_arg1" + + @pytest.fixture + def tool_with_bound_arg1( + self, mock_session: MagicMock, tool_details: dict[str, Any], bound_arg1_value: str + ) -> ToolboxTool: + """Provides a tool with 'arg1' statically bound.""" + bound_params = {"arg1": bound_arg1_value} + return ToolboxTool( + session=mock_session, + base_url=tool_details["base_url"], + name=tool_details["name"], + desc=tool_details["desc"], + params=tool_details["params"], + bound_params=bound_params, + ) + + async def test_bound_parameter_static_value_call( + self, + tool_with_bound_arg1: ToolboxTool, + mock_session: MagicMock, + tool_details: dict[str, Any], + configure_mock_response: Callable, + bound_arg1_value: str, + ): + """Test calling a tool with a statically bound parameter.""" + expected_result = "Bound call success!" + configure_mock_response(json_data={"result": expected_result}) + + opt_arg_val = 789 + req_kwarg_val = True + default_opt_val = tool_details["params"][1].default # Not used here, but for clarity + + # Call *without* providing arg1 + result = await tool_with_bound_arg1(opt_arg=opt_arg_val, req_kwarg=req_kwarg_val) + + assert result == expected_result + mock_session.post.assert_called_once_with( + tool_details["expected_url"], + # Payload should include the bound value for arg1 + json={"arg1": bound_arg1_value, "opt_arg": opt_arg_val, "req_kwarg": req_kwarg_val}, + ) + mock_session.post.return_value.__aenter__.return_value.json.assert_awaited_once() + + async def test_bound_parameter_static_value_introspection( + self, tool_with_bound_arg1: ToolboxTool, tool_details: dict[str, Any] + ): + """Verify the public signature excludes the bound parameter 'arg1'.""" + assert "arg1" not in tool_with_bound_arg1.__signature__.parameters + assert "arg1" not in tool_with_bound_arg1.__annotations__ + + # Check remaining parameters are present + assert "opt_arg" in tool_with_bound_arg1.__signature__.parameters + assert "req_kwarg" in tool_with_bound_arg1.__signature__.parameters + assert tool_with_bound_arg1.__signature__.parameters["opt_arg"].annotation == Optional[int] + assert tool_with_bound_arg1.__signature__.parameters["req_kwarg"].annotation == bool + + async def test_bound_parameter_callable_value_call( + self, + mock_session: MagicMock, + tool_details: dict[str, Any], + configure_mock_response: Callable, + ): + """Test calling a tool with a parameter bound to a callable.""" + callable_value = "dynamic_value" + callable_mock = MagicMock(return_value=callable_value) + bound_params = {"arg1": callable_mock} + + tool_bound_callable = ToolboxTool( + session=mock_session, + base_url=tool_details["base_url"], + name=tool_details["name"], + desc=tool_details["desc"], + params=tool_details["params"], + bound_params=bound_params, + ) + + expected_result = "Callable bound success!" + configure_mock_response(json_data={"result": expected_result}) + + opt_arg_val = 999 + req_kwarg_val = False + + # Call *without* providing arg1 + result = await tool_bound_callable(opt_arg=opt_arg_val, req_kwarg=req_kwarg_val) + + assert result == expected_result + # Verify the callable was executed exactly once + callable_mock.assert_called_once() + + mock_session.post.assert_called_once_with( + tool_details["expected_url"], + # Payload should include the *result* of the callable + json={"arg1": callable_value, "opt_arg": opt_arg_val, "req_kwarg": req_kwarg_val}, + ) + mock_session.post.return_value.__aenter__.return_value.json.assert_awaited_once() + + async def test_bound_parameter_callable_evaluation_error( + self, + mock_session: MagicMock, + tool_details: dict[str, Any], + ): + """Test that RuntimeError is raised if bound callable evaluation fails.""" + error_message = "Callable evaluation failed!" + + def failing_callable(): + raise ValueError(error_message) + + bound_params = {"arg1": failing_callable} + tool_bound_failing = ToolboxTool( + session=mock_session, + base_url=tool_details["base_url"], + name=tool_details["name"], + desc=tool_details["desc"], + params=tool_details["params"], + bound_params=bound_params, + ) + + with pytest.raises(RuntimeError) as exc_info: + await tool_bound_failing(opt_arg=1, req_kwarg=True) # Provide other args + + # Check that the original exception message is part of the RuntimeError + assert error_message in str(exc_info.value) + assert "Error evaluating argument 'arg1'" in str(exc_info.value) + + # Ensure the API call was *not* made + mock_session.post.assert_not_called() + + async def test_bound_parameter_conflict_error( + self, tool_with_bound_arg1: ToolboxTool, mock_session: MagicMock, bound_arg1_value: str + ): + """Test TypeError when providing an argument that is already bound.""" + conflicting_arg1_val = "call_time_value" + + with pytest.raises(TypeError) as exc_info: + # Attempt to provide 'arg1' again during the call + await tool_with_bound_arg1(arg1=conflicting_arg1_val, req_kwarg=True) + + assert "Cannot provide value during call for already bound argument(s): arg1" in str(exc_info.value) + + # Ensure the API call was *not* made + mock_session.post.assert_not_called() + + async def test_bound_parameter_overrides_default( + self, + mock_session: MagicMock, + tool_details: dict[str, Any], + configure_mock_response: Callable, + ): + """Test that a bound value for a parameter with a default overrides the default.""" + bound_opt_arg_value = 999 # Different from the default of 123 + bound_params = {"opt_arg": bound_opt_arg_value} + + tool_bound_default = ToolboxTool( + session=mock_session, + base_url=tool_details["base_url"], + name=tool_details["name"], + desc=tool_details["desc"], + params=tool_details["params"], + bound_params=bound_params, + ) + + expected_result = "Default override success!" + configure_mock_response(json_data={"result": expected_result}) + + arg1_val = "required_arg_val" + req_kwarg_val = True + + # Call *without* providing opt_arg + result = await tool_bound_default(arg1_val, req_kwarg=req_kwarg_val) + + assert result == expected_result + mock_session.post.assert_called_once_with( + tool_details["expected_url"], + # Payload should include the bound value for opt_arg, not the default + json={"arg1": arg1_val, "opt_arg": bound_opt_arg_value, "req_kwarg": req_kwarg_val}, + ) + + async def test_multiple_bound_parameters( + self, + mock_session: MagicMock, + tool_details: dict[str, Any], + configure_mock_response: Callable, + ): + """Test binding multiple parameters.""" + bound_arg1 = "multi_bound_1" + bound_opt_arg = 555 + bound_params = { + "arg1": bound_arg1, + "opt_arg": bound_opt_arg, + } + + tool_multi_bound = ToolboxTool( + session=mock_session, + base_url=tool_details["base_url"], + name=tool_details["name"], + desc=tool_details["desc"], + params=tool_details["params"], + bound_params=bound_params, + ) + + # Check introspection - only req_kwarg should remain + assert list(tool_multi_bound.__signature__.parameters.keys()) == ["req_kwarg"] + + expected_result = "Multi-bound success!" + configure_mock_response(json_data={"result": expected_result}) + + req_kwarg_val = False + # Call providing only the remaining unbound argument + result = await tool_multi_bound(req_kwarg=req_kwarg_val) + + assert result == expected_result + mock_session.post.assert_called_once_with( + tool_details["expected_url"], + # Payload should include both bound values and the called value + json={"arg1": bound_arg1, "opt_arg": bound_opt_arg, "req_kwarg": req_kwarg_val}, + ) \ No newline at end of file From cac3878dcb56e704de9cfc236ec3ba34593aa630 Mon Sep 17 00:00:00 2001 From: Twisha Bansal Date: Thu, 27 Mar 2025 19:08:35 +0530 Subject: [PATCH 03/13] basic bound params works as expected. --- packages/toolbox-core/tests/test_tool.py | 294 +++-------------------- 1 file changed, 34 insertions(+), 260 deletions(-) diff --git a/packages/toolbox-core/tests/test_tool.py b/packages/toolbox-core/tests/test_tool.py index 29799e63..34ae436c 100644 --- a/packages/toolbox-core/tests/test_tool.py +++ b/packages/toolbox-core/tests/test_tool.py @@ -12,13 +12,12 @@ # See the License for the specific language governing permissions and # limitations under the License. -from inspect import Signature +from inspect import Parameter, Signature from typing import Any, Callable, Optional from unittest.mock import AsyncMock, MagicMock import pytest -from toolbox_core.protocol import ParameterSchema from toolbox_core.tool import ToolboxTool @@ -30,26 +29,38 @@ def mock_session(self) -> MagicMock: # Added self return session @pytest.fixture - def tool_details(self) -> dict: - base_url = "http://fake-toolbox.com" - tool_name = "test_tool" - params = [ - ParameterSchema("arg1", Parameter.POSITIONAL_OR_KEYWORD, annotation=str), - ParameterSchema( + def tool_params(self) -> list[Parameter]: + return [ + Parameter("arg1", Parameter.POSITIONAL_OR_KEYWORD, annotation=str), + Parameter( "opt_arg", Parameter.POSITIONAL_OR_KEYWORD, default=123, annotation=Optional[int], ), + Parameter("req_kwarg", Parameter.KEYWORD_ONLY, annotation=bool), # Added back ] + + @pytest.fixture + def tool_details(self, tool_params: list[Parameter]) -> dict[str, Any]: + """Provides common details for constructing the test tool.""" + base_url = "http://fake-toolbox.com" + tool_name = "test_tool" + params = tool_params + full_signature = Signature(parameters=params, return_annotation=str) + public_signature = Signature(parameters=params, return_annotation=str) + full_annotations = {"arg1": str, "opt_arg": Optional[int], "req_kwarg": bool} + public_annotations = full_annotations.copy() + return { "base_url": base_url, "name": tool_name, "desc": "A tool for testing.", "params": params, - "signature": Signature(parameters=params, return_annotation=str), + "full_signature": full_signature, "expected_url": f"{base_url}/api/tool/{tool_name}/invoke", - "annotations": {"arg1": str, "opt_arg": Optional[int]}, + "public_signature": public_signature, + "public_annotations": public_annotations, } @pytest.fixture @@ -60,6 +71,7 @@ def tool(self, mock_session: MagicMock, tool_details: dict) -> ToolboxTool: name=tool_details["name"], desc=tool_details["desc"], params=tool_details["params"], + bound_params=None, ) @pytest.fixture @@ -82,9 +94,9 @@ async def test_initialization_and_introspection( assert tool.__name__ == tool_details["name"] assert tool.__doc__ == tool_details["desc"] assert tool._ToolboxTool__url == tool_details["expected_url"] - assert tool._ToolboxTool__session is tool._ToolboxTool__session - assert tool.__signature__ == tool_details["signature"] - assert tool.__annotations__ == tool_details["annotations"] + assert tool.__signature__ == tool_details["public_signature"] + assert tool.__annotations__ == tool_details["public_annotations"] + assert tool._ToolboxTool__bound_params == {} # assert hasattr(tool, "__qualname__") @pytest.mark.asyncio @@ -100,77 +112,13 @@ async def test_call_success( arg1_val = "test_value" opt_arg_val = 456 - result = await tool(arg1_val, opt_arg=opt_arg_val) - - assert result == expected_result - mock_session.post.assert_called_once_with( - tool_details["expected_url"], - json={"arg1": arg1_val, "opt_arg": opt_arg_val}, - ) - mock_session.post.return_value.__aenter__.return_value.json.assert_awaited_once() - - @pytest.mark.asyncio - async def test_call_success_with_defaults( - self, - tool: ToolboxTool, - mock_session: MagicMock, - tool_details: dict, - configure_mock_response: Callable, - ): - expected_result = "Default success!" - configure_mock_response({"result": expected_result}) - - arg1_val = "another_test" - default_opt_val = tool_details["params"][1].default - result = await tool(arg1_val) + req_kwarg_val = True + result = await tool(arg1_val, opt_arg=opt_arg_val, req_kwarg=req_kwarg_val) assert result == expected_result mock_session.post.assert_called_once_with( tool_details["expected_url"], - json={"arg1": arg1_val, "opt_arg": default_opt_val}, - ) - mock_session.post.return_value.__aenter__.return_value.json.assert_awaited_once() - - @pytest.mark.asyncio - async def test_call_api_error( - self, - tool: ToolboxTool, - mock_session: MagicMock, - tool_details: dict, - configure_mock_response: Callable, - ): - error_message = "Tool execution failed on server" - configure_mock_response({"error": error_message}) - default_opt_val = tool_details["params"][1].default - - with pytest.raises(Exception) as exc_info: - await tool("some_arg") - - assert str(exc_info.value) == error_message - mock_session.post.assert_called_once_with( - tool_details["expected_url"], - json={"arg1": "some_arg", "opt_arg": default_opt_val}, - ) - mock_session.post.return_value.__aenter__.return_value.json.assert_awaited_once() - - @pytest.mark.asyncio - async def test_call_missing_result_key( - self, - tool: ToolboxTool, - mock_session: MagicMock, - tool_details: dict, - configure_mock_response: Callable, - ): - fallback_response = {"status": "completed", "details": "some info"} - configure_mock_response(fallback_response) - default_opt_val = tool_details["params"][1].default - - result = await tool("value_for_arg1") - - assert result == fallback_response - mock_session.post.assert_called_once_with( - tool_details["expected_url"], - json={"arg1": "value_for_arg1", "opt_arg": default_opt_val}, + payload={"arg1": arg1_val, "opt_arg": opt_arg_val, "req_kwarg": req_kwarg_val}, ) mock_session.post.return_value.__aenter__.return_value.json.assert_awaited_once() @@ -189,6 +137,7 @@ async def test_call_invalid_arguments_type_error( mock_session.post.assert_not_called() + # Bound Params tests @pytest.fixture def bound_arg1_value(self) -> str: return "statically_bound_arg1" @@ -197,17 +146,16 @@ def bound_arg1_value(self) -> str: def tool_with_bound_arg1( self, mock_session: MagicMock, tool_details: dict[str, Any], bound_arg1_value: str ) -> ToolboxTool: - """Provides a tool with 'arg1' statically bound.""" bound_params = {"arg1": bound_arg1_value} return ToolboxTool( session=mock_session, base_url=tool_details["base_url"], name=tool_details["name"], desc=tool_details["desc"], - params=tool_details["params"], + params=tool_details["params"], # Use corrected params bound_params=bound_params, ) - + @pytest.mark.asyncio async def test_bound_parameter_static_value_call( self, tool_with_bound_arg1: ToolboxTool, @@ -221,190 +169,16 @@ async def test_bound_parameter_static_value_call( configure_mock_response(json_data={"result": expected_result}) opt_arg_val = 789 - req_kwarg_val = True - default_opt_val = tool_details["params"][1].default # Not used here, but for clarity + req_kwarg_val = True # The only remaining required arg - # Call *without* providing arg1 + # Call *without* providing arg1, but provide the others result = await tool_with_bound_arg1(opt_arg=opt_arg_val, req_kwarg=req_kwarg_val) assert result == expected_result mock_session.post.assert_called_once_with( tool_details["expected_url"], # Payload should include the bound value for arg1 - json={"arg1": bound_arg1_value, "opt_arg": opt_arg_val, "req_kwarg": req_kwarg_val}, - ) - mock_session.post.return_value.__aenter__.return_value.json.assert_awaited_once() - - async def test_bound_parameter_static_value_introspection( - self, tool_with_bound_arg1: ToolboxTool, tool_details: dict[str, Any] - ): - """Verify the public signature excludes the bound parameter 'arg1'.""" - assert "arg1" not in tool_with_bound_arg1.__signature__.parameters - assert "arg1" not in tool_with_bound_arg1.__annotations__ - - # Check remaining parameters are present - assert "opt_arg" in tool_with_bound_arg1.__signature__.parameters - assert "req_kwarg" in tool_with_bound_arg1.__signature__.parameters - assert tool_with_bound_arg1.__signature__.parameters["opt_arg"].annotation == Optional[int] - assert tool_with_bound_arg1.__signature__.parameters["req_kwarg"].annotation == bool - - async def test_bound_parameter_callable_value_call( - self, - mock_session: MagicMock, - tool_details: dict[str, Any], - configure_mock_response: Callable, - ): - """Test calling a tool with a parameter bound to a callable.""" - callable_value = "dynamic_value" - callable_mock = MagicMock(return_value=callable_value) - bound_params = {"arg1": callable_mock} - - tool_bound_callable = ToolboxTool( - session=mock_session, - base_url=tool_details["base_url"], - name=tool_details["name"], - desc=tool_details["desc"], - params=tool_details["params"], - bound_params=bound_params, - ) - - expected_result = "Callable bound success!" - configure_mock_response(json_data={"result": expected_result}) - - opt_arg_val = 999 - req_kwarg_val = False - - # Call *without* providing arg1 - result = await tool_bound_callable(opt_arg=opt_arg_val, req_kwarg=req_kwarg_val) - - assert result == expected_result - # Verify the callable was executed exactly once - callable_mock.assert_called_once() - - mock_session.post.assert_called_once_with( - tool_details["expected_url"], - # Payload should include the *result* of the callable - json={"arg1": callable_value, "opt_arg": opt_arg_val, "req_kwarg": req_kwarg_val}, + payload={"arg1": bound_arg1_value, "opt_arg": opt_arg_val, "req_kwarg": req_kwarg_val}, ) mock_session.post.return_value.__aenter__.return_value.json.assert_awaited_once() - async def test_bound_parameter_callable_evaluation_error( - self, - mock_session: MagicMock, - tool_details: dict[str, Any], - ): - """Test that RuntimeError is raised if bound callable evaluation fails.""" - error_message = "Callable evaluation failed!" - - def failing_callable(): - raise ValueError(error_message) - - bound_params = {"arg1": failing_callable} - tool_bound_failing = ToolboxTool( - session=mock_session, - base_url=tool_details["base_url"], - name=tool_details["name"], - desc=tool_details["desc"], - params=tool_details["params"], - bound_params=bound_params, - ) - - with pytest.raises(RuntimeError) as exc_info: - await tool_bound_failing(opt_arg=1, req_kwarg=True) # Provide other args - - # Check that the original exception message is part of the RuntimeError - assert error_message in str(exc_info.value) - assert "Error evaluating argument 'arg1'" in str(exc_info.value) - - # Ensure the API call was *not* made - mock_session.post.assert_not_called() - - async def test_bound_parameter_conflict_error( - self, tool_with_bound_arg1: ToolboxTool, mock_session: MagicMock, bound_arg1_value: str - ): - """Test TypeError when providing an argument that is already bound.""" - conflicting_arg1_val = "call_time_value" - - with pytest.raises(TypeError) as exc_info: - # Attempt to provide 'arg1' again during the call - await tool_with_bound_arg1(arg1=conflicting_arg1_val, req_kwarg=True) - - assert "Cannot provide value during call for already bound argument(s): arg1" in str(exc_info.value) - - # Ensure the API call was *not* made - mock_session.post.assert_not_called() - - async def test_bound_parameter_overrides_default( - self, - mock_session: MagicMock, - tool_details: dict[str, Any], - configure_mock_response: Callable, - ): - """Test that a bound value for a parameter with a default overrides the default.""" - bound_opt_arg_value = 999 # Different from the default of 123 - bound_params = {"opt_arg": bound_opt_arg_value} - - tool_bound_default = ToolboxTool( - session=mock_session, - base_url=tool_details["base_url"], - name=tool_details["name"], - desc=tool_details["desc"], - params=tool_details["params"], - bound_params=bound_params, - ) - - expected_result = "Default override success!" - configure_mock_response(json_data={"result": expected_result}) - - arg1_val = "required_arg_val" - req_kwarg_val = True - - # Call *without* providing opt_arg - result = await tool_bound_default(arg1_val, req_kwarg=req_kwarg_val) - - assert result == expected_result - mock_session.post.assert_called_once_with( - tool_details["expected_url"], - # Payload should include the bound value for opt_arg, not the default - json={"arg1": arg1_val, "opt_arg": bound_opt_arg_value, "req_kwarg": req_kwarg_val}, - ) - - async def test_multiple_bound_parameters( - self, - mock_session: MagicMock, - tool_details: dict[str, Any], - configure_mock_response: Callable, - ): - """Test binding multiple parameters.""" - bound_arg1 = "multi_bound_1" - bound_opt_arg = 555 - bound_params = { - "arg1": bound_arg1, - "opt_arg": bound_opt_arg, - } - - tool_multi_bound = ToolboxTool( - session=mock_session, - base_url=tool_details["base_url"], - name=tool_details["name"], - desc=tool_details["desc"], - params=tool_details["params"], - bound_params=bound_params, - ) - - # Check introspection - only req_kwarg should remain - assert list(tool_multi_bound.__signature__.parameters.keys()) == ["req_kwarg"] - - expected_result = "Multi-bound success!" - configure_mock_response(json_data={"result": expected_result}) - - req_kwarg_val = False - # Call providing only the remaining unbound argument - result = await tool_multi_bound(req_kwarg=req_kwarg_val) - - assert result == expected_result - mock_session.post.assert_called_once_with( - tool_details["expected_url"], - # Payload should include both bound values and the called value - json={"arg1": bound_arg1, "opt_arg": bound_opt_arg, "req_kwarg": req_kwarg_val}, - ) \ No newline at end of file From d652c5d11ab5a82e1c113f6a7842dcb812100f0d Mon Sep 17 00:00:00 2001 From: Twisha Bansal Date: Thu, 27 Mar 2025 19:15:01 +0530 Subject: [PATCH 04/13] fix merge issues --- packages/toolbox-core/tests/test_tool.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/toolbox-core/tests/test_tool.py b/packages/toolbox-core/tests/test_tool.py index 1c789232..20e3c108 100644 --- a/packages/toolbox-core/tests/test_tool.py +++ b/packages/toolbox-core/tests/test_tool.py @@ -94,9 +94,9 @@ async def test_initialization_and_introspection( assert tool.__name__ == tool_details["name"] assert tool.__doc__ == tool_details["desc"] assert tool._ToolboxTool__url == tool_details["expected_url"] - assert tool._ToolboxTool__session is tool._ToolboxTool__session - assert tool.__signature__ == tool_details["signature"] - assert tool.__annotations__ == tool_details["annotations"] + assert tool.__signature__ == tool_details["public_signature"] + assert tool.__annotations__ == tool_details["public_annotations"] + assert tool._ToolboxTool__bound_params == {} # assert hasattr(tool, "__qualname__") @pytest.mark.asyncio @@ -118,7 +118,7 @@ async def test_call_success( assert result == expected_result mock_session.post.assert_called_once_with( tool_details["expected_url"], - json={"arg1": arg1_val, "opt_arg": opt_arg_val}, + payload={"arg1": arg1_val, "opt_arg": opt_arg_val, "req_kwarg": req_kwarg_val}, ) mock_session.post.return_value.__aenter__.return_value.json.assert_awaited_once() From 4e6e7f892f998d28f63b8f5494ed7e34fb338fda Mon Sep 17 00:00:00 2001 From: Twisha Bansal Date: Thu, 27 Mar 2025 19:20:35 +0530 Subject: [PATCH 05/13] fix args --- packages/toolbox-core/src/toolbox_core/tool.py | 2 +- packages/toolbox-core/tests/test_tool.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/toolbox-core/src/toolbox_core/tool.py b/packages/toolbox-core/src/toolbox_core/tool.py index 8f675717..8d987953 100644 --- a/packages/toolbox-core/src/toolbox_core/tool.py +++ b/packages/toolbox-core/src/toolbox_core/tool.py @@ -177,7 +177,7 @@ async def __call__(self, *args: Any, **kwargs: Any) -> str: # Make the API call async with self.__session.post( self.__url, - payload=arguments_payload, + json=arguments_payload, ) as resp: try: ret = await resp.json() diff --git a/packages/toolbox-core/tests/test_tool.py b/packages/toolbox-core/tests/test_tool.py index 20e3c108..e7b4230d 100644 --- a/packages/toolbox-core/tests/test_tool.py +++ b/packages/toolbox-core/tests/test_tool.py @@ -118,7 +118,7 @@ async def test_call_success( assert result == expected_result mock_session.post.assert_called_once_with( tool_details["expected_url"], - payload={"arg1": arg1_val, "opt_arg": opt_arg_val, "req_kwarg": req_kwarg_val}, + json={"arg1": arg1_val, "opt_arg": opt_arg_val, "req_kwarg": req_kwarg_val}, ) mock_session.post.return_value.__aenter__.return_value.json.assert_awaited_once() @@ -178,6 +178,6 @@ async def test_bound_parameter_static_value_call( mock_session.post.assert_called_once_with( tool_details["expected_url"], # Payload should include the bound value for arg1 - payload={"arg1": bound_arg1_value, "opt_arg": opt_arg_val, "req_kwarg": req_kwarg_val}, + json={"arg1": bound_arg1_value, "opt_arg": opt_arg_val, "req_kwarg": req_kwarg_val}, ) mock_session.post.return_value.__aenter__.return_value.json.assert_awaited_once() From ffd382c411b6809fecd6f2535ed47121edf58b59 Mon Sep 17 00:00:00 2001 From: Twisha Bansal Date: Thu, 27 Mar 2025 19:21:25 +0530 Subject: [PATCH 06/13] lint --- packages/toolbox-core/tests/test_tool.py | 32 ++++++++++++++++-------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/packages/toolbox-core/tests/test_tool.py b/packages/toolbox-core/tests/test_tool.py index e7b4230d..6dfbd014 100644 --- a/packages/toolbox-core/tests/test_tool.py +++ b/packages/toolbox-core/tests/test_tool.py @@ -38,7 +38,9 @@ def tool_params(self) -> list[Parameter]: default=123, annotation=Optional[int], ), - Parameter("req_kwarg", Parameter.KEYWORD_ONLY, annotation=bool), # Added back + Parameter( + "req_kwarg", Parameter.KEYWORD_ONLY, annotation=bool + ), # Added back ] @pytest.fixture @@ -144,7 +146,10 @@ def bound_arg1_value(self) -> str: @pytest.fixture def tool_with_bound_arg1( - self, mock_session: MagicMock, tool_details: dict[str, Any], bound_arg1_value: str + self, + mock_session: MagicMock, + tool_details: dict[str, Any], + bound_arg1_value: str, ) -> ToolboxTool: bound_params = {"arg1": bound_arg1_value} return ToolboxTool( @@ -155,14 +160,15 @@ def tool_with_bound_arg1( params=tool_details["params"], # Use corrected params bound_params=bound_params, ) + @pytest.mark.asyncio async def test_bound_parameter_static_value_call( - self, - tool_with_bound_arg1: ToolboxTool, - mock_session: MagicMock, - tool_details: dict[str, Any], - configure_mock_response: Callable, - bound_arg1_value: str, + self, + tool_with_bound_arg1: ToolboxTool, + mock_session: MagicMock, + tool_details: dict[str, Any], + configure_mock_response: Callable, + bound_arg1_value: str, ): """Test calling a tool with a statically bound parameter.""" expected_result = "Bound call success!" @@ -172,12 +178,18 @@ async def test_bound_parameter_static_value_call( req_kwarg_val = True # The only remaining required arg # Call *without* providing arg1, but provide the others - result = await tool_with_bound_arg1(opt_arg=opt_arg_val, req_kwarg=req_kwarg_val) + result = await tool_with_bound_arg1( + opt_arg=opt_arg_val, req_kwarg=req_kwarg_val + ) assert result == expected_result mock_session.post.assert_called_once_with( tool_details["expected_url"], # Payload should include the bound value for arg1 - json={"arg1": bound_arg1_value, "opt_arg": opt_arg_val, "req_kwarg": req_kwarg_val}, + json={ + "arg1": bound_arg1_value, + "opt_arg": opt_arg_val, + "req_kwarg": req_kwarg_val, + }, ) mock_session.post.return_value.__aenter__.return_value.json.assert_awaited_once() From 3ec1cc3b5eb161b8c94ff483db2dbe786a7b4682 Mon Sep 17 00:00:00 2001 From: Twisha Bansal Date: Thu, 27 Mar 2025 19:26:27 +0530 Subject: [PATCH 07/13] test bind param method --- .../toolbox-core/src/toolbox_core/tool.py | 218 +++++++++--------- packages/toolbox-core/tests/test_tool.py | 42 ++++ 2 files changed, 151 insertions(+), 109 deletions(-) diff --git a/packages/toolbox-core/src/toolbox_core/tool.py b/packages/toolbox-core/src/toolbox_core/tool.py index 8d987953..906a0a61 100644 --- a/packages/toolbox-core/src/toolbox_core/tool.py +++ b/packages/toolbox-core/src/toolbox_core/tool.py @@ -197,112 +197,112 @@ async def __call__(self, *args: Any, **kwargs: Any) -> str: "result", str(ret) ) # Return string representation if 'result' key missing - # # --- Methods for adding state (return new instances) --- - # def _copy_with_updates( - # self: T, - # *, - # add_bound_params: dict[str, Union[Any, Callable[[], Any]]] | None = None, - # ) -> T: - # """Creates a new instance with updated bound params.""" - # new_bound_params = self.__bound_params.copy() - # if add_bound_params: - # new_bound_params.update(add_bound_params) - # - # return self.__class__( - # session=self.__session, - # base_url=self.__base_url, - # name=self.__name__, - # desc=self.__doc__ or "", - # params=self.__original_params, - # _bound_params=new_bound_params, - # ) - # - # def bind_params( - # self: T, - # params_to_bind: dict[str, Union[Any, Callable[[], Any]]], - # strict: bool = True, - # ) -> T: - # """ - # Returns a *new* tool instance with the provided parameters bound. - # - # Bound parameters are pre-filled values or callables that resolve to values - # when the tool is called. They are not part of the signature of the - # returned tool instance. - # - # Args: - # params_to_bind: A dictionary mapping parameter names to their - # values or callables that return the value. - # strict: If True (default), raises ValueError if attempting to bind - # a parameter that doesn't exist in the original tool signature - # or is already bound in this instance. If False, issues a warning. - # - # Returns: - # A new ToolboxTool instance with the specified parameters bound. - # - # Raises: - # ValueError: If strict is True and a parameter name is invalid or - # already bound. - # """ - # invalid_params: list[str] = [] - # duplicate_params: list[str] = [] - # original_param_names = {p.name for p in self.__original_params} - # - # for name in params_to_bind: - # if name not in original_param_names: - # invalid_params.append(name) - # elif name in self.__bound_params: - # duplicate_params.append(name) - # - # messages: list[str] = [] - # if invalid_params: - # messages.append( - # f"Parameter(s) {', '.join(invalid_params)} do not exist in the signature for tool '{self.__name__}'." - # ) - # if duplicate_params: - # messages.append( - # f"Parameter(s) {', '.join(duplicate_params)} are already bound in this instance of tool '{self.__name__}'." - # ) - # - # if messages: - # message = "\n".join(messages) - # if strict: - # raise ValueError(message) - # else: - # warnings.warn(message) - # # Filter out problematic params if not strict - # params_to_bind = { - # k: v - # for k, v in params_to_bind.items() - # if k not in invalid_params and k not in duplicate_params - # } - # - # if not params_to_bind: - # return self - # - # return self._copy_with_updates(add_bound_params=params_to_bind) - # - # def bind_param( - # self: T, - # param_name: str, - # param_value: Union[Any, Callable[[], Any]], - # strict: bool = True, - # ) -> T: - # """ - # Returns a *new* tool instance with the provided parameter bound. - # - # Convenience method for binding a single parameter. - # - # Args: - # param_name: The name of the parameter to bind. - # param_value: The value or callable for the parameter. - # strict: If True (default), raises ValueError if the parameter name - # is invalid or already bound. If False, issues a warning. - # - # Returns: - # A new ToolboxTool instance with the specified parameter bound. - # - # Raises: - # ValueError: If strict is True and the parameter name is invalid or - # already bound. - # """ - # return self.bind_params({param_name: param_value}, strict=strict) + # --- Methods for adding state (return new instances) --- + def _copy_with_updates( + self: T, + *, + add_bound_params: dict[str, Union[Any, Callable[[], Any]]] | None = None, + ) -> T: + """Creates a new instance with updated bound params.""" + new_bound_params = self.__bound_params.copy() + if add_bound_params: + new_bound_params.update(add_bound_params) + + return self.__class__( + session=self.__session, + base_url=self.__base_url, + name=self.__name__, + desc=self.__doc__ or "", + params=self.__original_params, + bound_params=new_bound_params, + ) + + def bind_params( + self: T, + params_to_bind: dict[str, Union[Any, Callable[[], Any]]], + strict: bool = True, + ) -> T: + """ + Returns a *new* tool instance with the provided parameters bound. + + Bound parameters are pre-filled values or callables that resolve to values + when the tool is called. They are not part of the signature of the + returned tool instance. + + Args: + params_to_bind: A dictionary mapping parameter names to their + values or callables that return the value. + strict: If True (default), raises ValueError if attempting to bind + a parameter that doesn't exist in the original tool signature + or is already bound in this instance. If False, issues a warning. + + Returns: + A new ToolboxTool instance with the specified parameters bound. + + Raises: + ValueError: If strict is True and a parameter name is invalid or + already bound. + """ + invalid_params: list[str] = [] + duplicate_params: list[str] = [] + original_param_names = {p.name for p in self.__original_params} + + for name in params_to_bind: + if name not in original_param_names: + invalid_params.append(name) + elif name in self.__bound_params: + duplicate_params.append(name) + + messages: list[str] = [] + if invalid_params: + messages.append( + f"Parameter(s) {', '.join(invalid_params)} do not exist in the signature for tool '{self.__name__}'." + ) + if duplicate_params: + messages.append( + f"Parameter(s) {', '.join(duplicate_params)} are already bound in this instance of tool '{self.__name__}'." + ) + + if messages: + message = "\n".join(messages) + if strict: + raise ValueError(message) + else: + warnings.warn(message) + # Filter out problematic params if not strict + params_to_bind = { + k: v + for k, v in params_to_bind.items() + if k not in invalid_params and k not in duplicate_params + } + + if not params_to_bind: + return self + + return self._copy_with_updates(add_bound_params=params_to_bind) + + def bind_param( + self: T, + param_name: str, + param_value: Union[Any, Callable[[], Any]], + strict: bool = True, + ) -> T: + """ + Returns a *new* tool instance with the provided parameter bound. + + Convenience method for binding a single parameter. + + Args: + param_name: The name of the parameter to bind. + param_value: The value or callable for the parameter. + strict: If True (default), raises ValueError if the parameter name + is invalid or already bound. If False, issues a warning. + + Returns: + A new ToolboxTool instance with the specified parameter bound. + + Raises: + ValueError: If strict is True and the parameter name is invalid or + already bound. + """ + return self.bind_params({param_name: param_value}, strict=strict) \ No newline at end of file diff --git a/packages/toolbox-core/tests/test_tool.py b/packages/toolbox-core/tests/test_tool.py index 6dfbd014..90ba5e89 100644 --- a/packages/toolbox-core/tests/test_tool.py +++ b/packages/toolbox-core/tests/test_tool.py @@ -193,3 +193,45 @@ async def test_bound_parameter_static_value_call( }, ) mock_session.post.return_value.__aenter__.return_value.json.assert_awaited_once() + + @pytest.fixture + def tool_with_bound_arg2( + self, + tool: ToolboxTool + ) -> ToolboxTool: + new_tool = tool.bind_params({"opt_arg": 88}) + return new_tool + + @pytest.mark.asyncio + async def test_bound_parameter_static_value_call2( + self, + tool_with_bound_arg2: ToolboxTool, + mock_session: MagicMock, + tool_details: dict[str, Any], + configure_mock_response: Callable, + bound_arg1_value: str, + ): + """Test calling a tool with a statically bound parameter.""" + expected_result = "Bound call success!" + configure_mock_response(json_data={"result": expected_result}) + + req_kwarg_val = True # The only remaining required arg + + # Call *without* providing arg1, but provide the others + result = await tool_with_bound_arg2( + arg1="random_val", req_kwarg=req_kwarg_val + ) + + assert result == expected_result + mock_session.post.assert_called_once_with( + tool_details["expected_url"], + # Payload should include the bound value for arg1 + json={ + "arg1": "random_val", + "opt_arg": 88, + "req_kwarg": req_kwarg_val, + }, + ) + mock_session.post.return_value.__aenter__.return_value.json.assert_awaited_once() + + From f5d7fd068243aae042a3180c42b91d30a348ca4d Mon Sep 17 00:00:00 2001 From: Twisha Bansal Date: Thu, 27 Mar 2025 19:28:02 +0530 Subject: [PATCH 08/13] lint --- packages/toolbox-core/src/toolbox_core/tool.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/toolbox-core/src/toolbox_core/tool.py b/packages/toolbox-core/src/toolbox_core/tool.py index 906a0a61..8d6a0245 100644 --- a/packages/toolbox-core/src/toolbox_core/tool.py +++ b/packages/toolbox-core/src/toolbox_core/tool.py @@ -49,7 +49,7 @@ def __init__( name: str, desc: str, params: list[Parameter], - bound_params: dict[str, Union[Any, Callable[[], Any]]] | None = None, + bound_params: Union[dict[str, Union[Any, Callable[[], Any]]], None] = None, ): """ Initializes a callable that will trigger the tool invocation through the Toolbox server. @@ -201,7 +201,7 @@ async def __call__(self, *args: Any, **kwargs: Any) -> str: def _copy_with_updates( self: T, *, - add_bound_params: dict[str, Union[Any, Callable[[], Any]]] | None = None, + add_bound_params: Union[dict[str, Union[Any, Callable[[], Any]]], None] = None, ) -> T: """Creates a new instance with updated bound params.""" new_bound_params = self.__bound_params.copy() From 501f76ce0de8d2e466bf809529f4efc57bceb7ed Mon Sep 17 00:00:00 2001 From: Twisha Bansal Date: Thu, 27 Mar 2025 19:30:24 +0530 Subject: [PATCH 09/13] lint --- .../toolbox-core/src/toolbox_core/tool.py | 2 +- packages/toolbox-core/tests/test_tool.py | 23 +++++++------------ 2 files changed, 9 insertions(+), 16 deletions(-) diff --git a/packages/toolbox-core/src/toolbox_core/tool.py b/packages/toolbox-core/src/toolbox_core/tool.py index 8d6a0245..4b8a35b4 100644 --- a/packages/toolbox-core/src/toolbox_core/tool.py +++ b/packages/toolbox-core/src/toolbox_core/tool.py @@ -305,4 +305,4 @@ def bind_param( ValueError: If strict is True and the parameter name is invalid or already bound. """ - return self.bind_params({param_name: param_value}, strict=strict) \ No newline at end of file + return self.bind_params({param_name: param_value}, strict=strict) diff --git a/packages/toolbox-core/tests/test_tool.py b/packages/toolbox-core/tests/test_tool.py index 90ba5e89..056ab1dd 100644 --- a/packages/toolbox-core/tests/test_tool.py +++ b/packages/toolbox-core/tests/test_tool.py @@ -195,21 +195,18 @@ async def test_bound_parameter_static_value_call( mock_session.post.return_value.__aenter__.return_value.json.assert_awaited_once() @pytest.fixture - def tool_with_bound_arg2( - self, - tool: ToolboxTool - ) -> ToolboxTool: + def tool_with_bound_arg2(self, tool: ToolboxTool) -> ToolboxTool: new_tool = tool.bind_params({"opt_arg": 88}) return new_tool @pytest.mark.asyncio async def test_bound_parameter_static_value_call2( - self, - tool_with_bound_arg2: ToolboxTool, - mock_session: MagicMock, - tool_details: dict[str, Any], - configure_mock_response: Callable, - bound_arg1_value: str, + self, + tool_with_bound_arg2: ToolboxTool, + mock_session: MagicMock, + tool_details: dict[str, Any], + configure_mock_response: Callable, + bound_arg1_value: str, ): """Test calling a tool with a statically bound parameter.""" expected_result = "Bound call success!" @@ -218,9 +215,7 @@ async def test_bound_parameter_static_value_call2( req_kwarg_val = True # The only remaining required arg # Call *without* providing arg1, but provide the others - result = await tool_with_bound_arg2( - arg1="random_val", req_kwarg=req_kwarg_val - ) + result = await tool_with_bound_arg2(arg1="random_val", req_kwarg=req_kwarg_val) assert result == expected_result mock_session.post.assert_called_once_with( @@ -233,5 +228,3 @@ async def test_bound_parameter_static_value_call2( }, ) mock_session.post.return_value.__aenter__.return_value.json.assert_awaited_once() - - From e94be7bd0e727e74514e9759132cd5d172caa629 Mon Sep 17 00:00:00 2001 From: Twisha Bansal Date: Thu, 27 Mar 2025 19:31:06 +0530 Subject: [PATCH 10/13] test bind with callable --- packages/toolbox-core/tests/test_tool.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/toolbox-core/tests/test_tool.py b/packages/toolbox-core/tests/test_tool.py index 056ab1dd..7512b379 100644 --- a/packages/toolbox-core/tests/test_tool.py +++ b/packages/toolbox-core/tests/test_tool.py @@ -196,7 +196,7 @@ async def test_bound_parameter_static_value_call( @pytest.fixture def tool_with_bound_arg2(self, tool: ToolboxTool) -> ToolboxTool: - new_tool = tool.bind_params({"opt_arg": 88}) + new_tool = tool.bind_params({"opt_arg": lambda: 88}) return new_tool @pytest.mark.asyncio From ffc58381e01c030231f2fcd1804408508732a40e Mon Sep 17 00:00:00 2001 From: Twisha Bansal Date: Thu, 27 Mar 2025 19:31:47 +0530 Subject: [PATCH 11/13] explain tried tests --- packages/toolbox-core/tests/test_tool.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/toolbox-core/tests/test_tool.py b/packages/toolbox-core/tests/test_tool.py index 7512b379..4ad2e8ab 100644 --- a/packages/toolbox-core/tests/test_tool.py +++ b/packages/toolbox-core/tests/test_tool.py @@ -196,7 +196,7 @@ async def test_bound_parameter_static_value_call( @pytest.fixture def tool_with_bound_arg2(self, tool: ToolboxTool) -> ToolboxTool: - new_tool = tool.bind_params({"opt_arg": lambda: 88}) + new_tool = tool.bind_params({"opt_arg": lambda: 88}) # Tried passing a string callable and it failed return new_tool @pytest.mark.asyncio From 8faaf1776c6f5aa9c5468420e704b6386aa7c9c8 Mon Sep 17 00:00:00 2001 From: Twisha Bansal Date: Thu, 27 Mar 2025 19:36:20 +0530 Subject: [PATCH 12/13] cleanup and lint --- .../toolbox-core/src/toolbox_core/tool.py | 37 +++++++++++-------- packages/toolbox-core/tests/test_tool.py | 4 +- 2 files changed, 24 insertions(+), 17 deletions(-) diff --git a/packages/toolbox-core/src/toolbox_core/tool.py b/packages/toolbox-core/src/toolbox_core/tool.py index 4b8a35b4..7f1ec07b 100644 --- a/packages/toolbox-core/src/toolbox_core/tool.py +++ b/packages/toolbox-core/src/toolbox_core/tool.py @@ -84,31 +84,36 @@ def __init__( self.__annotations__ = {p.name: p.annotation for p in visible_params} # TODO: self.__qualname__ ?? - def _resolve_callable_args( - self, callable_args: dict[str, Union[Any, Callable[[], Any]]] + def _evaluate_param_vals( + self, parameters_to_resolve: dict[str, Union[Any, Callable[[], Any]]] ) -> dict[str, Any]: """ - Evaluates any callable arguments. + Resolves parameter values, evaluating any callables. + + Iterates through the input dictionary, calling any callable values + to get their actual result. Non-callable values are kept as is. + + Args: + parameters_to_resolve: A dictionary where keys are parameter names + and values are either static values or callables returning a value. Returns: - A dictionary containing all args with callables resolved. + A dictionary containing all parameter names with their resolved, static values. Raises: - RuntimeError: If evaluating a callable argument fails. + RuntimeError: If evaluating a callable parameter value fails. """ - resolved_params: dict[str, Any] = {} - for name, value_or_callable in callable_args.items(): + resolved_parameters: dict[str, Any] = {} + for param_name, param_value in parameters_to_resolve.items(): try: - resolved_params[name] = ( - value_or_callable() - if callable(value_or_callable) - else value_or_callable + resolved_parameters[param_name] = ( + param_value() if callable(param_value) else param_value ) except Exception as e: raise RuntimeError( - f"Error evaluating argument '{name}' for tool '{self.__name__}': {e}" + f"Error evaluating parameter '{param_name}' for tool '{self.__name__}': {e}" ) from e - return resolved_params + return resolved_parameters def _prepare_arguments(self, *args: Any, **kwargs: Any) -> dict[str, Any]: """ @@ -127,17 +132,17 @@ def _prepare_arguments(self, *args: Any, **kwargs: Any) -> dict[str, Any]: or if arguments don't match the tool's signature. RuntimeError: If evaluating a bound parameter fails. """ - resolved_bound_params = self._resolve_callable_args(self.__bound_params) + evaluated_bound_params = self._evaluate_param_vals(self.__bound_params) # Check for conflicts between resolved bound params and keyword arguments - conflicts = resolved_bound_params.keys() & kwargs.keys() + conflicts = evaluated_bound_params.keys() & kwargs.keys() if conflicts: raise TypeError( f"Tool '{self.__name__}': Cannot provide value during call for already bound argument(s): {', '.join(conflicts)}" ) # Merge resolved bound parameters with provided keyword arguments - merged_kwargs = {**resolved_bound_params, **kwargs} + merged_kwargs = {**evaluated_bound_params, **kwargs} # Bind *args and merged_kwargs using the *original* full signature full_signature = Signature( diff --git a/packages/toolbox-core/tests/test_tool.py b/packages/toolbox-core/tests/test_tool.py index 4ad2e8ab..e2eccf0e 100644 --- a/packages/toolbox-core/tests/test_tool.py +++ b/packages/toolbox-core/tests/test_tool.py @@ -196,7 +196,9 @@ async def test_bound_parameter_static_value_call( @pytest.fixture def tool_with_bound_arg2(self, tool: ToolboxTool) -> ToolboxTool: - new_tool = tool.bind_params({"opt_arg": lambda: 88}) # Tried passing a string callable and it failed + new_tool = tool.bind_params( + {"opt_arg": lambda: 88} + ) # Tried passing a string callable and it failed return new_tool @pytest.mark.asyncio From 920cda629ace50c477554287cf38f1a3d588caf0 Mon Sep 17 00:00:00 2001 From: Twisha Bansal Date: Thu, 27 Mar 2025 19:39:42 +0530 Subject: [PATCH 13/13] cleanup --- packages/toolbox-core/src/toolbox_core/tool.py | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/packages/toolbox-core/src/toolbox_core/tool.py b/packages/toolbox-core/src/toolbox_core/tool.py index 7f1ec07b..01a5ff76 100644 --- a/packages/toolbox-core/src/toolbox_core/tool.py +++ b/packages/toolbox-core/src/toolbox_core/tool.py @@ -85,16 +85,16 @@ def __init__( # TODO: self.__qualname__ ?? def _evaluate_param_vals( - self, parameters_to_resolve: dict[str, Union[Any, Callable[[], Any]]] + self, params: dict[str, Union[Any, Callable[[], Any]]] ) -> dict[str, Any]: """ - Resolves parameter values, evaluating any callables. + Evaluate any callable parameter values. Iterates through the input dictionary, calling any callable values to get their actual result. Non-callable values are kept as is. Args: - parameters_to_resolve: A dictionary where keys are parameter names + params: A dictionary where keys are parameter names and values are either static values or callables returning a value. Returns: @@ -104,7 +104,7 @@ def _evaluate_param_vals( RuntimeError: If evaluating a callable parameter value fails. """ resolved_parameters: dict[str, Any] = {} - for param_name, param_value in parameters_to_resolve.items(): + for param_name, param_value in params.items(): try: resolved_parameters[param_name] = ( param_value() if callable(param_value) else param_value @@ -117,7 +117,7 @@ def _evaluate_param_vals( def _prepare_arguments(self, *args: Any, **kwargs: Any) -> dict[str, Any]: """ - Resolves bound parameters, merges with call arguments, and binds them + Evaluates parameters, merges with call arguments, and binds them to the tool's full signature. Args: @@ -132,16 +132,15 @@ def _prepare_arguments(self, *args: Any, **kwargs: Any) -> dict[str, Any]: or if arguments don't match the tool's signature. RuntimeError: If evaluating a bound parameter fails. """ - evaluated_bound_params = self._evaluate_param_vals(self.__bound_params) - # Check for conflicts between resolved bound params and keyword arguments - conflicts = evaluated_bound_params.keys() & kwargs.keys() + conflicts = self.__bound_params.keys() & kwargs.keys() if conflicts: raise TypeError( f"Tool '{self.__name__}': Cannot provide value during call for already bound argument(s): {', '.join(conflicts)}" ) - # Merge resolved bound parameters with provided keyword arguments + evaluated_bound_params = self._evaluate_param_vals(self.__bound_params) + # Merge params with provided keyword arguments merged_kwargs = {**evaluated_bound_params, **kwargs} # Bind *args and merged_kwargs using the *original* full signature