-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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.
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
Ability to integrate external agents into the framework #246
Changes from all commits
4dd8a31
05da68e
e77eb37
7699af9
7364a34
4f12aed
5585e66
598c3d6
ee46525
e381b2a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,52 @@ | ||
from abc import ABC, abstractmethod | ||
from typing import Optional, List, Any, Dict | ||
|
||
from pydantic import BaseModel, PrivateAttr, Field | ||
|
||
from crewai.utilities import I18N | ||
|
||
|
||
class AgentWrapperParent(ABC, BaseModel): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Really loved the generic agent interface. In a perfect world, this would be the only way the engine would refer to agents, right!? Should we call it There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just to make it clear, the dream here is that even There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good suggestion, that was exactly the idea :) |
||
_i18n: I18N = PrivateAttr(default=I18N()) | ||
data: Dict[str, Any] = Field( | ||
default_factory=dict, | ||
description="Data storage for children, as pydantic doesn't play well with inheritance.", | ||
) | ||
role: str = Field(description="Role of the agent", default="") | ||
allow_delegation: bool = Field( | ||
description="Allow delegation of tasks to other agents?", default=False | ||
) | ||
|
||
@property | ||
def i18n(self) -> I18N: | ||
if hasattr(self, "_agent") and hasattr(self._agent, "i18n"): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we prevent this kind of indirection by using some sort of interface? We are moving fast to become a strongly-typed library and this makes it very hard to make sure things are working properly. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If an offician AgentInterface existed, this could be part of it :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wanna draft something here? |
||
return self._agent.i18n | ||
else: | ||
return self._i18n | ||
|
||
@i18n.setter | ||
def i18n(self, value: I18N) -> None: | ||
if hasattr(self, "_agent") and hasattr(self._agent, "i18n"): | ||
self._agent.i18n = value | ||
else: | ||
self._i18n = value | ||
|
||
@abstractmethod | ||
def execute_task( | ||
self, | ||
task: str, | ||
context: Optional[str] = None, | ||
tools: Optional[List[Any]] = None, | ||
) -> str: | ||
pass | ||
|
||
@property | ||
@abstractmethod | ||
def tools(self) -> List[Any]: | ||
pass | ||
|
||
def set_cache_handler(self, cache_handler: Any) -> None: | ||
pass | ||
|
||
def set_rpm_controller(self, rpm_controller: Any) -> None: | ||
pass |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,52 @@ | ||
from typing import Any, Optional, List, Callable | ||
import warnings | ||
|
||
from langchain_core.messages import AIMessage | ||
from crewai.agents.agent_interface import AgentWrapperParent | ||
|
||
|
||
class LangchainCrewAgent(AgentWrapperParent): | ||
|
||
def __init__( | ||
self, | ||
agent: Any, | ||
role: str, | ||
allow_delegation: bool = False, | ||
tools: List[Any] | None = None, | ||
**data: Any, | ||
): | ||
super().__init__(role=role, allow_delegation=allow_delegation, **data) | ||
self.data.update(data) | ||
self.data["agent"] = agent | ||
# store tools by name to eliminate duplicates | ||
self.data["tools"] = {} | ||
self.tools = tools or [] | ||
|
||
def execute_task( | ||
self, | ||
task: str, | ||
context: Optional[List[str]] = None, | ||
tools: Optional[List[Any]] = None, | ||
) -> str: | ||
used_tools = self.tools + (tools or []) | ||
|
||
if context: | ||
context = [AIMessage(content=ctx) for ctx in context] | ||
else: | ||
context = [] | ||
# https://github.com/langchain-ai/langchain/discussions/17403 | ||
return self.data["agent"].invoke( | ||
{"input": task, "chat_history": context, "tools": used_tools} | ||
)["output"] | ||
|
||
@property | ||
def tools(self) -> List[Any]: | ||
return list(self.data["tools"].values()) | ||
|
||
@tools.setter | ||
def tools(self, tools: List[Any]) -> None: | ||
for tool in tools: | ||
if tool.name not in self.data["tools"]: | ||
self.data["tools"][tool.name] = tool | ||
else: | ||
warnings.warn(f"Tool {tool.name} already exists in the agent.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this breaking anything in terms of retro-compatibility? Not an issue, per se, but I wonder if there's any way around if that's the case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just thought that the natural kind of context is a list of past messages in a conversation, and it's more natural to pass that through as a list instead of squashing it into a str
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I love it. It makes sense! My concern is the breaking change. What if we moved it to an
str | List[str] | None
type?