fix(gateway): keep the audio path when a voice message transcribes successfully - #89201
fix(gateway): keep the audio path when a voice message transcribes successfully#89201lsshawn wants to merge 1 commit into
Conversation
…ccessfully
`_enrich_message_with_transcription` records where the audio file is on every
outcome EXCEPT the one that works:
- STT disabled -> "[The user sent a voice message: <path>]"
- transcription failed -> "[... the audio is available at: <path>]"
- exception raised -> same marker as failure
- transcription SUCCEEDED -> '"<transcript>"' <- path discarded
So the better STT gets, the less often the path survives: a box whose STT is
broken keeps every audio pointer, and a box whose STT works keeps none.
That matters because the message `content` is the only record of the path. The
messages table has no media column, so consumers recover attachments by parsing
these markers back out of the text. Once the marker is gone the recording is
unreachable even though the bytes are still in the cache — nothing else ever
wrote the filename down.
Downstream, an operator reviewing a conversation sees a transcript with no way
to replay it, which is exactly when replay matters: checking a quantity, an
address, or a tone the transcript flattened. On code-switched speech (here,
Malay/Cantonese/Mandarin/English in one sentence) the transcript is lossy and
the recording is the source of truth.
The fix appends `[User sent audio: <path>]` on its own line after the
transcript. Three properties are deliberate:
- The GRAMMAR is `_build_media_placeholder`'s existing audio marker
(run.py:2997), not the stt-disabled branch's prose. A media consumer
matching on placeholders does not match prose, so emitting the prose form
would put the path in front of the model while leaving the attachment just
as unrecoverable — a change that looks correct in the diff and fixes
nothing. This was found by running a real consumer's matcher, not by
reading the code.
- The transcript stays FIRST and verbatim, preserving NousResearch#41603's property that
the model replies to the words rather than narrating that a voice message
arrived.
- `to_agent_visible_cache_path` is applied for the same reason the failure
branches apply it: under a Docker terminal backend the agent sees the cache
at a different mount point. Outside Docker it returns its input unchanged.
Tests (tests/gateway/test_stt_config.py) pin the path, the grammar, the
ordering, the multi-clip case, and that the empty/inaudible sentinel (NousResearch#41603)
still emits NO media marker. Verified as genuine regression tests: with the
one-line fix reverted, 4 of the 5 fail; the fifth passes either way by design,
since it guards behaviour this change must not alter.
Related to the Vox Lockin campaign (NousResearch#78207), lane 07. Distinct from NousResearch#78196,
which preserves the marker only when transcription STALLS, not when it succeeds.
Reviewed by reviewer-e (AI automated review). Fixes a genuinely ironic data-loss path: every STT failure branch preserved the audio path while success dropped it — so better transcription made recordings less reachable, since Nit (non-blocking): gateway/run.py:24414 — the marker is hand-built as an f-string next to |
|
I filed #93982 for this before spotting your PR — triage linked them, and yours is the better fix. I'd landed on nearly the same change but used prose wording; your point about reusing I've been running your exact diff on a Telegram gateway since yesterday. It works — a real voice note now reaches the agent as: The model answers the content and doesn't narrate voice mode, so the behavior the original comment protects is preserved. One thing that will bite when CI runs. Two existing tests pin the old exact string and aren't in your diff:
Running your change against a pristine checkout of that file: Both are really asserting "transcribe and echo exactly once", which assert interrupt_text.startswith('"hello once"')assert len(_PendingVoiceAgent.messages) == 2
assert _PendingVoiceAgent.messages[0] == "initial turn"
assert _PendingVoiceAgent.messages[1].startswith('"hello once"')With those two edits the full Two smaller notes, take or leave:
Happy to push the test fix as a commit to your branch or a follow-up PR, whichever you prefer. |
Related #78207 #78196 #41603 #65745
What changed and why
_enrich_message_with_transcriptionrecords where the audio file is on every outcome except the one that works:[The user sent a voice message: <path>][... the audio is available at: <path>]"<transcript>"— path discardedSo the better STT gets, the less often the path survives. A box whose STT is broken keeps every audio pointer; a box whose STT works keeps none.
That matters because the message
contentis the only record of the path — themessagestable has no media column, so consumers recover attachments by parsing these markers back out of the text. Once the marker is gone the recording is unreachable even though the bytes are still sitting in the cache directory: nothing else ever wrote the filename down.Downstream this surfaces as an operator reviewing a conversation, seeing a transcript, and having no way to replay it — precisely when replay matters most: verifying a quantity, a delivery address, or a tone the transcript flattened. On code-switched speech (in our case Malay/Cantonese/Mandarin/English inside one sentence) the transcript is a lossy artefact and the recording is the source of truth.
The fix appends
[User sent audio: <path>]on its own line after the transcript.Three deliberate properties
The grammar is
_build_media_placeholder's existing audio marker (gateway/run.py:2997), not the stt-disabled branch's prose. A media consumer matching on placeholders does not match prose, so emitting[The user sent a voice message: …]here would put the path in front of the model while leaving the attachment just as unrecoverable — a change that looks correct in the diff and fixes nothing on screen. I found this by running a real consumer's matcher against both forms rather than by reading the code, andtest_successful_transcription_uses_the_media_placeholder_grammarpins it so a future reword can't silently undo the fix.The transcript stays first and verbatim, preserving #41603's property that the model replies to the words rather than narrating that a voice message arrived.
test_transcript_precedes_the_media_markerpins the ordering.to_agent_visible_cache_pathis applied for the same reason the failure branches apply it — under a Docker terminal backend the agent sees the cache at a different mount point, and handing it an unopenable host path would be worse than handing it nothing. Outside Docker the helper returns its input unchanged.How to test
Expected: all pass (67 here on Python 3.13 — 9 in
test_stt_config.py, plus 58 across the adjacent media/STT/transcription suites).Verified as genuine regression tests. With the one-line fix reverted, 4 of the 5 new tests fail:
The fifth (
test_empty_transcript_still_emits_no_media_marker) passes either way by design — it guards #41603's empty/inaudible sentinel, which this change must not alter, so it would be wrong for it to flip.Relationship to existing work
This sits in the Vox Lockin campaign (#78207), lane 07 (
gateway voice media delivery). It is distinct from #78196, which preserves the audio marker only when transcription stalls past its deadline — the success path is untouched there. I did not find an open issue or PR covering the successful-transcription case; happy to file one to reference if the campaign prefersFixes #<issue>linkage.