Skip to content

fix(device-ts): preserve accepted Whisper batches after stop - #20407

Merged
kodjima33 merged 2 commits into
BasedHardware:mainfrom
1aifanatic:fix/ts-whisper-inflight-stop
Oct 3, 2026
Merged

kodjima33 merged 2 commits into
BasedHardware:mainfrom
1aifanatic:fix/ts-whisper-inflight-stop

Conversation

@1aifanatic

@1aifanatic 1aifanatic commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

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 3f3af198ef2c implementation 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 1.3.14: 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.
  • TypeScript passed both the production tsconfig.json check 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.
  • No Whisper model, physical BLE device, credentials, or live service was used.
  • make setup installed the repository hooks and refreshed the task branch. Its unrelated locked backend install cannot build webrtcvad on this Windows host without MSVC; SDK checks and preflight use the existing test tooling and Python gate environment.
  • The full SDK suite also passed under Linux. Native Windows preflight intermittently timed out in the unchanged Noble subprocess test; the successful Linux run uses the same Bun version and unmodified test deadline.

Tests

sdks/device/typescript/src/stt/index.test.ts exercises 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-delivery concerns budgeted proactive context evaluation, and FC-accepted-session-completes-silently-without-payload concerns 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.

Review in cubic

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 3 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread sdks/device/typescript/src/stt/index.ts Outdated
@Git-on-my-level

Copy link
Copy Markdown
Collaborator

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:

  • src/stt/index.ts — the promise queue (queue = queue.then(job, job)) cleanly replaces the old flush(deliverWhenStopped) flag: accepted batches are always delivered in input order, and the try/catch now guards both opts.runner and opts.onTranscript, so an empty/throwing/rejecting runner or a throwing transcript callback skips that batch without stranding later ones or leaving an unhandled rejection on the final tail (this also resolves the earlier review thread about the callback sitting outside the guard). The if (stopped) return guard makes repeated stop() calls idempotent, and appendPcm after stop remains a no-op — both covered by tests.
  • src/stt/index.test.ts — good deterministic coverage: in-flight batch delivery through the shared factory, pending-full-batch-before-tail ordering, all three runner failure modes (empty/throw/reject), callback-error containment, and the pre-existing buffered-tail behavior as a control. Dependency-free and stable under repeated runs.
  • README.md — the new paragraph matches the verified behavior (input-order delivery, immediate stop() return, no further submission after stop); nothing aspirational.

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 need human response.

@Git-on-my-level Git-on-my-level added positive-signal Automation verified a genuine fix/quality contribution javascript Pull requests that update javascript code labels Oct 3, 2026
@1aifanatic

Copy link
Copy Markdown
Contributor Author

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.

@kodjima33 kodjima33 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trusted contributor (1aifanatic, long merged history). Real issue #12964, not self-filed. Scoped diff, CI clean, extensive regression tests. Confidence 5/5.

@kodjima33
kodjima33 merged commit 8bfac91 into BasedHardware:main Oct 3, 2026
45 of 46 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

javascript Pull requests that update javascript code positive-signal Automation verified a genuine fix/quality contribution

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TypeScript device SDK drops in-flight Whisper transcripts when stopped

3 participants