Skip to content

fix(feishu): attach ffprobe-derived duration to voice uploads - #40592

Closed
seamusmore wants to merge 1 commit into
NousResearch:mainfrom
seamusmore:feat/feishu-voice
Closed

fix(feishu): attach ffprobe-derived duration to voice uploads#40592
seamusmore wants to merge 1 commit into
NousResearch:mainfrom
seamusmore:feat/feishu-voice

Conversation

@seamusmore

@seamusmore seamusmore commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

Adds ffprobe duration probing to _send_uploaded_file_message, passing the duration (in ms) to both the file upload body and the message payload for audio-type messages. Falls back to the existing pure-Python OGG/Opus granule parser when ffprobe is unavailable.

Background

This PR originally carried six Feishu voice-path fixes. During rebase onto current main:

This PR now carries only the adapter-side duration handling that is not yet on main.

Changes

plugins/platforms/feishu/adapter.py:

  • ffprobe duration probing in _send_uploaded_file_message: probes the file with ffprobe via asyncio.to_thread (non-blocking), stores duration in ms as upload_duration.
  • Duration fallback: upload_duration or duration_ms — prefers ffprobe result, falls back to the existing pure-Python OGG/Opus granule-position parser (_get_audio_duration_ms) for .opus files.
  • Payload duration: when resolved_message_type == "audio" and upload_duration is available, the duration is included in the Feishu message payload ({"file_key": ..., "duration": ...}) so Feishu clients display the correct voice-bubble length.
  • Thread retry path: the 99992402 retry logic also uses payload_dict (with duration) instead of the bare {"file_key": ...}.

Fixes #16524, #8300.

Related

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists platform/feishu Feishu / Lark adapter comp/gateway Gateway runner, session dispatch, delivery tool/tts Text-to-speech and transcription labels Jun 6, 2026
seamusmore pushed a commit to seamusmore/hermes-agent that referenced this pull request Jun 9, 2026
Feishu requires duration (ms) during file upload (POST /im/v1/files)
for audio files, not just in the message payload. Previously only the
message send included duration via ffprobe; the file upload body was
missing it. Add duration extraction before upload and pass it to
_build_file_upload_body so the SDK includes it in CreateFileRequestBody.

This completes the fix from NousResearch#40592 which only added duration to the
message payload.

@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 identifying several real Feishu voice-path gaps. Current main still has the reported behaviors, but this branch needs a targeted salvage.

Problems

  • Feishu was migrated to the bundled plugin in 560010547; the live code is now plugins/platforms/feishu/adapter.py:2158, :3822, and :4536. gateway/platforms/feishu.py no longer exists, so these hunks need porting rather than a direct merge.
  • subprocess.run() blocks async adapter execution for up to 60 seconds in the PR at gateway/platforms/feishu.py:2072, and ffprobe blocks similarly at :4401 and :4446.
  • tests/gateway/test_feishu.py:2596 only covers an existing Opus upload; this change needs regression coverage for STT classification, conversion/duration handling, and cross-turn TTS dedup.

Suggested changes

  • Port the Feishu work to plugins/platforms/feishu/adapter.py, run ffmpeg/ffprobe off-loop, calculate duration once, and add focused tests.

Automated hermes-sweeper review.

Comment thread gateway/platforms/feishu.py Outdated
Comment thread gateway/platforms/feishu.py Outdated
@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages 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 14, 2026
@seamusmore seamusmore closed this Jul 14, 2026
@seamusmore seamusmore reopened this Jul 14, 2026
@seamusmore
seamusmore force-pushed the feat/feishu-voice branch 8 times, most recently from 329bfad to cb52407 Compare July 14, 2026 10:25
@seamusmore
seamusmore force-pushed the feat/feishu-voice branch 2 times, most recently from 969d269 to cb52407 Compare July 14, 2026 10:49
teknium1 pushed a commit that referenced this pull request Jul 28, 2026
…dio path dedup

Trimmed cherry-pick of PR #40592 (duration + dedup hunks only; the
voice-classification hunk duplicates #29235 and the send_voice Opus
rewrite is out of scope for this inbound-focused PR):

- adapter.py: ffprobe duration (off-loop) attached to Feishu voice
  uploads via _build_file_upload_body(duration=...) and the audio
  message payload (#16524, #8300)
- gateway/run.py: TTS dedup narrowed to the current turn;
  _enrich_message_with_transcription dedups repeated audio paths

Refs #40592 #16524 #8300
teknium1 pushed a commit that referenced this pull request Jul 28, 2026
…dio path dedup

Trimmed cherry-pick of PR #40592 (duration + dedup hunks only; the
voice-classification hunk duplicates #29235 and the send_voice Opus
rewrite is out of scope for this inbound-focused PR):

- adapter.py: ffprobe duration (off-loop) attached to Feishu voice
  uploads via _build_file_upload_body(duration=...) and the audio
  message payload (#16524, #8300)
- gateway/run.py: TTS dedup narrowed to the current turn;
  _enrich_message_with_transcription dedups repeated audio paths

Refs #40592 #16524 #8300
teknium1 pushed a commit that referenced this pull request Jul 28, 2026
…dio path dedup

Trimmed cherry-pick of PR #40592 (duration + dedup hunks only; the
voice-classification hunk duplicates #29235 and the send_voice Opus
rewrite is out of scope for this inbound-focused PR):

- adapter.py: ffprobe duration (off-loop) attached to Feishu voice
  uploads via _build_file_upload_body(duration=...) and the audio
  message payload (#16524, #8300)
- gateway/run.py: TTS dedup narrowed to the current turn;
  _enrich_message_with_transcription dedups repeated audio paths

Refs #40592 #16524 #8300
adapter.py:
- send_voice: async Opus conversion via asyncio.to_thread (off-loop)
- _resolve_normalized_message_type: AUDIO -> VOICE for STT routing
- _send_uploaded_file_message: ffprobe off-loop, duration reused
- _build_file_upload_body: optional duration parameter

gateway/run.py:
- _should_send_voice_reply: TTS dedup narrowed to current turn
- _enrich_message_with_transcription: audio path dedup

Tested: 5/5 single transcription, no event-loop blocking.
@isak-ialogics

Copy link
Copy Markdown
Contributor

Current head a81aafd no longer matches the advertised six-fix scope: GitHub’s diff is only plugins/platforms/feishu/adapter.py (+17/-4), adding ffprobe duration handling/payload data. It contains none of the claimed _resolve_normalized_message_type AUDIO→VOICE routing, send_voice Opus conversion, gateway _should_send_voice_reply / _enrich_message_with_transcription dedup changes, or regression tests. As-is, it therefore does not fix the voice-routing or dedup behavior named in the title/body. Suggested next action: restore those live-path hunks with focused regressions, or narrow the title/body to a duration-only fix before maintainer review.

@seamusmore seamusmore changed the title fix(feishu): voice routing to STT pipeline + dedup scope fix + missing duration fix(feishu): attach ffprobe-derived duration to voice uploads Jul 29, 2026
@seamusmore

Copy link
Copy Markdown
Contributor Author

The two failing CI checks are unrelated to this PR's changes:

  • Desktop E2E / Playwright E2E (Linux)correction-session-switch.spec.ts (Electron desktop test) times out after ~48s on both attempts. This PR only touches plugins/platforms/feishu/adapter.py; the failing test exercises session-switch UI behavior in the desktop app and has no dependency on the Feishu adapter.
  • All required checks pass — aggregate gate that fails because the E2E job above failed.

All relevant checks pass: Python tests (8/8 slices), ruff lints, Docker builds (amd64 + arm64), OSV scan. This appears to be a pre-existing flaky E2E test, not a regression from this diff.

@teknium1

Copy link
Copy Markdown
Contributor

Merged into main via consolidated salvage PR #73515 (merge c911a5f10f). Your Feishu duration/dedup hunks (ffprobe duration on voice uploads, TTS dedup narrowed to the current turn, audio-path dedup) were cherry-picked with your authorship; the voice-classification hunk duplicated #29235 and the outbound Opus rewrite was out of scope.

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
randlee pushed a commit to randlee/hermes-agent that referenced this pull request Aug 11, 2026
…dio path dedup

Trimmed cherry-pick of PR NousResearch#40592 (duration + dedup hunks only; the
voice-classification hunk duplicates NousResearch#29235 and the send_voice Opus
rewrite is out of scope for this inbound-focused PR):

- adapter.py: ffprobe duration (off-loop) attached to Feishu voice
  uploads via _build_file_upload_body(duration=...) and the audio
  message payload (NousResearch#16524, NousResearch#8300)
- gateway/run.py: TTS dedup narrowed to the current turn;
  _enrich_message_with_transcription dedups repeated audio paths

Refs NousResearch#40592 NousResearch#16524 NousResearch#8300
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists platform/feishu Feishu / Lark adapter 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 sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages 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.

feat: Include audio duration in Feishu voice message payload

4 participants