feat: implement Telegram typing indicator renewal during long agent operations#1753
feat: implement Telegram typing indicator renewal during long agent operations#1753praisonai-triage-agent[bot] wants to merge 3 commits into
Conversation
…perations - Add TypingIndicator utility class with configurable renewal interval - Update TelegramBot standalone mode to use typing renewal during agent.chat() - Update gateway polling handler to use typing renewal during agent.chat() - Enable ack_emoji (⏳) and done_emoji (✅) in onboard-generated configs - Typing indicator now renews every 4 seconds during long operations - Fixes issue where users thought bot was broken during 20-30s agent responses Fixes #1748 Co-authored-by: praisonai-triage-agent[bot] <praisonai-triage-agent[bot]@users.noreply.github.com>
|
@coderabbitai review |
|
/review |
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
✅ Actions performedReview triggered.
|
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR implements periodic renewal of Telegram typing indicators during long-running agent operations. It adds a ChangesTelegram typing indicator renewal
Sequence DiagramsequenceDiagram
participant User as Telegram User
participant Handler as Message Handler
participant Renewal as with_typing_renewal
participant Indicator as TypingIndicator
participant TelegramAPI as Telegram API
participant Chat as agent.session.chat()
User->>Handler: send message
Handler->>Renewal: execute chat with typing_func
Renewal->>Indicator: start(typing_func)
activate Indicator
par Parallel execution
Indicator->>TelegramAPI: send_action(typing)
loop Every 4 seconds
Indicator->>TelegramAPI: send_action(typing)
end
and
Renewal->>Chat: await operation (20–30s)
activate Chat
Chat-->>Renewal: response text
deactivate Chat
end
Renewal->>Indicator: cancel()
deactivate Indicator
Handler->>TelegramAPI: send response message
TelegramAPI-->>User: full answer appears
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
|
@copilot Do a thorough review of this PR. Read ALL existing reviewer comments above from Qodo, Coderabbit, and Gemini first — incorporate their findings. Review areas:
|
|
@claude You are the FINAL architecture reviewer. If the branch is under MervinPraison/PraisonAI (not a fork), you are able to make modifications to this branch and push directly. SCOPE: Focus ONLY on Python packages (praisonaiagents, praisonai). Do NOT modify praisonai-rust or praisonai-ts. Read ALL comments above from Gemini, Qodo, CodeRabbit, and Copilot carefully before responding. Phase 1: Review per AGENTS.md
Phase 2: FIX Valid Issues Phase 3: Final Verdict |
|
Claude finished @MervinPraison's task —— View job Final Architecture Review ✅Task List:
Phase 1: Architecture Review per AGENTS.md ✅✅ Protocol-Driven Design
✅ Backward Compatibility
✅ Performance Impact
Phase 2: Issues Fixed ✅CodeRabbit Docstring Coverage Issue ✅Fixed: Enhanced docstring for Committed: c41931b - docs: improve docstring coverage for typing indicator module Phase 3: Code Quality Review ✅✅ Implementation Quality
✅ Integration Points
✅ Test Coverage
Final Verdict: ✅ APPROVEDThis PR successfully implements typing indicator renewal following all AGENTS.md principles: ✅ Architecture: Properly layered, wrapper contains implementation Key Strengths:
Files Modified Summary:
Ready to merge 🚀 |
Enhance docstring for _typing_loop method to improve coverage and meet repository documentation standards. Co-authored-by: Mervin Praison <MervinPraison@users.noreply.github.com>
Greptile SummaryThis PR adds continuous typing indicator renewal for Telegram bots during long
Confidence Score: 4/5The change is safe to merge; the core renewal logic is well-structured and the cleanup path is reliable. The TypingIndicator implementation is correct and the cleanup path properly awaits the background task in a finally block. The only notable behaviour change is that the typing indicator is no longer sent during the debounce window — the old one-shot send happened before debounce, while the new renewal loop starts after it — which could leave a brief silent gap for bots with longer debounce windows. The test file uses the deprecated asyncio.get_event_loop() inside an async test where asyncio.get_running_loop() is the correct call. telegram.py and gateway/server.py are worth a second look for the debounce-period gap; test_typing_indicator.py for the deprecated event-loop accessor. Important Files Changed
Reviews (1): Last reviewed commit: "docs: improve docstring coverage for typ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| call_times = [] | ||
|
|
||
| async def _typing(): | ||
| call_times.append(asyncio.get_event_loop().time()) |
There was a problem hiding this comment.
asyncio.get_event_loop() emits a DeprecationWarning (and raises RuntimeError in 3.12 when called outside a running loop). Inside an async test the loop is running, so this works today, but asyncio.get_running_loop() is the correct API for use within an async context.
| call_times.append(asyncio.get_event_loop().time()) | |
| call_times.append(asyncio.get_running_loop().time()) |
| message_text = await self._debouncer.debounce(user_id, message_text) | ||
| response = await self._session.chat( | ||
| self._agent, user_id, message_text, | ||
| chat_id=str(update.message.chat_id) if update.message.chat_id else "", | ||
| user_name=user_name, | ||
| ) | ||
|
|
||
| # Show typing indicator with renewal during long operation | ||
| if self.config.typing_indicator: | ||
| from ._typing_indicator import with_typing_renewal | ||
|
|
||
| async def _typing_action(): | ||
| await update.message.chat.send_action("typing") | ||
|
|
||
| response = await with_typing_renewal( | ||
| typing_func=_typing_action, | ||
| operation_coro=self._session.chat( | ||
| self._agent, user_id, message_text, | ||
| chat_id=str(update.message.chat_id) if update.message.chat_id else "", | ||
| user_name=user_name, | ||
| ) | ||
| ) |
There was a problem hiding this comment.
The old code sent send_action("typing") before the debounce call, so users saw immediate feedback while waiting for the debounce window to expire. The new code only starts the renewal loop after debouncer.debounce() resolves, meaning there is no typing indicator during the debounce period. If the debounce window is configured for several seconds (common when users type in bursts), the bot appears silent until debounce completes. The same regression is present in gateway/server.py at the equivalent location.
Summary
This PR implements typing indicator renewal for Telegram bots during long agent.chat() operations, fixing the UX issue where users think the bot is broken during 20-30 second response delays.
Changes
Core Implementation
Key Features
UX Impact
Before: Users see silence for 20-30 seconds, assume bot is broken
After: Continuous feedback throughout operation with both ⏳ reaction and renewed typing indicators
Testing
Fixes #1748
Summary by CodeRabbit