-
Notifications
You must be signed in to change notification settings - Fork 4.3k
feat: support @function_tool on instance methods (closes #94) #3693
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 1 commit
395cbcb
b0b82cc
a7d51a9
19c2dd1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,7 +11,7 @@ | |
| from collections.abc import Awaitable, Callable, Mapping | ||
| from dataclasses import dataclass, field | ||
| from enum import Enum | ||
| from types import UnionType | ||
| from types import MethodType, UnionType | ||
| from typing import ( | ||
| TYPE_CHECKING, | ||
| Annotated, | ||
|
|
@@ -389,6 +389,16 @@ class FunctionTool: | |
| _emit_tool_origin: bool = field(default=True, kw_only=True, repr=False) | ||
| """Whether runtime item generation should emit tool origin metadata for this tool.""" | ||
|
|
||
| _bind_to_instance: Callable[[Any], FunctionTool] | None = field( | ||
| default=None, kw_only=True, repr=False, compare=False | ||
| ) | ||
| """Internal: builds an instance-bound copy of a method-backed tool (see __get__).""" | ||
|
|
||
| _bound_instances: weakref.WeakKeyDictionary[Any, FunctionTool] | None = field( | ||
| default=None, kw_only=True, repr=False, compare=False | ||
| ) | ||
| """Internal per-instance cache of bound tools for method-backed function tools.""" | ||
|
|
||
| @property | ||
| def qualified_name(self) -> str: | ||
| """Return the public qualified name used to identify this function tool.""" | ||
|
|
@@ -406,6 +416,25 @@ def __post_init__(self): | |
| ) | ||
| _validate_function_tool_timeout_config(self) | ||
|
|
||
| def __get__(self, instance: Any, owner: type[Any] | None = None) -> FunctionTool: | ||
| """Descriptor hook so ``@function_tool`` works on instance methods. | ||
|
|
||
| When the tool is a class attribute accessed via an instance, return a copy | ||
| bound to that instance (``self`` is supplied automatically and excluded | ||
| from the JSON schema). Tools that are not method-backed return unchanged. | ||
| """ | ||
| if instance is None or self._bind_to_instance is None: | ||
| return self | ||
| cache = self._bound_instances | ||
| if cache is None: | ||
| cache = weakref.WeakKeyDictionary() | ||
| self._bound_instances = cache | ||
| bound = cache.get(instance) | ||
| if bound is None: | ||
| bound = self._bind_to_instance(instance) | ||
| cache[instance] = bound | ||
| return bound | ||
|
|
||
| def __copy__(self) -> FunctionTool: | ||
| copied_tool = dataclasses.replace(self) | ||
| dataclass_field_names = {tool_field.name for tool_field in dataclasses.fields(FunctionTool)} | ||
|
|
@@ -552,6 +581,31 @@ def get_function_tool_origin(function_tool: FunctionTool) -> ToolOrigin | None: | |
| return function_tool._tool_origin or ToolOrigin(type=ToolOriginType.FUNCTION) | ||
|
|
||
|
|
||
| def _attach_self_binder( | ||
| tool: FunctionTool, | ||
| the_func: ToolFunction[...], | ||
| create: Callable[[Any], FunctionTool], | ||
| ) -> None: | ||
| """Let a method-backed function tool bind ``self`` when accessed via an instance. | ||
|
|
||
| If ``the_func``'s first parameter is ``self`` (i.e. it is an instance method | ||
| decorated with ``@function_tool``), record a binder that rebuilds the tool from | ||
| the method bound to a given instance. The bound method's signature omits | ||
| ``self``, so the schema and invocation are correct without further changes. | ||
| """ | ||
| try: | ||
| params = list(inspect.signature(the_func).parameters) | ||
| except (TypeError, ValueError): | ||
| return | ||
| if not params or params[0] != "self": | ||
| return | ||
|
|
||
| def _bind(instance: Any) -> FunctionTool: | ||
| return create(MethodType(the_func, instance)) | ||
|
|
||
| tool._bind_to_instance = _bind | ||
|
|
||
|
|
||
| @dataclass | ||
| class FileSearchTool: | ||
| """A hosted tool that lets the LLM search through a vector store. Currently only supported with | ||
|
|
@@ -1906,6 +1960,7 @@ async def _on_invoke_tool_impl(ctx: ToolContext[Any], input: str) -> Any: | |
| defer_loading=defer_loading, | ||
| sync_invoker=is_sync_function_tool, | ||
| ) | ||
| _attach_self_binder(function_tool, the_func, _create_function_tool) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Attaching the binder only after the unbound method has already gone through Useful? React with 👍 / 👎. |
||
| return function_tool | ||
|
|
||
| # If func is actually a callable, we were used as @function_tool with no parentheses | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,84 @@ | ||
| """@function_tool support on instance methods (#94).""" | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import json | ||
|
|
||
| from agents import Agent, FunctionTool, Runner, function_tool | ||
| from agents.tool_context import ToolContext | ||
| from tests.fake_model import FakeModel | ||
| from tests.test_responses import get_function_tool_call, get_text_message | ||
|
|
||
|
|
||
| class Calculator: | ||
| def __init__(self, base: int) -> None: | ||
| self.base = base | ||
|
|
||
| @function_tool | ||
| def add_to_base(self, x: int) -> int: | ||
| """Add x to the calculator's base.""" | ||
| return self.base + x | ||
|
|
||
|
|
||
| def _ctx(tool: FunctionTool) -> ToolContext: | ||
| return ToolContext(context=None, tool_name=tool.name, tool_call_id="1", tool_arguments="") | ||
|
|
||
|
|
||
| def test_instance_access_binds_self_and_drops_it_from_schema() -> None: | ||
| calc = Calculator(10) | ||
| tool = calc.add_to_base # descriptor __get__ -> instance-bound tool | ||
|
|
||
| assert isinstance(tool, FunctionTool) | ||
| properties = tool.params_json_schema.get("properties", {}) | ||
| assert "self" not in properties | ||
| assert "x" in properties | ||
|
|
||
|
|
||
| async def test_instance_method_tool_invokes_with_self() -> None: | ||
| calc = Calculator(10) | ||
| tool = calc.add_to_base | ||
| result = await tool.on_invoke_tool(_ctx(tool), json.dumps({"x": 5})) | ||
| assert result == 15 | ||
|
|
||
|
|
||
| async def test_distinct_instances_bind_independently() -> None: | ||
| ten, twenty = Calculator(10), Calculator(20) | ||
| assert await ten.add_to_base.on_invoke_tool(_ctx(ten.add_to_base), json.dumps({"x": 1})) == 11 | ||
| assert ( | ||
| await twenty.add_to_base.on_invoke_tool(_ctx(twenty.add_to_base), json.dumps({"x": 1})) | ||
| == 21 | ||
| ) | ||
|
|
||
|
|
||
| def test_bound_tool_is_cached_per_instance() -> None: | ||
| calc = Calculator(10) | ||
| assert calc.add_to_base is calc.add_to_base | ||
|
|
||
|
|
||
| def test_class_access_returns_unbound_tool() -> None: | ||
| # Accessing via the class (no instance) returns the original tool unchanged. | ||
| assert isinstance(Calculator.add_to_base, FunctionTool) | ||
|
|
||
|
|
||
| def test_module_level_function_tool_unaffected() -> None: | ||
| @function_tool | ||
| def free(x: int) -> int: | ||
| """A free function.""" | ||
| return x | ||
|
|
||
| assert isinstance(free, FunctionTool) | ||
| assert "x" in free.params_json_schema.get("properties", {}) | ||
|
|
||
|
|
||
| async def test_instance_method_tool_runs_in_agent() -> None: | ||
| calc = Calculator(100) | ||
| model = FakeModel() | ||
| model.add_multiple_turn_outputs( | ||
| [ | ||
| [get_function_tool_call("add_to_base", json.dumps({"x": 5}))], | ||
| [get_text_message("done")], | ||
| ] | ||
| ) | ||
| agent = Agent(name="A", instructions="x", model=model, tools=[calc.add_to_base]) | ||
| result = await Runner.run(agent, "add 5") | ||
| assert result.final_output == "done" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This per-instance cache breaks for common tool-holder classes and can also leak instances.
WeakKeyDictionaryrequires keys to be weak-referenceable and hashable, so a@dataclasstool class with defaulteq=Trueor a__slots__class without__weakref__raisesTypeErrorjust by evaluatinginstance.tool; for normal objects, the cached value is a boundFunctionToolwhose closure retainsMethodType(..., instance), so the weak key never becomes collectible. Consider avoiding the global per-instance cache or adding a fallback/cache design that does not strongly retain the instance.Useful? React with 👍 / 👎.