Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed metadata injection for local actions #1247

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Fixed metadata injection for local actions #1247

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9f05821
Fixed metadata injection for local actions
mkrjn99 Feb 5, 2025
fd586b3
Merge branch 'master' into fix/1246
sohamganatra Feb 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions python/composio/tools/toolset.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,11 @@ def add_metadata(self, action: Action, metadata: t.Optional[t.Dict]) -> t.Dict:
metadata.update(self._get_metadata(key=action))
return metadata

def add_local_action_metadata(self, action_type: ActionType, metadata: t.Optional[t.Dict]) -> t.Dict:
metadata = metadata or {}
metadata.update(self._get_metadata(key=action_type))
return metadata


class FileIOHelper(WithLogger):

Expand Down Expand Up @@ -1841,6 +1846,7 @@ def execute_action(
:param connected_account_id: Connection ID for executing the remote action
:return: Output object from the function call
"""
original_action_type = action
action = Action(action)
if self._version_lock is not None:
(action,) = self._version_lock.apply(actions=[action])
Expand All @@ -1860,6 +1866,12 @@ def execute_action(
action=action,
Copy link
Contributor

Choose a reason for hiding this comment

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

The new add_local_action_metadata seems to duplicate parts of add_metadata. In execute_action, local actions get metadata injected first via add_local_action_metadata and then via add_metadata. Is this double injection intentional? Consider conditionally skipping add_metadata for local actions.

request=params,
)

if action.is_local:
metadata = self._processor_helpers.add_local_action_metadata(
action_type=original_action_type, metadata=metadata
)

if not action.is_runtime:
params = self._processor_helpers.process_request(
action=action, request=params
Expand Down
33 changes: 33 additions & 0 deletions python/tests/test_tools/test_metadata_injection.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
from pydantic import BaseModel, Field

from composio import ComposioToolSet, action


class RequestClass(BaseModel):
pass


class ResponseClass(BaseModel):
pass

metadata_value = None
Copy link
Collaborator

Choose a reason for hiding this comment

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

Consider using a more descriptive name for the global variable and adding type hints:

test_captured_metadata: Optional[str] = None  # Used to capture metadata in test scenarios

This makes the test code more maintainable and its purpose clearer.

API_KEY_KEY = "api_key"
API_KEY_VALUE = "api_key_value"

@action(toolname="tool")
def action_metadata_injection(request: RequestClass, metadata: dict) -> ResponseClass:
"""
Test metadata injection.
"""
global metadata_value
metadata_value = metadata[API_KEY_KEY]
return ResponseClass()


def test_metadata_injection() -> None:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Consider adding more test cases to cover different scenarios:

  1. Test with None metadata
  2. Test with existing metadata to ensure proper merging
  3. Test with different action types

This would improve test coverage and ensure robustness of the metadata injection functionality.

toolset = ComposioToolSet(metadata={action_metadata_injection: {API_KEY_KEY: API_KEY_VALUE}})
toolset.execute_action(
action=action_metadata_injection,
params={},
)
assert (metadata_value == API_KEY_VALUE)