edison/chanora
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(ptt): disable missed-key-up watchdog on P0 (DEC-031, supersedes DEC-028)

Browse Source 
The watchdog spawned by ChanoraSession::start_audio cleared
ptt_held after 30 s of continuous PTT key-down. That was correct
for the 'OS lost the key-up event' failure mode the original
SAD-079 / DEC-028 was designed to catch, but it was the wrong
shape for real human speech: anyone holding the bound key for a
long answer got cut off mid-sentence.

For P0:
- Comment out the spawn site in ChanoraSession::start_audio with
  the rationale + the P1 redesign options under consideration
  (raised ceiling / OS key-state polling / RMS-silence fallback).
- Leave the MissedKeyUpWatchdog Rust type, its spawn / spawn_on_signal
  entry points, and all unit tests in chanora_audio::ptt unchanged
  so P1 can re-enable with the chosen detection strategy without
  re-implementing anything.

Spec: new DEC-031 in product-decision-register.md supersedes
DEC-028 for the v1 ship. DEC-028 stays in the register as
historical context. The §7 open-decisions log + §8 change history
get matching 0.9.12 rows.

Note: Mumble and TeamSpeak ship without a comparable watchdog —
the 30 s ceiling was stricter than industry baseline. The
underlying protection (OS-level key-up loss) is still worth
solving, just not with a fixed timeout.

cargo test --workspace --lib: 80 passed / 0 failed / 1 ignored
(unchanged; the watchdog unit tests still run because the type
itself is unchanged).
docs validator: clean (pre-existing 35-filename warning only).
This commit is contained in:
EdisonJwa
2026-05-16 01:04:40 +08:00
parent 6d4975bd6e
commit 45fec2310e
2 changed files with 28 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files
core/chanora_core/src/lib.rs
+25 -20
View File
@@ -562,26 +562,31 @@ impl ChanoraSession {
let controller = ptt::PttController::new(self.release_tail.clone());
state.ptt_controller = Some(controller.clone());
// Spawn the missed-key-up watchdog on first start_audio.
// It subscribes to the selector's `ptt_held` watch — NOT
// the real gate — so it only fires when the user has
// actually been holding the PTT key for >30 s. In
// Continuous mode `ptt_held` is never set true, so the
// watchdog never fires (the fix for the bug where
// Continuous transmission would disable itself after the
// 30 s default timeout).
{
let mut wd = self.ptt_watchdog.lock().await;
if wd.is_none() {
let signal = self.voice_selector.subscribe_ptt_held();
let selector = self.voice_selector.clone();
*wd = Some(chanora_audio::MissedKeyUpWatchdog::spawn_on_signal(
signal,
move || selector.set_ptt_held(false),
chanora_audio::MissedKeyUpWatchdog::DEFAULT_TIMEOUT,
));
}
}
// SAD-079 / DEC-028 missed-key-up watchdog: DISABLED for
// P0 per owner decision (2026-05-16). The original 30 s
// ceiling caused real users to be cut off mid-sentence in
// PTT mode whenever they spoke for longer than the
// timeout. The watchdog's purpose (catching OS-level
// key-up loss when the app loses focus / is minimised /
// hits App Nap) is real, but the fixed-timeout
// implementation is the wrong shape.
//
// P1 redesign options under consideration:
// * Raise ceiling to ~5 min (owner-tunable, per DEC-028)
// * Add Windows GetAsyncKeyState / macOS
// CGEventSourceKeyState / X11 XQueryKeymap polling so
// we detect the actual OS desync directly instead of
// timing out on legitimate long speech
// * Combine with an RMS-silence check once the audio
// level meter (P1) lands, so the watchdog only fires
// when the user has been "transmitting" silence for
// the entire window
//
// Until P1 picks one of those, we ship without the
// watchdog. The existing MissedKeyUpWatchdog code path
// and tests remain in place so the P1 work can re-enable
// it with the chosen detection strategy.
let _ = &self.ptt_watchdog;
// Apply any binding the user saved before audio was running
// (SDD-094 follow-up). Persistence + caching happen in
docs/governance/product-decision-register.md
+3
View File
@@ -60,6 +60,7 @@ A decision marked **Proposed / Owner Confirmation Required** is a recommended de
| DEC-028 | Missed-key-up watchdog requirement | **P0.** Resolves PTT-OPEN-006. The audio engine shall include a missed-key-up watchdog that clears `transmit_active` after a configured ceiling (default 30 s) when no key-up event arrives. The watchdog is a P0 release-gate item rather than an implementation-level concern because the failure mode (stuck transmission after the user has released the binding) is user-visible and reputation-relevant. | Accepted | Audio Owner + Software Architect | Prevents stuck-PTT bug class regardless of platform-input quirks. |
| DEC-029 | Flutter global-hotkey packages rejected for PTT | **Use the native Rust `DesktopPttBackend` trait + per-OS implementations (already specified in SDD-083 / SDD-084 / SDD-085); do not adopt `hotkey_manager`, `super_hot_key`, or any equivalent pub.dev package for PTT.** Rationale: those packages wrap the OS `RegisterHotKey` / `RegisterEventHotKey` semantics — they consume the key (suppressing it from other applications), they do not deliver a key-up event, and they do not support mouse side-buttons. PTT requires the opposite primitive (observe, do not consume, deliver both up and down). | Accepted | Software Architect + Audio Owner | Locks the v1 PTT capture path to the native backend; removes ambiguity for future maintainers tempted to "simplify" via a Flutter package. |
| DEC-030 | Voice activity detection deferred to P1 | **`TransmitMode::VoiceActivity` ships as a reserved enum variant with no v1 implementation.** The settings UI shows it as a disabled "coming soon" option. The actual implementation choice is deferred to a future baseline. Rationale: three viable backends were compared (RMS energy threshold — trivial but quality-poor; WebRTC VAD via the `webrtc-vad` crate — frozen-but-stable C++ BSD-3 dep, ~200 KB binary, industry baseline; Silero VAD via ONNX Runtime — best quality but ~8–16 MB binary uplift per platform plus an ONNX-runtime dependency surface). The v1 dependency-surface budget does not have room for the trade-off review required to pick correctly. Choosing too early risks either user-visible quality regression (RMS) or a forced ONNX adoption (Silero) before there is a comparable need for ML inference elsewhere (noise suppression, AEC). | Accepted | Audio Owner + Product Owner | Locks v1 to PTT + Continuous; preserves the enum surface so a P1 increment is non-breaking. |
| DEC-031 | Missed-key-up watchdog disabled on P0; redesign deferred to P1 | **The `MissedKeyUpWatchdog` is constructed and unit-tested but is NOT spawned by `ChanoraSession::start_audio` in the P0 baseline.** Supersedes DEC-028 for the v1 ship. Rationale: the original 30 s ceiling cut real users off mid-sentence when speaking through PTT for longer than the timeout — Mumble and TeamSpeak do not ship a comparable watchdog, so the protection is stricter than industry baseline while imposing a real UX cost. The watchdog's purpose (catching OS-level key-up loss when the app loses focus / is minimised / hits App Nap) remains valid; the fixed-timeout shape is the wrong implementation. P1 will reintroduce a redesigned variant using one of: (a) raised ceiling (~5 min, owner-tunable) only, (b) active OS-level key-state polling via `GetAsyncKeyState` / `CGEventSourceKeyState` / `XQueryKeymap` so we detect the actual OS desync directly, (c) audio-activity (RMS-silence) fallback once the level meter lands, or (d) a combination. The Rust unit tests for `MissedKeyUpWatchdog::spawn_on_signal` remain in `crates/chanora_audio/src/ptt.rs` so the P1 re-enable is non-breaking. | Accepted | Audio Owner + Product Owner | Eliminates a P0-class UX regression (long PTT speech cut off at 30 s) while preserving the implementation surface for P1. |
## 4. Accepted MVP Defaults
@@ -159,6 +160,7 @@ release but is not an open decision:
| DEC-028 | Audio Owner / Software Architect | Missed-key-up watchdog | Accepted (P0) | 2026-05-15 | Resolves PTT-OPEN-006. |
| DEC-029 | Software Architect / Audio Owner | Flutter global-hotkey packages rejected for PTT | Accepted | 2026-05-15 | Native `DesktopPttBackend` is the v1 PTT capture path; pub.dev hotkey packages consume the key, drop key-up events, and skip mouse side-buttons. |
| DEC-030 | Audio Owner / Product Owner | Voice activity detection deferred to P1 | Accepted (deferred to P1) | 2026-05-15 | `TransmitMode::VoiceActivity` reserved on the enum surface; UI shows "coming soon"; backend choice (RMS / WebRTC / Silero) deferred for dependency-surface review. |
| DEC-031 | Audio Owner / Product Owner | Missed-key-up watchdog disabled on P0; redesign deferred to P1 | Accepted (supersedes DEC-028 for P0) | 2026-05-16 | Watchdog implementation + tests retained; not spawned by `ChanoraSession::start_audio` in v1. P1 chooses between raised ceiling / OS key-state polling / RMS-silence fallback. |
## 8. Change History
@@ -218,3 +220,4 @@ release but is not an open decision:
| 0.9.9 | 2026-05-15 | Recorded six new accepted decisions DEC-023 through DEC-028 closing the gen2 desktop-PTT review's open questions PTT-OPEN-001 through PTT-OPEN-006: Windows Global PTT is P0/MVP (DEC-023), macOS Global PTT is P0/MVP with permission UX (DEC-024), the officially-tested Linux environment is GNOME-on-Wayland only (DEC-025), mouse side buttons are supported on Windows + macOS and Linux follows the portal (DEC-026), PTT diagnostics carry capability/availability only with no raw key codes (DEC-027), and the missed-key-up watchdog is a P0 release-gate requirement (DEC-028). No prior decision rows are mutated. |
| 0.9.10 | 2026-05-15 | Code-side promotion: the Linux GNOME-Wayland backend (DEC-025) is now a live `org.freedesktop.portal.GlobalShortcuts` session — `CreateSession` + `BindShortcuts` + `Activated` / `Deactivated` signal subscription scoped to the session handle, owned by a dedicated tokio task per backend instance. The Flutter "Configure" button on Linux portal delegates to the portal's own system dialog (Q3a) rather than the in-app `_PttBindingCaptureDialog`. Descriptor transitions broadcast via a `watch::Sender` consumed by `chanora_core::ChanoraSession::start_audio` and forwarded to `SessionEvent::PttCapability`. Cancellation / failure path downgrades to `L0Focused` and re-emits. No decision rows mutate. |
| 0.9.11 | 2026-05-15 | Added DEC-029 and DEC-030 covering the v1 audio + PTT lifecycle redesign. DEC-029 rejects Flutter global-hotkey packages (`hotkey_manager`, `super_hot_key`, equivalents) for PTT — they consume the key, do not deliver key-up, and do not support mouse side-buttons; the native Rust `DesktopPttBackend` already specified in SDD-083 / SDD-084 / SDD-085 is the v1 capture path. DEC-030 defers Voice Activity Detection to P1: `TransmitMode::VoiceActivity` ships as a reserved enum variant with no v1 implementation pending a backend trade-off review (RMS vs WebRTC VAD vs Silero VAD differ by ~8–16 MB and an ONNX-runtime dependency surface). No prior decision rows mutate; §5 impact matrix and §7 open-decision log gain matching rows. |
| 0.9.12 | 2026-05-16 | Added DEC-031: missed-key-up watchdog is disabled on P0 (supersedes DEC-028 for the v1 ship). The 30 s default ceiling spec'd in DEC-028 was cutting real PTT users off mid-sentence whenever a single utterance crossed the timeout; the watchdog's intent (catching OS-level key-up loss) remains valid, but the fixed-timeout implementation is the wrong shape. The `MissedKeyUpWatchdog` Rust type and its unit tests remain in `crates/chanora_audio/src/ptt.rs`; only the `ChanoraSession::start_audio` spawn is removed. P1 will choose between a raised ceiling, OS key-state polling (`GetAsyncKeyState` / `CGEventSourceKeyState` / `XQueryKeymap`), an RMS-silence fallback paired with the audio level meter, or a combination. DEC-028 stays in the register as historical context. |