Repository navigation
fix(device-ts): preserve accepted Whisper batches after stop - #20407
Conversation
There was a problem hiding this comment.
All reported issues were addressed across 3 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
Verified locally with Bun 1.3.14: the six new regression tests fail against current main's implementation and all pass on this head, confirming the #12964 defect (an in-flight full batch is dropped when stop() lands mid-run) is still live on main and fixed here. File-by-file:
Credit for the investigation trail on the previously closed #12971/#13613 — re-checking that the current base still drops in-flight batches and rebuilding the fix compactly against main was the right call. Left for human maintainer merge decision. Feedback generated by automated maintainer review on David's behalf. by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with |
|
Thanks for the detailed review. I verified that the six new regression tests fail on current main and pass on this head; the callback-throw case from the earlier review is included too. The queue now keeps accepted batches in order and contains both runner and callback failures. Appreciate the confirmation on the README and the remaining maintainer review. |
What changed and why
Fixes #12964. Stopping the TypeScript device SDK's local Whisper transcriber now preserves transcripts for accepted full batches and the final tail, delivered in input order. The implementation reuses the promise-queue pattern already present in the React Native Whisper adapter; empty or failed runner results do not strand subsequent batches, callback errors are contained even in the final batch, and stop still returns immediately.
The earlier work in #12971 and #13613 identified this defect and the ordering requirement. Both PRs are closed. Although #12971's closing comment says the production change is on main, the current
3f3af198ef2cimplementation still discards an in-flight full batch after stop. The new factory-level regression reproduces that behavior on this base. This PR credits those investigations and supplies a fresh, compact fix against current main.Product invariants affected
none
How it was verified
bun test sdks/device/typescript— 38 tests passed, 78 assertions. The five initial regression cases fail against the unchanged implementation; the existing buffered-tail control passes. A follow-up callback-error regression fails on the first PR commit and passes with the review fix, without intercepting unhandled rejections.tsconfig.jsoncheck and a separate strict check of all three test files with Bun 1.3.14 type definitions (the production config excludes tests). Existing cached compiler/type dependencies were used without changing package dependencies.make preflight— all 14 selected checks passed on Linux, including the SDK suite and draft PR metadata contracts.make setupinstalled the repository hooks and refreshed the task branch. Its unrelated locked backend install cannot buildwebrtcvadon this Windows host without MSVC; SDK checks and preflight use the existing test tooling and Python gate environment.Tests
sdks/device/typescript/src/stt/index.test.tsexercises the real shared factory and Whisper adapter with synthetic PCM and controlled runners: an in-flight full batch across stop, full-batch-before-tail ordering, repeated stop, post-stop input rejection, continuation after empty results, synchronous throws, and rejected promises, and safe completion when the first and final transcript callbacks throw. The suite already runs through the shared local/CI manifest.Failure class (fixes)
Failure-Class: none
The failed boundary is completion of SDK audio accepted before stop.
FC-active-only-guard-aborts-owed-deliveryconcerns budgeted proactive context evaluation, andFC-accepted-session-completes-silently-without-payloadconcerns sessions that never receive audio; neither covers this SDK runner-completion defect. This fix retains accepted work in the existing adapter and adds regression coverage to its existing suite; it introduces no new shared checker or registry guard.