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

Improve UX for action processor #709

Merged
merged 12 commits into from
Oct 18, 2024
Merged

Improve UX for action processor #709

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
017a1cf
Add test for pre and post processors
tushar-composio Oct 15, 2024
b6252ea
sort imports
tushar-composio Oct 15, 2024
4f9f492
Don't expect processors to return correct data type
tushar-composio Oct 16, 2024
9f39a61
Add `processors` field on `get_tools` and `execute_action`
tushar-composio Oct 17, 2024
d1b40fc
Merge branch 'master' into ENG-1726
tushar-composio Oct 17, 2024
f8f426e
formatting
tushar-composio Oct 17, 2024
36e2317
add a pytest.warns check
tushar-composio Oct 17, 2024
bf13af4
isort
tushar-composio Oct 17, 2024
2fc40f3
remove unused import
tushar-composio Oct 17, 2024
431ce02
install langchain extension during testing
tushar-composio Oct 17, 2024
8d2ce33
fix test
tushar-composio Oct 17, 2024
fe574c6
address review comments
tushar-composio Oct 18, 2024
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
53 changes: 46 additions & 7 deletions python/composio/tools/toolset.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,9 @@
MetadataType = t.Dict[_KeyType, t.Dict]
ParamType = t.TypeVar("ParamType")

# Enable deprecation warnings
warnings.simplefilter("always", DeprecationWarning)
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 a type hint for the warnings module to improve code readability and maintainability.



class ProcessorsType(te.TypedDict):
"""Request and response processors."""
Expand Down Expand Up @@ -251,18 +254,24 @@ def _limit_file_search_response(response: t.Dict) -> t.Dict:
self._api_key = None
self.logger.debug("`api_key` is not set when initializing toolset.")

self._processors = (
processors
if processors is not None
else {"post": {}, "pre": {}, "schema": {}}
)
if processors is not None:
warnings.warn(
Copy link
Collaborator

Choose a reason for hiding this comment

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

The warning message here could be more specific. Instead of just mentioning 'processors', it could say "Setting 'processors' directly on the ToolSet instance is deprecated..."

"Setting 'processors' on the ToolSet is deprecated, they should"
"be provided to the 'get_tools()' method instead.",
DeprecationWarning,
stacklevel=2,
)
self._processors: ProcessorsType = processors
else:
self._processors = {"post": {}, "pre": {}, "schema": {}}

self._metadata = metadata or {}
self._workspace_id = workspace_id
self._workspace_config = workspace_config
self._local_client = LocalClient()

if len(kwargs) > 0:
self.logger.info(f"Extra kwards while initializing toolset: {kwargs}")
self.logger.info(f"Extra kwargs while initializing toolset: {kwargs}")

self.logger.debug("Loading local tools")
load_local_tools()
Expand Down Expand Up @@ -542,7 +551,10 @@ def _serialize_execute_params(self, param: ParamType) -> ParamType:
return [self._serialize_execute_params(p) for p in param] # type: ignore

if isinstance(param, dict):
return {key: self._serialize_execute_params(val) for key, val in param.items()} # type: ignore
return {
key: self._serialize_execute_params(val) # type: ignore
for key, val in param.items()
}

raise ValueError(
"Invalid value found for execute parameters"
Expand Down Expand Up @@ -593,6 +605,12 @@ def _process(
f" through: {processor.__name__}"
)
data = processor(data)
# Users may not respect our type annotations and return something that isn't a dict.
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 comprehensive error handling for cases where processors might fail or return unexpected types. While you've added a warning for unexpected return types, it might be beneficial to have a more robust error handling strategy, possibly including custom exceptions or fallback behavior.

# If that happens we should show a friendly error message.
if not isinstance(data, t.Dict):
raise TypeError(
f"Expected {type_}-processor to return 'dict', got {type(data).__name__!r}"
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would suggest log a warning instead raising error since this can interrupt the agent execution and agent might get stuck in a loop, also as long as the return object is JSON serialisable there won't be any issue so just a warning should be fine. We can improve the documentation around this.

return data

def _process_request(self, action: Action, request: t.Dict) -> t.Dict:
Expand Down Expand Up @@ -628,6 +646,22 @@ def _process_schema_properties(self, action: Action, properties: t.Dict) -> t.Di
type_="schema",
)

def _merge_processors(self, processors: ProcessorsType) -> 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 a type hint for the processors parameter in the _merge_processors method signature.

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 a type hint for the processors parameter in the _merge_processors method to improve code readability and maintainability.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The _merge_processors method modifies the self._processors dictionary in-place. Consider returning a new dictionary instead of modifying the existing one to maintain immutability and prevent potential side effects.

Copy link
Collaborator

Choose a reason for hiding this comment

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

In the _merge_processors method, consider adding a type check for the processors parameter to ensure it's of type ProcessorsType. This will help catch potential errors early and improve type safety.

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 a docstring for the new _merge_processors method to explain its purpose and usage. This will help other developers understand the method's functionality and how to use it correctly.

for processor_type in self._processors.keys():
if processor_type in processors:
processor_type = t.cast(
te.Literal["pre", "post", "schema"], processor_type
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can extract te.Literal["pre", "post", "schema"] as ProcessorType

)
new_processors = processors[processor_type]

if processor_type in self._processors:
existing_processors = self._processors[processor_type]
else:
existing_processors = {}
self._processors[processor_type] = existing_processors

existing_processors.update(new_processors)

@_record_action_if_available
def execute_action(
self,
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 type hints for the processors parameter in the execute_action method. This will improve type checking and make the method signature more consistent with other parts of the codebase.

Expand All @@ -637,6 +671,8 @@ def execute_action(
entity_id: t.Optional[str] = None,
connected_account_id: t.Optional[str] = None,
text: t.Optional[str] = None,
*,
processors: t.Optional[ProcessorsType] = None,
) -> t.Dict:
"""
Execute an action on a given entity.
Expand All @@ -651,6 +687,9 @@ def execute_action(
"""
action = Action(action)
params = self._serialize_execute_params(param=params)
if processors is not None:
self._merge_processors(processors)

if not action.is_runtime:
params = self._process_request(action=action, request=params)
metadata = self._add_metadata(action=action, metadata=metadata)
Expand Down
5 changes: 5 additions & 0 deletions python/plugins/camel/composio_camel/toolset.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
from composio.constants import DEFAULT_ENTITY_ID
from composio.tools import ComposioToolSet as BaseComposioToolSet
from composio.tools.schema import OpenAISchema, SchemaType
from composio.tools.toolset import ProcessorsType


# pylint: enable=E0611
Expand Down Expand Up @@ -151,6 +152,8 @@ def get_tools(
apps: t.Optional[t.Sequence[AppType]] = None,
tags: t.Optional[t.List[TagType]] = None,
entity_id: t.Optional[str] = None,
*,
processors: t.Optional[ProcessorsType] = None,
) -> t.List[OpenAIFunction]:
"""
Get composio tools wrapped as Camel `OpenAIFunction` objects.
Expand All @@ -163,6 +166,8 @@ def get_tools(
:return: Composio tools wrapped as `OpenAIFunction` objects
"""
self.validate_tools(apps=apps, actions=actions, tags=tags)
if processors is not None:
self._merge_processors(processors)
return [
self._wrap_tool( # type: ignore
t.cast(
Expand Down
5 changes: 5 additions & 0 deletions python/plugins/claude/composio_claude/toolset.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from composio.constants import DEFAULT_ENTITY_ID
from composio.tools import ComposioToolSet as BaseComposioToolSet
from composio.tools.schema import ClaudeSchema, SchemaType
from composio.tools.toolset import ProcessorsType


class ComposioToolSet(
Expand Down Expand Up @@ -94,6 +95,8 @@ def get_tools(
actions: t.Optional[t.Sequence[ActionType]] = None,
apps: t.Optional[t.Sequence[AppType]] = None,
tags: t.Optional[t.List[TagType]] = None,
*,
processors: t.Optional[ProcessorsType] = None,
) -> t.List[ToolParam]:
"""
Get composio tools wrapped as OpenAI `ChatCompletionToolParam` objects.
Expand All @@ -105,6 +108,8 @@ def get_tools(
:return: Composio tools wrapped as `ChatCompletionToolParam` objects
"""
self.validate_tools(apps=apps, actions=actions, tags=tags)
if processors is not None:
self._merge_processors(processors)
return [
ToolParam(
**t.cast(
Expand Down
5 changes: 5 additions & 0 deletions python/plugins/griptape/composio_griptape/toolset.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

from composio import Action, ActionType, AppType, TagType
from composio.tools import ComposioToolSet as BaseComposioToolSet
from composio.tools.toolset import ProcessorsType
from composio.utils.shared import PYDANTIC_TYPE_TO_PYTHON_TYPE


Expand Down Expand Up @@ -135,6 +136,8 @@ def get_tools(
apps: t.Optional[t.Sequence[AppType]] = None,
tags: t.Optional[t.List[TagType]] = None,
entity_id: t.Optional[str] = None,
*,
processors: t.Optional[ProcessorsType] = None,
) -> t.List[BaseTool]:
"""
Get composio tools wrapped as GripTape `BaseTool` type objects.
Expand All @@ -147,6 +150,8 @@ def get_tools(
:return: Composio tools wrapped as `BaseTool` objects
"""
self.validate_tools(apps=apps, actions=actions, tags=tags)
if processors is not None:
self._merge_processors(processors)
return [
self._wrap_tool(
schema=tool.model_dump(
Expand Down
5 changes: 5 additions & 0 deletions python/plugins/langchain/composio_langchain/toolset.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

from composio import Action, ActionType, AppType, TagType
from composio.tools import ComposioToolSet as BaseComposioToolSet
from composio.tools.toolset import ProcessorsType
from composio.utils.pydantic import parse_pydantic_error
from composio.utils.shared import (
get_signature_format_from_schema_params,
Expand Down Expand Up @@ -154,6 +155,8 @@ def get_tools(
apps: t.Optional[t.Sequence[AppType]] = None,
tags: t.Optional[t.List[TagType]] = None,
entity_id: t.Optional[str] = None,
*,
processors: t.Optional[ProcessorsType] = None,
) -> t.Sequence[StructuredTool]:
"""
Get composio tools wrapped as Langchain StructuredTool objects.
Expand All @@ -166,6 +169,8 @@ def get_tools(
:return: Composio tools wrapped as `StructuredTool` objects
"""
self.validate_tools(apps=apps, actions=actions, tags=tags)
if processors is not None:
self._merge_processors(processors)
return [
self._wrap_tool(
schema=tool.model_dump(
Expand Down
5 changes: 5 additions & 0 deletions python/plugins/llamaindex/composio_llamaindex/toolset.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from composio import Action, ActionType, AppType
from composio import ComposioToolSet as BaseComposioToolSet
from composio import TagType
from composio.tools.toolset import ProcessorsType
from composio.utils.shared import get_pydantic_signature_format_from_schema_params


Expand Down Expand Up @@ -131,6 +132,8 @@ def get_tools(
apps: t.Optional[t.Sequence[AppType]] = None,
tags: t.Optional[t.List[TagType]] = None,
entity_id: t.Optional[str] = None,
*,
processors: t.Optional[ProcessorsType] = None,
) -> t.Sequence[FunctionTool]:
"""
Get composio tools wrapped as LlamaIndex FunctionTool objects.
Expand All @@ -143,6 +146,8 @@ def get_tools(
:return: Composio tools wrapped as `StructuredTool` objects
"""
self.validate_tools(apps=apps, actions=actions, tags=tags)
if processors is not None:
self._merge_processors(processors)
return [
self._wrap_tool(
schema=tool.model_dump(
Expand Down
5 changes: 5 additions & 0 deletions python/plugins/lyzr/composio_lyzr/toolset.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

from composio import Action, ActionType, AppType, TagType
from composio.tools import ComposioToolSet as BaseComposioToolSet
from composio.tools.toolset import ProcessorsType
from composio.utils.shared import (
get_signature_format_from_schema_params,
json_schema_to_model,
Expand Down Expand Up @@ -92,6 +93,8 @@ def get_tools(
apps: t.Optional[t.Sequence[AppType]] = None,
tags: t.Optional[t.List[TagType]] = None,
entity_id: t.Optional[str] = None,
*,
processors: t.Optional[ProcessorsType] = None,
) -> t.List[Tool]:
"""
Get composio tools wrapped as Lyzr `Tool` objects.
Expand All @@ -104,6 +107,8 @@ def get_tools(
:return: Composio tools wrapped as `Tool` objects
"""
self.validate_tools(apps=apps, actions=actions, tags=tags)
if processors is not None:
self._merge_processors(processors)
return [
self._wrap_tool(
schema=schema.model_dump(exclude_none=True),
Expand Down
5 changes: 5 additions & 0 deletions python/plugins/openai/composio_openai/toolset.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
from composio.constants import DEFAULT_ENTITY_ID
from composio.tools import ComposioToolSet as BaseComposioToolSet
from composio.tools.schema import OpenAISchema, SchemaType
from composio.tools.toolset import ProcessorsType


class ComposioToolSet(
Expand Down Expand Up @@ -101,6 +102,8 @@ def get_tools(
actions: t.Optional[t.Sequence[ActionType]] = None,
apps: t.Optional[t.Sequence[AppType]] = None,
tags: t.Optional[t.List[TagType]] = None,
*,
processors: t.Optional[ProcessorsType] = None,
) -> t.List[ChatCompletionToolParam]:
"""
Get composio tools wrapped as OpenAI `ChatCompletionToolParam` objects.
Expand All @@ -112,6 +115,8 @@ def get_tools(
:return: Composio tools wrapped as `ChatCompletionToolParam` objects
"""
self.validate_tools(apps=apps, actions=actions, tags=tags)
if processors is not None:
self._merge_processors(processors)
return [
ChatCompletionToolParam( # type: ignore
**t.cast(
Expand Down
5 changes: 5 additions & 0 deletions python/plugins/phidata/composio_phidata/toolset.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from pydantic import validate_call

from composio import Action, ActionType, AppType, TagType
from composio.tools.toolset import ProcessorsType

from composio_openai import ComposioToolSet as BaseComposioToolSet

Expand Down Expand Up @@ -68,6 +69,8 @@ def get_tools(
actions: t.Optional[t.Sequence[ActionType]] = None,
apps: t.Optional[t.Sequence[AppType]] = None,
tags: t.Optional[t.List[TagType]] = None,
*,
processors: t.Optional[ProcessorsType] = None,
) -> t.List[Function]:
"""
Get composio tools wrapped as Lyzr `Function` objects.
Expand All @@ -79,6 +82,8 @@ def get_tools(
:return: Composio tools wrapped as `Function` objects
"""
self.validate_tools(apps=apps, actions=actions, tags=tags)
if processors is not None:
self._merge_processors(processors)
return [
self._wrap_tool(
schema=schema.model_dump(
Expand Down
5 changes: 5 additions & 0 deletions python/plugins/praisonai/composio_praisonai/toolset.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from composio import Action, ActionType, AppType
from composio import ComposioToolSet as BaseComposioToolSet
from composio import TagType
from composio.tools.toolset import ProcessorsType


_openapi_to_python = {
Expand Down Expand Up @@ -187,6 +188,8 @@ def get_tools(
apps: t.Optional[t.Sequence[AppType]] = None,
tags: t.Optional[t.List[TagType]] = None,
entity_id: t.Optional[str] = None,
*,
processors: t.Optional[ProcessorsType] = None,
) -> t.List[str]:
"""
Get composio tools written as ParisonAi supported tools.
Expand All @@ -199,6 +202,8 @@ def get_tools(
:return: Name of the tools written
"""
self.validate_tools(apps=apps, actions=actions, tags=tags)
if processors is not None:
self._merge_processors(processors)
return [
self._write_tool(
schema=tool.model_dump(exclude_none=True),
Expand Down
Loading
Loading