-
Notifications
You must be signed in to change notification settings - Fork 516
Adding an option to isolate views in Group Chat #2244
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?
Conversation
|
|
|
I am still working on this PR. |
|
@r4881t - I like this idea. Do you think it would be worthwhile having the ability to filter what messages get shared. In your description you noted that tool responses can be large (agreed!), would being able to suppress the tool response only be helpful? |
Maybe. But if tool responses are suppressed, what else is there to add meaningful context. I can maybe think of a way where the param |
Thanks @r4881t , understand, let's leave it as is. Would you be able to add to the documentation in the Group Chat section? If time permits, a notebook example would be useful to show how this helps. |
| if not groupchat.isolate_agent_views: | ||
| for agent in groupchat.agents: | ||
| self.send(intro, agent, request_reply=False, silent=True) |
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.
dont think we need a if not groupchat.isolate_agent_views: case here , should be default behavior
| # broadcast the message to all agents except the speaker (unless isolate_agent_views is True) | ||
| if not groupchat.isolate_agent_views: | ||
| for agent in groupchat.agents: | ||
| if agent != speaker: | ||
| inter_reply = groupchat._run_inter_agent_guardrails( | ||
| src_agent_name=speaker.name, | ||
| dst_agent_name=agent.name, | ||
| message_content=message, | ||
| ) | ||
| self.send(replacement, agent, request_reply=False, silent=True) | ||
| else: | ||
| self.send(message, agent, request_reply=False, silent=True) | ||
| if inter_reply is not None: | ||
| replacement = ( | ||
| {"content": inter_reply, "name": speaker.name} | ||
| if not isinstance(inter_reply, dict) | ||
| else inter_reply | ||
| ) | ||
| self.send(replacement, agent, request_reply=False, silent=True) | ||
| else: | ||
| self.send(message, agent, request_reply=False, silent=True) |
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.
same here, don't need this case if not groupchat.isolate_agent_views this is default behavior, I suggest to keep this logic as fallback to when if groupchat.isolate_agent_views case not passes, which should serve as default behavior
| mask_llm_config: Optional LLM configuration for masking. | ||
| isolate_agent_views: If True, agents will only maintain their own message history | ||
| and will not receive messages from other agents. When False (default), all agents | ||
| receive all messages. When True, messages are still stored in groupchat.messages | ||
| for the GroupChatManager's view, but are not broadcast to other agents. |
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.
This PR is good , I have few logical questions from design point of view:
firstly let's look at a how a tools is being called on high level:
sys prompt + use prompt -> agent -> LLM review context -> tool choice -> tool execution -> return to agent / handoff to another agent -> end
so while this PR is treating context rotting + context window, I am seeing possibilities of context loss.
for e.g., let's look at a scenario say we have a GroupChat with [agent A, Agent B , Agent C]
we have enabled isolated agent views, which isolate agent chat history.
a tool call from agent B, requires context from agent A's output / agent A's tool output to be specific.
where the agent B tool call is recent, and the reference is lost in chat history for another agent.
i guess this is a situation we would likely hit. how should we handle this? i suppose. we can link this issue to this pull. #2242
|
@r4881t Nice PR! Are you still working on it? Could you review the comments and suggested changes from priyansh4320? Also please run pre-commit to fix the format issues. Thank you! |
|
@SirEntropy, could you help review this as well? I think it's a useful feature. |
priyansh4320
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.
@r4881t any updates on this PR?
Why are these changes needed?
In Group Chat, the default behavior is that all agents share the same message history. While this is suitable for most cases, some cases demand the agents to have isolated views of messages.
This pattern is more common in places where (a) tool call responses can be huge. (b) The agent's don't need implicit message history and rely on external ContextVar to manage the shared state/tasks.
Related issue number
Checks