-
Notifications
You must be signed in to change notification settings - Fork 6.6k
Add model_context
to SelectorGroupChat
for enhanced speaker selection
#6330
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?
Add model_context
to SelectorGroupChat
for enhanced speaker selection
#6330
Conversation
…ker selection (microsoft#6301) Signed-off-by: Abhijeetsingh Meena <abhijeet040403@gmail.com>
Hi @ekzhu, I’ve made some changes to use messages from Would really appreciate any feedback on the approach — also curious which context class you'd prefer as the default. |
self._model_context = model_context | ||
else: | ||
# TODO: finalize the best default context class | ||
self._model_context = BufferedChatCompletionContext(buffer_size=5) |
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.
Let's use UnboundedChatCompletionContext
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.
Done! changed it to use UnboundedChatCompletionContext
async def select_speaker(self, thread: List[BaseAgentEvent | BaseChatMessage]) -> str: | ||
"""Selects the next speaker in a group chat using a ChatCompletion client, | ||
with the selector function as override if it returns a speaker name. | ||
|
||
A key assumption is that the agent type is the same as the topic type, which we use as the agent name. | ||
""" | ||
# TODO: A hacky solution - Update model context from _message_thread at every speaker selection | ||
# Add last BaseChatMessage to model context |
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.
Split the part of handle_agent_response
in the base class that deal with updating message thread into a separate method: update_message_thread
, which updates the message thread.
Lines 145 to 153 in d051da5
# Append the message to the message thread and construct the delta. | |
delta: List[BaseAgentEvent | BaseChatMessage] = [] | |
if message.agent_response.inner_messages is not None: | |
for inner_message in message.agent_response.inner_messages: | |
self._message_thread.append(inner_message) | |
delta.append(inner_message) | |
self._message_thread.append(message.agent_response.chat_message) | |
delta.append(message.agent_response.chat_message) | |
Then, in SelectorGroupChatManager
, override this method to also update the message context.
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 with handle_start
, we also need to update model context there.
Lines 118 to 119 in d051da5
self._message_thread.extend(message.messages) | |
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.
Done!
Created a update_message_thread
method in BaseGroupChatManager
to update the message thread and overrode it in SelectorGroupChatManager
to update the model_context
@Ethan0456 I realized that #6350 may be doing a similar thing to this PR but from message thread point of view. Let's pause this PR for now and let's see if we can address context size problem using #6350 first. |
As an AutoGen user who has been eagerly looking forward to this PR, I wanted to share my thoughts in detail. It's a bit long, but I hope it's clear. I would appreciate any feedback after reading. Community NeedBased on ongoing community feedback, I believe there is a clear need for internal message summarization and management functionality within Personal Use CaseThat said, I’m sharing my perspective here not as a contributor, but as a user who practically needs this functionality. Limitations of #6350While #6350 does address a similar issue, its TTL cutoff approach simply limits the number of messages. This doesn’t quite meet the need for summarizing or selectively preserving internal messages. Specifically, in the case of Why model_context Works Better for MeThe model_context-based approach proposed in this PR, particularly using Concern About Expanding #6350 ScopeIf #6350 were to expand beyond TTL cutoff into more complex message preservation or summarization, it might blur the responsibility between simple message cleanup and full history management. This could make the purpose of each mechanism less clear. ConclusionTherefore, I personally see #6350 as a clean and focused solution for trimming unnecessary messages, and I’m very supportive of that contribution moving forward. However, this PR enables more precise conversation flow control through internal message summarization and history context management, and it’s something I was also looking forward to seeing merged. I believe the two are not in conflict—they solve different problems and can complement each other well. Additional NoteAutoGen’s model_context structure is already designed to allow users to customize message management without requiring external extensions. That said, tools like the community extension |
Hi @ekzhu, @SongChiYoung, I also believe that A Hypothetical ExampleFor example, a (hypothetical) workflow—similar to what @SongChiYoung described—could involve maintaining a list of This approach may not be achievable with the current design proposed in PR #6350—not to say that the PR isn't useful, but rather that it targets a different problem space. Another workflow where Hypothesis-Driven Agent CollaborationScenario: You're orchestrating a team of LLM agents, each responsible for a different stage of scientific reasoning—such as hypothesis generation, experiment design, result analysis, and reflection. Why not traditional How This enables goal-aware, context-rich memory selection, compared to a more straightforward time-based truncation approach, like the one proposed in PR #6350. Would love to hear your thoughts on this! |
@Ethan0456 @SongChiYoung Great points made. Let's resume work here. There are many complaints about SelectorGroupChat, we can try to improve it here. |
- Added `update_message_thread` method in `BaseGroupChatManager` to manage message thread updates. - Replaced direct `_message_thread` modifications with calls to this method. - Overrode `update_message_thread` in `SelectorGroupChat` to also update the `model_context`. Signed-off-by: Abhijeetsingh Meena <abhijeet040403@gmail.com>
delta.append(inner_message) | ||
self._message_thread.append(message.agent_response.chat_message) | ||
await self.update_message_thread([message.agent_response.chat_message]) |
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.
To simplify, you can just call await self.update_message_thread(delta)
after this?
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.
Done! now message thread is updated only once with delta
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.
Let's add some unit tests to show that model context is being managed, validate it using ReplayChatCompletionClient
which records the calls.
Hi @ekzhu, I've updated the code based on your suggestion and added a unit test to validate the selector group chat with model context. Please let me know if you have any additional suggestions for improvement. |
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.
Can you resolve the merge conflict with the main branch?
else: | ||
agent_name = participants[0] | ||
self._previous_speaker = agent_name | ||
trace_logger.debug(f"Selected speaker: {agent_name}") | ||
return agent_name | ||
|
||
async def _select_speaker(self, roles: str, participants: List[str], history: str, max_attempts: int) -> str: | ||
def construct_message_history( | ||
self, message_history: Sequence[Union[BaseChatMessage, BaseAgentEvent, UserMessage, AssistantMessage]] |
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.
At this point, there shouldn't be any BaseChatMessage
or BaseAgentEvent
in the input list right?
@@ -453,6 +502,7 @@ def __init__( | |||
candidate_func: Optional[CandidateFuncType] = None, | |||
custom_message_types: List[type[BaseAgentEvent | BaseChatMessage]] | None = None, | |||
emit_team_events: bool = False, | |||
model_context: ChatCompletionContext | None = 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.
We need to update the API doc (argument list) and include a code example of using a custom model context.
agent2 = AssistantAgent("agent2", model_client=agent_two_model_client, description="Assistant agent 2") | ||
|
||
termination = TextMentionTermination("TERMINATE") | ||
team = SelectorGroupChat( |
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 don't see the model context is being customized here?
Why are these changes needed?
This PR enhances the
SelectorGroupChat
class by introducing a newmodel_context
parameter to support more context-aware speaker selection.Changes
model_context: ChatCompletionContext | None
parameter toSelectorGroupChat
.UnboundedChatCompletionContext
when None is provided likeAssistantAgent
._select_speaker
to prepend context messages frommodel_context
to the main thread history.construct_message_history
.Related issue number
Closes Issue #6301, enabling the group chat manager to utilize
model_context
for richer, more informed speaker selection decisions.Checks