Conversation
b33c912 to
a8fd4d8
Compare
The clarify-reply interception in the gateway only reads `(event.text or "").strip()`. A voice reply to a pending `clarify` prompt has empty `text`, so it falls through: the clarify never resolves and the audio is processed as an unrelated new turn — the user's answer is silently dropped. Transcribe the voice reply via the existing `_enrich_message_with_transcription` pipeline and use the transcript as the clarify answer, echoed back as 🎙️ "...". If transcription yields nothing usable, the clarify stays pending and the user is asked to reply in text, instead of the answer being lost. Builds on existing primitives (MessageType.VOICE, _enrich_message_with_transcription); no new dependencies. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
a8fd4d8 to
edb4dcd
Compare
|
Hello @liuhao1024, |
|
I tested the Matrix/SenseVoice case from #52998 against current There is one remaining gap: Matrix voice events can carry a cached filename such as I added two end-to-end
A tested implementation is available in Verification on that commit:
For one-click integration, I opened Adridot/hermes-agent#1 against the exact author branch. It contains only the tested delta at bc8ad6a: 84/84 related tests pass on the author head, and 91/91 pass after merging it with current upstream main at 861d69c. The upstream review remains consolidated here. |
Transcribe voice media before resolving a pending clarify even when the event text contains a cached filename. Accept only non-empty raw transcripts so failed STT leaves the prompt pending. Cover both Telegram empty-text voice events and Matrix cached-filename events. Refs NousResearch#52998.
fix(gateway): handle Matrix voice clarify filenames
There was a problem hiding this comment.
Pull request overview
This PR fixes a gateway edge case where a user’s voice reply to a pending clarify prompt could not be captured (because event.text is empty), causing the clarify to remain unresolved and the reply to be mishandled as a new turn. The change extends the clarify interception path in gateway/run.py to transcribe voice media and resolve the pending clarify using the raw transcript, with a fallback message when transcription yields no usable text.
Changes:
- Add a voice-aware clarify interception path that detects voice-like audio attachments, runs
_enrich_message_with_transcription, and resolves the clarify with the raw transcript. - Echo back the transcript to the chat (🎙️
"...") so the user can confirm what was understood. - Add tests validating that (a) a voice reply resolves a pending clarify via transcript and (b) failed transcription keeps clarify pending and prompts for text.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
gateway/run.py |
Transcribes voice replies during pending-clarify interception and uses transcript to resolve (or prompts for text if transcription fails). |
tests/gateway/test_telegram_audio_vs_voice.py |
Adds async tests covering clarify-resolution via voice transcript and non-resolution on transcription failure. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| _echo_adapter = self.adapters.get(source.platform) | ||
| _echo_meta = {"thread_id": source.thread_id} if source.thread_id else None | ||
| if _echo_adapter: |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for covering the pending-clarify voice path and incorporating the Matrix cached-filename follow-up from the discussion. The premise is still present on current main: gateway/run.py:9090-9107 only resolves a clarify from non-empty event.text.
Problems
gateway/run.py:7512on the PR head unconditionally posts the raw transcript. This bypassesstt.echo_transcripts: false, whose documented purpose is to avoid posting raw transcripts (website/docs/user-guide/configuration.md:1552). Existing echo paths are gated by_should_echo_stt_transcripts()(gateway/run.py:10463,15329), andtests/gateway/test_stt_transcript_echo_config.py:57-70enforces that invariant.
Suggested changes
- Gate the new clarify echo with
_should_echo_stt_transcripts()and add a regression that voice clarify still resolves while no echo is sent when the setting is false.
Automated hermes-sweeper review.
| if _echo_adapter: | ||
| for _tx in _clean_transcripts: | ||
| try: | ||
| await _echo_adapter.send( |
There was a problem hiding this comment.
Please gate this echo with _should_echo_stt_transcripts(). stt.echo_transcripts: false is documented to suppress raw transcript posts, and existing gateway echo paths all preserve that setting.
|
I verified both currently unresolved review findings against the generated merge of current Red:
The minimal production changes are: - _echo_meta = {"thread_id": source.thread_id} if source.thread_id else None
+ _echo_meta = self._thread_metadata_for_source(
+ source, self._reply_anchor_for_event(event),
+ )
- if _echo_adapter:
+ if _echo_adapter and self._should_echo_stt_transcripts():The two behavioral regressions assert the complete contract: Green on the current-main merge tree:
The source branch predates |
Address review feedback on NousResearch#50925: - Gate the clarify 🎙️ transcript echo behind _should_echo_stt_transcripts() so stt.echo_transcripts: false is honored, matching every other echo path. - Build the echo metadata with _thread_metadata_for_source() and the event reply anchor so Telegram DM-topic echoes keep direct_messages_topic_id, the reply anchor, and the topic fallback flag instead of a bare thread_id. - Regressions: echo disabled still resolves the clarify with no send; DM-topic echo preserves the full routing metadata. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Addressed the two outstanding echo findings in b5ba9fd (after merging current
Added the two behavioral regressions (echo disabled → clarify still resolves, no send; DM-topic echo → full routing metadata preserved). |
What does this PR do?
The gateway's clarify-reply interception only reads
(event.text or "").strip(). A voice reply to a pendingclarifyprompt has emptytext, so it falls through: the clarify never resolves and the audio is processed as an unrelated new turn — the user's answer is silently dropped.This transcribes a voice reply via the existing
_enrich_message_with_transcriptionpipeline and resolves the clarify with the raw transcript, echoing it back as🎙️ "<transcript>"so the user can confirm what was understood. If STT yields nothing usable, the clarify stays pending and the user is asked to reply in text, instead of the answer being lost.Design note: this deliberately calls
_enrich_message_with_transcription(low-level) rather than the canonical_prepare_inbound_message_text. The latter returns the agent-facing wrapper text (...voice message..., sender prefixes, channel context) and has a native-image-buffer side effect — both wrong for a value read only as a clarify answer. Prior art #31525 took the_prepare_inbound_message_textroute; this avoids the wrapper pollution and the side effect, and stays voice-targeted so an image/document reply isn't mistaken for the answer.Related Issue
No single tracked issue to close. Related (clarify replies interrupted / not accepted): #27564, #39694. Prior art for the same gap (self-closed by its author, unmerged): #31525.
Type of Change
Changes Made
gateway/run.py— in the clarify-reply interception, transcribe a voice reply and resolve the pending clarify with the raw transcript (echoed as🎙️ "..."); keep the clarify pending with a text-reply nudge when STT yields nothing usable.tests/gateway/test_telegram_audio_vs_voice.py— two tests: a voice reply resolves a pending clarify with the transcript, and a failed transcription keeps the clarify pending.How to Test
clarifyprompt (agent asks a blocking question).🎙️ "<transcript>", and resolves the clarify.Automated:
pytest tests/gateway/test_telegram_audio_vs_voice.py -q→ 7 passed.Checklist
Code
fix(scope):)pytest tests/ -qand all tests pass — ran the affected filetests/gateway/test_telegram_audio_vs_voice.py(7 passed) +py_compile gateway/run.py; did not run the full suite in this environmentDocumentation & Housekeeping
cli-config.yaml.exampleif I added/changed config keys — N/A (no config keys)CONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — N/AScreenshots / Logs