Skip to content

fix(cli): avoid return in voice cleanup finally - #32193

Open
l0h1nth wants to merge 1 commit into
NousResearch:mainfrom
l0h1nth:fix/remove-return-in-finally
Open

fix(cli): avoid return in voice cleanup finally#32193
l0h1nth wants to merge 1 commit into
NousResearch:mainfrom
l0h1nth:fix/remove-return-in-finally

Conversation

@l0h1nth

@l0h1nth l0h1nth commented May 25, 2026

Copy link
Copy Markdown

Removes a return statement from a finally block in cli.py.

On Python 3.14, this emits a SyntaxWarning and fails when warnings are treated as errors. The behavior is preserved because _voice_continuous is already set to False before the auto-restart guard runs.

Tested:

  • python -Werror::SyntaxWarning -m py_compile cli.py
  • python -m compileall -q cli.py

@alt-glitch alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have labels May 25, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Duplicate of #21100 (and #21261, #23709). Same fix: remove return-in-finally in voice cleanup for Python 3.14+ SyntaxWarning. Issue tracked at #21088.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused fix. The premise is still present on current main: cli.py:11313 begins the cleanup finally, and cli.py:11336 returns from it. Removing that return preserves the intended stop behavior because cli.py:11333 has already set _voice_continuous = False, while the subsequent restart guard at cli.py:11344 requires that flag to be true.

Problems

  • Regression coverage is missing for the three-silent-cycle threshold. tests/tools/test_voice_cli_integration.py:1251-1258 covers restart after one silent cycle, but no test verifies that the third cycle stops continuous mode without restarting.

Suggested changes

  • Add a threshold regression test that invokes _voice_stop_and_transcribe() three times with silent recordings and asserts continuous mode is disabled and recording only restarted after the first two cycles.

Automated hermes-sweeper review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants