Skip to content

fix(feishu): include audio duration in uploads - #8631

Closed
sgaofen wants to merge 1 commit into
NousResearch:mainfrom
sgaofen:codex/feishu-audio-duration
Closed

fix(feishu): include audio duration in uploads#8631
sgaofen wants to merge 1 commit into
NousResearch:mainfrom
sgaofen:codex/feishu-audio-duration

Conversation

@sgaofen

@sgaofen sgaofen commented Apr 12, 2026

Copy link
Copy Markdown
Contributor

Summary

  • attach best-effort audio duration metadata when Feishu uploads voice messages
  • probe duration from WAV headers first, then fall back to ffprobe or mutagen when available
  • add regression tests for both upload-body duration plumbing and WAV duration probing

Root Cause

send_voice() reused the generic Feishu file-upload path, but the upload body never included audio duration metadata. Feishu's native audio message rendering depends on that field, so successful uploads could still display with missing or incorrect duration.

Closes #8300.

Validation

  • python3 -m py_compile gateway/platforms/feishu.py tests/gateway/test_feishu.py
  • uv run --extra dev pytest tests/gateway/test_feishu.py -q -k 'send_voice_uploads_opus_and_sends_audio_message or probe_audio_duration_uses_wav_header'
  • uv run --extra dev pytest tests/gateway/test_feishu.py -q -k 'not test_build_event_handler_registers_reaction_and_card_processors'

Platform Tested

  • macOS 15.x (Apple Silicon)

Contribution Guide Notes

  • Reviewed CONTRIBUTING.md and checked for existing open PRs before submitting this scoped change.
  • Ran the targeted verification commands listed above for this PR. I have not claimed a full repo-wide pytest tests/ -q pass unless explicitly noted.

@sgaofen sgaofen changed the title [codex] include audio duration in Feishu uploads fix(feishu): include audio duration in uploads Apr 12, 2026
@sgaofen
sgaofen marked this pull request as ready for review April 13, 2026 00:53
@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 Apr 28, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Overlaps with #16736 which also fixes #8300/#16524 (Feishu audio duration). Both PRs add mutagen/ffprobe probe chain. Recommend consolidating to avoid duplicate merge.

@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 the focused Feishu regression coverage. The underlying gap remains on current main: plugins/platforms/feishu/adapter.py:4559-4563 builds audio upload bodies without duration, and tests/gateway/test_feishu.py:2527-2575 has no duration assertion.

Problems

  • The diff targets gateway/platforms/feishu.py, but current main relocated that adapter to plugins/platforms/feishu/adapter.py in 552adbe0827c32df8ed9bb19e908c26eff43add7; the PR is currently DIRTY.
  • The added probe performs WAV reads and potentially subprocess.run() synchronously inside the async upload path. Current adapter SDK work uses _run_blocking at plugins/platforms/feishu/adapter.py:1692-1695.
  • The added ffprobe call has no timeout.

Suggested changes

  • Port the change and tests to the plugin adapter/current test imports.
  • Execute the probe through the adapter executor and give ffprobe a finite timeout while preserving the best-effort fallback.

Automated hermes-sweeper review.

)
try:
duration_ms: Optional[int] = None
if resolved_message_type == "audio":

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.

_probe_audio_duration_ms() can read files and launch ffprobe; this async upload path should await executor-backed work (for example the adapter's _run_blocking) rather than run the probe on the event loop.

return max(1, int(duration_ms))
except Exception:
pass

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.

Please give this ffprobe invocation a finite timeout. Without one, a hung executable can indefinitely delay outbound voice delivery.

@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 12, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Closing as stale target: this PR patches gateway/platforms/feishu.py, which was deleted when the platform adapters were rewritten as plugins — the live code now lives in plugins/platforms/*/adapter.py, so this diff can no longer apply.

Feishu upload duration is being salvaged via #40592 against the live plugin adapter — and credit where due: this PR was the earliest report/fix of the missing-duration problem.

Thanks @sgaofen — if anything in your change isn't covered by the salvage noted above, please resubmit against the current plugin adapter.

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.

Feishu outbound audio upload omits duration, causing incorrect/0s duration display

3 participants