Skip to content

feat: add qwen asr - #2383

Merged
wangyoucao577 merged 4 commits into
mainfrom
feature/yuyidi/qwen_asr
Nov 4, 2026
Merged

wangyoucao577 merged 4 commits into
mainfrom
feature/yuyidi/qwen_asr

Conversation

@diyuyi-agora

Copy link
Copy Markdown
Contributor

No description provided.

@github-actions

This comment was marked as outdated.

@github-actions

This comment was marked as outdated.

@github-actions

This comment was marked as outdated.

@github-actions

This comment was marked as outdated.

@github-actions

This comment was marked as outdated.

@github-actions

This comment was marked as outdated.

@github-actions

github-actions Bot commented Nov 3, 2026

Copy link
Copy Markdown

Review: qwen_asr_python

I reviewed this against docs/ai/L1/L2/asr_plugin_design_review.md. I found no MUST-level code violations in the extension. The items below need attention before merge.

1. This PR rewrites the review baseline it is judged against (process blocker)
docs/ai/L1/L2/asr_plugin_design_review.md and docs/ai/L1/L2/testing.md drop "A bounded retry strategy MUST escalate to fatal" and the "Retry ceiling reached → FATAL_ERROR" row, and make unlimited non-fatal retry the default. Under the current guide, _reconnect_loop (no ceiling) is arguably already allowed. Even so, changing the framework-wide ASR merge contract inside a vendor PR with an empty description skips the "framework contract change with migration" path the guide requires. Please split the doc change into its own docs: PR and get maintainer sign-off there. That PR should also explain how existing bounded-retry plugins relate to the new default.

2. No guarder or standalone test evidence (§13.2)
The PR description is empty. Please attach results from task test-extension EXTENSION=agents/ten_packages/extension/qwen_asr_python and task asr-guarder-test EXTENSION=qwen_asr_python CONFIG_DIR=tests/configs. The guarder two-cycle, reconnect, invalid-credential, and dump-equality checks matter most here because finalize ends the vendor task and starts a new one.

3. The audio consumer blocks on vendor round-trips under _swap_lock (performance, §6.2/§9.3)

  • send_audio() takes _swap_lock for every frame (extension.py:306). On a session change or when no task is ready, it awaits client.finish_task() and client.start_task() inside that lock (_prepare_task_for_session_locked, extension.py:835-860). Each of those calls can wait up to connect_timeout_s (10 s).
  • _finalize_impl holds _swap_lock while it awaits finish_task() for up to finalize_timeout_ms (extension.py:381-394). Every send_audio call queues behind it for that whole time.
  • If the socket drops while one of these waits is in flight, _handle_close runs on the reader task and also waits for _swap_lock. Disconnect reporting and reconnect are then delayed until the 5–10 s timeout fires, instead of starting immediately.

This does not deadlock, but the pipeline stalls for several seconds at each turn boundary and on each failure. Two possible fixes: set a "task transition" flag under the lock and do the vendor wait outside it (frames are already rebuffered through _keep_buffer_frame), or have the client close path set _task_failed so waiters fail fast.

4. Minor

  • _send_audio_locked computes duration as int(len / (sr/1000*2)) (extension.py:827-829). It ignores input_audio_channels()/input_audio_sample_width() (unlike _keep_buffer_frame), and truncating per frame lets the timeline drift for frame sizes that are not a whole number of milliseconds. Use the §6.4 formula and accumulate in float, or in bytes.
  • In start_connection, buffered frames flushed to the vendor are dumped only when ready_to_report_connected is true (extension.py:237-248). If the generation check fails after a successful flush, that audio was sent but never dumped, which can break dump byte equality.
  • manifest.json has no trailing newline.

ASR checklist

Item Result
Lifecycle (super calls, init-failure latch, stop cancels tasks) Pass
Connection state (connected only after task-started, generation guard, on_disconnected on failures) Pass
Buffering (Keep mode, 10 MB bound, per-frame session metadata on flush, flush after recovery) Pass
Finalize (finish-task/task-finished, timeout, always one asr_finalize_end with saved id/metadata, serialized) Pass (see 3 for latency)
Reconnect (single owner, capped backoff, fatal latch, stop cancel, ConnectionClosedOK emits no error) Pass under current guide; policy change needs separate review (1)
Result shape (metadata.session_id, asr_info, BCP-47 language, empty/heartbeat results dropped) Pass
Metrics (connect delay after handshake, base TTFW/TTLW, cumulative timeline across tasks) Pass (minor duration rounding, 4)
Tests (standalone tests cover finalize/reconnect/stale/redaction/timeline) Standalone tests present; guarder evidence missing (2)

@TEN-framework TEN-framework deleted a comment from github-actions Bot Nov 3, 2026
@wangyoucao577
wangyoucao577 merged commit b46e8cb into main Nov 4, 2026
42 checks passed
@wangyoucao577
wangyoucao577 deleted the feature/yuyidi/qwen_asr branch November 4, 2026 03:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants