Skip to content

Breaking: build callable-class activity tools from __call__ in OpenAI Agents - #206

Open
campoy wants to merge 6 commits into
mainfrom
fix/openai-agents-python-activity-tool-callable-class
Open

campoy wants to merge 6 commits into
mainfrom
fix/openai-agents-python-activity-tool-callable-class

Conversation

@campoy

@campoy campoy commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

What was changed

activity_as_tool now builds a callable-class Activity's tool from its __call__, not from its constructor.

  • When given a class, it builds the schema from __call__ as an instance binds it, keeps the class name as the tool name, and takes the description from __call__'s docstring or that of a __call__ it overrides, else the class's own docstring, but not the Name(...) one that dataclasses and namedtuples generate. __call__ must be a function, staticmethod, classmethod or functools.partialmethod: binding it drops the receiver of a method, classmethod or partialmethod and keeps every parameter of a staticmethod. Calls then pass the model's arguments to __call__, which the Worker's registered instance runs.
  • __call__'s annotations are resolved in its own module, keeping Annotated metadata, so string annotations and from __future__ import annotations work. Methods passed as Class.method now get the same resolution; before, a string annotation failed with NameError. They still drop a first parameter named self, as on main and in the SDK.
  • These now fail with ApplicationError(..., "invalid_tool"): an instance of a callable-class activity in place of its class, a class whose __call__ is missing or of another kind, a __call__ whose signature or type hints can't be read, and a __call__ with keyword-only parameters or **kwargs, which the tool can't pass.
  • Functions are unchanged, and so is the scheduled Activity name.
  • The fn docstring now names the three forms.

No public name or option changes.

Breaking change: the change has no version gate. We are not adding version gates to these fixes: breaking old histories is accepted with the plugins' 1.0 releases. The runs affected had the constructor's schema, which gave the right result only when the constructor's parameters matched __call__'s. A run that called such a tool before this change recorded the constructor's arguments. Built from __call__, the tool rejects them on replay, returns an argument error to the model instead of scheduling the Activity, and the replay fails with [TMPRL1100]. This holds whether the tool was built in Workflow code or once at import time. Runs that only used function or Class.method tools are not affected. Also, a class whose __call__ takes a parameter with no JSON schema now fails tool construction, as a function Activity with that parameter does; before, it got the constructor's schema. The same holds for the classes in the invalid_tool bullet above.

Upgrade: let runs that called a callable-class Activity tool passed as its class finish on the earlier version before you upgrade, or keep them there with Worker Versioning.

Why?

See #205. Workflow code references a callable-class Activity by its class, as workflow.execute_activity_class does, but activity_as_tool built the tool from inspect.signature(cls), the constructor's. The model was offered the constructor's parameters and its values went to __call__ by position: with a one-string constructor and a one-string call, the run completed with a wrong result and no error. Other constructors failed tool construction in Workflow code, or failed every call with TypeError.

Checklist

  1. Closes [Bug] openai_agents: activity_as_tool builds a callable-class Activity's tool from its constructor #205

  2. How was this tested:

    • test_tool_workflow in tests/test_openai.py gains a callable-class Activity, ActivityWeatherClass (constructor takes conditions, __call__ takes city: "CityName", a string naming a module-level alias), passed to activity_as_tool as its class and registered as ActivityWeatherClass("Cloudy"). The test now also checks that the class's Activity is scheduled with the model's city.
      • On main at ffda4f6 it fails: assert [] == [['"Tokyo"']] (the model's city failed the constructor schema, so the Activity was never scheduled).
      • Without the annotation resolution, it and the replay of the existing tools-workflow-history.json fail with NameError: name 'CityName' is not defined.
      • Both pass with the change.
    • test_callable_class_tool_schema checks the schema and description for a @staticmethod __call__(self: str) on an abc.ABC subclass and a @classmethod __call__(cls, self: str) (both offer self), a __call__ set with functools.partialmethod, an undocumented and a documented dataclass, and a __call__ that overrides a documented one. It checks the schema only, because before Python 3.13 the SDK registers an async partialmethod __call__ as sync. test_invalid_callable_class_tool checks the invalid_tool errors for a class with no __call__, a zero-parameter function as __call__, keyword-only and **kwargs parameters, and an instance.
    • ActivityWeatherService.get_weather_method now takes city: "CityName", so test_tool_workflow and the ToolsWorkflow replays cover the Class.method annotation resolution.
    • With e0e076a's activity_as_tool, 8 of these 21 tests fail: the two self parameters (empty schema), the documented dataclass and the overriding __call__ (wrong description), the **kwargs and instance cases (raw UserError and AttributeError), and the two cases whose error message changed.
    • Locally, the full suite passes (402 tests), and make lint (ruff check --select I, ruff format --check, pyright, mypy, basedpyright, pydocstyle) passes. These ran in a cached environment with uv 0.13.1, outside the plugin's required-version, with UV_RUN pointed at it. CI covers make lint and make test.
    • [Bug] openai_agents: activity_as_tool builds a callable-class Activity's tool from its constructor #205's reproduction exits 1 on 1.1.0, on main at c664a34 and on temporalio.contrib.openai_agents in temporalio 1.33.0. On this branch the model is offered parcel_id and the Activity returns parcel P-1717 via acme-post.
    • Replay: tests/histories/tools-workflow-callable-class-history.json, recorded on this branch, has the model call ActivityWeatherClass (23 events), and test_replay replays it on Python 3.10, 3.12, 3.13 and 3.14. A version of it recorded with all three class tools (in e0e076a) failed on main's activity_as_tool with [TMPRL1100], while tools-workflow-history.json replayed.
    • A History recorded on main from a ToolsWorkflow run whose model called the class tool fails on this branch with [TMPRL1100] Nondeterminism error: Activity type of scheduled event 'ActivityWeatherClass' does not match activity type of activity command 'invoke_model_activity'. Earlier checks on a gated version of this change, with Workers with no Workflow cache, covered a class tool built in Workflow code and one built at import time, each for one and two turns around a held Activity, and a function tool as a control. Without the gate, all eight main histories with a class tool failed with [TMPRL1100], and the two function-tool histories replayed.
  3. Any docs updates needed?
    The activity_as_tool docstring is updated here. Release notes should say which runs stop replaying (see "Breaking change").

Not covered:

  • A functools.partialmethod __call__ runs only as far as the SDK supports it. The SDK registers an async one as sync before Python 3.13, and it reads no argument types for it (arg_types == [None]), so arguments that aren't JSON primitives are not converted.
  • The SDK's _type_hints_from_func finds a __call__ or method receiver by the name self, so for __call__(this, loc: Loc) the Worker decodes loc without its type (reported in review, not rerun here). That is the SDK's, and affects workflow.execute_activity_class too.
  • Not changed here: function_schema doesn't get the caller's strict_json_schema.
  • On Python 3.10, Class.method tools now wrap a parameter with a None default in Optional, as function tools already do there.
  • Inferred, not run: when the constructor's schema cannot be built (a constructor that takes a client object), building the tool failed in Workflow code before this change. A Workflow that caught that error and offered another tool now builds the class tool instead, so a run that took that path before the upgrade can fail replay.
  • Passing an instance, the form the Worker registers, still fails in Workflow code with AttributeError. Workflow code references the class, so this PR leaves it alone.
  • temporalio 1.34.x still bundles the old code as temporalio.contrib.openai_agents. No backport here.

Open questions:

  • Should temporalio 1.34.x get a backport?

Draft while the breaking change and this question are open.

@campoy
campoy force-pushed the fix/openai-agents-python-activity-tool-callable-class branch from 0af8613 to 7fe708b Compare October 28, 2026 21:55
@campoy

campoy commented Oct 28, 2026

Copy link
Copy Markdown
Contributor Author

The rebase is pushed as 7fe708b, with the replay-test conflict resolved. CI is green. Local lint and all 10 selected tests (nine replay cases and the installed-source check) pass.

One blocking review finding:

[P1] Preserve annotation resolution when wrapping __call__. In workflow.py:186–188, copying __annotations__ onto functools.partial loses the original function's globals. With postponed annotations and a user-defined return type, even tool construction fails. This also affects an explicit string annotation such as -> "Weather".

I ran a callable class whose constructor and __call__ both take city: str, with __call__ returning a Weather dataclass and from __future__ import annotations enabled. The output was:

base: schema fields=['city']
PR: NameError: name 'Weather' is not defined

I also changed the existing ActivityWeatherClass.__call__ return annotation to the string "Weather" and replayed tools-workflow-callable-class-history.json using UnsandboxedWorkflowRunner. The base implementation replayed it successfully; this PR failed the workflow activation with the same NameError at schema = function_schema(fn).

The new schema is built before the patch gate is checked, so the legacy path cannot protect these older runs. Resolve the annotations on the original __call__, preserving Annotated metadata, before wrapping it. Please add coverage for postponed annotations and replay of that case.

@campoy
campoy force-pushed the fix/openai-agents-python-activity-tool-callable-class branch from 5ea0c2a to 68ad367 Compare October 28, 2026 22:38
@campoy

campoy commented Oct 28, 2026

Copy link
Copy Markdown
Contributor Author

Fixed in 68ad367, now rebased on main and without the version gate.

activity_as_tool now resolves __call__'s annotations in its own module with typing.get_type_hints(fn.__call__, include_extras=True) before wrapping it, so Annotated metadata is kept.

Coverage: ActivityWeatherClass.__call__ now takes city: "CityName", a string naming a module-level alias. Without the fix, test_tool_workflow and the replay of tools-workflow-history.json fail with NameError: name 'CityName' is not defined; with it they pass.

With the gate gone, I removed tools-workflow-callable-class-history.json. It recorded the constructor's arguments, so it now fails with [TMPRL1100], as the description's "Breaking change" note says.

activity_as_tool accepted a callable-class activity by its class, the
reference workflow code uses, but built the tool from the class's
signature, which is its constructor's. The model was offered the
constructor's parameters and its values went to __call__ by position,
so a run could complete with a wrong result.

For a class, build the schema from __call__ without self, keep the
class name, and take the description from __call__'s docstring (or the
class docstring). Resolve __call__'s annotations in its own module,
since the partial that wraps it has no module globals.

Breaking change: there is no version gate. A run that called such a
tool before this change recorded the constructor's arguments. On replay
the tool rejects them and the replay fails with a non-determinism
error. Let those runs finish before upgrading, or use Worker
Versioning.
@campoy
campoy force-pushed the fix/openai-agents-python-activity-tool-callable-class branch from 68ad367 to a42ec46 Compare October 28, 2026 22:44
@campoy
campoy marked this pull request as ready for review October 28, 2026 23:06
@campoy
campoy requested a review from a team as a code owner October 28, 2026 23:06

@brianstrauch brianstrauch left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One P2 finding on callable-class argument handling.

if inspect.isclass(fn):
# Workflow code references a callable-class activity by its class, but the worker's
# registered instance runs __call__, so build the tool from __call__ without `self`.
call = functools.partial(fn.__call__, None)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Preserve arguments for static and class __call__ methods

Unconditionally binding None assumes fn.__call__ has an unbound instance receiver. For @staticmethod async def __call__(city: str) and @classmethod async def __call__(cls, city: str), the exposed signature is already (city: str), so this consumes the real city parameter. I reproduced both with a constructor taking city: str: the base tool exposes city, while this branch exposes an empty schema and schedules the activity with args=[], producing TypeError: ...__call__() missing 1 required positional argument: 'city'. The SDK recognizes both registered instances with argument types [str].

Please bind a dummy receiver only for an unbound instance method, preserving static and already-bound class methods, and add regression coverage for both forms.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, fixed in 8dffd34. activity_as_tool now drops the receiver only when __call__ is a plain function on the class (inspect.isfunction(inspect.getattr_static(cls, "__call__"))), so a @staticmethod or @classmethod __call__ keeps all its parameters.

Coverage: test_tool_workflow gains ActivityWeatherStaticCall and ActivityWeatherClassMethodCall, each with __call__(city: str), and checks that both are scheduled with the model's city. Without the fix, both tools offer an empty schema and the Workflow fails on the ActivityWeatherStaticCall Activity. With it, the full suite (390 tests) and the make lint steps pass locally.

…tools

activity_as_tool dropped the first parameter of every callable-class
__call__, assuming an unbound `self`. A @staticmethod or @classmethod
__call__ has no `self` to drop, so the tool lost its first real
parameter and the Activity failed with a missing argument. Drop the
receiver only when __call__ is a plain function.

@brianstrauch brianstrauch left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One P2 finding on callable classes using functools.partialmethod for call. Lint and 59 focused tests pass; an additional regression probe fails on this revision.

Comment on lines +179 to +182
if inspect.isfunction(inspect.getattr_static(fn, "__call__")):
call = functools.partial(fn.__call__, None)
else:
call = functools.partial(fn.__call__)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Handle partialmethod implementations of __call__

A functools.partialmethod descriptor can still expose an unbound instance receiver, but inspect.isfunction(inspect.getattr_static(fn, "__call__")) returns false for it. For example:

@activity.defn
class Weather:
    def __init__(self, city: str) -> None:
        pass

    async def run(self, conditions: str, city: str) -> str:
        return f"{city}: {conditions}"

    __call__ = functools.partialmethod(run, "sunny")

Here Weather.__call__ has signature (self, city: str). The new wrapper leaves self in place, so execution falls through to the existing receiver-removal block, which raises AttributeError: 'functools.partial' object has no attribute '__temporal_activity_definition' at line 198. Consequently, activity_as_tool(Weather, ...) fails during tool construction.

I verified that the base implementation exposes city and completes a call through a local worker registered with Weather("Tokyo"), returning Tokyo: sunny; this revision fails before scheduling the activity. Please handle the receiver for partialmethod descriptors and add regression coverage.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, fixed in abaae82. Rather than adding another descriptor check, activity_as_tool now binds the class's __call__ descriptor to a stand-in instance, as the registered instance would bind it. Python then drops self from a method or partialmethod and keeps every parameter of a static or class method. Annotations and the docstring come from the function underneath, so your example offers city: string with run's docstring.

Coverage: a new test_partialmethod_call_tool_drops_only_self checks the schema and description for __call__ = functools.partialmethod(get_weather, "Rainy"). On 8dffd34 it fails with the same AttributeError. It's a schema test, not part of test_tool_workflow, because on Python 3.10 the SDK registers an async partialmethod __call__ as sync (is_async=False), so the Worker can't run it there. With the change, all four forms offer {'city': 'string'} on Python 3.10, 3.13 and 3.14. Locally, the full suite (391 tests) and the make lint steps pass.

Checking for a plain-function __call__ left `self` in a
functools.partialmethod __call__, so building its tool failed with
AttributeError. Bind the __call__ descriptor to a stand-in instance
instead, so Python drops `self` where the registered instance would:
from a method or partialmethod, but not from a static or class method.
@campoy
campoy requested a review from brianstrauch October 29, 2026 01:41

@DABH DABH left a comment •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review of the __call__-based class tool. Each finding below was reproduced against the PR build (Python 3.10 to 3.14, SDK 1.34.0, Agents SDK 0.22.0) unless marked otherwise. Items already raised in earlier review rounds (static/class method receivers, partialmethod receiver, annotation resolution) are not repeated.

Not anchored to a diff line:

  • Replay coverage. tests/histories/tools-workflow-history.json has no ActivityWeather* schedules, so no committed history replays an invoked class tool. A later change to the scheduled command (for example scheduling under schema.name, or re-adding constructor args) keeps test_openai_replay.py green while production histories break.
  • Same bug elsewhere. python/google_genai/.../workflow.py still builds class tools from the constructor, so #205 applies there too and may deserve its own issue.
  • Title. The PR is a deliberate breaking change but the title has no Breaking: prefix. Release notes are generated from commit subjects, so the squash subject should carry it.

"Input activity must have a name to be made into a tool",
"invalid_tool",
)
if inspect.isclass(fn):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replay compatibility (judgment call). This switches the tool's schema and arguments with no workflow.patched branch. run_activity always executes inside the workflow, so an invoke-time gate is feasible: build both the legacy constructor-derived schema and the new __call__ schema once, then branch inside run_activity on workflow.patched(...). Build-time gating is impossible because activity_as_tool can run at import. I understand from the thread this was removed on purpose; if so, please state in the PR body why that is acceptable for a GA plugin (for example that every affected run was already producing wrong results), since a run whose model already called a class tool now fails replay with [TMPRL1100].

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's deliberate. We're not adding version gates to these fixes: breaking old histories is accepted with the plugins' 1.0 releases. The runs affected had the constructor's schema, which gave the right result only when the constructor's parameters matched __call__'s. I've added that to the "Breaking change" note in the description.

# registered instance runs __call__, so build the tool from __call__ as an instance binds
# it: without the `self` of a method or partialmethod, and with every parameter of a
# static or class method. The tool never calls this stand-in binding.
method = inspect.getattr_static(fn, "__call__")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keyword-only __call__ parameters are offered to the model and then dropped. The SDK's keyword-only check at @activity.defn time inspects the constructor for a class, so async def __call__(self, *, city: str) decorates fine. function_schema then advertises city as required, to_call_args returns it in the kwargs half, and run_activity discards that half (the comment at the call site says the SDK already rejected keyword-only parameters, which is no longer true for classes). The activity is scheduled with args=[] and the worker fails every attempt with TypeError: missing 1 required keyword-only argument. Same for functools.partialmethod(get, conditions="Rainy"), whose signature turns later parameters keyword-only. Suggest rejecting inspect.Parameter.KEYWORD_ONLY parameters of the bound __call__ here with the same ApplicationError(..., "invalid_tool") the function uses.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in e0e076a. activity_as_tool now rejects keyword-only parameters of the bound __call__, including the ones a partialmethod keyword creates, with ApplicationError(..., "invalid_tool"). test_invalid_callable_class_tool covers it.

# static or class method. The tool never calls this stand-in binding.
method = inspect.getattr_static(fn, "__call__")
if hasattr(method, "__get__"):
method = method.__get__(object(), fn)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Malformed classes raise raw exceptions instead of ApplicationError('invalid_tool'). @activity.defn class NoCall: def __init__(self, c: str): ... is accepted by the SDK, getattr_static returns the metaclass slot type.__call__, and binding it to object() raises TypeError: descriptor '__call__' for 'type' objects doesn't apply to a 'object' object. A zero-parameter __call__ = zero raises ValueError: invalid method signature later. Inside workflow code these become workflow task failures that retry indefinitely. Please guard for a class-level __call__ and wrap signature errors in ApplicationError naming the class. (Passing the registered instance, which the PR body calls out of scope, has the same opaque AttributeError on __name__; a one-line invalid_tool error would be kinder.)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in e0e076a. A class with no __call__ in its MRO now raises ApplicationError("Activity class X has no __call__ method", "invalid_tool"), and a signature error such as the zero-parameter __call__ raises ApplicationError("Cannot build a tool from X: invalid method signature", "invalid_tool"). Both are cases in test_invalid_callable_class_tool. I left passing the instance as it is on main, since this PR doesn't touch that path.

method = inspect.getattr_static(fn, "__call__")
if hasattr(method, "__get__"):
method = method.__get__(object(), fn)
call = functools.partial(method)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two stand-in builders that disagree. This partial plus hand-copied __name__/__annotations__/__doc__ duplicates the existing self branch below, which still copies raw string annotations. Result: the Class.method form (which the new docstring advertises) still raises NameError: name 'Weather' is not defined under from __future__ import annotations, while the class form now works (reproduced). One helper that builds a single stand-in with resolved hints, called from both branches, removes the divergence. function_schema(fn, name_override=...) also exists, so setattr(call, '__name__', ...) is not needed.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in e0e076a. The class branch now only picks the callable and the function to read annotations and docs from. The self check and one stand-in, with resolved hints, name and docstring, are shared by both forms. So Class.method resolves postponed annotations too: ActivityWeatherService.get_weather_method now takes city: "CityName", and on abaae82 that fails test_tool_workflow and both ToolsWorkflow replays with NameError.

I kept setting __name__: generate_func_documentation in agents/function_schema.py reads func.__name__ even when name_override is given, so a partial without one fails.

# A partial has no module globals, so resolve postponed annotations against the
# function's own.
func = method
while isinstance(func, functools.partial):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Python 3.14 (the CI max) silently mis-builds a bare functools.partial __call__. On 3.14 functools.partial is a method descriptor whose __get__ returns a bound method wrapping the partial, so this loop stops at the method and the tool is built from functools.partial's own annotations and docstring: __call__ = functools.partial(free, 'x') gives properties == {} and description 'Create a new function with partial application of the given arguments and keywords.' with no error. On 3.10 to 3.13 the same class gives the right city schema, so behavior diverges across the CI matrix. Unwrap types.MethodType via __func__ as well as partial layers.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unwrapped in e0e076a: annotations and the docstring now come from the function under bound methods as well as partials, so the description is free's.

The empty schema on 3.14 is what the instance accepts, though. There the partial binds like a method and gets the instance as its next argument, so inspect.signature(instance) is (). On 3.10 to 3.13 the class doesn't reach the tool: @activity.defn raises TypeError, because the SDK can't read a partial's type hints. So neither version can run this form, and I didn't add a test for it. I listed it under "Not covered".

while isinstance(func, functools.partial):
func = func.func
call.__annotations__ = typing.get_type_hints(func, include_extras=True)
call.__doc__ = func.__doc__ or fn.__doc__

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docstring fallback misbehaves in both directions. (1) It picks up synthesized class docs: @activity.defn @dataclass class DC: conditions: str with an undocumented __call__ yields description "DC(conditions: 'str')", the constructor signature, which also becomes the activity summary. (2) It reads only the class's own __doc__, so a documented base class is lost, whereas the pre-PR function_schema(cls) used inspect.getdoc, which walks the MRO: class Derived(Base) with a documented Base now gets description == ''. Suggest inspect.getdoc(func) and no dataclass-synthesized fallback.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in e0e076a. The description is __call__'s own docstring, then inspect.getdoc(cls), which walks base classes, but not for a dataclass, whose generated docstring is its constructor's signature. A dataclass with its own class docstring loses the fallback too; documenting __call__ covers that. I didn't call inspect.getdoc on __call__: for an undocumented one it walks up to object.__call__ and returns "Call self as a function.". test_callable_class_tool_schema now covers a dataclass and a class documented on its base class.

func = func.func
call.__annotations__ = typing.get_type_hints(func, include_extras=True)
call.__doc__ = func.__doc__ or fn.__doc__
fn = typing.cast(Callable, call)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The existing self branch now runs on this stand-in and can crash. When the bound __call__ still begins with self (for example functools.singledispatchmethod, whose __get__ returns a plain function), execution falls into the self branch below, which does getattr(fn, '__temporal_activity_definition') on the partial and raises AttributeError: 'functools.partial' object has no attribute '__temporal_activity_definition'. Nothing reads that copied attribute after ret is computed at the top of the function, so deleting that setattr removes the crash. typing.cast here is also redundant.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in e0e076a. The unused __temporal_activity_definition copy and the typing.cast are gone, and the shared self check now runs on the bound callable before the stand-in is built. A singledispatchmethod __call__ now offers city on 3.10, 3.12, 3.13 and 3.14.

events.append(e)

assert len(events) == 11
attrs = e.activity_task_scheduled_event_attributes

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This reads activity_task_scheduled_event_attributes on every history event and relies on protobuf returning an empty default message so activity_type.name == '' fails the startswith. The rest of the suite guards the same read with e.HasField("activity_task_scheduled_event_attributes") (for example further down this file). Using elif e.HasField(...) makes the intent explicit.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in e0e076a.



def test_partialmethod_call_tool_drops_only_self():
# On Python 3.10 the SDK runs an async partialmethod as sync, so test_tool_workflow can't

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two corrections for the partialmethod form. (1) The version note is too narrow: inspect.iscoroutinefunction(cls.__call__) is False on 3.10, 3.11 and 3.12 and True only from 3.13 (measured), so the SDK registers the activity as sync on all three, and Worker(activities=[PM()]) then demands an activity_executor. (2) Support is schema-only beyond JSON primitives: the SDK's _type_hints_from_func sees only the unannotated partialmethod wrapper (arg_types == [None]), so a non-primitive parameter such as a dataclass is decoded on the worker as a plain dict and __call__ fails with AttributeError: 'dict' object has no attribute .... Consider scoping the docstring claim and adjusting this comment to "before Python 3.13".

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I measured is_async=False on 3.10 and 3.12 and True on 3.13 and 3.14, so the comment now says "Before Python 3.13". The argument decoding is the SDK's, so I listed it under "Not covered" in the description. The fn docstring doesn't mention partialmethod, so I left it as is.

…orms

Address review findings on callable-class activity tools:

- Build one stand-in for both the class and the Class.method forms,
  with annotations resolved in the function's own module, so a
  Class.method with string annotations no longer fails with NameError.
  Drop the unused __temporal_activity_definition copy, which crashed
  when a bound __call__ still started with `self`.
- Raise ApplicationError("invalid_tool") for a class with no __call__,
  a __call__ whose signature can't be read, and keyword-only
  parameters, which the tool can't pass.
- Read annotations and docs from under bound methods as well as
  partials. Take the description from __call__'s own docstring, then
  the class's or a base class's, but not a dataclass's generated one.
- Add a ToolsWorkflow History whose model calls each class tool to the
  replay test.
@campoy

campoy commented Oct 29, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the review. The fixes are in e0e076a, and the inline items are answered in their threads.

  • Replay coverage: added tests/histories/tools-workflow-callable-class-history.json, recorded on this branch with the model calling ActivityWeatherClass, ActivityWeatherStaticCall and ActivityWeatherClassMethodCall. test_replay replays it. With main's activity_as_tool it fails with [TMPRL1100].
  • google_genai: agreed. From reading the code, its activity_as_tool also builds from inspect.signature(fn), the constructor for a class. That belongs in its own issue rather than this PR.
  • Title: now starts with Breaking:.

@campoy campoy changed the title openai_agents: build callable-class activity tools from __call__ Breaking: build callable-class activity tools from __call__ in OpenAI Agents Oct 29, 2026

@brianstrauch brianstrauch left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two remaining findings, reproduced against e0e076a on Python 3.14.4. make lint and 16 focused tests passed, including all nine replay histories. Additional probes fail for the cases below; corresponding baseline checks pass.

partial,
"__temporal_activity_definition",
getattr(fn, "__temporal_activity_definition"),
if params and params[0].name == "self":

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Preserve real parameters named self after descriptor binding

The class branch has already bound the descriptor, so a remaining parameter named self can be an actual input rather than a receiver. For example, both @staticmethod async def __call__(self: str) and @classmethod async def __call__(cls, self: str) are valid, but this check consumes their required input. I reproduced both producing properties == {}. The tool consequently schedules with no arguments, leaving the registered callable missing its required parameter.

As a baseline control, classes with a city: str constructor and either of these call signatures successfully receive "Tokyo" through the base implementation's tool. Please track whether an unbound receiver remains, instead of applying the name-only heuristic to every already-bound class callable, and cover both signatures.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in bfc2003. __call__ must now be a function, staticmethod, classmethod or functools.partialmethod, and binding any of them already drops the receiver, so the class form no longer strips a parameter named self. test_callable_class_tool_schema covers @staticmethod __call__(self: str) and @classmethod __call__(cls, self: str); both offer self.

func = func.func if isinstance(func, functools.partial) else func.__func__
doc = func.__doc__
# A dataclass without a docstring gets its constructor's signature as one.
if not doc and not dataclasses.is_dataclass(fn):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Preserve explicit dataclass documentation in the fallback

dataclasses.is_dataclass(fn) identifies all dataclasses, including ones with an explicitly written class docstring; it does not distinguish a generated constructor signature. Consequently, @activity.defn @dataclass class Weather with a class docstring such as "Get the weather for a city." and an undocumented __call__ now produces description == "". I reproduced the same class retaining its description on the base implementation. The guard also suppresses inherited documentation for dataclass subclasses.

This removes the description the model uses to select the tool. Please filter the synthesized constructor documentation while preserving explicit class documentation and the nearest documented base, and add a documented-dataclass regression case.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in bfc2003. Only the Name(...) docstring that dataclasses and namedtuples write is skipped now, so an explicit dataclass docstring is used again. ActivityWeatherDocumentedDataclass covers it.

I didn't keep the nearest documented base, though. As DABH found, walking the bases sends library docstrings (abc.ABC, typing.Generic) to the model and into the Activity summary. The description is now __call__'s docstring, or that of a __call__ it overrides, else the class's own.

@DABH DABH left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Second pass, on the current head. The previous round's keyword-only gap, the dead attribute copy, the postponed-annotation gap for Class.method, the 3.14 functools.partial binding, the raw errors for malformed classes, the missing replay history and the Breaking: subject are all addressed. Lint and the suite pass. The findings below were reproduced in the plugin venv on 3.14 (and on a 3.10 interpreter where the version matters).

Severity: P1 = wrong result or stuck run in realistic use; P2 = correctness or robustness in an edge case; P3 = docs, tests, cleanup.

Where the remaining problems cluster: the new docstring fallback (lines 197-200) now has three independent failure modes, and the plugin's own __call__ resolution differs from the SDK's _type_hints_from_func, so the schema the model fills and the types the Worker decodes can disagree with no error at tool-build time (__call__(this, loc: Loc) offers loc as an object schema while ret.arg_types is [None, Loc], so the worker decodes raw and __call__ receives a dict; a partialmethod __call__ gets arg_types=[None]). The right depth is one resolution shared by both sides (upstreaming this binding into temporalio.common._type_hints_from_func); until then the plugin holds ret.arg_types and can refuse the disagreement it can see (arg_types is None, a length mismatch with the schema's parameters, or a None hint for a non-primitive parameter) with invalid_tool, turning the PR's 'not covered' caveats into enforced ones. The keyword-only check at line 213 is compensation for the same SDK gap (@activity.defn validates the constructor's signature, not __call__'s); a comment naming that reason would help the next reader.

As noted last round, google_genai (workflow.py:79-91, byte-for-byte the pre-PR code), strands_agents (_temporal_activity_tool.py:28-29) and google_adk (workflow.py:159,222,252) still build a callable class's tool from inspect.signature(cls), so #205 stays reachable in three siblings; worth tracking issues.

func = func.func if isinstance(func, functools.partial) else func.__func__
doc = func.__doc__
# A dataclass without a docstring gets its constructor's signature as one.
if not doc and not dataclasses.is_dataclass(fn):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: the dataclass guard discards every dataclass docstring, not just the generated one. dataclasses only writes Name(sig) into __doc__ when the docstring is empty, so an explicit docstring is never the generated text, yet this guard skips the fallback for the whole dataclass family (and for plain subclasses of a dataclass, since is_dataclass is true through inherited __dataclass_fields__). @activity.defn @dataclass class Lookup: """Look up a parcel.\n\nArgs:\n parcel_id: ...""" with an undocumented __call__ gives tool.description == '' and loses the per-parameter descriptions and the activity summary, where the pre-PR function_schema(cls) returned 'Look up a parcel.' (reproduced, 8 of 8 finders). The test enshrines the symptom ((ActivityWeatherDataclass, "")) and has no explicit-docstring dataclass case. Compare fn.__doc__ against f"{fn.__name__}{inspect.signature(fn)}".replace(" -> None", "") (the exact string dataclasses writes) and skip only that; a typing.NamedTuple base produces a similar generated doc (NTTool(conditions,)) that is not filtered today.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in bfc2003 (see Brian's thread). A docstring is skipped only when it starts with Name( on a dataclass or a tuple subclass, which covers dataclasses and namedtuples; your NTTool now gets ''. I used the prefix instead of rebuilding the exact signature string. There are now cases for both a documented and an undocumented dataclass.

doc = func.__doc__
# A dataclass without a docstring gets its constructor's signature as one.
if not doc and not dataclasses.is_dataclass(fn):
doc = inspect.getdoc(fn)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: inspect.getdoc(fn) walks the class MRO, so unrelated library docstrings become the tool description and the activity summary. A callable class with no docstring on __call__ or itself that derives from abc.ABC yields 'Helper class that provides a standard way to create an ABC using\ninheritance.'; from typing.Generic, the multi-paragraph Generic docstring (reproduced). Any user class deriving from a documented framework base (clients, mixins, pydantic models) sends that base's docstring to the model and records it as the ScheduleActivity summary. If the intent is the nearest documented user base, walk fn.__mro__ explicitly; otherwise use only fn.__doc__.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in bfc2003: the class fallback is the class's own docstring only, so abc.ABC's and typing.Generic's no longer reach the model. ActivityWeatherStaticCall derives from abc.ABC and expects an empty description.

func = call
while isinstance(func, (functools.partial, types.MethodType)):
func = func.func if isinstance(func, functools.partial) else func.__func__
doc = func.__doc__

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: the inheritance is inverted. func.__doc__ reads only the resolved __call__'s own attribute, so a subclass that overrides a documented base __call__ without repeating the docstring gets '' (or the base class docstring from line 200), while inspect.getdoc semantics would inherit the method docstring as function_schema does for plain functions (reproduced: inspect.getdoc(vars(Derived)['__call__']) returns the base text). The same applies to the Class.method form overriding a documented base method. Walk fn.__mro__ for the first __call__ in vars(c) with a docstring; do not use inspect.getdoc(cls.__call__), which falls through to type.__call__ and returns 'Call self as a function.' for a docless __call__.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in bfc2003: the description is the first docstring among the __call__s in fn.__mro__ (functions, staticmethods, classmethods and partialmethods), then the class's own. ActivityWeatherDerived overrides a documented base __call__ and gets that docstring, not the base class's. The Class.method form now reads inspect.getdoc, so it inherits a base method's docstring too.

partial,
"__temporal_activity_definition",
getattr(fn, "__temporal_activity_definition"),
if params and params[0].name == "self":

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: the receiver is still dropped by the literal parameter name self, now on top of descriptor binding. For the Class.method form, a method whose first parameter is named anything else (async def get(this, city: str)) is exposed to the model as a required this parameter, to_call_args passes the model's value positionally, and the worker's bound method fails with TypeError: takes 2 positional arguments but 3 were given on every attempt (reproduced). For the class form the strip runs after __get__ binding, so @staticmethod async def __call__(self: str, city: str) has a real positional parameter removed: the tool offers only city, arg_types is [str, str], and the worker fails with 'missing 1 required positional argument' (reproduced). Bind the method form the same way (fn.__get__(object()) drops the first parameter regardless of its name, resolves string annotations in the method's module, and forwards __name__/__doc__, producing an identical FuncSchema for every tested form), and do not apply the name strip inside the class branch.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the class form, fixed in bfc2003 (see Brian's thread): no name check after binding.

For Class.method I kept the name check. The plugin gets a plain function there, the same as a module-level one, so it can't bind it without knowing it's a method. The name is also how main and the SDK's _type_hints_from_func find the receiver: for get(this, city: str) the SDK already records [None, str] as the argument types, whatever schema the plugin builds. I'd fix that in the SDK.

raise ApplicationError(
f"Activity class {fn.__name__} has no __call__ method", "invalid_tool"
)
call = method.__get__(object(), fn) if hasattr(method, "__get__") else method

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: only ValueError is translated, so descriptor and type errors still escape as raw exceptions. Binding to a bare object() makes C slot wrappers and any descriptor that type-checks its instance raise TypeError: @activity.defn class PartialSub(functools.partial): pass (nearest __call__ in the MRO is functools.partial's slot wrapper) raises TypeError: descriptor '__call__' for 'functools.partial' objects doesn't apply to a 'object' object, a cached/bound-caching descriptor whose __get__ checks isinstance(obj, cls) raises likewise, and __call__ = 42 (accepted by @activity.defn, which inspects the constructor) raises TypeError: 42 is not a callable object from inspect.signature (all reproduced on 3.10 and 3.14). Inside a workflow these are task failures retried forever rather than the promised invalid_tool. Catch TypeError alongside ValueError around the bind and the signature read. Two related version splits: on 3.13 functools.partial.__get__ exists but only emits FutureWarning: functools.partial will be a method descriptor... and returns itself, so a __call__ = functools.partial(...) class warns on every tool build (and fails under -W error); and a callable instance as __call__ (__call__ = Impl()) makes typing.get_type_hints raise TypeError on 3.10-3.12 while returning {} on 3.14. The hasattr(method, '__get__') fallback is otherwise untested; either bind unconditionally and document that __call__ must be a function, staticmethod or classmethod, or add a callable-instance case to test_callable_class_tool_schema.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in bfc2003. A __call__ that isn't a function, staticmethod, classmethod or functools.partialmethod over a function now gets invalid_tool before any binding. That covers your functools.partial subclass, a callable instance, a bare functools.partial on 3.14 and a singledispatchmethod (which last round offered city; it's now rejected). With no binding of a functools.partial, there's no 3.13 FutureWarning either. Binding, the signature read and get_type_hints are in one try that turns NameError, TypeError and ValueError into invalid_tool. On 3.10 to 3.14 @activity.defn already rejects __call__ = 42 here, with TypeError.

# own module, and the activity's name and docstring.
stand_in = functools.partial(call)
setattr(stand_in, "__name__", fn.__name__)
stand_in.__annotations__ = typing.get_type_hints(func, include_extras=True)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: get_type_hints here is outside the try/except, and on 3.10 it changes the method-path schema. (1) For a partialmethod __call__ whose underlying method has a TYPE_CHECKING-only annotation, @activity.defn accepts the class (the SDK sees the annotation-less _method wrapper) but this call raises NameError out of workflow code (reproduced), a task-failure loop rather than invalid_tool. (2) On Python 3.10 only, get_type_hints wraps a parameter with a None default in Optional[...], so class Svc: @activity.defn async def m(self, city: Annotated[str, 'The city'] = None) now yields anyOf [string, null] with the Annotated description lost (the SDK's _strip_annotated only strips a top-level Annotated), where the old partial (no __code__) gave {'type': 'string', 'description': 'The city'}; on 3.11+ both give str. Plain-function tools already behave this way on 3.10, so this only needs a decision: accept the 3.10 divergence for methods, or unwrap the 3.10 Optional when the raw annotation is not Optional.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(1) Fixed in bfc2003: get_type_hints is inside the try, so a TYPE_CHECKING-only annotation gets invalid_tool.

(2) I'll accept the 3.10 difference. Class.method now resolves hints the way plain function tools always have, so on 3.10 both wrap a None default in Optional; 3.12 and later keep str and the description.

call: Callable = fn
func: Callable = fn
doc = fn.__doc__
if inspect.isclass(fn):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: the instance form still crashes with a raw AttributeError. activity_as_tool(ActivityWeatherClass('Cloudy')), the exact object passed to Worker(activities=[...]), passes _Definition.from_callable (the definition is found through the class attribute), skips this branch, and dies in generate_func_documentation with AttributeError: 'ActivityWeatherClass' object has no attribute '__name__' (reproduced); inside a workflow that is a task failure retried forever, and only AgentsWorkflowError is in workflow_failure_exception_types. The PR body leaves this out of scope, which is fine, but then not inspect.isclass(fn) and not inspect.isroutine(fn) should raise invalid_tool naming the three supported forms, or map the instance to type(fn). The docstring's Raises: section (line 150) also still lists only the 'not properly decorated' case and none of the new invalid_tool cases (no __call__, unreadable signature, keyword-only parameters).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in bfc2003: passing an instance of a callable-class activity raises ApplicationError("Pass the activity class X, not an instance of it", "invalid_tool"), and Raises: lists the new invalid_tool cases. test_invalid_callable_class_tool covers the instance.

call = functools.partial(call, None)
params = params[1:]
# run_activity passes only positional arguments.
if any(p.kind is inspect.Parameter.KEYWORD_ONLY for p in params):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3 simplification: this check, the params = params[1:] slice that feeds it, and the ValueError translation at line 203 are reachable only for classes. @activity.defn already runs inspect.signature(fn) on a decorated function or method and raises TypeError('Activity cannot have keyword-only arguments'), so for every non-class form these branches are dead; for a class, after method.__get__(object(), fn) the first parameter is never self, so the strip at line 209 is unreachable in the class branch. Moving the try/except and the keyword-only check inside if inspect.isclass(fn): (with the comment that the SDK validated the constructor's signature, not __call__'s) and binding the method form with fn.__get__(object()) removes the call is not fn gate, the two stacked partials, the hand-copied __name__/__annotations__/__doc__ trio, and the call/func/doc prologue initialised at the top and overwritten; a restructured version produced identical FuncSchema fields and identical ApplicationError messages for all 15 forms tried, including the three invalid classes. An alternative that also removes the pre-resolved annotations dict: one stand-in carrying __wrapped__ and __signature__ (functools.update_wrapper plus stand_in.__signature__ = sig.replace(parameters=params)), so function_schema's own inspect.signature/get_type_hints handle every form and the plugin no longer has to stay in lock-step with what the Agents SDK reads.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly done in bfc2003. Binding, the signature read and the keyword-only check (now with a comment on why it's needed) live in the class branch. The method form keeps its own self check (see the thread on line 209). Both build their stand-in with one _tool_stand_in helper, so the call is not fn gate and the shared prologue are gone. I kept the resolved-annotations dict rather than __wrapped__ and __signature__, since it's the version tested across these forms.

stand_in.__annotations__ = typing.get_type_hints(func, include_extras=True)
stand_in.__doc__ = doc
fn = stand_in
schema = function_schema(fn)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3 (pre-existing, adjacent): function_schema(fn) never forwards the caller's strict_json_schema, so ensure_strict_json_schema always runs and raises a raw agents.exceptions.UserError ('additionalProperties should not be set for object types ...') for a dict[str, Any] or **kwargs parameter even when the user passed strict_json_schema=False (reproduced with a class __call__(self, city: str, **extra: str)). The new keyword-only guard also does not cover VAR_KEYWORD, whose values run_activity would drop. Pass strict_json_schema=strict_json_schema through and treat VAR_KEYWORD like keyword-only.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

**kwargs in __call__ is now rejected with the keyword-only check (ActivityWeatherKwargsCall). Passing strict_json_schema through to function_schema changes the schema of every tool built with strict_json_schema=False, so I'd rather do that in its own PR.

# test_tool_workflow can't run this form; the tool's schema is the part this plugin builds.
(ActivityWeatherPartialMethodCall, "Get the weather for a given city."),
# Not the docstring a dataclass writes from its constructor.
(ActivityWeatherDataclass, ""),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3 (tests): the dataclass case pins the empty description (see line 199); add an explicit-docstring dataclass expecting its docstring and a generated-docstring dataclass expecting the base/empty fallback. Running ActivityWeatherStaticCall and ActivityWeatherClassMethodCall through the end-to-end workflow duplicates what this parametrized schema test already proves per binding form (the end-to-end leg, run_activity passing positionals and the SDK calling instance(*args), is the same code path for all three), at the cost of coordinated edits in five places plus the new 1,081-line replay history; keeping one class form in ToolsWorkflow and adding the other two here would be cheaper while the history file is still new. And test_tool_workflow now has 17 copies of the six-line index-based assert block; a decoded results list zipped against an expected list reports the failing index and makes a new tool two list entries.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in bfc2003: the static and class forms moved from ToolsWorkflow into test_callable_class_tool_schema, the History is re-recorded with only ActivityWeatherClass (23 events), and there are documented and undocumented dataclass cases. I left the index-based asserts in test_tool_workflow as they are. That's the test's existing style, and this PR now adds two of them.

…n fallback

Address review findings on callable-class activity tools:

- Accept a __call__ that is a function, staticmethod, classmethod or
  functools.partialmethod, and raise ApplicationError("invalid_tool")
  for anything else. Binding these already drops the receiver, so the
  class form no longer strips a real parameter named `self`.
- Turn NameError and TypeError, as well as ValueError, from binding,
  the signature read and type hints into invalid_tool, and reject
  **kwargs in __call__ like keyword-only arguments.
- Reject an instance of a callable-class activity, and list the
  invalid_tool cases in the docstring.
- Take the description from __call__ or a __call__ it overrides, else
  the class's own docstring, skipping only the one dataclasses and
  namedtuple generate. Base classes' docstrings, such as abc.ABC's, are
  no longer used.
- Test the static and class forms in the schema test instead of
  ToolsWorkflow, and re-record the History with the one class tool.
@campoy

campoy commented Oct 29, 2026

Copy link
Copy Markdown
Contributor Author

Thanks both. The fixes are in bfc2003, and the inline items are answered in their threads.

  • SDK argument types: the cases where the schema and ret.arg_types disagree (a receiver not named self, partialmethod's [None]) come from temporalio.common._type_hints_from_func. It also drives workflow.execute_activity_class, so I'd fix it in the SDK rather than mirror its rules here. The keyword-only check now has a comment saying why the plugin needs it.
  • Sibling plugins: agreed that google_genai, strands_agents and google_adk need their own issues for the same bug.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] openai_agents: activity_as_tool builds a callable-class Activity's tool from its constructor

3 participants