fix(telegram): stop typing after final reply - #33631
Conversation
|
Thanks for this. A fix for the post-delivery typing-stop boundary in This PR overlaps that area but also touches paths #37556 didn't (e.g. the stream-consumer / final-delivery path), so I'm not closing it — could you rebase on current |
89863f9 to
0a39c62
Compare
|
@kshitijk4poor Sorry for the late reply — I only just saw your comment. I rebased this branch onto current
I updated this PR branch to keep the remaining delta focused on those paths and added regression coverage for:
Validation I ran locally: python -m pytest tests/gateway/test_telegram_post_send_typing.py \
tests/gateway/test_telegram_footer_post_send_typing.py \
tests/gateway/test_base_topic_sessions.py \
tests/gateway/test_run_progress_topics.py -q -o addopts=Result: So after rebasing, I do still see a remaining gap that this PR covers; the branch is now updated to reflect only that residual fix. |
0a39c62 to
e81d5e4
Compare
|
@kshitijk4poor Thanks again — I tightened this PR further to make the residual scope clearer. Why I still think this is a meaningful follow-up after #37556:
I also strengthened the regression coverage and updated the PR body accordingly. In particular, the tests now explicitly show that Latest targeted validation: python -m pytest tests/gateway/test_telegram_post_send_typing.py \
tests/gateway/test_telegram_footer_post_send_typing.py \
tests/gateway/test_base_topic_sessions.py \
tests/gateway/test_run_progress_topics.py -q -o addopts=Result: |
|
For convenience, here is the scope comparison against the other related PRs mentioned on this thread:
Why I think that matters:
Additional strengthening I added while rebasing this branch:
So the value proposition of this PR is not “solve the whole typing lifecycle”; it is “keep the already-merged |
Related to canonical issue #29175 and part of the saturated Telegram "typing indicator lingers after final reply" cluster: competing (not duplicate) with #33606, #29172, #29595, #25210, #24983, #21688, and the merged base.py fix #37556 (which this PR is intentionally narrower than). This PR keeps typing-suppression semantics separate from |
|
Thanks for narrowing the remaining streamed-footer path. Current main does still have that specific gap: Problems
Suggested changes
Automated hermes-sweeper review. |
|
@teknium1 Thanks — that review was correct. I had over-tightened the adapter-side suppression rule and broken the existing terminal final-send contract that uses I pushed a follow-up commit to fix that:
Validation run locally after the change: python -m pytest tests/gateway/test_telegram_post_send_typing.py \
tests/gateway/test_telegram_footer_post_send_typing.py \
tests/gateway/test_base_topic_sessions.py \
tests/gateway/test_run_progress_topics.py \
tests/gateway/test_telegram_format.py \
tests/gateway/test_stream_consumer_thread_routing.py -q -o addopts=''Result: So the branch is now positioned as: keep the long-standing |
ac99923 to
3c3b904
Compare
Problem
After the final Telegram reply is delivered, Hermes can still appear to be typing for a few extra seconds.
#37556fixed the post-delivery callback boundary inbase.py, but there is still a smaller Telegram-specific residual gap after that fix:Why this PR is intentionally narrow
There are broader proposals in this area (for example
#33606/#29172), but after#37556the remaining gap here is smaller and more localized. This PR keeps the delta focused on that residual Telegram behavior instead of expanding into stream-consumer / typing-lifecycle redesign.This also differs from
#29595: that PR reusesmetadata["notify"]as the final-send marker, while this PR preserves the existingnotify=Trueterminal-send contract and adds a dedicatedsuppress_post_send_typingflag for footer-specific residual paths.Fix
Keep Telegram post-send typing suppression coherent by treating these as terminal-send markers:
notify=Truefor the existing final-send contract used by stream-consumer pathssuppress_post_send_typing=Truefor trailing-footer / residual terminal-send paths that are final but not notify-drivenCovered paths:
plugins/platforms/telegram/adapter.pygateway/run.pyWhy the dedicated flag still helps
notifyalready means "deliver this as a notify-worthy/user-visible message"suppress_post_send_typingmeans "this send is terminal and must not re-arm Telegram typing", even when the send is not notification-drivenTests
Added / updated regression coverage for:
tests/gateway/test_telegram_post_send_typing.pynotify=Truefinal sends still suppress typing refreshsuppress_post_send_typing=Truealso suppress typing refreshtests/gateway/test_telegram_footer_post_send_typing.pysuppress_post_send_typing=Truetests/gateway/test_base_topic_sessions.pytests/gateway/test_run_progress_topics.py#37556callback-boundary coveragetests/gateway/test_telegram_format.pynotify=Truecontract remains intacttests/gateway/test_stream_consumer_thread_routing.pynotify=TrueValidation
python -m pytest tests/gateway/test_telegram_post_send_typing.py \ tests/gateway/test_telegram_footer_post_send_typing.py \ tests/gateway/test_base_topic_sessions.py \ tests/gateway/test_run_progress_topics.py \ tests/gateway/test_telegram_format.py \ tests/gateway/test_stream_consumer_thread_routing.py -q -o addopts=''Result:
168 passed