Skip to content

fix(stt): scope upload size limits to remote providers - #65823

Closed
ypwcharles wants to merge 1 commit into
NousResearch:mainfrom
ypwcharles:fix/stt-local-file-size-limit
Closed

fix(stt): scope upload size limits to remote providers#65823
ypwcharles wants to merge 1 commit into
NousResearch:mainfrom
ypwcharles:fix/stt-local-file-size-limit

Conversation

@ypwcharles

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes a bug where transcribe_audio() applies the 25 MiB remote upload cap to all providers, including local, local_command, and user-configured command providers that never upload files. A 26 MiB WAV file processed locally by faster-whisper gets rejected before the dispatcher is even reached.

The fix moves provider resolution before the size check: local/command providers skip the upload-cap enforcement, while remote built-ins, plugin providers, and unknown providers continue to enforce it.

Before (current main):

validate_audio_file()  ← enforces 25 MiB for ALL providers
  ↓
get_provider()
  ↓
local / openai / plugin / ...

After:

validate_audio_file(enforce_size_limit=False)  ← path/symlink/format only
  ↓
get_provider()
  ↓
if NOT local → validate_audio_file_size()  ← 25 MiB cap
  ↓
dispatch to provider

Related Issue

N/A — discovered during local STT workflow investigation with large meeting recordings.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • tools/transcription_tools.py: Added _is_local_stt_provider() helper and _validate_audio_file_size() extracted from the original monolithic _validate_audio_file(). Size validation now happens after provider resolution, only for non-local providers.
  • tests/tools/test_transcription_tools.py: Added oversized file tests for local and local_command providers confirming they reach the dispatcher.
  • tests/tools/test_transcription_command_providers.py: Added oversized configured command provider test.
  • tests/tools/test_transcription_plugin_dispatch.py: Refactored 9 E2E tests to use real tmp_path files instead of mocking _validate_audio_file() — the validation chain now actually runs. Added oversized plugin rejection test.

How to Test

  1. scripts/run_tests.sh tests/tools/test_transcription.py tests/tools/test_transcription_tools.py tests/tools/test_transcription_command_providers.py tests/tools/test_transcription_plugin_dispatch.py -k "not test_timeout_returns_clean_error" -q215 passed, 0 failed.
  2. The one excluded test (test_timeout_returns_clean_error) is a pre-existing baseline failure caused by the shared venv lacking psutil — triggers the live-system kill guard. Verified identical failure on clean upstream/main with no local changes.
  3. Manual probe: 26 MiB sparse WAV + stt.provider: local → enters dispatcher. Same file + stt.provider: openai → returns "File too large" without calling _transcribe_openai.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits
  • I searched for existing PRs to make sure this is not a duplicate
  • My PR contains only changes related to this fix
  • I've run the focused test suite — 215 passed, 0 failed
  • I've added tests for my changes
  • I've tested on: Ubuntu 24.04 (WSL2), Python 3.11.15, pytest 9.0.3

Documentation & Housekeeping

  • I've updated relevant documentation — or N/A
  • I've updated cli-config.yaml.example — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md — or N/A
  • I've considered cross-platform impact (Windows, macOS) — or N/A
  • I've updated tool descriptions/schemas — or N/A

Screenshots / Logs

# Manual probe output (26 MiB sparse WAV):
local provider:  success=True, calls=1, error=None
openai provider: success=False, calls=0, error="File too large: 25.0MB (max 25MB)"

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists tool/tts Text-to-speech and transcription labels Jul 16, 2026

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review Summary

Verdict: Approved

Scopes STT upload size limits to remote providers only (local file processing unaffected). Clean, well-scoped fix. No security concerns.


Reviewed by Hermes Agent

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for isolating a real ordering bug in transcribe_audio(): current main validates the 25 MiB limit before _get_provider() (tools/transcription_tools.py:1732-1745).

Problems

  • tools/voice_mode.py:891-896 still chunks every oversized WAV before provider resolution. tests/tools/test_voice_mode.py:792-830 verifies this path while returning provider: "local"; the proposed transcription_tools.py change therefore does not give CLI local recordings the requested direct dispatch.
  • The new command-provider exemption classifies all type: command providers as local. The documented contract supports curl-style pipelines (website/docs/user-guide/features/tts.md:542), so this also removes the cap for command providers that upload remotely.

Suggested changes

  • Make voice-mode chunking provider-aware and add local-versus-remote oversized recording coverage.
  • Limit the command exemption to provably local providers, or add an explicit documented configuration signal for command-provider upload behavior.

Automated hermes-sweeper review.

Comment thread tools/transcription_tools.py Outdated
@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 18, 2026
@ypwcharles
ypwcharles force-pushed the fix/stt-local-file-size-limit branch from 9c95c42 to 7932931 Compare July 19, 2026 17:57
@ypwcharles

Copy link
Copy Markdown
Contributor Author

Addressed review feedback: tightened _is_local_stt_provider() to only exempt local and local_command — configured command providers (including curl-style remote pipelines) now go through the upload size cap like any other non-local provider.

Changes:

  • tools/transcription_tools.py: _is_local_stt_provider() simplified to a two-member whitelist
  • tests/tools/test_transcription_command_providers.py: oversized command provider test now asserts rejection (not bypass)

Post-fix validation: 215 passed, 0 failed (canonical focused suite).

@ypwcharles
ypwcharles force-pushed the fix/stt-local-file-size-limit branch from 7932931 to d8e4a68 Compare July 24, 2026 01:58
@ypwcharles

Copy link
Copy Markdown
Contributor Author

Addressed voice-mode chunking feedback: transcribe_recording() now trusts transcribe_audio() provider-aware validation — always attempts direct transcription first, only chunks on "File too large" error. Local providers (faster-whisper, whisper.cpp) no longer get blindly chunked.

Changes:

  • tools/voice_mode.py: removed pre-check chunking; lazy chunk on size error only
  • tests/tools/test_voice_mode.py: updated 2 existing chunking tests + added 3 regression tests (local skip, remote chunk, other error no chunk)

Post-fix: 70 passed, 6 failed (6 pre-existing DetectAudioEnvironment/SubprocessTimeoutKill failures unrelated to this change). New head: d8e4a6866.

@teknium1

Copy link
Copy Markdown
Contributor

Merged into main via consolidated salvage PR #73510 (merge c0c5dac531). Your change scoping the 25MB upload cap to remote providers only was cherry-picked with your authorship — local whisper no longer rejects big files.

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

@teknium1 teknium1 closed this Jul 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Medium — degraded but workaround exists 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.

4 participants