Store reference to original function when creating FunctionTool

[varunursekar]

When @function_tool is used, no reference to the original function is retained, which makes testing the function directly cumbersome. The original function is now just stored as an attribute of FunctionTool:

@function_tool
def my_func(x: str, y: str) -> str:
  return x + y

# works like the original function
my_func.func('hello ', 'world!')

The alternative is to require users to define helper functions duplicating signatures and doc strings:

# using double definitions instead
def _my_func(x: str, y: str) -> str:
  return x + y

@function_tool
def my_func(x: str, y: str) -> str:
  return _my_func(x, y)

[seratch]

Thanks for sending this pull request. Ideally, enabling my_func('hello ', 'world!') is the best. I will also explore solutions for it. As for your changes, make lint seems to be failing. Can you resolve it?

[varunursekar]

@seratch Thanks for the quick response.

To achieve the functionality you are describing we could adopt the following approach:

import inspect
from dataclasses import dataclass
import functools


@dataclass
class FunctionTool:
    ....
    func: ToolFunction[...] | None = None

    def __post_init__(self):
        if self.func:
          functools.update_wrapper(self, self.func)

    def __call__(self, *args, **kwargs):
        if not self.func:
          raise FunctionNotSetError()
        return self.func(*args, **kwargs)

def func(x: str) -> str:
  """this is a test"""
    return x

func = FunctionTool(...,func=func)

sig = inspect.signature(func)
sig_func = inspect.signature(func.func)

assert sig == sig_func
assert func.__doc__ == func.func.__doc__

This will ensure that signatures and docs of the original function are retained. What do you think?

[seratch]

Yeah you're right. I actually did the same for Slack's official SDKs before joining OpenAI :)

If you could apply the approach rather than holding func object in FunctionTool instance, it should be better for dev experience. I don't think this approach could cause any side effects but we can verify if nothing is broken due to that.

[varunursekar]

@seratch I implemented the solution I proposed. A few notes:

1. I explored the idea of passing in func as an init-only variable that is not stored as an attribute of the class. One way would be to override the __init__ method of the dataclass - but I decided against this because it requires copying all arguments and their defaults in order, which seemed not great for maintainability. Another option would be to use dataclass InitVars. However, storing the function as a private attribute _func seemed simpler to me.
2. While the __call__ method of FunctionTool is sync, it technically also handles the async case since it will simply return a coroutine, which the user can await.
3. Caveat to 2: The downside is that the user doesn't have a way of inspecting that the FunctionTool returns a coroutine, since inspect.iscoroutinefunction doesn't return True for async callable classes (only async functions) and FunctionTool.__call__ is sync. One way around this would be to create a subclass AsyncFunctionTool that has an async __call__ method instead. However, I don't think this should be too much of an issue since the behaviour should be apparent from the original function definition itself.

[varunursekar]

Seems like this test is unrelated but was still failing? Can you see any reason why my changes would cause this test to error? For some reason pytest interpreted test_tool as a fixture because the name began with test - I changed the name to _test_tool and this fixed the issue.

[seratch]

file /home/runner/work/openai-agents-python/openai-agents-python/tests/extensions/memory/test_advanced_sqlite_session.py, line 22
  @function_tool
  async def test_tool(query: str) -> str:
      """A test tool for testing tool call tracking."""
      return f"Tool result for: {query}"
E       fixture 'query' not found
>       available fixtures: _class_scoped_runner, _function_scoped_runner, _module_scoped_runner, _package_scoped_runner, _session_scoped_runner, agent, anyio_backend, anyio_backend_name, anyio_backend_options, cache, capfd, capfdbinary, caplog, capsys, capsysbinary, capteesys, class_mocker, clear_default_runner, clear_openai_settings, clear_span_processor, disable_real_model_clients, doctest_namespace, ensure_openai_api_key, event_loop_policy, free_tcp_port, free_tcp_port_factory, free_udp_port, free_udp_port_factory, mocker, module_mocker, monkeypatch, package_mocker, pytestconfig, record_property, record_testsuite_property, record_xml_attribute, recwarn, session_mocker, setup_span_processor, shutdown_trace_provider, snapshot_check, tmp_path, tmp_path_factory, tmpdir, tmpdir_factory, unused_tcp_port, unused_tcp_port_factory, unused_udp_port, unused_udp_port_factory, usage_data
>       use 'pytest --fixtures [testpath]' for help on them.

This looks like an issue with the combination of the changes here and pytest.fixture mechanism. While the failure affects only tests so far, I'd like to avoid ignoring the error because similar issues could arise with some other libraries.

[varunursekar]

Yeah it seems like it's an issue with functools.update_wrapper - pytest.fixture also uses this and I suspect there is some bad interaction between the two. Will investigate.

[github-actions]

This PR is stale because it has been open for 10 days with no activity.

[seratch]

@varunursekar If you're still interested in working on this, we're happy to have your enhancement here!

[varunursekar]

@seratch It seems like wraps was indeed having some side-effect on pytest collection. If we keep __name__ and __doc__, pytest will still raise warnings because many of the tools used in the test have the prefix 'test' that is picked up by pytest. Arguably it may be good practice to change the names of those tools to something like dummy_tool to avoid this. If you prefer, I can just remove the __name__ and __doc__ dressing entirely.

[github-actions]

This PR is stale because it has been open for 10 days with no activity.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: caa28733af

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Store wrapped function when decorator is parameterized

When @function_tool(...) is used with parentheses, the outer func is None, but _create_function_tool stores _func=func. That leaves _func unset for the common decorator-with-args path, so FunctionTool.__post_init__ won’t copy __name__/__doc__, and the new __call__ will raise AttributeError even though the tool was created from a real function. This breaks the stated goal for users who pass any decorator options (e.g. @function_tool(name_override=...)). The wrapped function should be stored from the inner the_func/real_func instead.

Useful? React with 👍 / 👎.