Skip to content

fix(stt): check_voice_requirements() should recognize all STT providers - #41657

Closed
zehuaw1 wants to merge 3 commits into
NousResearch:mainfrom
zehuaw1:fix/stt-status-check-all-providers
Closed

zehuaw1 wants to merge 3 commits into
NousResearch:mainfrom
zehuaw1:fix/stt-status-check-all-providers

Conversation

@zehuaw1

@zehuaw1 zehuaw1 commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Problem

/voice status only checks for local, groq, and openai providers. Any other valid provider — local_command, mistral, xai, elevenlabs, or a custom command provider — falls through to the generic MISSING message, even when transcription works perfectly.

The function _has_any_command_stt_provider() was already defined in transcription_tools.py but never imported in voice_mode.py.

Fix

tools/voice_mode.py — 11 insertions, 1 deletion:

  • Import _has_any_command_stt_provider from transcription_tools.py
  • Add elif branches for local_command, mistral, xai, elevenlabs
  • Add generic catch-all via _has_any_command_stt_provider() for arbitrary custom command providers

Risk

None. Existing providers (local, groq, openai) behave identically. Code change is purely additive — the new elif branches sit between the existing ones and the else clause.

Related: #41659

The /voice status command only checked for 'local', 'groq', and 'openai'
providers. Any other valid provider (local_command, mistral, xai,
elevenlabs, or custom command providers) fell through to the generic
MISSING message — even when transcription worked perfectly.

- Import _has_any_command_stt_provider (already defined, never imported)
- Add elif branches for local_command, mistral, xai, elevenlabs
- Add generic catch-all via _has_any_command_stt_provider() for
  arbitrary custom command providers
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have tool/tts Text-to-speech and transcription comp/tools Tool registry, model_tools, toolsets labels Jun 8, 2026
@zehuaw1
zehuaw1 marked this pull request as ready for review June 8, 2026 01:22
@alt-glitch alt-glitch mentioned this pull request Jun 11, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing a real /voice status gap: current main only reports local, groq, and openai as OK in tools/voice_mode.py:1160-1171, while tools/transcription_tools.py:776-828 and :1666-1695 support additional native providers.

Problems

  • The new _has_any_command_stt_provider(stt_config) branch is not tied to the selected provider. That helper returns true for any configured command provider (tools/transcription_tools.py:355-361), so an unrelated command entry can make an unknown or plugin-selected provider appear OK.
  • The current dispatcher also supports registered plugin STT providers (tools/transcription_tools.py:1713-1737), which the proposed status branches do not cover.
  • tests/tools/test_voice_mode.py:444-499 has no coverage for these added provider-status paths.

Suggested changes

  • Resolve the selected command provider specifically, and add tests covering selected versus unrelated command providers plus the plugin path.

Automated hermes-sweeper review.

Comment thread tools/voice_mode.py
@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 14, 2026
zehuaw1 added 2 commits July 14, 2026 12:28
PR review feedback:
- Replace _has_any_command_stt_provider() with selected-provider
  check via _resolve_command_stt_provider_config()
- Add _check_plugin_stt_provider() for plugin-registered backends
- Add tests: selected command, unrelated command (should NOT pass),
  and plugin provider path
@teknium1

Copy link
Copy Markdown
Collaborator

Merged into main via consolidated salvage PR #73510 (merge c0c5dac531). Your check_voice_requirements() fix recognizing all STT providers (3 commits) was cherry-picked with your authorship.

Your contribution is credited to you in git history. Thank you! Closing this PR as merged-via-salvage.

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

Labels

comp/tools Tool registry, model_tools, toolsets P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades tool/tts Text-to-speech and transcription type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants