Repository navigation
fix(dart-sdk): report a Parakeet stream closed before ready - #22347
apoorvdarshan wants to merge 3 commits into
Conversation
No workflow or manifest lane ran the tests in sdks/device/dart, so the package's STT fixes were only ever tested on a contributor's machine. The Hygiene manifest runner has no Flutter toolchain, so add a path-scoped workflow like sdk-cpp.yml that sets up the pinned Flutter 3.44.5 and runs flutter test for the package, and record it in the CI budget inventory. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ParakeetTranscriber drops PCM until the server sends ready. The server can accept the socket and then close it before ready (1013 capacity_full or service_not_ready in backend/parakeet/main.py). The stream listener only marked the stream drained, so every later appendPcm dropped its chunk and stop() completed normally: the caller lost the session's audio with no error to retry on or fall back from. When the stream ends before ready and before stop(), keep why it ended (close code and reason, or the stream error) and throw it as an SttStreamClosedException from appendPcm and stop(). A stream that is still waiting for ready keeps dropping PCM. This mirrors the Go SDK fix in BasedHardware#22301. stt.dart and stt_test.dart are reformatted with dart format --line-length 120, which CI's Formatting job applies to them at the latest language version because the package has no resolved config. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Verdict: the production fix is correct, well-scoped, and well tested - the Dart twin of the Go fix in #22301. Two non-blocking nits on the failure-class record; the only merge gate is maintainer sign-off on the new CI workflow. What I verified, file by file:
Blast radius: confined to the Dart device SDK package - the production Omi app uses native Pigeon BLE and no in-repo code calls these APIs, so the new throws can only reach external package consumers. Cost of the CI lane is ~1 run/day at ~5 minutes. USER IMPACT: none directly - this hardens a device-SDK package; a device maker using the Dart SDK now gets a clear error carrying the server's close code instead of silently losing a whole recording session when Parakeet is at capacity. Still wanted: the failure-class record fixes above, and maintainer sign-off on the new CI workflow itself (new lane + third-party action subosito/flutter-action@v2), which is the only thing between this and an approval. by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with |
|
Verdict: the production fix is correct and nicely scoped; nothing blocking from my side — two follow-ups below, plus the workflow-review label because this adds CI.
API note for SDK consumers: USER IMPACT: none for app users — Dart SDK consumers get a clear error (with the server's close code) instead of silently losing a whole session's audio when Parakeet refuses the stream. What I'd still want as a reviewer: (1) the new Dart workflow green on this PR before merge, (2) the FC record reconciled with where its prevention test actually lands. Workflow additions ( by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with |
* fix(device-ts): report a Parakeet stream closed before ready createParakeetTranscriber drops PCM until the server sends ready. The Parakeet server can accept the socket and then close it before ready (1013 capacity_full or service_not_ready in backend/parakeet/main.py). The transcriber had no close handler before stop(), so appendPcm kept returning and dropping every chunk, and stop() returned normally: the caller lost the session's audio with no error to retry on or fall back from. Record the close when the socket closes before ready and before stop(), and throw it as an SttStreamClosedError (closeCode, closeReason) from appendPcm and stop(); stop() throws after it has torn the socket down. A stream still waiting for ready keeps dropping PCM. This mirrors the Go and Dart SDK fixes in #22301 and #22347. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * ci: run with ci:full Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
kodjima33
left a comment
There was a problem hiding this comment.
Fixes #22346: Parakeet transcriber silently dropped audio on a pre-ready close with no error. Good root cause + tests, CI green. Adds a new CI workflow (.github/workflows/sdk-dart.yml) and is labelled workflow-review -- leaving for Nik to sign off on the CI change before merge.
|
Verdict: the production fix is correct, tightly scoped, and mirrors the merged Go twin (#22301); this head already carries a maintainer approval. One follow-through item on the new CI workflow below — nothing blocking. What I verified on 51f305b:
CI follow-through: no "Dart Device SDK" check-run exists on this head (the app-level "Dart Analyze & Tests" is a different job and skipped), so the new suite has not actually executed in CI on this PR — expected for a fork PR that adds the workflow file itself. Please confirm the workflow's first green run on main after merge, or on the next PR touching sdks/device/dart/, before relying on it. Blast radius: an API contract change for sdks/device/dart consumers only — USER IMPACT: Dart device SDK users get a clear, retryable error carrying the server's close code when transcription is refused before the session starts, instead of silently losing the whole session's audio. Still wanted as a reviewer: the post-merge confirmation of sdk-dart.yml's first green run, and the FC artifact addition above. Neither blocks. — automated review on behalf of the maintainers by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with |
What changed and why
Fixes #22346
ParakeetTranscriberin the Dart device SDK (sdks/device/dart) drops PCM until the server sends{"type":"ready"}. The Parakeet server can accept the WebSocket and then close it beforeready: 1013service_not_readywhile the GPU worker loads, 1013capacity_fullorallocation_rejectedfrom admission, or 1011 (stream_transcribeinbackend/parakeet/main.py). The stream listener'sonDoneandonErroronly marked the stream drained, andappendPcmonly checked_ready. Every laterappendPcmdropped its chunk andstop()completed normally, so the caller lost the whole session's audio with no error to retry on or fall back from. #22301 fixes the same bug in the Go SDK.ready, and beforestop(), the transcriber keeps why: the channel's close code and reason, or the stream error for a failed connection.appendPcmthrows it as a newSttStreamClosedException(closeCode,closeReason,error), andstop()throws it after it has closed the socket. A stream still waiting forreadykeeps dropping PCM, and a close that answers our ownfinalizeduringstop()is not reported. Deepgram is unchanged.sdks/device/dart, and the Hygiene runner that executes manifest checks has no Flutter toolchain. A new path-scoped.github/workflows/sdk-dart.yml, likesdk-cpp.ymlandsdk-rust.yml, sets up the pinned Flutter 3.44.5 and runsflutter testin the package..github/ci-budget.yamlrecords it at 1 run/day x 5 minutes.Product invariants affected
none
How it was verified
origin/main(0a132902c6): theParakeetTranscriberlistener,appendPcmandstop()insdks/device/dart/lib/stt/stt.dart, and the server's closes beforereadyinbackend/parakeet/main.pyandbackend/parakeet/admission.py.web_socket_channel3.0.3, whichWebSocketChannel.connectuses: on a close frame it storescloseCodeandcloseReasonbefore it closes the stream, so both are set whenonDoneruns, and a failed connection arrives as aWebSocketChannelExceptionstream error followed by done.stop()tests end the fake stream only in response tofinalize, afterstop()has marked the transcriber stopped, so they still complete without an error.python3 .github/scripts/check_ci_budget.py --base origin/mainpasses with the new entries.dart format --line-length 120 --set-exit-if-changed(Flutter 3.44.5's Dart 3.12.2, run from the repository root like the Formatting job) reports both Dart files unchanged.flutter testlocally. The newDart Device SDKworkflow runs the package's suite on this PR, and the rest of CI runs withci:full.Tests
sdks/device/dart/test/stt_test.dart:Parakeet reports a stream the server closed before ready: the fake channel closes with 1013capacity_fullbeforeready, as the hosted server does at capacity. A chunk appended whilereadyis pending is dropped without an error. After the close,appendPcmandstop()both throwSttStreamClosedExceptionwith code 1013 and reasoncapacity_full, no PCM is sent, and the socket is closed. Onmain,appendPcmreturns normally andstop()completes, so the test fails.Parakeet reports a stream that failed before ready: the stream errors and ends beforeready; both calls throw the exception carrying that error.Both were written to fail without the fix and run in CI; they have not been run locally.
UI evidence
UI-Evidence: none -- Dart device SDK library change with no UI; the regression tests above cover the behaviour
Failure class (fixes)
Failure-Class: new
Failure-class transition narrative
New class
FC-readiness-gate-swallows-terminal-close. It is the class #22301 (open) declares for the Go SDK instance. It is not onmainyet, so this PR adds a byte-identical copy of that file (blob048aec1a3d), and whichever of the two PRs merges second adds nothing new to it. If #22301 merges first, this PR's declaration becomesFailure-Class: FC-readiness-gate-swallows-terminal-close.stop()report it once the stream has ended. HereParakeet reports a stream the server closed before readycloses the stream beforereadyand checks both calls.stt.dartonorigin/mainbefore this change.evidence_prsis empty because neither PR has merged.New guards
.github/workflows/sdk-dart.ymlruns the Dart device SDK's tests, including the regression tests above. Instance: #13627 fixed two Dart SDK STT bugs (#13625, #13635) with tests that its PR validated only with a localflutter testrun, because no CI job selectssdks/device/dart; this PR's regression tests would not run anywhere either. It is not a manifest lane because the Hygiene job that runs manifest checks has no Flutter toolchain, so, like the C++ and Rust SDKs, the package gets its own path-scoped workflow.Scoped cleanups
With no resolved package config for
sdks/device/dart, the Formatting job formats these files at the latest Dart language version and rejectsstt.dartandstt_test.dartas they are onmain. Runningdart format --line-length 120on them reflows some existing lines (constructor calls and list literals); those hunks are formatting only.🤖 Generated with Claude Code