fix(gateway): set duration on Telegram voice/audio so long clips don'… - #36020
fix(gateway): set duration on Telegram voice/audio so long clips don'…#36020szafranski wants to merge 3 commits into
Conversation
61c7702 to
34bc595
Compare
austinpickett
left a comment
There was a problem hiding this comment.
Code Review
tl;dr: The implementation is clean and correct. One blocker before merging: the branch needs a rebase onto current main.
Stale base
git diff origin/main...pr-36020 --name-only reports 4,203 changed files — this is entirely stale-base pollution. The branch forked off an old commit (10b4cfeac — fix history leakage) that is hundreds of commits behind main. The actual PR commit (34bc595) contains exactly 2 files, 265 lines net-new. Please rebase onto main so CI can run cleanly and reviewers see only the real diff.
Implementation review (gateway/platforms/telegram.py, +77 lines)
_coerce_duration_seconds() — correct and tight. Handles float-string inputs (ffprobe returns a string), rejects 0/negative, returns None for anything unusable.
_probe_voice_duration_seconds() — the three-tier fallback chain is well-structured:
- stdlib
wavefor.wav— zero external deps, early-exit on success. mutagen— opportunistic; the bareexcept Exception: passcorrectly keeps it optional.ffprobesubprocess guarded byshutil.which— correct 5-second timeout.
All three tiers degrade gracefully to None, which restores pre-PR behavior (Telegram's own metadata, possibly absent). Non-breaking ✓.
send_voice integration — asyncio.to_thread is the right call for the blocking probe before touching the file handle. duration=_duration_secs is passed to both the send_voice (.ogg/.opus) and send_audio (.mp3/.m4a) paths. When _duration_secs is None, python-telegram-bot omits the field from the API request, matching prior behavior. ✓
One minor note: the wave import inside the function is redundant — wave is imported at module level in the test file and is stdlib, but the in-function import is harmless. No action needed.
Tests (tests/gateway/test_telegram_voice_duration.py, +188 lines)
Hermetic and thorough:
- 4× parametrized WAV rounding cases using real stdlib
wavewrites. - mutagen path exercised via
sys.modulesinjection (no real dep required). None-length, 0-length, missing mutagen, and missing ffprobe all returnNone.- 8×
_coerce_duration_secondscontract cases. - Async integration tests confirm
durationreachesbot.send_voice/bot.send_audiokwargs. - Graceful-fallback test confirms
duration=Nonewhen probe fails.
20/20 coverage as claimed. ✓
Summary
The fix is correct, targeted, and well-tested. Blocking item: please rebase onto current main — once that's done and CI is green, this is ready to merge.
34bc595 to
ef7fcb2
Compare
austinpickett
left a comment
There was a problem hiding this comment.
Fix is correct and the rebase landed clean — mergeable: MERGEABLE. Setting duration on Telegram voice/audio so long clips aren't cut off is a real fix. Approving.
|
Almost there — only one failing check is blocking merge: Contributor Attribution Check. The commit message includes a Quick fix — amend the commit to drop that line and force-push: git commit --amend # remove the Co-Authored-By: Claude Opus 4.8 line, save
git push --force-with-leaseOnce that lands and the attribution check goes green, this is ready to merge. |
ef7fcb2 to
1daaee6
Compare
|
Thanks,
Should be done:
https://github.com/NousResearch/hermes-agent/actions/runs/27344817745
czw., 11 cze 2026 o 13:44 Austin Pickett ***@***.***>
napisał(a):
… *austinpickett* left a comment (NousResearch/hermes-agent#36020)
<#36020 (comment)>
Almost there — only one failing check is blocking merge: *Contributor
Attribution Check*.
The commit message includes a Co-Authored-By: Claude Opus 4.8 trailer,
which is what tripped it. All other checks (tests ×6, lint, typecheck,
supply chain, Docker, e2e) are green.
Quick fix — amend the commit to drop that line and force-push:
git commit --amend # remove the Co-Authored-By: Claude Opus 4.8 line, save
git push --force-with-lease
Once that lands and the attribution check goes green, this is ready to
merge.
—
Reply to this email directly, view it on GitHub
<#36020?email_source=notifications&email_token=AFUHCAAJT4P4KXJUCODKAN347KLQDA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTINRYGAYTMMBQGQZ2M4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#issuecomment-4680160043>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AFUHCAGGUASXH5RBXXPFGFT47KLQDAVCNFSNUABGKJSXA33TNF2G64TZHMYTAMRUGU2TIMRWG45US43TOVSTWNBVGU4DKNJTGE3TRILWAI>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
Almost there — the last failing check is Contributor Attribution, and the fix is one line you'll need to add since it maps your email to your GitHub username. The check found your commit-author email Please add this line to the "p.fabiszewski@gmail.com": "szafranski",If your actual GitHub username differs from Commit + push that to this branch and the attribution check goes green. Everything else (tests, lint, typecheck, the duration fix itself) is already passing — this is the only blocker. Thanks! |
Attribution check still failingCI is all green except This is a maintainer-side fix. Please add: "p.fabiszewski@gmail.com": "szafranski",to the (My APPROVED review from earlier still stands — the fix itself is correct.) |
|
now ok? :)
czw., 11 cze 2026 o 16:55 Austin Pickett ***@***.***>
napisał(a):
… *austinpickett* left a comment (NousResearch/hermes-agent#36020)
<#36020 (comment)>
Attribution check still failing
CI is *all green* except check-attribution: the contributor email
***@***.*** is not in the AUTHOR_MAP in scripts/release.py.
This is a maintainer-side fix. Please add:
***@***.***": "szafranski",
to the AUTHOR_MAP dict in scripts/release.py (around line 47) and push
the commit. Once that lands, the check will pass and this PR is merge-ready.
(My APPROVED review from earlier still stands — the fix itself is correct.)
—
Reply to this email directly, view it on GitHub
<#36020?email_source=notifications&email_token=AFUHCAAIMVIQUVLQYMNZR6D47LB57A5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTINRYGE4DQNJXHA4KM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#issuecomment-4681885788>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AFUHCAAL7AYAEBHA5J4DFML47LB57AVCNFSNUABGKJSXA33TNF2G64TZHMYTAMRUGU2TIMRWG45US43TOVSTWNBVGU4DKNJTGE3TRILWAI>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
Sorry for bothering but I just wanted to ask is there anything more I should do? |
2b50c6d to
0c441a9
Compare
|
Sorry for the repeated attribution-check issue on my side. I cleaned up the branch history and force-pushed a linear branch on top of current upstream/main. The AI co-author trailer has been removed from the AUTHOR_MAP commit, and the PR diff is now limited to the intended Telegram duration fix, its regression tests, and the AUTHOR_MAP entry. The new workflow runs are currently waiting for maintainer approval/re-run ( |
0c441a9 to
3ff1e3f
Compare
|
@austinpickett please excuse but I wanted to make sure I didn't make some mess here, since I see some "dismissed" info above (image attached). Should I do anything more right now? |
acab62e to
d30b698
Compare
|
…t show 0:00 Telegram only auto-derives a voice/audio clip's duration from container metadata for short recordings; clips longer than ~4:50 are delivered with duration 0 and render as 0:00 in the player. Probe the length locally (stdlib wave -> mutagen -> ffprobe) and pass duration explicitly to sendVoice/sendAudio. Best-effort: when nothing can read the file we omit duration and fall back to Telegram's prior behavior. Extracts and hardens the Telegram-only part of the stale, Piper-bundled PR NousResearch#7815 (ffprobe-only, predates the send_voice retry/anchor refactor); relates to NousResearch#8508.
d30b698 to
f800325
Compare
teknium1
left a comment
There was a problem hiding this comment.
Thanks for isolating the duration probe and preserving the adapter's retry/anchor flow. The reported adapter behavior is still present on current main: plugins/platforms/telegram/adapter.py:5861-5869 and :5887-5895 call the Bot API without duration, so this targets a live defect.
Problems
- The fix does not cover the standalone Telegram sender.
tools/send_message_tool.py:849-856routes Telegram sends to_send_telegram, including non-gateway contexts as documented intests/tools/test_send_message_telegram_proxy.py:3-5. Its voice/audio sends and retry sends attools/send_message_tool.py:1327-1333and:1357-1363still omitduration. Long clips sent through that path retain the reported behavior.
Suggested changes
- Share the probe between the adapter and
_send_telegram, then forward the result through both initial and retrysend_voice/send_audiocalls. - Add standalone
_send_telegram.oggand.mp3regression coverage alongside the adapter tests.
Automated hermes-sweeper review.
| @@ -5864,6 +5938,7 @@ async def send_voice( | |||
| "voice": audio_file, | |||
There was a problem hiding this comment.
Please apply the same duration propagation to the standalone Telegram sender. tools/send_message_tool.py:849-856 routes non-gateway Telegram sends through _send_telegram, whose send_voice/send_audio initial and retry calls at lines 1327-1333 and 1357-1363 still omit duration.
|
Addressed: the standalone send_message_tool path now reuses the Telegram duration probe for .ogg sendVoice and .mp3 sendAudio, including the topic retry path. Verified with .venv/bin/python -m pytest tests/gateway/test_telegram_voice_duration.py -q (22 passed) and ruff. |

What does this PR do?
Telegram only auto-derives a voice/audio clip's duration from container metadata for
short recordings. Clips longer than ~4:50 are delivered with duration 0 and render as
0:00 in the player. This probes the clip length locally and passes
durationexplicitly to
sendVoice/sendAudio, so the bubble shows the real time. When thelength can't be determined we omit
durationand fall back to Telegram's prior behavior,so this never regresses the short-clip path.
This extracts and hardens the Telegram-only part of the stale, Piper-bundled PR #7815
(ffprobe-only, predates the current
send_voiceretry/anchor refactor).Related Issue
Fixes #36005
Relates to #8508. Supersedes the Telegram portion of #7815.
Type of Change
Changes Made
plugins/platforms/telegram/adapter.py:_probe_voice_duration_seconds()— best-effort length via stdlibwave→mutagen→ffprobe(theffprobebranch is guarded byshutil.which)._coerce_duration_seconds()— round to whole positive seconds, elseNone.send_voicenow runs the probe off-thread (asyncio.to_thread) and forwardsdurationto both the.ogg/.opus(send_voice) and.mp3/.m4a(send_audio) paths.mutagenis used opportunistically (it isn't a declareddep), with
wave/ffprobecovering its absence.tests/gateway/test_telegram_voice_duration.py: new hermetic test suite (20 tests).How to Test
scripts/run_tests.sh tests/gateway/test_telegram_voice_duration.py→ 20 passed (macOS & Linux).0:00. After: it shows the real length. Verified on Linux —a 293 s clip now renders as 4:53 (screenshot below).
Checklist
Code
fix(gateway): ...)scripts/run_tests.sh tests/gateway/test_telegram_voice_duration.py→ 20/20)Documentation & Housekeeping
cli-config.yaml.exampleif I added/changed config keys — N/A (no config keys)CONTRIBUTING.md/AGENTS.mdif I changed architecture — N/Ashutil.whichguardsffprobe;waveis stdlib;mutagenis optional. Validated on macOS + Linux.Screenshots / Logs