feat: support @function_tool on instance methods (closes #94)#3693
feat: support @function_tool on instance methods (closes #94)#3693fede-kamel wants to merge 4 commits into
Conversation
Make FunctionTool a descriptor: when a method decorated with @function_tool is accessed via an instance, return a copy bound to that instance. The bound method's signature omits self, so the JSON schema and invocation are correct and self is supplied automatically. Module-level function tools and class access are unaffected; bound tools are cached per instance. Closes openai#94
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 395cbcb004
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| bound = cache.get(instance) | ||
| if bound is None: | ||
| bound = self._bind_to_instance(instance) | ||
| cache[instance] = bound |
There was a problem hiding this comment.
Avoid weak-key caching of bound method tools
This per-instance cache breaks for common tool-holder classes and can also leak instances. WeakKeyDictionary requires keys to be weak-referenceable and hashable, so a @dataclass tool class with default eq=True or a __slots__ class without __weakref__ raises TypeError just by evaluating instance.tool; for normal objects, the cached value is a bound FunctionTool whose closure retains MethodType(..., 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 👍 / 👎.
| 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.
Bind methods before validating context parameters
Attaching the binder only after the unbound method has already gone through function_schema means instance tools cannot use the existing context-parameter feature. A method like def lookup(self, ctx: RunContextWrapper[Any], x: int) -> str is rejected during class definition because the schema builder sees ctx as a non-first context parameter after self, even though the bound method's valid signature would have ctx first. The method should be bound or have self skipped before schema validation for method-backed tools.
Useful? React with 👍 / 👎.
- Drop the per-instance WeakKeyDictionary cache: it raised TypeError for unhashable / __slots__ tool-holder classes and strongly retained instances via the bound closure. __get__ now rebuilds the bound tool per access (cheap; typically accessed once when building the agent). - Support methods that take RunContextWrapper: function_schema gains an opt-in skip_self so a leading self is skipped before context-position validation, fixing 'context param at non-first position' for def m(self, ctx, x). Gated by a __qualname__-based method check so a module-level function whose first arg is literally 'self' is unaffected. - Tests for context-taking methods and the module-level self guard.
Summary
Makes
@function_toolwork on instance methods. Today, decorating a method and passing the bound method fails becauseselfends up in the JSON schema (and isn't supplied at call time):This implements the descriptor protocol on
FunctionTool: when a method-backed tool is accessed via an instance,__get__returns a copy bound to that instance. Binding rebuilds the tool from the bound method (whose signature omitsself), so the schema and invocation are correct andselfis passed automatically. Bound tools are cached per instance.@function_toolfunctions are unaffected (only tools whose first parameter isselfget a binder; class/non-instance access returns the tool unchanged).This addresses @rm-openai's "I'll take a look" on the issue and removes the need for the
function_tool(instance.method)workaround.Test plan
tests/test_function_tool_methods.py:selfexcluded from schema; invocation suppliesself; distinct instances bind independently; per-instance caching; class-level access returns the unbound tool; module-level functions unaffected; and an end-to-endRunner.runwith aFakeModelcalling the bound method.tests/test_function_tool.py+tests/test_function_schema.py(75 tests) still pass — no regression.make format,make lint,make typecheck(mypy + pyright) clean. Docs example added under Tools › Function tools.Issue number
Closes #94
Checks
make lintandmake format