Skip to content

weird on_messages and on_messages_stream abstract relationship in BaseChatAgent #5125

@bio-mlhui

Description

@bio-mlhui

In BaseChatAgent implementation, the default implementation for ```on_messages`` is blank:

    @abstractmethod
    async def on_messages(self, messages: Sequence[ChatMessage], cancellation_token: CancellationToken) -> Response:
        ...

and the default implementation for on_messages_stream is based on on_messages:

    async def on_messages_stream(
        self, messages: Sequence[ChatMessage], cancellation_token: CancellationToken
    ) -> AsyncGenerator[AgentEvent | ChatMessage | Response, None]:
        response = await self.on_messages(messages, cancellation_token)
        for inner_message in response.inner_messages or []:
            yield inner_message
        yield response

However, in AssistantAgent, the relationship is reversed: the implementation of on_messages is based on on_messages_stream:

    async def on_messages(self, messages: Sequence[ChatMessage], cancellation_token: CancellationToken) -> Response:
        async for message in self.on_messages_stream(messages, cancellation_token):
            if isinstance(message, Response):
                return message
        raise AssertionError("The stream should have returned the final result.")

    async def on_messages_stream(
        self, messages: Sequence[ChatMessage], cancellation_token: CancellationToken
    ) -> AsyncGenerator[AgentEvent | ChatMessage | Response, None]:
     many_codes_following
            )

I think it is more elegant to reverse their relationship in BaseChatAgent. When the developer want to cutomize a new BaseChatAgent, the developer only needs to instantiate the on_messages_stream method.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions