fix(dingtalk): extract ASR recognition from 1:1 voice messages - #49546
fix(dingtalk): extract ASR recognition from 1:1 voice messages#49546zh-xl-kang wants to merge 1 commit into
Conversation
8b6fc6d to
9a82f15
Compare
80be470 to
1a4392f
Compare
|
Hi maintainers 👋 — just a gentle follow-up on this PR. It's been open for 11 days with no review yet. This is a small, well-tested fix (+132 lines, 9 new regression tests, all 77 DingTalk tests passing) that addresses a real gap: 1:1 DM voice messages were silently dropped because the empty-text gate in The fix is complementary to the existing group-chat voice note handling (commit 93734c2) — it covers the separate Would appreciate a look when you have a moment. cc @teknium1 @kevinskysunny |
1a4392f to
379f442
Compare
|
Quick follow-up — this PR has been open for 12 days and the last bump was yesterday. Happy to provide additional context, split the change into smaller chunks, or address any concerns if there's something blocking the review. The change is isolated to the DingTalk adapter (no core/gateway changes) and fully tested. Any guidance on review timeline would be appreciated. 🙏 |
DingTalk 1:1 voice messages arrive with msgtype='audio' and empty text/rich_text fields. The server-side ASR result lives in extensions.content.recognition (or extensions.recognition on some SDK versions). Without this fix, the empty-text gate in _on_message silently dropped voice messages in DM conversations. Changes: - _extract_text: pull recognition text from extensions when msgtype='audio' and text/rich_text are empty, with three-level fallback (content.recognition → extensions.recognition → placeholder) - _extract_media: classify msgtype='audio' as MessageType.AUDIO (not TEXT) so the gateway routes through STT when recognition is empty. downloadCode is intentionally NOT added to media_urls — it is an OSS code, not a local file path. - Correct misleading STT comment in _extract_media. - Fix 9 audio test imports: gateway.platforms.dingtalk → plugins.platforms.dingtalk.adapter (post-migration path from 5600105). - Add test_never_returns_empty_string: verifies _extract_text never returns empty for audio messages across 4 fallback paths. Adds 9 regression tests covering all fallback paths, edge cases (non-dict extensions, whitespace-only recognition), and the downloadCode-not-in-media_urls invariant.
379f442 to
1798f2b
Compare
teknium1
left a comment
There was a problem hiding this comment.
Thanks for isolating the 1:1 DingTalk ASR path. The recognition extraction targets a real current-main gap: plugins/platforms/dingtalk/adapter.py:663-670 calls _extract_text() and drops an event with no text or media, while current _extract_text() (:724-749) never reads extensions.
Problems
- The new
MessageType.AUDIOclassification does not route this message through STT.gateway/run.py:10414-10422explicitly keepsAUDIOout ofaudio_paths, and this PR intentionally leavesmedia_urlsempty. TheTestAudioMessageTypeClassificationdocstring therefore asserts behavior the gateway does not provide.
Suggested changes
- Retain the ASR-text extraction, but remove or justify the AUDIO classification; it is not needed for recognized text to clear the existing empty-message gate.
- Add an adapter-level
_on_messageregression asserting the ASR text reacheshandle_message, rather than only helper-level assertions.
This is an automated hermes-sweeper review.
| # empty, the placeholder text "[Voice message — transcription | ||
| # unavailable]" is used; no secondary STT is attempted. | ||
| msg_type = MessageType.AUDIO | ||
|
|
There was a problem hiding this comment.
MessageType.AUDIO does not route through STT: gateway/run.py:10414-10422 explicitly excludes AUDIO from audio_paths, and this branch supplies no media URL. Please remove this classification or document a verified downstream use; the ASR text extraction already prevents the empty-message drop.
|
Closing — these issues have been addressed in the latest upstream release. Thanks for the review feedback. |
What does this PR do?
DingTalk 1:1 voice messages arrive with
msgtype="audio"and emptytext/rich_textfields. The server-side ASR result lives inextensions.content.recognition(orextensions.recognitionon some SDK versions). Without this fix, the empty-text gate in_on_messagesilently dropped voice messages in DM conversations.This restores 1:1 voice support by reading the recognition text from
extensionsand classifyingmsgtype="audio"so the gateway can route through STT when the ASR result is empty. It complements — and does not overlap with — the earlier group-chat voice fix (see below).Related Issue
Fixes #
Type of Change
Changes Made
plugins/platforms/dingtalk/adapter.py—_extract_text(): pull recognition text fromextensionswhenmsgtype="audio"andtext/rich_textare empty, with a three-level fallback:extensions.content.recognition(primary path, current SDK)extensions.recognition(fallback for some SDK versions)"[Voice message — transcription unavailable]"(placeholder so the message is not dropped)plugins/platforms/dingtalk/adapter.py—_extract_media(): classifymsgtype="audio"asMessageType.AUDIO(notTEXT) so the gateway routes through STT when recognition is empty.downloadCodeis intentionally not added tomedia_urls— it is an OSS code, not a local file path, and the STT pipeline would fail trying to open it.tests/gateway/test_dingtalk.py— 9 new regression tests (see below).Relationship to existing fix
Commit
93734c26e(fix(dingtalk): transcribe native voice notes) fixed voice messages arriving via rich-texttype=voiceitems in group chats (where the bot is @mentioned). This PR fixes the separate path for 1:1 DM voice messages wheremsgtype="audio"and the ASR result is inextensions, not rich-text. The two fixes are complementary and do not overlap.How to Test
msgtype="audio"and emptytext/rich_text._on_messagesilently drops the message.extensions(or a placeholder is used) and the message is classified asMessageType.AUDIO, so it is processed / routed through STT instead of dropped.Automated: 9 new regression tests cover the primary
extensions.content.recognitionpath, theextensions.recognitionfallback, the no-recognition → placeholder fallback, edge cases (empty extensions, non-dict extensions, whitespace-only recognition),MessageType.AUDIOclassification, and thedownloadCodenot-in-media_urlsinvariant (2 tests). All existing DingTalk tests (68) continue to pass — 77 passed total.Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests passDocumentation & Housekeeping
docs/, docstrings) — or N/Acli-config.yaml.exampleif I added/changed config keys — or N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/A