fix: tool name showing as `function_template` in phidata #1213

Devanshusisodiya · 2025-01-20T15:15:23Z

Phidata toolkit was showing composio toolnames as function_template. Changed function signature to the lowercase enum to fix this.

Important

Fix tool name display issue by setting __name__ to lowercase in function_template and update return types in get_actions and get_tools.

Behavior:
- Fix tool name display issue by setting __name__ to lowercase in function_template in toolset.py.
- Update get_actions and get_tools in toolset.py to return t.List[t.Union[Tool, Toolkit, t.Callable, t.Dict, Function]].
Imports:
- Correct import for Assistant in phidata_demo.py to from phi.assistant.assistant import Assistant.

^{This description was created by}^{for 59f4d77. It will automatically update as commits are pushed.}

vercel · 2025-01-20T15:15:28Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
composio	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jan 20, 2025 3:30pm

entelligence-ai-pr-reviews · 2025-01-20T15:16:04Z

python/plugins/phidata/composio_phidata/toolset.py

@@ -83,6 +85,7 @@ def function_template(*args, **kwargs) -> str:
        # Apply the signature and annotations to the function
        func.__signature__ = sig
        func.__annotations__ = annotations
+        func.__setattr__("__name__", name.lower())


The __setattr__ call is unsafe and could allow attribute injection. Use a direct attribute assignment with func.__name__ = name.lower() instead.

📝 Committable Code Suggestion

‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change

func.__setattr__("__name__", name.lower())

func.__name__ = name.lower()

can't assign __name__ attribute of type ToolFunction defined by phidata

entelligence-ai-pr-reviews · 2025-01-20T15:16:05Z

python/plugins/phidata/composio_phidata/toolset.py

+    def get_actions(
+        self, actions: t.Sequence[ActionType]
+    ) -> t.List[t.Union[Tool, Toolkit, t.Callable, t.Dict, Function]] | None:


Return type annotation uses | operator which requires Python 3.10+. Should use Union from typing module for better compatibility across Python versions.

📝 Committable Code Suggestion

‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change

def get_actions(

self, actions: t.Sequence[ActionType]

) -> t.List[t.Union[Tool, Toolkit, t.Callable, t.Dict, Function]] | None:

def get_actions(

self, actions: t.Sequence[ActionType]

) -> t.Union[t.List[t.Union[Tool, Toolkit, t.Callable, t.Dict, Function]], None]:

entelligence-ai-pr-reviews · 2025-01-20T15:16:06Z

python/plugins/phidata/composio_phidata/toolset.py

        *,
        processors: t.Optional[ProcessorsType] = None,
        check_connected_accounts: bool = True,
-    ) -> t.List[Toolkit]:
+    ) -> t.List[t.Union[Tool, Toolkit, t.Callable, t.Dict, Function]] | None:


Returning None in get_actions() and get_tools() could cause runtime errors if callers expect a List. Should return empty list instead of None for consistency.

📝 Committable Code Suggestion

‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change

*,

processors: t.Optional[ProcessorsType] = None,

check_connected_accounts: bool = True,

) -> t.List[Toolkit]:

) -> t.List[t.Union[Tool, Toolkit, t.Callable, t.Dict, Function]] | None:

) -> t.List[t.Union[Tool, Toolkit, t.Callable, t.Dict, Function]]:

not returning None anymore, will always return List[Tool | ToolKit, Callable, Function]

ellipsis-dev

❌ Changes requested. Reviewed everything up to d7f52f2 in 46 seconds

More details

Looked at 60 lines of code in 2 files
Skipped 0 files when reviewing.
Skipped posting 1 drafted comments based on config settings.

1. python/plugins/phidata/composio_phidata/toolset.py:136

Draft comment:
The return type in the function signature includes | None, but the docstring specifies List[Toolkit]. Consider updating the docstring to reflect the possibility of a None return value.
Reason this comment was not posted:
Marked as duplicate.

Workflow ID: wflow_IFTo5eEFw3zU20aG

Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

ellipsis-dev · 2025-01-20T15:16:21Z

python/plugins/phidata/composio_phidata/toolset.py

-    def get_actions(self, actions: t.Sequence[ActionType]) -> t.List[Toolkit]:
+    def get_actions(
+        self, actions: t.Sequence[ActionType]
+    ) -> t.List[t.Union[Tool, Toolkit, t.Callable, t.Dict, Function]] | None:


The return type in the function signature includes | None, but the docstring specifies List[Toolkit]. Consider updating the docstring to reflect the possibility of a None return value.

not returning None anymore, will always return List[Tool | ToolKit, Callable, Function]

shreysingla11 · 2025-01-20T15:16:56Z

python/plugins/phidata/composio_phidata/toolset.py

-    def get_actions(self, actions: t.Sequence[ActionType]) -> t.List[Toolkit]:
+    def get_actions(
+        self, actions: t.Sequence[ActionType]
+    ) -> t.List[t.Union[Tool, Toolkit, t.Callable, t.Dict, Function]] | None:


The return type annotation t.List[t.Union[Tool, Toolkit, t.Callable, t.Dict, Function]] | None indicates this method can return None, but there's no documentation or clear indication of when None would be returned. Consider:

Adding documentation about when None is returned

Adding runtime validation for the None case

Updating the docstring to reflect this possibility

not returning None anymore, will always return List[Tool | ToolKit, Callable, Function]

shreysingla11 · 2025-01-20T15:17:05Z

python/plugins/phidata/phidata_demo.py



 toolset = ComposioToolSet()
 composio_tools = toolset.get_tools(
    actions=[Action.GITHUB_STAR_A_REPOSITORY_FOR_THE_AUTHENTICATED_USER]
 )

-assistant = Assistant(tools=composio_tools, show_tool_calls=True)
+assistant = Assistant(run_id=None, tools=composio_tools, show_tool_calls=True)


The addition of run_id=None parameter should be documented. Consider adding a comment explaining why this parameter is needed and what it does. This will help future maintainers understand the purpose of this change.

kaavee315 · 2025-01-20T15:17:14Z

python/plugins/phidata/composio_phidata/toolset.py

-    def get_actions(self, actions: t.Sequence[ActionType]) -> t.List[Toolkit]:
+    def get_actions(
+        self, actions: t.Sequence[ActionType]
+    ) -> t.List[t.Union[Tool, Toolkit, t.Callable, t.Dict, Function]] | None:


Why this chance?

assumed lowercase as mentioned in phidata's chat.

shreysingla11 · 2025-01-20T15:17:20Z

Code Review Summary

Changes Overview

Fixed tool name display by adding proper name attribution using __setattr__
Updated return type annotations in get_actions and get_tools methods
Modified imports for better specificity and added new tool-related imports
Updated Assistant initialization with run_id parameter

Positive Aspects

✅ Fixes the core issue of tool name display
✅ Improves type safety with more specific return types
✅ Better code formatting and readability
✅ Maintains backward compatibility with proper deprecation notices

Suggestions for Improvement

Document when methods might return None in the new return type annotations
Add explanation for the run_id=None parameter in phidata_demo.py
Consider updating docstrings to reflect the new return types

Overall Assessment

The changes are well-structured and solve the immediate issue while improving type safety. The code quality is good, with proper attention to backward compatibility. With the suggested documentation improvements, this PR would be ready for merge.

Rating: 8/10 - Good quality changes with room for minor documentation improvements.

ellipsis-dev

👍 Looks good to me! Incremental review on 59f4d77 in 33 seconds

More details

Looked at 36 lines of code in 2 files
Skipped 0 files when reviewing.
Skipped posting 1 drafted comments based on config settings.

1. python/plugins/phidata/composio_phidata/toolset.py:111

Draft comment:
The return type hint should not include | None since the method always returns a list. Apply this change to both get_actions and get_tools methods.
Reason this comment was not posted:
Comment looked like it was already resolved.

Workflow ID: wflow_21N8ZFymPk9sVQIQ

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

fix: tool name not showing as

d7f52f2

Devanshusisodiya requested a review from angrybayblade as a code owner January 20, 2025 15:15

entelligence-ai-pr-reviews bot reviewed Jan 20, 2025

View reviewed changes

ellipsis-dev bot reviewed Jan 20, 2025

View reviewed changes

shreysingla11 reviewed Jan 20, 2025

View reviewed changes

kaavee315 reviewed Jan 20, 2025

View reviewed changes

fix: review fixes

59f4d77

vercel bot deployed to Preview January 20, 2025 15:30 View deployment

ellipsis-dev bot reviewed Jan 20, 2025

View reviewed changes

Devanshusisodiya changed the title ~~fix: tool name not showing as function_template~~ fix: tool name showing as function_template Jan 20, 2025

Devanshusisodiya changed the title ~~fix: tool name showing as function_template~~ fix: tool name showing as function_template in phidata Jan 20, 2025

kaavee315 approved these changes Jan 20, 2025

View reviewed changes

kaavee315 merged commit 620689c into master Jan 20, 2025
25 of 26 checks passed

kaavee315 deleted the phidata_fix branch January 20, 2025 18:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: tool name showing as `function_template` in phidata #1213

fix: tool name showing as `function_template` in phidata #1213

Devanshusisodiya commented Jan 20, 2025 •

edited by ellipsis-dev bot

Loading

vercel bot commented Jan 20, 2025 •

edited

Loading

entelligence-ai-pr-reviews bot Jan 20, 2025

Devanshusisodiya Jan 20, 2025

entelligence-ai-pr-reviews bot Jan 20, 2025

Devanshusisodiya Jan 20, 2025

entelligence-ai-pr-reviews bot Jan 20, 2025

Devanshusisodiya Jan 20, 2025

ellipsis-dev bot left a comment

ellipsis-dev bot Jan 20, 2025

Devanshusisodiya Jan 20, 2025

shreysingla11 Jan 20, 2025

Devanshusisodiya Jan 20, 2025

shreysingla11 Jan 20, 2025

Devanshusisodiya Jan 20, 2025

kaavee315 Jan 20, 2025

Devanshusisodiya Jan 20, 2025

shreysingla11 commented Jan 20, 2025

ellipsis-dev bot left a comment

	func.__setattr__("__name__", name.lower())
	func.__name__ = name.lower()

fix: tool name showing as function_template in phidata #1213

fix: tool name showing as function_template in phidata #1213

Conversation

Devanshusisodiya commented Jan 20, 2025 • edited by ellipsis-dev bot Loading

vercel bot commented Jan 20, 2025 • edited Loading

entelligence-ai-pr-reviews bot Jan 20, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

entelligence-ai-pr-reviews bot Jan 20, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

entelligence-ai-pr-reviews bot Jan 20, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ellipsis-dev bot left a comment

Choose a reason for hiding this comment

ellipsis-dev bot Jan 20, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shreysingla11 commented Jan 20, 2025

Code Review Summary

Changes Overview

Positive Aspects

Suggestions for Improvement

Overall Assessment

ellipsis-dev bot left a comment

Choose a reason for hiding this comment

fix: tool name showing as `function_template` in phidata #1213

fix: tool name showing as `function_template` in phidata #1213

Devanshusisodiya commented Jan 20, 2025 •

edited by ellipsis-dev bot

Loading

vercel bot commented Jan 20, 2025 •

edited

Loading