fix: Telegram show typing#640
Conversation
Typing was fired once right before send(), making it invisible to users. Now starts a 4s typing loop on message receipt and cancels it on reply.
📝 WalkthroughWalkthroughThe changes implement per-chat typing indicator management in the Telegram channel. A new Changes
Sequence DiagramsequenceDiagram
participant User
participant TelegramChannel
participant AsyncTask as Asyncio Task Loop
participant TelegramAPI
User->>TelegramChannel: Message arrives (enqueue)
TelegramChannel->>TelegramChannel: _start_typing(chat_id)
TelegramChannel->>AsyncTask: Create background _typing_loop task
loop Every 4 seconds
AsyncTask->>TelegramAPI: _send_chat_action("typing")
TelegramAPI-->>AsyncTask: ✓ Typing indicator sent
end
TelegramChannel->>TelegramChannel: Message processing complete
TelegramChannel->>TelegramChannel: _stop_typing(chat_id)
TelegramChannel->>AsyncTask: task.cancel()
AsyncTask-->>TelegramChannel: Task terminated
TelegramChannel->>TelegramAPI: send (text/media)
TelegramAPI-->>User: Message delivered
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves a bug in the Telegram channel where the typing indicator was not effectively displayed to users. It introduces a robust mechanism to show a persistent 'typing...' status from the moment a message is received until the bot sends its reply. This enhancement significantly improves the user experience by providing real-time feedback and indicating that the bot is actively processing their request, making interactions feel more natural and responsive. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/copaw/app/channels/telegram/channel.py (1)
452-459: Consider adding a maximum typing duration as a safeguard.If message processing fails or hangs before
send()is reached, the typing loop continues indefinitely until a new message arrives or the channel stops. Adding a timeout (e.g., 60-120s) could prevent phantom typing indicators in edge cases.💡 Optional: Add max duration timeout
async def _typing_loop(self, chat_id: str) -> None: - """Repeatedly send 'typing' action every 4s until cancelled.""" + """Repeatedly send 'typing' action every 4s until cancelled or timeout.""" + max_duration = 120 # seconds + elapsed = 0 try: - while self._application: + while self._application and elapsed < max_duration: await self._send_chat_action(chat_id, "typing") await asyncio.sleep(4) + elapsed += 4 except asyncio.CancelledError: pass🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/copaw/app/channels/telegram/channel.py` around lines 452 - 459, The _typing_loop currently can run indefinitely if message handling stalls; modify async def _typing_loop(self, chat_id: str) to enforce a maximum typing duration (e.g., 60–120 seconds) by recording start time and breaking the loop when elapsed exceeds a configurable max_typing_seconds (or default to 60); keep sending typing via _send_chat_action and sleeping as before but exit the loop (or cancel) when the timeout is reached to avoid phantom typing indicators, and ensure any existing asyncio.CancelledError handling remains intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/copaw/app/channels/telegram/channel.py`:
- Around line 452-459: The _typing_loop currently can run indefinitely if
message handling stalls; modify async def _typing_loop(self, chat_id: str) to
enforce a maximum typing duration (e.g., 60–120 seconds) by recording start time
and breaking the loop when elapsed exceeds a configurable max_typing_seconds (or
default to 60); keep sending typing via _send_chat_action and sleeping as before
but exit the loop (or cancel) when the timeout is reached to avoid phantom
typing indicators, and ensure any existing asyncio.CancelledError handling
remains intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5d69d108-98bc-4be3-aea6-b3ff5fa0858f
📒 Files selected for processing (1)
src/copaw/app/channels/telegram/channel.py
There was a problem hiding this comment.
Code Review
This pull request aims to fix an issue where the Telegram typing indicator was not visible, by implementing a typing loop that starts upon message receipt and cancels upon reply. However, this introduces potential resource exhaustion and resource leak vulnerabilities. Specifically, the typing loop lacks a timeout, which could lead to a Denial of Service if the agent hangs, and a race condition in the stop() method might lead to leaked typing tasks during shutdown. A key issue identified is that the typing indicator could run indefinitely if the bot fails to generate a response, leading to a poor user experience and potential resource issues.
| async def _typing_loop(self, chat_id: str) -> None: | ||
| """Repeatedly send 'typing' action every 4s until cancelled.""" | ||
| try: | ||
| while self._application: | ||
| await self._send_chat_action(chat_id, "typing") | ||
| await asyncio.sleep(4) | ||
| except asyncio.CancelledError: | ||
| pass |
There was a problem hiding this comment.
The _typing_loop currently runs indefinitely, sending a 'typing' action every 4 seconds. This presents a resource exhaustion vulnerability: if the agent hangs or fails to respond, the typing indicator will continue indefinitely, potentially leading to resource exhaustion and rate-limiting by Telegram. This also negatively impacts user experience, as the bot would appear stuck. A timeout mechanism is crucial to prevent this.
async def _typing_loop(self, chat_id: str) -> None:
"""Repeatedly send 'typing' action every 4s for a limited time."""
try:
# Loop for a limited time (e.g., 7 iterations * 4s = 28s) to avoid
# showing the typing indicator indefinitely if no reply is sent.
for _ in range(7):
if not self._application:
break
await self._send_chat_action(chat_id, "typing")
await asyncio.sleep(4)
except asyncio.CancelledError:
# This is the expected way to stop the loop when a reply is sent.
pass| for cid in list(self._typing_tasks): | ||
| self._stop_typing(cid) |
There was a problem hiding this comment.
The stop() method cancels all current typing tasks at the beginning of the shutdown process. However, the polling task is not cancelled until line 630, and the application is not stopped until line 640. There is a race condition where a new message can arrive and trigger _start_typing() after the initial cancellation loop but before the polling task is fully stopped. This results in new typing tasks being created that are never cancelled, leading to a resource leak.
Description
Typing was fired once right before send(), making it invisible to users.
Now starts a 4s typing loop on message receipt and cancels it on reply.
Related Issue: Fixes #387
Security Considerations: [If applicable, e.g. channel auth, env/config handling]
Type of Change
Component(s) Affected
Checklist
pre-commit run --all-fileslocally and it passespytestor as relevant) and they passTesting
show_typing=Falsedisables the indicatorAdditional Notes
[Optional: any other context]
Summary by CodeRabbit
Release Notes