Fix(Tool): Class method with @tool decorator is also accepted as a valid Tool.

[Ponprabhakar-pg]

Description

Problem

When using a class method decorated with @tool and passing it as a tool to an Agent instance, the system raises an "unrecognized tool specification" error. This occurs because bound methods (class methods accessed through an instance) are not properly recognized by the Agent's tool handling mechanism.

Solution

This PR fixes the issue by improving the tool registration process to properly handle bound methods. The implementation now correctly identifies and processes class methods that have been decorated with @tool when they're passed as instance methods.

Changes

Enhanced tool registration to recognize bound methods
Added proper handling for class methods decorated with @tool
Ensured compatibility with both standalone functions and class methods

Testing

Verified that the following pattern now works correctly:

class MyClass:
    @tool
    def my_tool_method(self, query: str):
        # method implementation
        return result

instance = MyClass()
agent = Agent(tools=[instance.my_tool_method])

This change maintains backward compatibility while extending support for object-oriented usage patterns.

Related Issues

#199
[Link to related issues using #issue-number format]

Documentation PR

Not needed.
[Link to related associated PR in the agent-docs repo]

Type of Change

• Bug fix

Testing

[How have you tested the change?]

• hatch fmt --linter
• hatch fmt --formatter
• hatch test --all
• Verify that the changes do not break functionality or introduce warnings in consuming repositories: agents-docs, agents-tools, agents-cli

Checklist

• I have read the CONTRIBUTING document
• I have added tests that prove my fix is effective or my feature works
• I have updated the documentation accordingly
• I have added an appropriate example to the documentation to outline the feature
• My changes generate no new warnings
• Any dependent changes have been merged and published

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[dbschmigelski]

Hi, thanks for the contribution.

I believe you should be able to remove some of the tracing changes here with a rebase and reseting the hatch env. I believe open-telemetry/opentelemetry-python#4615 resolved some of the issue.

For the change to registry.py would you mind adding a unit test so we can prevent regressions.

[Ponprabhakar-pg]

Hi, thanks for the contribution.

I believe you should be able to remove some of the tracing changes here with a rebase and reseting the hatch env. I believe open-telemetry/opentelemetry-python#4615 resolved some of the issue.

For the change to registry.py would you mind adding a unit test so we can prevent regressions.

Hi, thanks for the feedback!

I've rebased the branch and reset the hatch environment as suggested. The tracing-related changes have been cleaned up accordingly.

I've also added a unit test for the changes in registry.py to help guard against regressions.

Please let me know if there's anything else you'd like me to adjust!

[dbschmigelski]

Sorry for the churn but there are a few nits.

1. I appreciate the integration test but I'm not sure it is necessary. I think we should move to the tests/ directory and just include a unit test. I think the coverage there, particularly if we test direct tool access should be sufficient
2. Instead of using print statements to separate tests lets use separate methods. You can look at the other unit tests like for style https://github.com/strands-agents/sdk-python/blob/main/tests/strands/tools/test_registry.py. I think the separate test names will be self documentating instead of the prints here. Adding to that I'm not sure we need the logger in this case like you have above. And we likely want to use pytest fixtures for each method - again look to other unit tests in the package for style.

Thanks and sorry for the added revisions

[Ponprabhakar-pg]

Sorry for the churn but there are a few nits.

1. I appreciate the integration test but I'm not sure it is necessary. I think we should move to the tests/ directory and just include a unit test. I think the coverage there, particularly if we test direct tool access should be sufficient
2. Instead of using print statements to separate tests lets use separate methods. You can look at the other unit tests like for style https://github.com/strands-agents/sdk-python/blob/main/tests/strands/tools/test_registry.py. I think the separate test names will be self documentating instead of the prints here. Adding to that I'm not sure we need the logger in this case like you have above. And we likely want to use pytest fixtures for each method - again look to other unit tests in the package for style.

Thanks and sorry for the added revisions

Hi Dean,
I've reverted the integration test case and added a new unit test case in appropriate folder with pytest fixtures as mentioned, let me know if I need to updated anything further?

Thanks!

[dbschmigelski]

Thanks, I apologize for not giving a more formal review on the actual strategy. I'm inclined to agree with @thinkinginmath's comment #199 (comment)

the SDK cannot register it as a tool because it’s expecting a function object, not a bound method of a python class.
you should either have a standalone function, or change your method to staticmethod
@staticmethod
@tool
def reasoning_plan_generator(query: str): // note: no reference to self.

We have the draft PR #162 which I believe will more comprehensively capture the requirements of the issue.

I am going to close this PR then mark the draft PR in the issue you created. In the draft PR what we want to align on is syntax mostly.

Again, I apologize for the back and forth, as I should have identified this earlier.