mirror of
https://github.com/openai/codex.git
synced 2026-10-27 18:48:37 +00:00
Track voice session lifecycle metrics in the TUI (#43934)
## What changed - Record voice session starts, connections, failures, endings, and active duration under `codex.voice.session.*`. - Deduplicate failure accounting and distinguish errors during startup retry cleanup from errors during a requested stop. - Record failures on rejected voice operations and disconnects, and clear the owned session when a stop request is rejected. ## Testing Add regression tests for failure accounting during retry cleanup and session cleanup after rejected stop requests, including replay-only threads. GitOrigin-RevId: d67cab5ff1e8fc85ce7a3d61a1763d38a5d537f0
This commit is contained in:
committed by
copyberry
parent
5e3f0ee94b
commit
fba22e9a2a
@@ -849,6 +849,7 @@ impl App {
|
||||
let message = format!("Voice conversation failed: {err:#}");
|
||||
if is_realtime_stop {
|
||||
if self.chat_widget.thread_id() == realtime_stop_thread_id {
|
||||
self.chat_widget.record_realtime_failure();
|
||||
self.chat_widget.reset_realtime_conversation();
|
||||
self.chat_widget.add_error_message(message);
|
||||
}
|
||||
|
||||
@@ -62,3 +62,39 @@ async fn rejected_voice_start_does_not_leave_local_capture_active() -> Result<()
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn rejected_voice_stop_clears_the_owned_session() -> Result<()> {
|
||||
for replay_only in [false, true] {
|
||||
let (mut app, _events, _ops) = make_test_app_with_channels().await;
|
||||
let (mut app_server, requests, proxy) =
|
||||
start_recording_remote_app_server(&app.config).await?;
|
||||
let thread_id = ThreadId::new();
|
||||
app.chat_widget
|
||||
.handle_thread_session_quiet(test_thread_session(
|
||||
thread_id,
|
||||
app.config.cwd.to_path_buf(),
|
||||
));
|
||||
crate::chatwidget::activate_voice_for_thread(&mut app.chat_widget, thread_id);
|
||||
if replay_only {
|
||||
app.app_server_target = AppServerTarget::Remote {
|
||||
endpoint: crate::resolve_remote_addr("ws://127.0.0.1:9")?,
|
||||
};
|
||||
app.active_thread_id = Some(thread_id);
|
||||
app.ensure_thread_channel(thread_id).mark_replay_only();
|
||||
}
|
||||
let mut tui = crate::tui::test_support::make_test_tui()?;
|
||||
Box::pin(app.handle_event(
|
||||
&mut tui,
|
||||
&mut app_server,
|
||||
AppEvent::CodexOp(Op::RealtimeConversationStop { thread_id }),
|
||||
))
|
||||
.await?;
|
||||
|
||||
assert!(!app.chat_widget.may_receive_realtime_transcripts());
|
||||
assert!(recorded_params(&requests, "thread/realtime/stop").is_empty());
|
||||
app_server.shutdown().await?;
|
||||
proxy.await??;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -482,9 +482,11 @@ impl App {
|
||||
self.chat_widget
|
||||
.restore_undelivered_realtime_speech(*delivery_id);
|
||||
}
|
||||
if let AppCommand::RealtimeConversationStart { thread_id, .. } = &op
|
||||
if let AppCommand::RealtimeConversationStart { thread_id, .. }
|
||||
| AppCommand::RealtimeConversationStop { thread_id } = &op
|
||||
&& self.chat_widget.thread_id() == Some(*thread_id)
|
||||
{
|
||||
self.chat_widget.record_realtime_failure();
|
||||
self.chat_widget.reset_realtime_conversation();
|
||||
}
|
||||
self.chat_widget
|
||||
@@ -506,9 +508,11 @@ impl App {
|
||||
self.chat_widget
|
||||
.restore_undelivered_realtime_speech(*delivery_id);
|
||||
}
|
||||
if let AppCommand::RealtimeConversationStart { thread_id, .. } = &op
|
||||
if let AppCommand::RealtimeConversationStart { thread_id, .. }
|
||||
| AppCommand::RealtimeConversationStop { thread_id } = &op
|
||||
&& self.chat_widget.thread_id() == Some(*thread_id)
|
||||
{
|
||||
self.chat_widget.record_realtime_failure();
|
||||
self.chat_widget.reset_realtime_conversation();
|
||||
}
|
||||
self.chat_widget.add_error_message(
|
||||
|
||||
@@ -88,7 +88,7 @@ fn can_retain_realtime_speech(turn_id: &str, item: &ThreadItem) -> bool {
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)]
|
||||
enum RealtimeConversationPhase {
|
||||
pub(super) enum RealtimeConversationPhase {
|
||||
#[default]
|
||||
Inactive,
|
||||
Starting,
|
||||
@@ -129,7 +129,7 @@ enum StartupRetry {
|
||||
#[derive(Default)]
|
||||
pub(super) struct RealtimeConversationUiState {
|
||||
startup_retry: StartupRetry,
|
||||
phase: RealtimeConversationPhase,
|
||||
pub(super) phase: RealtimeConversationPhase,
|
||||
recover_late_transcripts: bool,
|
||||
attempt_id: u64,
|
||||
thread_id: Option<ThreadId>,
|
||||
@@ -137,6 +137,8 @@ pub(super) struct RealtimeConversationUiState {
|
||||
startup_abort: Option<AbortHandle>,
|
||||
backend_started: bool,
|
||||
webrtc_connected: bool,
|
||||
active_since: Option<Instant>,
|
||||
failure_recorded: bool,
|
||||
microphone_muted: bool,
|
||||
microphone_level: usize,
|
||||
speaker_level: usize,
|
||||
@@ -278,6 +280,8 @@ impl ChatWidget {
|
||||
return;
|
||||
};
|
||||
|
||||
self.session_telemetry
|
||||
.counter("codex.voice.session.start", /*inc*/ 1, &[]);
|
||||
self.start_realtime_conversation(thread_id);
|
||||
}
|
||||
|
||||
@@ -312,6 +316,7 @@ impl ChatWidget {
|
||||
) {
|
||||
return;
|
||||
}
|
||||
self.finish_realtime_session_metrics();
|
||||
self.restore_all_undelivered_realtime_speech();
|
||||
|
||||
// Stopping may prevent final transcript events from arriving.
|
||||
@@ -327,6 +332,7 @@ impl ChatWidget {
|
||||
return;
|
||||
};
|
||||
if !self.submit_op(AppCommand::RealtimeConversationStop { thread_id }) {
|
||||
self.record_realtime_failure();
|
||||
self.reset_realtime_conversation();
|
||||
}
|
||||
} else {
|
||||
@@ -376,6 +382,7 @@ impl ChatWidget {
|
||||
thread_id,
|
||||
offer_sdp: offer.offer_sdp.into(),
|
||||
}) {
|
||||
self.record_realtime_failure();
|
||||
self.reset_realtime_conversation();
|
||||
}
|
||||
}
|
||||
@@ -432,6 +439,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.reset_realtime_conversation();
|
||||
} else {
|
||||
self.add_info_message(
|
||||
@@ -457,6 +465,9 @@ impl ChatWidget {
|
||||
return;
|
||||
}
|
||||
self.realtime_conversation.phase = RealtimeConversationPhase::Active;
|
||||
self.realtime_conversation.active_since = Some(Instant::now());
|
||||
self.session_telemetry
|
||||
.counter("codex.voice.session.connected", /*inc*/ 1, &[]);
|
||||
let running_delegation =
|
||||
self.turn_lifecycle
|
||||
.last_turn_id
|
||||
@@ -1525,10 +1536,30 @@ impl ChatWidget {
|
||||
}
|
||||
}
|
||||
|
||||
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) {
|
||||
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.stop_realtime_conversation();
|
||||
self.add_error_message(message);
|
||||
}
|
||||
@@ -1558,10 +1589,18 @@ impl ChatWidget {
|
||||
self.finish_realtime_partial_transcripts();
|
||||
let muted = self.realtime_conversation.microphone_muted;
|
||||
let thread_id = self.realtime_conversation.thread_id;
|
||||
self.reset_realtime_conversation();
|
||||
if (retry_after_stop || retry_after_early_close)
|
||||
&& let Some(thread_id) = thread_id.filter(|id| Some(*id) == self.thread_id())
|
||||
let retry_thread_id = if retry_after_stop || retry_after_early_close {
|
||||
thread_id.filter(|id| Some(*id) == self.thread_id())
|
||||
} else {
|
||||
None
|
||||
};
|
||||
if retry_thread_id.is_none()
|
||||
&& (reason.is_none() || matches!(reason.as_deref(), Some("transport_closed" | "error")))
|
||||
{
|
||||
self.record_realtime_failure();
|
||||
}
|
||||
self.reset_realtime_conversation();
|
||||
if let Some(thread_id) = retry_thread_id {
|
||||
// The old backend is closed. A late peer result belongs to its attempt ID.
|
||||
self.realtime_conversation.startup_retry = StartupRetry::Used;
|
||||
self.realtime_conversation.microphone_muted = muted;
|
||||
@@ -1585,7 +1624,20 @@ impl ChatWidget {
|
||||
self.request_redraw();
|
||||
}
|
||||
|
||||
fn finish_realtime_session_metrics(&mut self) {
|
||||
if let Some(active_since) = self.realtime_conversation.active_since.take() {
|
||||
self.session_telemetry
|
||||
.counter("codex.voice.session.ended", /*inc*/ 1, &[]);
|
||||
self.session_telemetry.record_duration(
|
||||
"codex.voice.session.duration",
|
||||
active_since.elapsed(),
|
||||
&[],
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn reset_realtime_conversation(&mut self) -> Option<ThreadId> {
|
||||
self.finish_realtime_session_metrics();
|
||||
let should_refresh_terminal_title = self.realtime_conversation.phase
|
||||
!= RealtimeConversationPhase::Inactive
|
||||
&& self.last_terminal_title.is_some();
|
||||
|
||||
@@ -0,0 +1,30 @@
|
||||
//! Failure accounting distinguishes internal retry cleanup from a requested stop.
|
||||
|
||||
use super::super::StartupRetry;
|
||||
use super::*;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
#[tokio::test]
|
||||
async fn terminal_error_during_retry_cleanup_records_one_failure() {
|
||||
for (retry, expected_failure) in [
|
||||
(StartupRetry::WaitingForStop, true),
|
||||
(StartupRetry::Used, false),
|
||||
] {
|
||||
let (mut chat, _sender, _events, _ops) = make_chatwidget_manual_with_sender().await;
|
||||
activate_voice(&mut chat);
|
||||
chat.realtime_conversation.phase = RealtimeConversationPhase::Stopping;
|
||||
chat.realtime_conversation.startup_retry = retry;
|
||||
|
||||
chat.on_realtime_error("connection lost".into());
|
||||
|
||||
assert_eq!(
|
||||
chat.realtime_conversation.failure_recorded,
|
||||
expected_failure
|
||||
);
|
||||
chat.on_realtime_error("duplicate error".into());
|
||||
assert_eq!(
|
||||
chat.realtime_conversation.failure_recorded,
|
||||
expected_failure
|
||||
);
|
||||
}
|
||||
}
|
||||
@@ -3,6 +3,8 @@
|
||||
|
||||
#[path = "realtime/recording_controls_tests.rs"]
|
||||
mod recording_controls_tests;
|
||||
#[path = "realtime/session_metrics_tests.rs"]
|
||||
mod session_metrics_tests;
|
||||
|
||||
use super::RealtimeConversationPhase;
|
||||
use crate::app_command::AppCommand;
|
||||
|
||||
@@ -1,11 +1,19 @@
|
||||
//! Disconnects preserve editable input but never automatically retry queued submissions.
|
||||
|
||||
use super::realtime::RealtimeConversationPhase;
|
||||
use super::*;
|
||||
|
||||
impl ChatWidget {
|
||||
pub(crate) fn pause_for_disconnect(&mut self) {
|
||||
// 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();
|
||||
}
|
||||
let _ = self.reset_realtime_conversation();
|
||||
if let Some(questions) = &mut self.bottom_pane.questions {
|
||||
questions.delivery_enabled = false;
|
||||
|
||||
Reference in New Issue
Block a user