From 28ba80eb4d429546deabfe03ae394df633f337ff Mon Sep 17 00:00:00 2001 From: Om Singhal Date: Fri, 18 Sep 2026 04:25:32 -0400 Subject: [PATCH] fix(tools): stop partial tools colliding and send the __call__ docstring FunctionTool declared every functools.partial as `partial` with the docstring of functools.partial itself, so two partial tools were registered under the same name and only the last one could be called. The name now comes from the wrapped callable, and so does the docstring unless the partial carries its own. A callable object documented only on `__call__` reported that docstring as `tool.description` but sent no description in its declaration, because the JSON schema builder read `func.__doc__` directly. The lookup CallableSpec already did now lives in `get_callable_doc`, and the declaration uses it too. --- .../adk/tools/_function_tool_declarations.py | 8 +++- src/google/adk/utils/_callable_utils.py | 38 +++++++++++++------ tests/unittests/tools/test_function_tool.py | 37 ++++++++++++++++++ .../tools/test_function_tool_declarations.py | 32 ++++++++++++++++ tests/unittests/utils/test_callable_utils.py | 25 ++++++++++++ 5 files changed, 128 insertions(+), 12 deletions(-) diff --git a/src/google/adk/tools/_function_tool_declarations.py b/src/google/adk/tools/_function_tool_declarations.py index ebb6af7de5c..b928073fcff 100644 --- a/src/google/adk/tools/_function_tool_declarations.py +++ b/src/google/adk/tools/_function_tool_declarations.py @@ -25,6 +25,7 @@ from __future__ import annotations import collections.abc +import functools import inspect import logging from typing import Any @@ -40,6 +41,7 @@ from pydantic import create_model from pydantic import fields as pydantic_fields +from ..utils._callable_utils import get_callable_doc from ..utils.variant_utils import get_google_llm_variant from ..utils.variant_utils import GoogleLLMVariant @@ -104,6 +106,8 @@ def get_callable_name(func: Callable[..., Any]) -> str: """Returns the name a callable is advertised and registered under. Callable objects carry no `__name__`, so they fall back to their class name. + A functools.partial without a `__name__` of its own takes the name of the + callable it wraps, so two partials of different functions do not collide. This is the single source of truth for both the declaration sent to the model and the key the tool is registered under: if the two disagree, the model is told about a tool it cannot invoke. @@ -114,6 +118,8 @@ def get_callable_name(func: Callable[..., Any]) -> str: Returns: The name to use for the callable. """ + if isinstance(func, functools.partial) and not hasattr(func, '__name__'): + return get_callable_name(func.func) return getattr(func, '__name__', None) or func.__class__.__name__ @@ -334,7 +340,7 @@ def build_function_declaration_with_json_schema( ) # Handle Callable functions - description = inspect.cleandoc(func.__doc__) if func.__doc__ else None + description = get_callable_doc(func) or None func_name = get_callable_name(func) declaration = types.FunctionDeclaration( name=func_name, diff --git a/src/google/adk/utils/_callable_utils.py b/src/google/adk/utils/_callable_utils.py index 44b221d7919..312f16f8da5 100644 --- a/src/google/adk/utils/_callable_utils.py +++ b/src/google/adk/utils/_callable_utils.py @@ -33,6 +33,7 @@ logger = logging.getLogger("google_adk." + __name__) _DEFAULT_CALL_DOC = inspect.getdoc(type.__call__) +_DEFAULT_PARTIAL_DOC = inspect.getdoc(functools.partial) _METHOD_WRAPPER_TYPES = ( type((1).__add__), @@ -109,6 +110,31 @@ def get_type_hints_cached(func: Callable[..., Any]) -> dict[str, Any]: return hints +def get_callable_doc(func: Callable[..., Any]) -> str: + """Returns the cleaned docstring of a callable, or "" if it has none. + + A functools.partial without a docstring of its own resolves to the wrapped + callable's docstring, and a callable object without one falls back to its + `__call__` docstring. CallableSpec uses this for FunctionTool's description, + and `build_function_declaration_with_json_schema` uses it for the + declaration sent to the model. That covers only the JSON schema path: with + JSON_SCHEMA_FOR_FUNC_DECL off, `from_function_with_options` still reads + `func.__doc__` directly, so the two can differ there. + """ + doc = inspect.getdoc(func) or "" + if isinstance(func, functools.partial) and doc == _DEFAULT_PARTIAL_DOC: + return get_callable_doc(func.func) + if not doc and not _is_routine(func) and not isinstance(func, type): + call_method = getattr(func, "__call__", None) + if call_method is not None: + call_doc = inspect.getdoc(call_method) or "" + if call_doc and call_doc != _DEFAULT_CALL_DOC: + doc = call_doc + if doc == _DEFAULT_CALL_DOC: + doc = "" + return doc + + class CallableSpec: """Unified specification and introspection for a callable. @@ -131,18 +157,8 @@ def __init__(self, func: Callable[..., Any] | None) -> None: self.func = func self.unwrapped_func = unwrap_callable(func) if func is not None else None - # Docstring resolution (prioritize direct func.__doc__, then func.__call__.__doc__) if func is not None: - doc = inspect.getdoc(func) or "" - if not doc and not _is_routine(func) and not isinstance(func, type): - call_method = getattr(func, "__call__", None) - if call_method is not None: - call_doc = inspect.getdoc(call_method) or "" - if call_doc and call_doc != _DEFAULT_CALL_DOC: - doc = call_doc - if doc == _DEFAULT_CALL_DOC: - doc = "" - self.doc = doc + self.doc = get_callable_doc(func) # Context parameter detection self.context_param_name = context_utils.find_context_parameter(func) diff --git a/tests/unittests/tools/test_function_tool.py b/tests/unittests/tools/test_function_tool.py index b0abc57ed37..105f463661a 100644 --- a/tests/unittests/tools/test_function_tool.py +++ b/tests/unittests/tools/test_function_tool.py @@ -13,6 +13,7 @@ # limitations under the License. from enum import Enum +import functools import inspect from typing import Any from typing import Optional @@ -24,6 +25,7 @@ from google.adk.agents.invocation_context import InvocationContext from google.adk.features import FeatureName from google.adk.features._feature_registry import temporary_feature_override +from google.adk.models.llm_request import LlmRequest from google.adk.sessions.session import Session from google.adk.tools.function_tool import _build_declaration_cached from google.adk.tools.function_tool import FunctionTool @@ -167,6 +169,41 @@ async def test_run_async_with_tool_context_async_callable(): assert tool.description == "Async call doc" +def test_callable_declaration_uses_call_docstring(): + """The declaration carries the same __call__ docstring as tool.description.""" + tool = FunctionTool(AsyncCallableWith1ArgAndToolContext()) + + declaration = tool._get_declaration() # pylint: disable=protected-access + + assert declaration.description == "Async call doc" + + +def test_partial_tools_do_not_shadow_each_other(): + """Partials of different functions keep their own names and docstrings.""" + + def get_weather(api_key: str, city: str) -> str: + """Returns the current weather for a city.""" + return city + + def get_forecast(api_key: str, city: str, days: int = 3) -> str: + """Returns a forecast for a city.""" + return city + + llm_request = LlmRequest() + llm_request.append_tools([ + FunctionTool(functools.partial(get_weather, "key")), + FunctionTool(functools.partial(get_forecast, "key")), + ]) + + declarations = llm_request.config.tools[0].function_declarations + assert [d.name for d in declarations] == ["get_weather", "get_forecast"] + assert [d.description for d in declarations] == [ + "Returns the current weather for a city.", + "Returns a forecast for a city.", + ] + assert list(llm_request.tools_dict) == ["get_weather", "get_forecast"] + + @pytest.mark.asyncio async def test_run_async_without_tool_context_async_func(): """Test that run_async calls the function without tool_context when tool_context is not in signature (async function).""" diff --git a/tests/unittests/tools/test_function_tool_declarations.py b/tests/unittests/tools/test_function_tool_declarations.py index 408a371fe19..c226a59ce28 100644 --- a/tests/unittests/tools/test_function_tool_declarations.py +++ b/tests/unittests/tools/test_function_tool_declarations.py @@ -23,6 +23,7 @@ from collections.abc import Sequence import dataclasses from enum import Enum +import functools import os from typing import Any from typing import AsyncGenerator @@ -788,6 +789,37 @@ def undocumented(x: int) -> int: }, ) + def test_callable_object_docstring_on_call(self): + """Test callable object documented only on __call__.""" + + class OrderLookup: + + def __call__(self, order_id: str) -> str: + """Looks up an order by its id.""" + return order_id + + decl = build_function_declaration_with_json_schema(OrderLookup()) + + self.assertEqual(decl.name, "OrderLookup") + self.assertEqual(decl.description, "Looks up an order by its id.") + + def test_partial_uses_wrapped_name_and_docstring(self): + """Test functools.partial is declared as the function it wraps.""" + + def get_weather(api_key: str, city: str) -> str: + """Returns the current weather for a city.""" + return city + + decl = build_function_declaration_with_json_schema( + functools.partial(get_weather, "key") + ) + + self.assertEqual(decl.name, "get_weather") + self.assertEqual( + decl.description, "Returns the current weather for a city." + ) + self.assertEqual(list(decl.parameters_json_schema["properties"]), ["city"]) + class TestComplexFunction(parameterized.TestCase): """Test the complex function from the user's prototype.""" diff --git a/tests/unittests/utils/test_callable_utils.py b/tests/unittests/utils/test_callable_utils.py index 23b445dd3a3..a2827e7617e 100644 --- a/tests/unittests/utils/test_callable_utils.py +++ b/tests/unittests/utils/test_callable_utils.py @@ -175,6 +175,31 @@ def __call__(self, x: int) -> int: assert CallableSpec(UndocCallable()).doc == "" +def test_callable_spec_partial_doc(): + """A partial resolves doc from the callable it wraps unless it has its own.""" + + def wrapped(a: int, b: int) -> int: + """Wrapped function docstring.""" + return a + b + + class DocCallable: + + def __call__(self, a: int, b: int) -> int: + """Call method docstring.""" + return a + b + + documented = functools.partial(wrapped, 1) + documented.__doc__ = "Partial docstring." + + assert CallableSpec(functools.partial(wrapped, 1)).doc == ( + "Wrapped function docstring." + ) + assert CallableSpec(functools.partial(DocCallable(), 1)).doc == ( + "Call method docstring." + ) + assert CallableSpec(documented).doc == "Partial docstring." + + def test_callable_spec_raises_on_unintrospectable_callable(): """CallableSpec raises ValueError when signature of unintrospectable callable is accessed.""" spec = CallableSpec(dir)