TypingIndicator simplification

[rodrigobr-msft]

This pull request refactors the TypingIndicator class to simplify its usage and improve reliability. The main changes include removing asynchronous locking and state management, updating the API to use explicit start() and stop() methods, and revising the typing indicator logic to be scoped to a single conversation turn. The associated tests have been updated to reflect the new API and ensure robust behavior.

TypingIndicator class refactor:

• Reworked TypingIndicator to accept TurnContext and interval in the constructor, removed internal state and locking, and replaced the asynchronous start/stop methods with synchronous start() and stop() methods. The indicator now sends typing activities at regular intervals using an internal asyncio task, and raises exceptions if started or stopped in an invalid state. (typing_indicator.py)
• Updated usage in agent_application.py to match the new API: replaced await typing.start(context) and await typing.stop() with typing.start() and typing.stop(). (agent_application.py) [1] [2]

Testing improvements:

• Rewrote and expanded tests for TypingIndicator to cover new behaviors, including correct activity sending, error handling for invalid state transitions, and multiple start/stop cycles. (test_typing_indicator.py)
• Added new tests for activity format and ensured that stopping the indicator prevents further typing activities. (test_typing_indicator.py)

Code style and imports:

• Minor import and formatting improvements in typing_indicator.py.

[Copilot]

Pull Request Overview

This PR refactors the TypingIndicator class from an asyncio-based implementation to a threading-based implementation using threading.Timer. The main changes include replacing the continuous asyncio task loop with a single-shot timer and simplifying the start/stop logic.

• Changed from asyncio-based task management to threading-based Timer
• Replaced continuous loop with single timer execution
• Changed default interval from 1 second to 10 units (with unit mismatch in log message)

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull Request Overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull Request Overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.

Comments suppressed due to low confidence (2)

libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/agent_application.py:736

• The stop() method is now synchronous and should not be awaited. Remove the await keyword: typing.stop()

                typing.stop()

libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/typing_indicator.py:11

• Import of 'datetime' is not used.
  Import of 'timedelta' is not used.

from datetime import datetime, timedelta

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull Request Overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 26 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull Request Overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

Comments suppressed due to low confidence (1)

tests/hosting_core/app/test_typing_indicator.py:11

• Import of 'TurnContext' is not used.

from microsoft_agents.hosting.core import TurnContext

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull Request Overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull Request Overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull Request Overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull Request Overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

There's a race condition in the _run method. If stop() is called immediately after start() but before the task begins executing _run(), then when _run() starts:

• Line 40: running_task = self._task will be None (since stop() set it to None)
• Line 42: while None is self._task evaluates to while None is None, which is True
• The loop will execute and try to send a typing activity even though stop() was already called

A more robust approach would be to initialize running_task to the current task object:

async def _run(self) -> None:
    """Sends typing indicators at regular intervals."""
    running_task = asyncio.current_task()
    try:
        while running_task is self._task:
            await self._context.send_activity(Activity(type=ActivityTypes.typing))
            await asyncio.sleep(self._interval)
    except asyncio.CancelledError:
        pass

This ensures the identity check works correctly even if stop() is called before _run() executes.

[Copilot]

The inline comment has a typo: "10ms interval_seconds" should be "10ms interval" (the parameter name shouldn't appear in the comment describing the value).

Suggested change

	indicator = TypingIndicator(context, interval_seconds=0.01) # 10ms interval_seconds
	indicator = TypingIndicator(context, interval_seconds=0.01) # 10ms interval

[Copilot]

Missing error handling in the _run() method. If send_activity() raises an exception (e.g., network error, bot framework error), the task will crash and stop sending typing indicators without any logging or graceful handling.

The old implementation caught exceptions and logged them. Consider adding try-except handling around the send_activity call:

try:
    await self._context.send_activity(Activity(type=ActivityTypes.typing))
except Exception as e:
    logger.error("Error sending typing activity: %s", e)
    # Optionally: break or continue depending on desired behavior

Suggested change

	await self._context.send_activity(Activity(type=ActivityTypes.typing))
	try:
	await self._context.send_activity(Activity(type=ActivityTypes.typing))
	except Exception as e:
	logger.error("Error sending typing activity: %s", e)