ReviewOS

also looking at this

craft-native/craft

feat(ios): the speech pair closes the last deferral — ratchet 28 -> 26

#101
Merged glennmichael123 wants to merge feat/ios-speech-pair into main
10 files +1287 -573
10 days ago

startListening and stopListening move from the Swift shim into bridge_mobile_speech.zig, which until now served nothing on purpose. An earlier round researched both actions in full, found four blockers, and wrote down five preconditions rather than shipping. All five are met here, so the file goes from a record of a decision to the implementation it described.

80 of 106 actions in Zig. Ratchet 28 → 26.

The realtime tap

-[AVAudioNode installTapOnBus:bufferSize:format:block:] runs its block on the audio I/O thread, which nothing else in this tree touches. Swift's recognitionRequest?.append(buffer) gets a free ARC retain for the duration of the call; Zig gets nothing, so releasing the request while a tap call is in flight is a use-after-free on a realtime thread.

The handoff is a published pointer plus an in-flight counter, and every access is .seq_cst rather than acquire/release. That is not caution — the dangerous interleaving is Dekker's: the tap increments then loads the pointer while teardown clears the pointer then loads the counter, and acquire/release permits both sides to see the other's old value. That case is precisely "teardown believes no tap is running while a tap holds a pointer it is about to free". One xchg per audio buffer (~86/sec) is not worth reasoning around.

Teardown stops the engine, removes the tap, clears the pointer, then drains for a bounded 20ms. Engine-then-pointer in that order, not the reverse: clearing first would silently drop buffers the tap had already delivered. If the drain ever fails, the request is leaked rather than released, and the fact is logged.

Two crashes that were waiting

  • With no input route, -outputFormatForBus:0 answers a format whose sampleRate and channelCount are both 0, and installTapOnBus: then raises an NSException — an uncatchable SIGABRT. Swift aborts there today (CraftApp.swift:2535-2538); formatCarriesInput refuses instead, using the check AVAudioEngine.h:459-461 spells out.
  • The recognition handler fires on an internal Speech queue, so it hops to the main queue before anything touches AVAudioEngine.

Verified on a simulator, not asserted from the headers

The fixture drives both actions and asserts two new events. craftSpeechStart is emitted on the far side of the whole chain — plist guard, requestAuthorization:'s block on an arbitrary queue, the main-queue hop, the audio session, a live SFSpeechRecognizer, a non-zero input format, the recognition task, the tap installed on the audio I/O thread, and AVAudioEngine actually starting. Any link short of the last emits craftSpeechError instead.

Both assertions were mutation-tested: forcing startAndReturnError: to fail produces the i=74 refusal note and fails on i=68; removing the end emit fails on i=72. The second mutation initially reported the wrong cause — openSettings is chained off craftSpeechEnd and its assertion ran first — so the speech block now sits ahead of it and the failure names the real thing.

One finding worth recording. simctl privacy documents nine services and none is speech, but it does not validate the argument: raw kTCCServiceSpeechRecognition is accepted where speech-recognition is not. Without it the run stopped dead at a "would like to access Speech Recognition" alert no script can answer — which a screenshot of the simulator showed, and the logs did not.

Divergences, each in the module header

  • The plist key stands in for the config flag. NSSpeechRecognitionUsageDescription is written from enableSpeechRecognition and nothing else (no ||), so a missing key means the flag was off — and the message emitted is the one Swift's nil-recognizer guard would have emitted. The check runs before requestAuthorization:, where Swift's runs after, because asking without the key terminates the process rather than failing.
  • A zero format reports "Audio engine failed" rather than inventing a fifth message Swift has never sent.
  • A second startListening while running stops first. Swift cancels only the task and then installs a second tap on bus 0, which raises.
  • A stop that stops nothing still emits craftSpeechEnd. Swift's stopSpeechRecognition() has no early return, and that event is what a page's listening indicator binds to.
  • A recognition error still emits no craftSpeechError — Swift discards the NSError, Android does not, and inventing a string here would prejudge a contract question that belongs to the JS surface.

triggerHapticChecked becomes pub for one caller: CraftApp.swift:2544 and :2558 fire a light impact directly, bypassing the enableHaptics gate the dispatcher applies to case "haptic". That bypass is the spec's, not a choice this port makes.

Verification

  • zig build test, zig fmt --check, zig build, build-ios, build-ios-simulator — all green
  • packages/ios/fixtures/zig-slice/build-and-run.sh — PASS, 39 assertions, from a clean uninstall/install

10 changed files on the files tab, with 0 review threads.

Sign in to comment on this pull request.