Python: Fix agent samples (#11278)

### Motivation and Context

<!-- Thank you for your contribution to the semantic-kernel repo!
Please help reviewers and future users, providing the following
information:
  1. Why is this change required?
  2. What problem does it solve?
  3. What scenario does it contribute to?
  4. If it fixes an open issue, please link to the issue here.
-->
This
[sample](https://github.com/microsoft/semantic-kernel/blob/main/python/samples/concepts/agents/chat_completion_agent/chat_completion_agent_function_termination.py)
was throwing an error due to duplicating tool messages in the chat
history.

### Description

<!-- Describe your changes, the overall approach, the underlying design.
These notes will help understanding how your code works. Thanks! -->
Fix a few agent samples.

### Contribution Checklist

<!-- Before submitting this PR, please make sure: -->

- [x] The code builds clean without any errors or warnings
- [x] The PR follows the [SK Contribution
Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [x] All unit tests pass, and I have added new tests where possible
- [x] I didn't break anyone 😄

Author: TaoChenOSU

python/samples/concepts/agents/chat_completion_agent/chat_completion_agent_function_termination.py

@@ -99,8 +99,7 @@ async def main():
     print("================================")
 
     # 4. Print out the chat history to view the different types of messages
-    chat_history = await thread.get_messages()
-    for message in chat_history.messages:
+    async for message in thread.get_messages():
         _write_content(message)
 
     """

python/samples/concepts/agents/chat_completion_agent/chat_completion_agent_summary_history_reducer_single_agent.py

@@ -3,14 +3,9 @@
 import asyncio
 import logging
 
-from semantic_kernel.agents import (
-    ChatCompletionAgent,
-    ChatHistoryAgentThread,
-)
+from semantic_kernel.agents import ChatCompletionAgent, ChatHistoryAgentThread
 from semantic_kernel.connectors.ai.open_ai import AzureChatCompletion
-from semantic_kernel.contents import (
-    ChatHistorySummarizationReducer,
-)
+from semantic_kernel.contents import ChatHistorySummarizationReducer
 
 """
 The following sample demonstrates how to implement a truncation chat 
@@ -55,14 +50,13 @@ async def main():
         # Attempt reduction
         is_reduced = await thread.reduce()
         if is_reduced:
-            print(f"@ History reduced to {len(thread.messages)} messages.")
+            print(f"@ History reduced to {len(thread)} messages.")
 
-        print(f"@ Message Count: {len(thread.messages)}\n")
+        print(f"@ Message Count: {len(thread)}\n")
 
         # If reduced, print summary if present
         if is_reduced:
-            chat_history = await thread.get_messages()
-            for msg in chat_history.messages:
+            async for msg in thread.get_messages():
                 if msg.metadata and msg.metadata.get("__summary__"):
                     print(f"\tSummary: {msg.content}")
                     break

python/semantic_kernel/agents/autogen/autogen_conversable_agent.py

@@ -157,12 +157,8 @@ async def get_response(
         )
         assert thread.id is not None  # nosec
 
-        chat_history = ChatHistory()
-        async for message in thread.get_messages():
-            chat_history.add_message(message)
-
         reply = await self.conversable_agent.a_generate_reply(
-            messages=[message.to_dict() for message in chat_history.messages],
+            messages=[message.to_dict() async for message in thread.get_messages()],
             **kwargs,
         )
 
@@ -214,17 +210,14 @@ async def invoke(
         )
         assert thread.id is not None  # nosec
 
-        chat_history = ChatHistory()
-        async for message in thread.get_messages():
-            chat_history.add_message(message)
-
         if recipient is not None:
             if not isinstance(recipient, AutoGenConversableAgent):
                 raise AgentInvokeException(
                     f"Invalid recipient type: {type(recipient)}. "
                     "Recipient must be an instance of AutoGenConversableAgent."
                 )
 
+            messages = [message async for message in thread.get_messages()]
             chat_result = await self.conversable_agent.a_initiate_chat(
                 recipient=recipient.conversable_agent,
                 clear_history=clear_history,
@@ -233,7 +226,7 @@ async def invoke(
                 max_turns=max_turns,
                 summary_method=summary_method,
                 summary_args=summary_args,
-                message=chat_history.messages[-1].content,  # type: ignore
+                message=messages[-1].content,  # type: ignore
                 **kwargs,
             )
 
@@ -248,7 +241,7 @@ async def invoke(
                 )
         else:
             reply = await self.conversable_agent.a_generate_reply(
-                messages=[message.to_dict() for message in chat_history.messages],
+                messages=[message.to_dict() async for message in thread.get_messages()],
             )
 
             logger.info("Called AutoGenConversableAgent.a_generate_reply.")

python/semantic_kernel/agents/chat_completion/chat_completion_agent.py

@@ -231,11 +231,9 @@ async def create_channel(
         if thread.id is None:
             await thread.create()
 
-        chat_history = ChatHistory()
-        async for message in thread.get_messages():
-            chat_history.add_message(message)
+        messages = [message async for message in thread.get_messages()]
 
-        return ChatHistoryChannel(messages=chat_history.messages, thread=thread)
+        return ChatHistoryChannel(messages=messages, thread=thread)
 
     @trace_agent_get_response
     @override
@@ -280,9 +278,7 @@ async def get_response(
         if not responses:
             raise AgentInvokeException("No response from agent.")
 
-        response = responses[-1]
-        await thread.on_new_message(response)
-        return AgentResponseItem(message=response, thread=thread)
+        return AgentResponseItem(message=responses[-1], thread=thread)
 
     @trace_agent_invocation
     @override
@@ -321,7 +317,6 @@ async def invoke(
             chat_history.add_message(message)
 
         async for response in self._inner_invoke(thread, chat_history, arguments, kernel, **kwargs):
-            await thread.on_new_message(response)
             yield AgentResponseItem(message=response, thread=thread)
 
     @trace_agent_invocation
@@ -411,6 +406,9 @@ async def invoke_stream(
 
         await self._capture_mutated_messages(agent_chat_history, message_count_before_completion, thread)
         if role != AuthorRole.TOOL:
+            # Tool messages will be automatically added to the chat history by the auto function invocation loop
+            # if it's the response (i.e. terminated by a filter), thus we need to avoid notifying the thread about
+            # them multiple times.
             await thread.on_new_message(
                 ChatMessageContent(
                     role=role if role else AuthorRole.ASSISTANT, content="".join(response_builder), name=self.name
@@ -468,6 +466,11 @@ async def _inner_invoke(
 
         for response in responses:
             response.name = self.name
+            if response.role != AuthorRole.TOOL:
+                # Tool messages will be automatically added to the chat history by the auto function invocation loop
+                # if it's the response (i.e. terminated by a filter),, thus we need to avoid notifying the thread about
+                # them multiple times.
+                await thread.on_new_message(response)
             yield response
 
     async def _prepare_agent_chat_history(

python/semantic_kernel/core_plugins/math_plugin.py

@@ -19,30 +19,27 @@ class MathPlugin:
     @kernel_function(name="Add")
     def add(
         self,
-        input: Annotated[int, "the first number to add"],
-        amount: Annotated[int, "the second number to add"],
-    ) -> Annotated[int, "the output is a number"]:
+        input: Annotated[int | str, "The first number to add"],
+        amount: Annotated[int | str, "The second number to add"],
+    ) -> Annotated[int, "The result"]:
         """Returns the Addition result of the values provided."""
         if isinstance(input, str):
             input = int(input)
         if isinstance(amount, str):
             amount = int(amount)
-        return MathPlugin.add_or_subtract(input, amount, add=True)
+
+        return input + amount
 
     @kernel_function(name="Subtract")
     def subtract(
         self,
-        input: Annotated[int, "the first number"],
-        amount: Annotated[int, "the number to subtract"],
-    ) -> int:
+        input: Annotated[int | str, "The number to subtract from"],
+        amount: Annotated[int | str, "The number to subtract"],
+    ) -> Annotated[int, "The result"]:
         """Returns the difference of numbers provided."""
         if isinstance(input, str):
             input = int(input)
         if isinstance(amount, str):
             amount = int(amount)
-        return MathPlugin.add_or_subtract(input, amount, add=False)
 
-    @staticmethod
-    def add_or_subtract(input: int, amount: int, add: bool) -> int:
-        """Helper function to perform addition or subtraction based on the add flag."""
-        return input + amount if add else input - amount
+        return input - amount

python/tests/unit/agents/chat_completion/test_chat_completion_agent.py

@@ -180,7 +180,7 @@ async def test_invoke(kernel_with_ai_service: tuple[Kernel, ChatCompletionClient
     assert messages[0].message.content == "Processed Message"
 
 
-async def test_invoke_tool_call_added(kernel_with_ai_service: tuple[Kernel, ChatCompletionClientBase]):
+async def test_invoke_tool_call_not_added(kernel_with_ai_service: tuple[Kernel, ChatCompletionClientBase]):
     kernel, mock_ai_service_client = kernel_with_ai_service
     agent = ChatCompletionAgent(
         kernel=kernel,
@@ -211,15 +211,14 @@ async def mock_get_chat_message_contents(
     assert messages[1].message.content == "Processed Message 2"
 
     thread: ChatHistoryAgentThread = messages[-1].thread
-    history = ChatHistory()
-    async for message in thread.get_messages():
-        history.add_message(message)
-
-    assert len(history.messages) == 5
-    assert history.messages[1].content == "Processed Message 1"
-    assert history.messages[2].content == "Processed Message 2"
-    assert history.messages[1].name == "TestAgent"
-    assert history.messages[2].name == "TestAgent"
+    thread_messages = [message async for message in thread.get_messages()]
+
+    assert len(thread_messages) == 4
+    assert thread_messages[0].content == "test"
+    assert thread_messages[1].content == "Processed Message 1"
+    assert thread_messages[2].content == "Processed Message 2"
+    assert thread_messages[1].name == "TestAgent"
+    assert thread_messages[2].name == "TestAgent"
 
 
 async def test_invoke_no_service_throws(kernel: Kernel):