diff --git a/core/chanora_core/src/lib.rs b/core/chanora_core/src/lib.rs index 90664fe..84924bf 100644 --- a/core/chanora_core/src/lib.rs +++ b/core/chanora_core/src/lib.rs @@ -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 diff --git a/docs/governance/product-decision-register.md b/docs/governance/product-decision-register.md index a1914dd..b715b37 100644 --- a/docs/governance/product-decision-register.md +++ b/docs/governance/product-decision-register.md @@ -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. |