Conversation
Competing/complementary with #50533 (which also makes |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the focused Discord VC fix. The premise is confirmed on current main: plugins/platforms/discord/adapter.py:2925 still sets PLAYBACK_TIMEOUT = 120, used by the mixer and legacy waits at :2955, :2974, and :2992.
Problems
gateway/run.py:12852in this diff usesmax(0, value), so a negativevoice_reply_max_charssilently disables the spoken-text cap. The documentation only defines0as the opt-out; treat negative values as invalid and fall back to the default.tests/gateway/test_voice_command.py:2124changes aninspect.getsource()assertion.AGENTS.md:1358prohibits source-reading tests; this should be behavioral coverage instead.
Suggested changes
- Add negative-value fallback coverage for
voice_reply_max_chars. - Replace the source assertion with a real configured-timeout playback test; keep coverage for both mixer and legacy paths where practical.
Automated hermes-sweeper review.
| return max(0, value) | ||
|
|
||
| @staticmethod | ||
| def _clamp_spoken_voice_reply(text: str, max_chars: int) -> str: |
There was a problem hiding this comment.
Negative values currently become 0, which disables the clamp despite only 0 being documented as the opt-out. Treat negative values as invalid and fall back to 1200; add a regression test for -1.
| assert "_voice_playback_timeout" in source, \ | ||
| "play_in_voice_channel must use the configurable playback timeout helper" | ||
|
|
||
| def test_playback_timeout_constant_exists(self): |
There was a problem hiding this comment.
Please replace this source-text assertion with behavioral coverage. AGENTS.md:1358 prohibits tests that read source code because they couple to implementation spelling rather than the playback-timeout contract.
What does this PR do?
Fixes Discord voice-channel TTS readbacks getting cut off by Hermes' hardcoded playback watchdog on longer assistant responses.
This PR does two things:
discord.voice_playback_timeout_seconds(default: 300s)discord.voice_reply_max_chars(default: 1200), while preserving the full assistant response as the normal Discord text replyThis is intended for long Discord VC replies where TTS generation succeeds, but playback is stopped by Hermes before the audio finishes.
Related Issue
No issue filed.
Related/overlap: #50533 also addresses configurable Discord voice playback timeouts. This PR additionally clamps the spoken VC companion reply so long text remains available in Discord without forcing the bot to read the whole thing aloud.
Type of Change
Changes Made
plugins/platforms/discord/adapter.pydiscord.voice_playback_timeout_secondsfrom configgateway/run.pydiscord.voice_reply_max_charshermes_cli/config.pytests/gateway/test_voice_command.pytests/gateway/test_discord_voice_mixer.pywebsite/docs/guides/use-voice-mode-with-hermes.mdHow to Test
uv run --with pytest --with pytest-asyncio --with numpy python -m pytest tests/gateway/test_voice_command.py tests/gateway/test_discord_voice_mixer.py -q -o 'addopts='Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests passDocumentation & Housekeeping
docs/, docstrings) — or N/Acli-config.yaml.exampleif I added/changed config keys — or N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/AScreenshots / Logs
Targeted test run:
Observed symptom before the fix: