-
Notifications
You must be signed in to change notification settings - Fork 20
Alui/multiagent #141
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
base: main
Are you sure you want to change the base?
Alui/multiagent #141
Conversation
|
@mikegros This draft demonstrates how to include the planning and execution agents in a supervisor agent (that I call Changes:
A good way to start reviewing the changes would be to pull the branch and then try to run pytest -s tests/agents/test_multiagent |
|
I think this looks great overall. A couple things:
|
src/ursa/agents/execution_agent.py
Outdated
| llm: BaseChatModel, | ||
| agent_memory: Optional[Any | AgentMemory] = None, | ||
| log_state: bool = False, | ||
| extra_tools: Optional[list[Callable[..., Any]]] = None, |
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.
See my comment in the PR
|
@mikegros Thanks for reviewing! My main hesitation with this PR is I don't know if the memory is being properly handled. In this implementation, the |
I will look at that closer, but as a quick response, you could give all the tool agents the same checkpointer database, as long as they all have their own thread_id (which is the way the CLI handles checkpointing). Then they could all keep a message history. |
Alex's recent refactor got rid of the _action method and made invoke a method of each agent directly. Removed _action and this should pass the other CI tests. Co-authored-by: lui-arthur
There was one I missed on the planning agent. Co-authored-by: lui-arthur
…ittle more updating soon.
- Passing checkpointer to the subagents so that they can keep a history.
- Fixed a JSON Decode Error that could happen on the LLM output.
- Removed some potential bad character passing to json.loads
mikegros
left a comment
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 didnt realize I had filled out this review but not submitted it. I think this is mostly good to merge. Especially since it is listed as "experimental" so if memory aspects aren't being handled ideally, that can be part of the iteration that happens between now and when we would make it no-longer-experimental.
| return call_agent | ||
|
|
||
|
|
||
| class Ursa: |
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.
Should this inherit BaseAgent for usage metrics or anything else?
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.
We talked about this, so I dont think we need to make any changes.
No description provided.