From 322bbf4d8486efd7dbbcf49598711a9e3fefc282 Mon Sep 17 00:00:00 2001 From: Benjamin Carlsson Date: Sat, 10 Oct 2026 02:45:04 +0000 Subject: [PATCH] Classify voice session failures and record terminal outcomes (#52756) ## Why Voice failure metrics lacked cause and lifecycle phase, and session durations were recorded before the final outcome was known. A failed control send could also overwrite a more specific WebRTC error. ## What changed - Tag `codex.voice.session.failure` with bounded `cause` and `phase` values, recording at most one failure per session. - Capture duration when stopping and emit `codex.voice.session.duration` at cleanup with a `success` or `failure` outcome. - Carry typed WebRTC errors into the TUI, preserve existing errors when control sends fail, and avoid duplicate errors after startup failure. - Record app-server disconnect failures for background voice sessions as well as the visible session. ## Testing Add coverage for terminal outcome and failure metric attributes, preservation of specific startup and audio-session errors, and startup failures without duplicate control-channel errors. GitOrigin-RevId: 5aeda6e766093e3fce7206560aa83b6f7770dddb --- codex-rs/Cargo.lock | 1 + codex-rs/realtime-webrtc/src/session.rs | 26 +++-- codex-rs/realtime-webrtc/src/session_tests.rs | 27 ++++- .../realtime-webrtc/tests/session_actor.rs | 11 +- codex-rs/tui/Cargo.toml | 1 + codex-rs/tui/src/app/event_dispatch.rs | 17 ++- codex-rs/tui/src/app/reconnect.rs | 1 + codex-rs/tui/src/app/thread_routing.rs | 6 +- codex-rs/tui/src/app_event.rs | 8 +- codex-rs/tui/src/chatwidget.rs | 1 + codex-rs/tui/src/chatwidget/protocol.rs | 2 +- codex-rs/tui/src/chatwidget/realtime.rs | 109 +++++++++++++----- .../chatwidget/realtime/failure_metrics.rs | 90 +++++++++++++++ .../chatwidget/realtime/recording_controls.rs | 15 ++- .../realtime/session_metrics_tests.rs | 109 +++++++++++++++++- codex-rs/tui/src/chatwidget/realtime_tests.rs | 1 + .../chatwidget/realtime_tests/lifecycle.rs | 54 ++++++++- codex-rs/tui/src/chatwidget/reconnect.rs | 18 +-- 18 files changed, 431 insertions(+), 66 deletions(-) create mode 100644 codex-rs/tui/src/chatwidget/realtime/failure_metrics.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 530060483e..10c3e869ae 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -4867,6 +4867,7 @@ dependencies = [ "libc", "objc2-app-kit", "objc2-foundation", + "opentelemetry_sdk", "pathdiff", "pretty_assertions", "pulldown-cmark", diff --git a/codex-rs/realtime-webrtc/src/session.rs b/codex-rs/realtime-webrtc/src/session.rs index badbf41e38..c50f3a8755 100644 --- a/codex-rs/realtime-webrtc/src/session.rs +++ b/codex-rs/realtime-webrtc/src/session.rs @@ -39,7 +39,7 @@ enum Command { struct State { microphone: AtomicU16, speaker: AtomicU16, - error: Mutex>, + error: Mutex>, } struct Owner { @@ -137,6 +137,7 @@ impl RealtimeWebrtcSession { std::thread::Builder::new() .name("voice-session".into()) .spawn(move || { + let mut receiver = receiver; let task = async { let host = report_failure( ConnectionError::HelperStartup, @@ -151,7 +152,7 @@ impl RealtimeWebrtcSession { offer .send(Ok(sdp.into_sdp())) .map_err(|_| anyhow::anyhow!("voice startup cancelled"))?; - run(host, receiver, &state, &controls, selection).await + run(host, &mut receiver, &state, &controls, selection).await }; let result = runtime.block_on(Abortable::new(Abortable::new(task, abort), stopped)); if let Ok(Ok(Err(error))) = result { @@ -164,7 +165,7 @@ impl RealtimeWebrtcSession { .error .lock() .unwrap_or_else(std::sync::PoisonError::into_inner) = - Some(failure.to_string()); + Some((failure, failure.to_string())); } })?; let offer_sdp = result @@ -232,7 +233,7 @@ impl RealtimeWebrtcSessionHandle { } } - pub fn take_error(&self) -> Option { + pub fn take_error(&self) -> Option<(ConnectionError, String)> { self.0 .state .error @@ -251,13 +252,18 @@ impl RealtimeWebrtcSessionHandle { fn send(&self, command: Command) -> Result<()> { if self.0.sender.try_send(command).is_err() { - *self + let mut error = self .0 .state .error .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner) = - Some("Voice control channel unavailable.".into()); + .unwrap_or_else(std::sync::PoisonError::into_inner); + if error.is_none() { + *error = Some(( + ConnectionError::Failed, + "Voice control channel unavailable.".into(), + )); + } self.close(); anyhow::bail!("voice control channel unavailable"); } @@ -267,7 +273,7 @@ impl RealtimeWebrtcSessionHandle { async fn run( mut host: VoiceHost, - mut commands: mpsc::Receiver, + commands: &mut mpsc::Receiver, state: &State, controls: &Mutex, selection: AudioDeviceSelection, @@ -283,7 +289,7 @@ async fn run( let startup = async { let mut host = report_failure(ConnectionError::Transport, host.apply_answer(sdp).await)?; host = report_failure(ConnectionError::AudioDevices, host.open_devices(selection.clone()).await)?; - let applied = startup_controls(&mut commands, controls, |initial| { + let applied = startup_controls(commands, controls, |initial| { host.begin_audio_controls(initial) }); let applied = report_failure(ConnectionError::AudioControls, applied)?; @@ -297,7 +303,7 @@ async fn run( .copied().unwrap_or(ConnectionError::Failed); let _ = complete.send(Err(failure)); // This failure is delivered by the startup completion only. - return Ok(()); + return std::future::pending().await; } }; connected = true; diff --git a/codex-rs/realtime-webrtc/src/session_tests.rs b/codex-rs/realtime-webrtc/src/session_tests.rs index 87227e6516..8e22d3b455 100644 --- a/codex-rs/realtime-webrtc/src/session_tests.rs +++ b/codex-rs/realtime-webrtc/src/session_tests.rs @@ -43,7 +43,32 @@ fn controls_preserve_order_and_independent_settings_and_overflow_closes() { } assert!(handle.set_microphone_muted(/*muted*/ false).is_err()); assert!(handle.0.stop.is_aborted()); - assert!(handle.take_error().is_some()); + assert_eq!( + handle.take_error(), + Some(( + ConnectionError::Failed, + "Voice control channel unavailable.".into() + )) + ); +} + +#[test] +fn failed_control_send_preserves_existing_error() { + let (handle, commands, _) = handles(); + *handle.0.state.error.lock().unwrap() = Some(( + ConnectionError::AudioSession, + "voice audio session stopped unexpectedly".into(), + )); + drop(commands); + + assert!(handle.set_microphone_muted(/*muted*/ true).is_err()); + assert_eq!( + handle.take_error(), + Some(( + ConnectionError::AudioSession, + "voice audio session stopped unexpectedly".into() + )) + ); } #[tokio::test] diff --git a/codex-rs/realtime-webrtc/tests/session_actor.rs b/codex-rs/realtime-webrtc/tests/session_actor.rs index bc00c50adc..88d4f4a6b5 100644 --- a/codex-rs/realtime-webrtc/tests/session_actor.rs +++ b/codex-rs/realtime-webrtc/tests/session_actor.rs @@ -84,8 +84,11 @@ fn startup_controls_meters_and_helper_loss() -> Result<()> { error.is_some() })?; assert_eq!( - error.as_deref(), - Some("voice audio session stopped unexpectedly") + error, + Some(( + codex_realtime_webrtc::ConnectionError::AudioSession, + "voice audio session stopped unexpectedly".into() + )) ); assert_eq!(handle.take_error(), None); Ok(()) @@ -145,9 +148,11 @@ fn device_failure_reaches_startup_completion_without_duplicate_error() -> Result started.handle.apply_answer_sdp("synthetic-answer".into()), Err(codex_realtime_webrtc::ConnectionError::AudioDevices) ); + started.handle.set_microphone_muted(/*muted*/ true)?; + assert_eq!(started.handle.take_error(), None); + started.handle.close(); #[cfg(unix)] common::wait_for_helper_reaped(&root)?; - assert_eq!(started.handle.take_error(), None); Ok(()) } diff --git a/codex-rs/tui/Cargo.toml b/codex-rs/tui/Cargo.toml index 45f5866515..ea0894b3e1 100644 --- a/codex-rs/tui/Cargo.toml +++ b/codex-rs/tui/Cargo.toml @@ -181,6 +181,7 @@ chrono = { workspace = true, features = ["serde"] } futures = { workspace = true } http = { workspace = true } insta = { workspace = true } +opentelemetry_sdk = { workspace = true } pretty_assertions = { workspace = true } rand = { workspace = true } serial_test = { workspace = true } diff --git a/codex-rs/tui/src/app/event_dispatch.rs b/codex-rs/tui/src/app/event_dispatch.rs index 659e67a6ea..05f4a1bade 100644 --- a/codex-rs/tui/src/app/event_dispatch.rs +++ b/codex-rs/tui/src/app/event_dispatch.rs @@ -1124,6 +1124,17 @@ impl App { self.active_thread_id = visible_thread; } if let Err(err) = result { + if is_realtime_stop { + let chat_widget = match self.background_voice.as_deref_mut() { + Some(owner) if parked_voice => owner, + _ => &mut self.chat_widget, + }; + if chat_widget.thread_id() == realtime_stop_thread_id { + chat_widget.record_realtime_failure( + crate::chatwidget::RealtimeFailureCause::AppServerRequest, + ); + } + } if self.recover_transport_error(&err) { return Ok(AppRunControl::Continue); } @@ -1150,12 +1161,14 @@ impl App { let message = format!("Voice conversation failed: {err:#}"); if is_realtime_stop { if chat_widget.thread_id() == realtime_stop_thread_id { - chat_widget.record_realtime_failure(); chat_widget.reset_realtime_conversation(); chat_widget.add_realtime_error(message); } } else { - chat_widget.on_realtime_error(message); + chat_widget.on_realtime_error( + message, + crate::chatwidget::RealtimeFailureCause::AppServerRequest, + ); } tracing::error!(error = ?err, "realtime conversation request failed"); } else if handled { diff --git a/codex-rs/tui/src/app/reconnect.rs b/codex-rs/tui/src/app/reconnect.rs index 5e0e66ac1d..00a9853c56 100644 --- a/codex-rs/tui/src/app/reconnect.rs +++ b/codex-rs/tui/src/app/reconnect.rs @@ -191,6 +191,7 @@ impl App { self.chat_widget.restore_user_message_to_composer(message); } if let Some(owner) = self.background_voice.as_mut() { + owner.record_realtime_disconnect_failure(); owner.reset_realtime_conversation(); } self.retire_background_voice(); diff --git a/codex-rs/tui/src/app/thread_routing.rs b/codex-rs/tui/src/app/thread_routing.rs index 6264272a9f..9197df86bf 100644 --- a/codex-rs/tui/src/app/thread_routing.rs +++ b/codex-rs/tui/src/app/thread_routing.rs @@ -486,7 +486,8 @@ impl App { | AppCommand::RealtimeConversationStop { thread_id } = &op && self.chat_widget.thread_id() == Some(*thread_id) { - self.chat_widget.record_realtime_failure(); + self.chat_widget + .record_realtime_failure(crate::chatwidget::RealtimeFailureCause::LocalRequest); self.chat_widget.reset_realtime_conversation(); } self.chat_widget @@ -512,7 +513,8 @@ impl App { | AppCommand::RealtimeConversationStop { thread_id } = &op && self.chat_widget.thread_id() == Some(*thread_id) { - self.chat_widget.record_realtime_failure(); + self.chat_widget + .record_realtime_failure(crate::chatwidget::RealtimeFailureCause::LocalRequest); self.chat_widget.reset_realtime_conversation(); } self.chat_widget.add_error_message( diff --git a/codex-rs/tui/src/app_event.rs b/codex-rs/tui/src/app_event.rs index 7a388a223f..4422be5cd7 100644 --- a/codex-rs/tui/src/app_event.rs +++ b/codex-rs/tui/src/app_event.rs @@ -69,6 +69,12 @@ use codex_realtime_webrtc::StartedRealtimeWebrtcSession; use crate::history_cell::HistoryCell; +#[derive(Debug)] +pub(crate) struct RealtimeWebrtcStartupFailure { + pub message: String, + pub cause: codex_realtime_webrtc::ConnectionError, +} + /// Global voice controls always apply to the one call's owner. #[derive(Clone, Copy, Debug)] pub(crate) enum VoiceControl { @@ -1233,7 +1239,7 @@ pub(crate) enum AppEvent { RealtimeWebrtcOfferCreated { thread_id: ThreadId, attempt_id: u64, - result: Result, + result: Result, }, /// Result of establishing the WebRTC connection for an active voice attempt. diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index ea0a5ffd54..2c02a6d344 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -357,6 +357,7 @@ mod realtime_settings; mod realtime_split_flap; pub(crate) use realtime::MAX_REPLAY_TRANSCRIPT_CELLS; pub(crate) use realtime::MAX_TRANSCRIPT_BYTES; +pub(crate) use realtime::RealtimeFailureCause; pub(crate) use realtime::RealtimeTranscriptRecord; pub(crate) use realtime::is_private_realtime_agent_item; pub(crate) use realtime::realtime_delegation_display_text; diff --git a/codex-rs/tui/src/chatwidget/protocol.rs b/codex-rs/tui/src/chatwidget/protocol.rs index 9e884d213c..0065ef473b 100644 --- a/codex-rs/tui/src/chatwidget/protocol.rs +++ b/codex-rs/tui/src/chatwidget/protocol.rs @@ -345,7 +345,7 @@ impl ChatWidget { } ServerNotification::ThreadRealtimeError(notification) => { if !from_replay { - self.on_realtime_error(notification.message); + self.on_realtime_error(notification.message, RealtimeFailureCause::Backend); } } ServerNotification::ThreadRealtimeClosed(notification) => { diff --git a/codex-rs/tui/src/chatwidget/realtime.rs b/codex-rs/tui/src/chatwidget/realtime.rs index 6215611a08..bb2a48e527 100644 --- a/codex-rs/tui/src/chatwidget/realtime.rs +++ b/codex-rs/tui/src/chatwidget/realtime.rs @@ -3,9 +3,12 @@ //! Interleaved speakers retain separate displays so settled caption text never reanimates. //! Speech recovery suppresses stale queued answers while preserving unspoken text fallbacks. +mod failure_metrics; mod recording_controls; mod transcript_replay; +pub(crate) use failure_metrics::RealtimeFailureCause; + use super::ChatWidget; use super::HistoryCell; use super::PARENT_OWNED_INPUT_MESSAGE; @@ -13,6 +16,7 @@ use super::realtime_split_flap::SplitFlapTranscriptCell; use super::realtime_split_flap::VoiceAmplitudeHistory; use crate::app_command::AppCommand; use crate::app_event::AppEvent; +use crate::app_event::RealtimeWebrtcStartupFailure; use crate::bottom_pane::VoiceStripPhase; use crate::bottom_pane::VoiceStripState; use crate::history_cell; @@ -144,6 +148,7 @@ pub(super) struct RealtimeConversationUiState { backend_started: bool, webrtc_connected: bool, active_since: Option, + finished_duration: Option, failure_recorded: bool, microphone_muted: bool, microphone_level: usize, @@ -333,8 +338,16 @@ impl ChatWidget { .map(|channels| channels.as_slice().to_vec()), }; std::thread::spawn(move || { - let result = RealtimeWebrtcSession::start(abort_registration, selection) - .map_err(|error| error.to_string()); + let result = + RealtimeWebrtcSession::start(abort_registration, selection).map_err(|error| { + RealtimeWebrtcStartupFailure { + cause: error + .downcast_ref::() + .copied() + .unwrap_or(codex_realtime_webrtc::ConnectionError::Failed), + message: error.to_string(), + } + }); app_event_tx.send(AppEvent::RealtimeWebrtcOfferCreated { thread_id, attempt_id, @@ -352,7 +365,7 @@ impl ChatWidget { ) { return; } - self.finish_realtime_session_metrics(); + self.capture_realtime_session_duration(); self.restore_all_undelivered_realtime_speech(); // Stopping may prevent final transcript events from arriving. @@ -368,7 +381,7 @@ impl ChatWidget { return; }; if !self.submit_op(AppCommand::RealtimeConversationStop { thread_id }) { - self.record_realtime_failure(); + self.record_realtime_failure(RealtimeFailureCause::LocalRequest); self.reset_realtime_conversation(); } } else { @@ -383,7 +396,7 @@ impl ChatWidget { &mut self, thread_id: ThreadId, attempt_id: u64, - result: Result, + result: Result, ) { if self.realtime_conversation.phase != RealtimeConversationPhase::Starting || self.realtime_conversation.attempt_id != attempt_id @@ -397,7 +410,10 @@ impl ChatWidget { let offer = match result { Ok(offer) => offer, Err(error) => { - self.on_realtime_error(format!("Failed to start voice mode: {error}")); + self.on_realtime_error( + format!("Failed to start voice mode: {}", error.message), + error.cause.into(), + ); return; } }; @@ -405,8 +421,18 @@ impl ChatWidget { if self.realtime_conversation.microphone_muted && let Err(error) = offer.handle.set_microphone_muted(/*muted*/ true) { + let failure = offer.handle.take_error(); offer.handle.close(); - self.on_realtime_error(format!("Failed to restore microphone mute: {error}")); + let (message, cause) = match failure { + Some((cause, error)) => { + (format!("Voice conversation failed: {error}"), cause.into()) + } + None => ( + format!("Failed to restore microphone mute: {error}"), + RealtimeFailureCause::AudioControls, + ), + }; + self.on_realtime_error(message, cause); return; } self.realtime_conversation.handle = Some(offer.handle); @@ -418,7 +444,7 @@ impl ChatWidget { thread_id, offer_sdp: offer.offer_sdp.into(), }) { - self.record_realtime_failure(); + self.record_realtime_failure(RealtimeFailureCause::LocalRequest); self.reset_realtime_conversation(); } } @@ -475,7 +501,7 @@ impl ChatWidget { self.realtime_conversation.startup_retry = StartupRetry::WaitingForStop; self.refresh_terminal_title(); if !self.submit_op(AppCommand::RealtimeConversationStop { thread_id }) { - self.record_realtime_failure(); + self.record_realtime_failure(RealtimeFailureCause::LocalRequest); self.reset_realtime_conversation(); } else { self.add_info_message( @@ -485,7 +511,22 @@ impl ChatWidget { } return; } - self.on_realtime_error(format!("Failed to connect voice mode: {error}")); + let stored_error = self + .realtime_conversation + .handle + .as_ref() + .and_then(RealtimeWebrtcSessionHandle::take_error); + let (message, cause) = match stored_error { + Some((cause, message)) => ( + format!("Voice conversation failed: {message}"), + cause.into(), + ), + None => ( + format!("Failed to connect voice mode: {error}"), + error.into(), + ), + }; + self.on_realtime_error(message, cause); return; } @@ -1005,7 +1046,10 @@ impl ChatWidget { text: text.into(), }) { self.restore_undelivered_realtime_speech(delivery_id); - self.on_realtime_error("Failed to deliver the voice response.".to_string()); + self.on_realtime_error( + "Failed to deliver the voice response.".to_string(), + RealtimeFailureCause::SpeechDelivery, + ); } else { self.realtime_conversation.pending_speech.retain(|pending| { pending.turn_id != turn_id || pending.state != PendingSpeechState::AwaitingTurn @@ -1656,29 +1700,18 @@ impl ChatWidget { cells } - pub(crate) fn record_realtime_failure(&mut self) { - if self.realtime_conversation.phase == RealtimeConversationPhase::Inactive - || self.realtime_conversation.failure_recorded - { - return; - } - self.realtime_conversation.failure_recorded = true; - self.session_telemetry - .counter("codex.voice.session.failure", /*inc*/ 1, &[]); - } - pub(super) fn realtime_retry_cleanup_pending(&self) -> bool { self.realtime_conversation.startup_retry == StartupRetry::WaitingForStop } - pub(crate) fn on_realtime_error(&mut self, message: String) { + pub(crate) fn on_realtime_error(&mut self, message: String, cause: RealtimeFailureCause) { if self.realtime_conversation.phase == RealtimeConversationPhase::Inactive { return; } if self.realtime_conversation.phase != RealtimeConversationPhase::Stopping || self.realtime_retry_cleanup_pending() { - self.record_realtime_failure(); + self.record_realtime_failure(cause); } self.stop_realtime_conversation(); self.add_realtime_error(message); @@ -1717,7 +1750,13 @@ impl ChatWidget { if retry_thread_id.is_none() && (reason.is_none() || matches!(reason.as_deref(), Some("transport_closed" | "error"))) { - self.record_realtime_failure(); + let cause = match reason.as_deref() { + Some("transport_closed") => RealtimeFailureCause::TransportClosed, + Some("error") => RealtimeFailureCause::Backend, + None => RealtimeFailureCause::UnknownClose, + Some(_) => RealtimeFailureCause::Backend, + }; + self.record_realtime_failure(cause); } let failed = self.realtime_conversation.failure_recorded; self.reset_realtime_conversation(); @@ -1748,14 +1787,28 @@ impl ChatWidget { self.request_redraw(); } + fn capture_realtime_session_duration(&mut self) { + if self.realtime_conversation.finished_duration.is_none() + && let Some(active_since) = self.realtime_conversation.active_since.take() + { + self.realtime_conversation.finished_duration = Some(active_since.elapsed()); + } + } + fn finish_realtime_session_metrics(&mut self) { - if let Some(active_since) = self.realtime_conversation.active_since.take() { + self.capture_realtime_session_duration(); + if let Some(duration) = self.realtime_conversation.finished_duration.take() { + let outcome = if self.realtime_conversation.failure_recorded { + "failure" + } else { + "success" + }; self.session_telemetry .counter("codex.voice.session.ended", /*inc*/ 1, &[]); self.session_telemetry.record_duration( "codex.voice.session.duration", - active_since.elapsed(), - &[], + duration, + &[("outcome", outcome)], ); } } diff --git a/codex-rs/tui/src/chatwidget/realtime/failure_metrics.rs b/codex-rs/tui/src/chatwidget/realtime/failure_metrics.rs new file mode 100644 index 0000000000..e2c1227004 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/realtime/failure_metrics.rs @@ -0,0 +1,90 @@ +//! Bounded failure classification for locally owned voice sessions. +//! Each session records at most one cause, while lifecycle phase remains an independent fact. + +use super::RealtimeConversationPhase; +use crate::chatwidget::ChatWidget; + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum RealtimeFailureCause { + AppServerDisconnect, + AppServerRequest, + AudioControls, + AudioDevices, + AudioSession, + Backend, + ConnectionUnknown, + HelperStartup, + NegotiationTimedOut, + LocalRequest, + RuntimeInitialization, + Shutdown, + SpeechDelivery, + Transport, + TransportClosed, + UnknownClose, +} + +impl RealtimeFailureCause { + fn as_str(self) -> &'static str { + match self { + Self::AppServerDisconnect => "app_server_disconnect", + Self::AppServerRequest => "app_server_request", + Self::AudioControls => "audio_controls", + Self::AudioDevices => "audio_devices", + Self::AudioSession => "audio_session", + Self::Backend => "backend", + Self::ConnectionUnknown => "connection_unknown", + Self::HelperStartup => "helper_startup", + Self::NegotiationTimedOut => "negotiation_timed_out", + Self::LocalRequest => "local_request", + Self::RuntimeInitialization => "runtime_initialization", + Self::Shutdown => "shutdown", + Self::SpeechDelivery => "speech_delivery", + Self::Transport => "transport", + Self::TransportClosed => "transport_closed", + Self::UnknownClose => "unknown_close", + } + } +} + +impl From for RealtimeFailureCause { + fn from(error: codex_realtime_webrtc::ConnectionError) -> Self { + match error { + codex_realtime_webrtc::ConnectionError::NegotiationTimedOut => { + Self::NegotiationTimedOut + } + codex_realtime_webrtc::ConnectionError::Failed => Self::ConnectionUnknown, + codex_realtime_webrtc::ConnectionError::HelperStartup => Self::HelperStartup, + codex_realtime_webrtc::ConnectionError::RuntimeInitialization => { + Self::RuntimeInitialization + } + codex_realtime_webrtc::ConnectionError::Transport => Self::Transport, + codex_realtime_webrtc::ConnectionError::AudioDevices => Self::AudioDevices, + codex_realtime_webrtc::ConnectionError::AudioControls => Self::AudioControls, + codex_realtime_webrtc::ConnectionError::AudioSession => Self::AudioSession, + codex_realtime_webrtc::ConnectionError::Shutdown => Self::Shutdown, + } + } +} + +impl ChatWidget { + pub(crate) fn record_realtime_failure(&mut self, cause: RealtimeFailureCause) { + if self.realtime_conversation.phase == RealtimeConversationPhase::Inactive + || self.realtime_conversation.failure_recorded + { + return; + } + self.realtime_conversation.failure_recorded = true; + let phase = match self.realtime_conversation.phase { + RealtimeConversationPhase::Inactive => "inactive", + RealtimeConversationPhase::Starting => "starting", + RealtimeConversationPhase::Active => "active", + RealtimeConversationPhase::Stopping => "stopping", + }; + self.session_telemetry.counter( + "codex.voice.session.failure", + /*inc*/ 1, + &[("cause", cause.as_str()), ("phase", phase)], + ); + } +} diff --git a/codex-rs/tui/src/chatwidget/realtime/recording_controls.rs b/codex-rs/tui/src/chatwidget/realtime/recording_controls.rs index 7e84dee420..e299500840 100644 --- a/codex-rs/tui/src/chatwidget/realtime/recording_controls.rs +++ b/codex-rs/tui/src/chatwidget/realtime/recording_controls.rs @@ -8,7 +8,16 @@ impl ChatWidget { let muted = !self.realtime_conversation.microphone_muted; if let Some(handle) = self.realtime_conversation.handle.as_ref() { if let Err(error) = handle.set_microphone_muted(muted) { - self.on_realtime_error(format!("Failed to update microphone: {error}")); + let (message, cause) = match handle.take_error() { + Some((cause, error)) => { + (format!("Voice conversation failed: {error}"), cause.into()) + } + None => ( + format!("Failed to update microphone: {error}"), + RealtimeFailureCause::AudioControls, + ), + }; + self.on_realtime_error(message, cause); return; } } else if self.realtime_conversation.phase != RealtimeConversationPhase::Starting { @@ -136,8 +145,8 @@ impl ChatWidget { let Some(handle) = self.realtime_conversation.handle.as_ref() else { return; }; - if let Some(error) = handle.take_error() { - self.on_realtime_error(format!("Voice conversation failed: {error}")); + if let Some((cause, error)) = handle.take_error() { + self.on_realtime_error(format!("Voice conversation failed: {error}"), cause.into()); return; } if self.realtime_conversation.phase == RealtimeConversationPhase::Starting { diff --git a/codex-rs/tui/src/chatwidget/realtime/session_metrics_tests.rs b/codex-rs/tui/src/chatwidget/realtime/session_metrics_tests.rs index 07c60bb34c..1604b410ce 100644 --- a/codex-rs/tui/src/chatwidget/realtime/session_metrics_tests.rs +++ b/codex-rs/tui/src/chatwidget/realtime/session_metrics_tests.rs @@ -2,7 +2,14 @@ use super::super::StartupRetry; use super::*; +use codex_otel::MetricsClient; +use codex_otel::MetricsConfig; +use opentelemetry_sdk::metrics::InMemoryMetricExporter; +use opentelemetry_sdk::metrics::data::AggregatedMetrics; +use opentelemetry_sdk::metrics::data::MetricData; use pretty_assertions::assert_eq; +use std::collections::BTreeMap; +use std::time::Instant; #[tokio::test] async fn terminal_error_during_retry_cleanup_records_one_failure() { @@ -15,16 +22,114 @@ async fn terminal_error_during_retry_cleanup_records_one_failure() { chat.realtime_conversation.phase = RealtimeConversationPhase::Stopping; chat.realtime_conversation.startup_retry = retry; - chat.on_realtime_error("connection lost".into()); + chat.on_realtime_error( + "connection lost".into(), + RealtimeFailureCause::ConnectionUnknown, + ); assert_eq!( chat.realtime_conversation.failure_recorded, expected_failure ); - chat.on_realtime_error("duplicate error".into()); + chat.on_realtime_error( + "duplicate error".into(), + RealtimeFailureCause::ConnectionUnknown, + ); assert_eq!( chat.realtime_conversation.failure_recorded, expected_failure ); } } + +#[tokio::test] +async fn completed_voice_duration_uses_terminal_success_or_failure_outcome() { + for (failed, expected_outcome) in [(false, "success"), (true, "failure")] { + let (mut chat, _sender, _events, _ops) = make_chatwidget_manual_with_sender().await; + let metrics = MetricsClient::new( + MetricsConfig::in_memory( + "test", + "codex-cli", + env!("CARGO_PKG_VERSION"), + InMemoryMetricExporter::default(), + ) + .with_runtime_reader(), + ) + .expect("in-memory metrics should initialize"); + chat.session_telemetry = chat + .session_telemetry + .clone() + .with_metrics_without_metadata_tags(metrics); + activate_voice(&mut chat); + chat.realtime_conversation.active_since = Some(Instant::now()); + chat.capture_realtime_session_duration(); + if failed { + chat.record_realtime_disconnect_failure(); + } + + chat.reset_realtime_conversation(); + + let metrics = chat + .session_telemetry + .snapshot_metrics() + .expect("runtime metrics snapshot should be available"); + let duration = metrics + .scope_metrics() + .flat_map(opentelemetry_sdk::metrics::data::ScopeMetrics::metrics) + .find(|metric| metric.name() == "codex.voice.session.duration") + .expect("voice duration metric should be recorded"); + let duration_attributes = match duration.data() { + AggregatedMetrics::F64(MetricData::Histogram(histogram)) => histogram + .data_points() + .next() + .expect("voice duration should have one datapoint") + .attributes() + .map(|attribute| { + ( + attribute.key.as_str().to_string(), + attribute.value.as_str().to_string(), + ) + }) + .collect::>(), + _ => panic!("voice duration should be an f64 histogram"), + }; + assert_eq!( + duration_attributes, + BTreeMap::from([("outcome".to_string(), expected_outcome.to_string())]) + ); + + let failure = metrics + .scope_metrics() + .flat_map(opentelemetry_sdk::metrics::data::ScopeMetrics::metrics) + .find(|metric| metric.name() == "codex.voice.session.failure"); + if failed { + let failure_attributes = match failure + .expect("failed voice session should record a failure") + .data() + { + AggregatedMetrics::U64(MetricData::Sum(sum)) => sum + .data_points() + .next() + .expect("voice failure should have one datapoint") + .attributes() + .map(|attribute| { + ( + attribute.key.as_str().to_string(), + attribute.value.as_str().to_string(), + ) + }) + .collect::>(), + _ => panic!("voice failure should be a u64 sum"), + }; + assert_eq!( + failure_attributes, + BTreeMap::from([ + ("cause".to_string(), "app_server_disconnect".to_string()), + ("phase".to_string(), "active".to_string()), + ]) + ); + } else { + assert!(failure.is_none()); + } + } +} diff --git a/codex-rs/tui/src/chatwidget/realtime_tests.rs b/codex-rs/tui/src/chatwidget/realtime_tests.rs index 78c53630df..0ada7c233c 100644 --- a/codex-rs/tui/src/chatwidget/realtime_tests.rs +++ b/codex-rs/tui/src/chatwidget/realtime_tests.rs @@ -7,6 +7,7 @@ mod recording_controls_tests; mod session_metrics_tests; use super::RealtimeConversationPhase; +use super::RealtimeFailureCause; use crate::app_command::AppCommand; use crate::app_event::AppEvent; use crate::chatwidget::ChatWidget; diff --git a/codex-rs/tui/src/chatwidget/realtime_tests/lifecycle.rs b/codex-rs/tui/src/chatwidget/realtime_tests/lifecycle.rs index bf7d71e293..bbeb21683c 100644 --- a/codex-rs/tui/src/chatwidget/realtime_tests/lifecycle.rs +++ b/codex-rs/tui/src/chatwidget/realtime_tests/lifecycle.rs @@ -1,6 +1,7 @@ //! Voice startup, shutdown, and retry maintain one active owned session. use super::*; +use crate::app_event::RealtimeWebrtcStartupFailure; use pretty_assertions::assert_eq; #[tokio::test] @@ -148,7 +149,10 @@ async fn audio_failure_cancels_pending_voice_and_reports_the_device_error() { chat.realtime_conversation.phase = RealtimeConversationPhase::Starting; chat.realtime_conversation.startup_abort = Some(abort); - chat.on_realtime_error("speaker stream failed: device disconnected".to_string()); + chat.on_realtime_error( + "speaker stream failed: device disconnected".to_string(), + RealtimeFailureCause::AudioSession, + ); commit_realtime_history_events(&mut chat, &mut events); let cell = std::iter::from_fn(|| events.try_recv().ok()) @@ -198,7 +202,10 @@ async fn canceled_offer_is_ignored_after_a_new_start_attempt() { chat.on_realtime_webrtc_offer_created( ThreadId::new(), /*attempt_id*/ 1, - Err("canceled offer failed".to_string()), + Err(RealtimeWebrtcStartupFailure { + message: "superseded failure".into(), + cause: codex_realtime_webrtc::ConnectionError::Failed, + }), ); assert_eq!( @@ -211,6 +218,38 @@ async fn canceled_offer_is_ignored_after_a_new_start_attempt() { ); } +#[tokio::test] +async fn voice_startup_failure_preserves_the_specific_error() { + let (mut chat, _sender, mut events, _ops) = make_chatwidget_manual_with_sender().await; + chat.realtime_conversation.phase = RealtimeConversationPhase::Starting; + chat.realtime_conversation.attempt_id = 1; + + chat.on_realtime_webrtc_offer_created( + ThreadId::new(), + /*attempt_id*/ 1, + Err(RealtimeWebrtcStartupFailure { + message: "voice package unavailable".into(), + cause: codex_realtime_webrtc::ConnectionError::Failed, + }), + ); + + commit_realtime_history_events(&mut chat, &mut events); + let cell = std::iter::from_fn(|| events.try_recv().ok()) + .find_map(|event| match event { + AppEvent::InsertHistoryCell(cell) => Some(cell), + _ => None, + }) + .expect("voice should report the startup failure"); + insta::assert_snapshot!( + cell.display_lines(/*width*/ 80) + .into_iter() + .map(|line| line.to_string()) + .collect::>() + .join("\n"), + @"■ Failed to start voice mode: voice package unavailable" + ); +} + #[tokio::test] async fn voice_becomes_active_only_after_backend_and_current_peer_are_ready() { let (mut chat, _sender, mut events, _ops) = make_chatwidget_manual_with_sender().await; @@ -507,10 +546,13 @@ async fn failure_cleanup_does_not_attribute_stop_to_the_user() { // A local failure initiates backend cleanup; its acknowledgement is still "requested". chat.realtime_conversation.phase = RealtimeConversationPhase::Stopping; chat.realtime_conversation.failure_recorded = true; - chat.on_realtime_error(format!( - "Failed to connect voice mode: {}", - codex_realtime_webrtc::ConnectionError::AudioDevices - )); + chat.on_realtime_error( + format!( + "Failed to connect voice mode: {}", + codex_realtime_webrtc::ConnectionError::AudioDevices + ), + RealtimeFailureCause::AudioDevices, + ); chat.on_realtime_conversation_closed(Some("requested".into())); assert_eq!( chat.realtime_conversation.phase, diff --git a/codex-rs/tui/src/chatwidget/reconnect.rs b/codex-rs/tui/src/chatwidget/reconnect.rs index 4cde2de18f..f181fd541f 100644 --- a/codex-rs/tui/src/chatwidget/reconnect.rs +++ b/codex-rs/tui/src/chatwidget/reconnect.rs @@ -6,6 +6,16 @@ use super::*; use crate::bottom_pane::RestrictedInputMode; impl ChatWidget { + pub(crate) fn record_realtime_disconnect_failure(&mut self) { + if matches!( + self.realtime_conversation.phase, + RealtimeConversationPhase::Starting | RealtimeConversationPhase::Active + ) || self.realtime_retry_cleanup_pending() + { + self.record_realtime_failure(RealtimeFailureCause::AppServerDisconnect); + } + } + pub(crate) fn pause_for_disconnect(&mut self) { self.permission_discovery = None; self.invalidate_permission_discovery(); @@ -13,13 +23,7 @@ impl ChatWidget { self.cancel_image_submission(); // The app-server transport can fail while the separate WebRTC helper // still sends microphone audio. Retire local media before showing offline UI. - if matches!( - self.realtime_conversation.phase, - RealtimeConversationPhase::Starting | RealtimeConversationPhase::Active - ) || self.realtime_retry_cleanup_pending() - { - self.record_realtime_failure(); - } + self.record_realtime_disconnect_failure(); let _ = self.reset_realtime_conversation(); if let Some(questions) = &mut self.bottom_pane.questions { questions.delivery_enabled = false;