fix(stt): better error logging and smarter DM when STT lazy-install fails - #46127
fix(stt): better error logging and smarter DM when STT lazy-install fails#46127damiankluk wants to merge 4 commits into
Conversation
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Approved
Improves STT lazy-install failure logging and adds smarter DM message when STT fails to install.
Looks Good
- Better error context for debugging STT installation failures.
- Cleaner user-facing messaging for install failures.
- Well-scoped change.
Reviewed by Hermes Agent
Three improvements to the voice-transcription setup flow: 1. tools/transcription_tools.py - Log lazy-install failures at WARNING instead of DEBUG, and include actionable guidance about venv permission issues (the most common cause of silent STT failures). 2. gateway/run.py - Smart DM message: check the actual stt config before sending setup instructions. If stt.enabled is already true and provider is 'local', skip the redundant 'set stt.enabled' advice and show a permission-aware install hint instead. 3. gateway/run.py - Agent note now includes a config-aware hint about why STT is unavailable (e.g. 'provider is local but faster-whisper failed to install').
kill -0 returns success on zombie processes (the PID still occupies the process table), causing the restart watcher to loop forever. Check /proc/PID/status for State:Z and bail out early. Also cap the wait at 120 s (600 * 0.2s) as a safety net. Fixes the /restart command hanging on Telegram when the old gateway becomes a zombie because the dashboard (PID 1) doesn't reap children.
… workflow_dispatch
teknium1
left a comment
There was a problem hiding this comment.
Thanks for investigating the lazy-install failure path. The operator-logging concern is still present: tools/transcription_tools.py:228 logs the caught install exception at DEBUG.
Problems
- Current main intentionally removed the hardcoded STT failure DM:
gateway/run.py:10478-10486documents that it caused duplicate, wrong-language replies. The PR restores that behavior. - Current main also intentionally keeps provider/setup errors out of the LLM-visible message (
gateway/run.py:15258-15271), because those strings persist in conversation history. The PR's config-aware hint reverses that contract;tests/gateway/test_stt_config.py:84-104covers the neutral-marker behavior. - Docker now routes lazy installs to writable durable storage (
Dockerfile:300-309;tests/tools/test_dockerfile_immutable_install.py:81-110), so the/opt/hermes/.venvremediation needs redesign.
Suggested changes
- Salvage only a tested warning-level lazy-install diagnostic, while preserving the neutral gateway message path.
- Treat Docker packaging and the unrelated restart/workflow commits as separate current-main changes.
Automated hermes-sweeper review.
| "This is often a permission issue: the Hermes process user cannot " | ||
| "write to the virtual environment. Try running manually as the " | ||
| "venv owner: `stat -c '%%u' '$(dirname $(dirname $(which python3)))'` " | ||
| "then `su - <owner> -c 'VIRTUAL_ENV=/opt/hermes/.venv " |
There was a problem hiding this comment.
This remediation hardcodes the Docker image venv, but current Docker redirects lazy installs to writable /opt/data/lazy-packages (Dockerfile:300-309). Please avoid emitting /opt/hermes/.venv commands here; keep any warning diagnostic environment-independent or derive the actual lazy-install target.
Log lazy-install failures at WARNING instead of DEBUG, with actionable guidance about venv write-permission issues (the most common cause of silent STT failures). Salvaged from PR #46127 (transcription_tools half only — the gateway DM hunks are superseded by main's neutral-marker enrichment design, and the Docker/CI files were unrelated scope). (cherry picked from commit d3e07bd, reduced)
|
Merged into main via consolidated salvage PR #73510 (merge Your contribution is credited to you in git history. Thank you! Closing this PR as merged-via-salvage. |
Log lazy-install failures at WARNING instead of DEBUG, with actionable guidance about venv write-permission issues (the most common cause of silent STT failures). Salvaged from PR NousResearch#46127 (transcription_tools half only — the gateway DM hunks are superseded by main's neutral-marker enrichment design, and the Docker/CI files were unrelated scope). (cherry picked from commit d3e07bd, reduced)
Problem
When a user sends a voice message and faster-whisper is not installed, the gateway:
uv pip installset stt.enabled: trueeven when it is alreadytrueChanges
tools/transcription_tools.py_try_lazy_install_stt(): Log failures at WARNING instead of DEBUG, with the actual exception message and actionable guidance for the most common cause — the Hermes process user cannot write to the virtual environment (UID mismatch in Docker/user-namespace setups).gateway/run.py— STT failure DM (sent to user)stt.enabled: false→ says so specificallystt.provider: localor auto-detected → shows a permission-aware install commandgateway/run.py— Agent context noteRoot cause investigated
In the reporters environment (Docker with user-namespace mapping), the gateway process runs as
uid=1000but the Hermes venv (/opt/hermes/.venv/) is owned byuid=10000. All lazy-installs fail silently becauseuv pip installcannot write to the venv. The.envfile now includesPYTHONPATH=/home/hermes/.hermes/venv_extpointing to a writable extension directory where faster-whisper has been pre-installed.Testing
PYTHONPATHincludes the extension directory