fix: preview disc stuck at .processing after stop
The previous code path for the keyboard preview's ASR controller
cancelled the consumer task at the exact moment it closed the
audio stream:
asrTask?.cancel() // ← kills the .final consumer
asrTask = nil
...
bufferContinuation?.finish() // tells ASR "no more audio"
The cancellation cascaded: the for-await on the events stream
exited → the AsyncStream's `continuation.onTermination` fired →
ASR.cancel() ran → producer task was marked cancelled → the
producer's `if !Task.isCancelled { yield(.final) }` guard
suppressed the .final event. Net result: nobody told the UI to
leave `.processing`, and the disc sat there forever.
Fix (4 changes):
1) `stop()` no longer cancels the consumer. The consumer task
exits naturally when the events stream finishes, sees the
`.final` event the producer still yields, and transitions
the phase out of `.processing`. This is the primary fix.
2) `start()` cancels any leftover `asrTask` at the entry point
as a safety net — covers the "user smashes the disc twice
quickly" race where a previous consumer is still draining.
3) `stop()` schedules a 3-second safety-net Task: if the ASR
pipeline never produces a `.final` (analyzer hang, system
glitch), force the phase back to `.idle` so the user isn't
stuck. Normal recordings complete well under 3 seconds, so
the timeout is only hit on the unhappy path.
4) `KeyboardPreviewSheet` adds `.onDisappear { asr.stop() }`
so closing the sheet mid-recording releases the
AVAudioSession and mic. `stop()` is idempotent (no-op on
non-recording phases), safe to call here.
State machine: `phase = .processing` now has TWO transition
paths out — the consumer receiving `.final` (fast path) and
the 3-second safety net (fallback). Both are required; the
fast path is the common case, the fallback is the
"guaranteed-progress" guarantee.
Testability: `asrTask` was `private`; relaxed to `internal` so
the regression test in
`OSGKeyboardTests/PreviewASRControllerStateTests.swift` can
install a known consumer task and assert `stop()` does not
cancel it. The class is `@MainActor` so Swift 6 isolation
rules still prevent production code outside the class from
racing on it.
Tests:
- `testStopDoesNotCancelConsumerTask` — primary fix regression.
- `testStopIsIdempotent` — `.onDisappear` after a manual stop
doesn't misbehave.
- 27/27 tests pass (25 existing + 2 new).
- BUILD SUCCEEDED.
🤖 Generated with Claude Code
This commit is contained in:
@@ -0,0 +1,70 @@
|
||||
// PreviewASRControllerStateTests.swift
|
||||
// OSGKeyboard · Tests
|
||||
//
|
||||
// Locks in the state-machine contract for the keyboard preview's
|
||||
// ASR controller. The previous bug was: `stop()` called
|
||||
// `asrTask?.cancel()` on the consumer task that was waiting for the
|
||||
// ASR `.final` event. The cancellation cascaded through
|
||||
// `continuation.onTermination → self.cancel()` and suppressed the
|
||||
// `.final` yield, leaving the controller in `.processing` forever
|
||||
// because no one scheduled the transition out. The fix is: don't
|
||||
// cancel on stop, let the consumer naturally see the `.final` and
|
||||
// flip the phase.
|
||||
//
|
||||
// These tests don't drive a real ASR pipeline — they verify the
|
||||
// state-machine contract directly. The class is `@MainActor`
|
||||
// isolated, so all assertions happen on main. `asrTask` is
|
||||
// `internal` (not `private`) precisely so this file can install a
|
||||
// known consumer task and observe whether `stop()` cancels it.
|
||||
//
|
||||
// Note on the 3-second processing-timeout safety net: that
|
||||
// fallback (a `Task { sleep 3s; if .processing → .idle }` block
|
||||
// inside `stop()`) is not unit-tested here — testing it would
|
||||
// require either a 3-second test or extracting the policy to a
|
||||
// testable helper. The primary fix above removes the original
|
||||
// bug outright; the timeout is a defensive safety net for a
|
||||
// separate class of failure (analyzer hang) and is straightforward
|
||||
// enough to read that a test adds little. If a future change
|
||||
// touches the safety-net block, re-introduce a test that mocks
|
||||
// the ASR to never yield `.final` and asserts the timeout fires.
|
||||
|
||||
import XCTest
|
||||
@testable import OSGKeyboard
|
||||
import AVFoundation
|
||||
|
||||
@MainActor
|
||||
final class PreviewASRControllerStateTests: XCTestCase {
|
||||
|
||||
/// Core fix: `stop()` MUST NOT cancel `asrTask`. The consumer
|
||||
/// task is the only place the `.final` event can land, and
|
||||
/// cancelling it leaves the UI in `.processing` forever. This
|
||||
/// is the regression test for the original bug — if it ever
|
||||
/// fails, the disc is stuck again.
|
||||
func testStopDoesNotCancelConsumerTask() {
|
||||
let controller = PreviewASRController()
|
||||
let consumerTask = Task<Void, Never> {}
|
||||
controller.asrTask = consumerTask
|
||||
|
||||
controller.stop()
|
||||
|
||||
XCTAssertFalse(
|
||||
consumerTask.isCancelled,
|
||||
"stop() must not cancel the consumer task; the .final event needs the consumer alive to land"
|
||||
)
|
||||
}
|
||||
|
||||
/// Idempotency: calling `stop()` twice (e.g. `.onDisappear`
|
||||
/// fires after the user already stopped manually) must not
|
||||
/// misbehave — no double-cancel, no extra state transitions.
|
||||
/// This is the contract `.onDisappear` relies on.
|
||||
func testStopIsIdempotent() {
|
||||
let controller = PreviewASRController()
|
||||
let consumerTask = Task<Void, Never> {}
|
||||
controller.asrTask = consumerTask
|
||||
|
||||
controller.stop()
|
||||
controller.stop()
|
||||
|
||||
XCTAssertFalse(consumerTask.isCancelled)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user