From e636b686c12ade1c3ab547a29e7d1a9f3fff8647 Mon Sep 17 00:00:00 2001 From: Eric Gustin <34000337+EricGustin@users.noreply.github.com> Date: Tue, 18 Feb 2025 13:27:49 -0800 Subject: [PATCH] Add `@tool.deprecated` (#247) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## PR Description Add the ability to mark a tool as deprecated and display the warning in the user's runtime. This PR also lays the foundation for future work for emitting other levels of logs (debug, info, etc) that occur during the tool's execution. NOTE: Updates to the Arcade Clients (Python and JS) still need to be done before the deprecation warning is emitted, but this PR needs to be merged before those updates! Let's cross our fingers that we'll never need to deprecate `@tool.deprecated`! ### Example 1. Mark your tool as deprecated ```python from typing import Annotated from arcade.sdk import tool @tool.deprecated("Use the 'Math.AddInt' tool instead.") # order of decorators does not matter @tool def add( a: Annotated[int, "The first number"], b: Annotated[int, "The second number"] ) -> Annotated[int, "The sum of the two numbers"]: """ Add two numbers together """ return a + b ``` 2. Call the deprecated tool ```python from arcadepy import Arcade client = Arcade() tool_input = {"a": 9001, "b": 42} response = client.tools.execute( tool_name="Math.Add", input=tool_input, user_id="me@example.com", ) print(f"The result of adding {tool_input['a']} and {tool_input['b']} is: {response.output.value}") ``` 3. Observe the DeprecationWarning: ``` ❯ python examples/call_a_tool_directly.py /Users/ericgustin/repos/Team/arcade-ai/examples/call_a_tool_directly.py:22: DeprecationWarning: 'Math.Add' is deprecated: Use the `Math.AddInt` tool instead. response = client.tools.execute( The result of adding 9001 and 42 is: 9043 ``` --- arcade/arcade/core/catalog.py | 2 + arcade/arcade/core/executor.py | 13 ++++- arcade/arcade/core/output.py | 15 +++-- arcade/arcade/core/schema.py | 23 ++++++++ arcade/arcade/core/utils.py | 9 +++ arcade/arcade/sdk/tool.py | 11 ++++ arcade/tests/core/test_executor.py | 34 +++++++++++- arcade/tests/sdk/test_tool_decorator.py | 74 +++++++++++++++++++++++++ 8 files changed, 174 insertions(+), 7 deletions(-) diff --git a/arcade/arcade/core/catalog.py b/arcade/arcade/core/catalog.py index 5ea7042a..383eefbf 100644 --- a/arcade/arcade/core/catalog.py +++ b/arcade/arcade/core/catalog.py @@ -382,6 +382,7 @@ class ToolCatalog(BaseModel): tool_name = snake_to_pascal_case(raw_tool_name) fully_qualified_name = FullyQualifiedName.from_toolkit(tool_name, toolkit_definition) + deprecation_message = getattr(tool, "__tool_deprecation_message__", None) return ToolDefinition( name=tool_name, @@ -393,6 +394,7 @@ class ToolCatalog(BaseModel): requirements=ToolRequirements( authorization=auth_requirement, ), + deprecation_message=deprecation_message, ) diff --git a/arcade/arcade/core/executor.py b/arcade/arcade/core/executor.py index ebb19d34..0bc38bde 100644 --- a/arcade/arcade/core/executor.py +++ b/arcade/arcade/core/executor.py @@ -12,7 +12,7 @@ from arcade.core.errors import ( ToolSerializationError, ) from arcade.core.output import output_factory -from arcade.core.schema import ToolCallOutput, ToolContext, ToolDefinition +from arcade.core.schema import ToolCallLog, ToolCallOutput, ToolContext, ToolDefinition class ToolExecutor: @@ -29,6 +29,15 @@ class ToolExecutor: """ Execute a callable function with validated inputs and outputs via Pydantic models. """ + # only gathering deprecation log for now + tool_call_logs = [] + if definition.deprecation_message is not None: + tool_call_logs.append( + ToolCallLog( + message=definition.deprecation_message, level="warning", subtype="deprecation" + ) + ) + try: # serialize the input model inputs = await ToolExecutor._serialize_input(input_model, **kwargs) @@ -50,7 +59,7 @@ class ToolExecutor: output = await ToolExecutor._serialize_output(output_model, results) # return the output - return output_factory.success(data=output) + return output_factory.success(data=output, logs=tool_call_logs) except RetryableToolError as e: return output_factory.fail_retry( diff --git a/arcade/arcade/core/output.py b/arcade/arcade/core/output.py index 5cea3f8d..ba8c56e3 100644 --- a/arcade/arcade/core/output.py +++ b/arcade/arcade/core/output.py @@ -1,6 +1,7 @@ from typing import TypeVar -from arcade.core.schema import ToolCallError, ToolCallOutput +from arcade.core.schema import ToolCallError, ToolCallLog, ToolCallOutput +from arcade.core.utils import coerce_empty_list_to_none T = TypeVar("T") @@ -14,9 +15,11 @@ class ToolOutputFactory: self, *, data: T | None = None, + logs: list[ToolCallLog] | None = None, ) -> ToolCallOutput: value = getattr(data, "result", "") if data else "" - return ToolCallOutput(value=value) + logs = coerce_empty_list_to_none(logs) + return ToolCallOutput(value=value, logs=logs) def fail( self, @@ -24,6 +27,7 @@ class ToolOutputFactory: message: str, developer_message: str | None = None, traceback_info: str | None = None, + logs: list[ToolCallLog] | None = None, ) -> ToolCallOutput: return ToolCallOutput( error=ToolCallError( @@ -31,7 +35,8 @@ class ToolOutputFactory: developer_message=developer_message, can_retry=False, traceback_info=traceback_info, - ) + ), + logs=coerce_empty_list_to_none(logs), ) def fail_retry( @@ -42,6 +47,7 @@ class ToolOutputFactory: additional_prompt_content: str | None = None, retry_after_ms: int | None = None, traceback_info: str | None = None, + logs: list[ToolCallLog] | None = None, ) -> ToolCallOutput: return ToolCallOutput( error=ToolCallError( @@ -50,7 +56,8 @@ class ToolOutputFactory: can_retry=True, additional_prompt_content=additional_prompt_content, retry_after_ms=retry_after_ms, - ) + ), + logs=coerce_empty_list_to_none(logs), ) diff --git a/arcade/arcade/core/schema.py b/arcade/arcade/core/schema.py index cbea0319..daa8087b 100644 --- a/arcade/arcade/core/schema.py +++ b/arcade/arcade/core/schema.py @@ -191,6 +191,9 @@ class ToolDefinition(BaseModel): requirements: ToolRequirements """The requirements (e.g. authorization) for the tool to run.""" + deprecation_message: Optional[str] = None + """The message to display when the tool is deprecated.""" + def get_fully_qualified_name(self) -> FullyQualifiedName: return FullyQualifiedName(self.name, self.toolkit.name, self.toolkit.version) @@ -259,6 +262,24 @@ class ToolCallRequest(BaseModel): """The context for the tool invocation.""" +class ToolCallLog(BaseModel): + """A log that occurred during the tool invocation.""" + + message: str + """The user-facing warning message.""" + + level: Literal[ + "debug", + "info", + "warning", + "error", + ] + """The level of severity for the log.""" + + subtype: Optional[Literal["deprecation"]] = None + """Optional field for further categorization of the log.""" + + class ToolCallError(BaseModel): """The error that occurred during the tool invocation.""" @@ -294,6 +315,8 @@ class ToolCallOutput(BaseModel): value: Union[str, int, float, bool, dict, list[str]] | None = None """The value returned by the tool.""" + logs: list[ToolCallLog] | None = None + """The logs that occurred during the tool invocation.""" error: ToolCallError | None = None """The error that occurred during the tool invocation.""" requires_authorization: ToolCallRequiresAuthorization | None = None diff --git a/arcade/arcade/core/utils.py b/arcade/arcade/core/utils.py index 61fd1868..0e4c2eaf 100644 --- a/arcade/arcade/core/utils.py +++ b/arcade/arcade/core/utils.py @@ -78,3 +78,12 @@ def does_function_return_value(func: Callable) -> bool: visitor = ReturnVisitor() visitor.visit(tree) return visitor.returns_value + + +def coerce_empty_list_to_none(lst: list[Any] | None) -> list[Any] | None: + """ + Coerces empty lists to None, otherwise returns the list unchanged. + """ + if isinstance(lst, list) and len(lst) == 0: + return None + return lst diff --git a/arcade/arcade/sdk/tool.py b/arcade/arcade/sdk/tool.py index 641095ae..d2a7f0dd 100644 --- a/arcade/arcade/sdk/tool.py +++ b/arcade/arcade/sdk/tool.py @@ -59,3 +59,14 @@ def tool( if func: return decorator(func) return decorator + + +def _tool_deprecated(message: str) -> Callable: + def decorator(func: Callable) -> Callable: + func.__tool_deprecation_message__ = message # type: ignore[attr-defined] + return func + + return decorator + + +tool.deprecated = _tool_deprecated # type: ignore[attr-defined] diff --git a/arcade/tests/core/test_executor.py b/arcade/tests/core/test_executor.py index 30f0a04a..2154723d 100644 --- a/arcade/tests/core/test_executor.py +++ b/arcade/tests/core/test_executor.py @@ -4,7 +4,7 @@ import pytest from arcade.core.catalog import ToolCatalog from arcade.core.executor import ToolExecutor -from arcade.core.schema import ToolCallError, ToolCallOutput, ToolContext +from arcade.core.schema import ToolCallError, ToolCallLog, ToolCallOutput, ToolContext from arcade.sdk import tool from arcade.sdk.errors import RetryableToolError, ToolExecutionError @@ -15,6 +15,13 @@ def simple_tool(inp: Annotated[str, "input"]) -> Annotated[str, "output"]: return inp +@tool.deprecated("Use simple_tool instead") +@tool +def simple_deprecated_tool(inp: Annotated[str, "input"]) -> Annotated[str, "output"]: + """Simple tool that is deprecated""" + return inp + + @tool def retryable_error_tool() -> Annotated[str, "output"]: """Tool that raises a retryable error""" @@ -43,6 +50,7 @@ def bad_output_error_tool() -> Annotated[str, "output"]: catalog = ToolCatalog() catalog.add_tool(simple_tool, "simple_toolkit") +catalog.add_tool(simple_deprecated_tool, "simple_toolkit") catalog.add_tool(retryable_error_tool, "simple_toolkit") catalog.add_tool(exec_error_tool, "simple_toolkit") catalog.add_tool(unexpected_error_tool, "simple_toolkit") @@ -54,6 +62,20 @@ catalog.add_tool(bad_output_error_tool, "simple_toolkit") "tool_func, inputs, expected_output", [ (simple_tool, {"inp": "test"}, ToolCallOutput(value="test")), + ( + simple_deprecated_tool, + {"inp": "test"}, + ToolCallOutput( + value="test", + logs=[ + ToolCallLog( + message="Use simple_tool instead", + level="warning", + subtype="deprecation", + ) + ], + ), + ), ( retryable_error_tool, {}, @@ -110,6 +132,7 @@ catalog.add_tool(bad_output_error_tool, "simple_toolkit") ], ids=[ "simple_tool", + "simple_deprecated_tool", "retryable_error_tool", "exec_error_tool", "unexpected_error_tool", @@ -152,3 +175,12 @@ def check_output(output: ToolCallOutput, expected_output: ToolCallOutput): # normal tool execution else: assert output.value == expected_output.value + + # check logs + output_logs = output.logs or [] + expected_logs = expected_output.logs or [] + assert len(output_logs) == len(expected_logs) + for output_log, expected_log in zip(output_logs, expected_logs): + assert output_log.message == expected_log.message + assert output_log.level == expected_log.level + assert output_log.subtype == expected_log.subtype diff --git a/arcade/tests/sdk/test_tool_decorator.py b/arcade/tests/sdk/test_tool_decorator.py index f86b0166..8136a84c 100644 --- a/arcade/tests/sdk/test_tool_decorator.py +++ b/arcade/tests/sdk/test_tool_decorator.py @@ -115,3 +115,77 @@ def test_tool_decorator_with_auth_failure(auth_class, auth_kwargs): ) def test_tool(x, y): return x + y + + +def test_tool_deprecated_ordering_no_auth(): + """ + Checks the behavior of @tool.deprecated when used before and after the @tool decorator. + The order of the decorators should not matter. + """ + message = "Deprecated: please use new_tool instead." + + @tool.deprecated(message) + @tool + def func_deprecated_after(x): + """Test description for func_deprecated_after""" + return x + + assert hasattr(func_deprecated_after, "__tool_deprecation_message__") + assert func_deprecated_after.__tool_deprecation_message__ == message + assert func_deprecated_after.__tool_name__ == "FuncDeprecatedAfter" + assert ( + func_deprecated_after.__tool_description__ == "Test description for func_deprecated_after" + ) + assert func_deprecated_after.__tool_requires_auth__ is None + + @tool + @tool.deprecated(message) + def func_deprecated_before(x): + """Test description for func_deprecated_before""" + return x + + assert hasattr(func_deprecated_before, "__tool_deprecation_message__") + assert func_deprecated_before.__tool_deprecation_message__ == message + assert func_deprecated_before.__tool_name__ == "FuncDeprecatedBefore" + assert ( + func_deprecated_before.__tool_description__ == "Test description for func_deprecated_before" + ) + assert func_deprecated_before.__tool_requires_auth__ is None + + +def test_tool_deprecated_ordering_with_auth(): + """ + Checks the behavior of @tool.deprecated when used with authentication. + The order of the decorators should not matter. + """ + message = "Deprecated: please use new_tool instead." + + @tool.deprecated(message) + @tool(requires_auth=OAuth2(id="my_auth_id", scopes=["test_scope"])) + def func_deprecated_after_auth(x): + """Test description for func_deprecated_after_auth""" + return x + + assert hasattr(func_deprecated_after_auth, "__tool_deprecation_message__") + assert func_deprecated_after_auth.__tool_deprecation_message__ == message + assert func_deprecated_after_auth.__tool_name__ == "FuncDeprecatedAfterAuth" + assert ( + func_deprecated_after_auth.__tool_description__ + == "Test description for func_deprecated_after_auth" + ) + assert func_deprecated_after_auth.__tool_requires_auth__ is not None + + @tool(requires_auth=OAuth2(id="my_auth_id", scopes=["test_scope"])) + @tool.deprecated(message) + def func_deprecated_before_auth(x): + """Test description for func_deprecated_before_auth""" + return x + + assert hasattr(func_deprecated_before_auth, "__tool_deprecation_message__") + assert func_deprecated_before_auth.__tool_deprecation_message__ == message + assert func_deprecated_before_auth.__tool_name__ == "FuncDeprecatedBeforeAuth" + assert ( + func_deprecated_before_auth.__tool_description__ + == "Test description for func_deprecated_before_auth" + ) + assert func_deprecated_before_auth.__tool_requires_auth__ is not None