mirror of
https://github.com/openai/codex.git
synced 2026-10-27 18:48:37 +00:00
turn steer rejection semantics
This commit is contained in:
@@ -125,7 +125,7 @@ pub enum TurnSteerRejectionReason {
|
||||
|
||||
#[derive(Clone)]
|
||||
pub struct CodexTurnSteerEvent {
|
||||
pub expected_turn_id: String,
|
||||
pub expected_turn_id: Option<String>,
|
||||
pub accepted_turn_id: Option<String>,
|
||||
pub num_input_images: usize,
|
||||
pub result: TurnSteerResult,
|
||||
@@ -570,7 +570,7 @@ struct CodexTurnEventRequest {
|
||||
#[derive(Serialize)]
|
||||
struct CodexTurnSteerEventParams {
|
||||
thread_id: String,
|
||||
expected_turn_id: String,
|
||||
expected_turn_id: Option<String>,
|
||||
accepted_turn_id: Option<String>,
|
||||
product_client_id: Option<String>,
|
||||
num_input_images: usize,
|
||||
|
||||
@@ -18,6 +18,7 @@ use super::TrackEventRequest;
|
||||
use super::TrackEventsContext;
|
||||
use super::TurnCompletedInput;
|
||||
use super::TurnStartedInput;
|
||||
use super::TurnSteerRejectionReason;
|
||||
use super::TurnSteerResult;
|
||||
use super::codex_app_metadata;
|
||||
use super::codex_plugin_metadata;
|
||||
@@ -319,7 +320,7 @@ fn turn_steer_event_serializes_expected_shape() {
|
||||
event_params: codex_turn_steer_event_params(
|
||||
&tracking,
|
||||
CodexTurnSteerEvent {
|
||||
expected_turn_id: "turn-2".to_string(),
|
||||
expected_turn_id: Some("turn-2".to_string()),
|
||||
accepted_turn_id: Some("turn-2".to_string()),
|
||||
num_input_images: 2,
|
||||
result: TurnSteerResult::Accepted,
|
||||
@@ -349,6 +350,48 @@ fn turn_steer_event_serializes_expected_shape() {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rejected_turn_steer_event_serializes_expected_shape() {
|
||||
let tracking = TrackEventsContext {
|
||||
model_slug: "gpt-5".to_string(),
|
||||
thread_id: "thread-3".to_string(),
|
||||
turn_id: "turn-3".to_string(),
|
||||
};
|
||||
let event = TrackEventRequest::TurnSteer(CodexTurnSteerEventRequest {
|
||||
event_type: "codex_turn_steer_event",
|
||||
event_params: codex_turn_steer_event_params(
|
||||
&tracking,
|
||||
CodexTurnSteerEvent {
|
||||
expected_turn_id: Some("turn-expected".to_string()),
|
||||
accepted_turn_id: None,
|
||||
num_input_images: 1,
|
||||
result: TurnSteerResult::Rejected,
|
||||
rejection_reason: Some(TurnSteerRejectionReason::ExpectedTurnMismatch),
|
||||
created_at: 1_716_000_124,
|
||||
},
|
||||
),
|
||||
});
|
||||
|
||||
let payload = serde_json::to_value(&event).expect("serialize rejected turn steer event");
|
||||
|
||||
assert_eq!(
|
||||
payload,
|
||||
json!({
|
||||
"event_type": "codex_turn_steer_event",
|
||||
"event_params": {
|
||||
"thread_id": "thread-3",
|
||||
"expected_turn_id": "turn-expected",
|
||||
"accepted_turn_id": null,
|
||||
"product_client_id": originator().value,
|
||||
"num_input_images": 1,
|
||||
"result": "rejected",
|
||||
"rejection_reason": "expected_turn_mismatch",
|
||||
"created_at": 1_716_000_124
|
||||
}
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn turn_started_then_completed_emits_turn_event() {
|
||||
let tracking = TrackEventsContext {
|
||||
|
||||
+111
-16
@@ -220,6 +220,18 @@ impl SteerInputError {
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
fn to_turn_steer_rejection_reason(&self) -> TurnSteerRejectionReason {
|
||||
match self {
|
||||
Self::NoActiveTurn(_) => TurnSteerRejectionReason::NoActiveTurn,
|
||||
Self::ExpectedTurnMismatch { .. } => TurnSteerRejectionReason::ExpectedTurnMismatch,
|
||||
Self::ActiveTurnNotSteerable { turn_kind } => match turn_kind {
|
||||
NonSteerableTurnKind::Review => TurnSteerRejectionReason::NonSteerableReview,
|
||||
NonSteerableTurnKind::Compact => TurnSteerRejectionReason::NonSteerableCompact,
|
||||
},
|
||||
Self::EmptyInput => TurnSteerRejectionReason::EmptyInput,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Notes from the previous real user turn.
|
||||
@@ -357,6 +369,8 @@ use codex_analytics::CodexTurnEvent;
|
||||
use codex_analytics::CodexTurnSteerEvent;
|
||||
use codex_analytics::InitializationMode;
|
||||
use codex_analytics::InvocationType;
|
||||
use codex_analytics::TrackEventsContext;
|
||||
use codex_analytics::TurnSteerRejectionReason;
|
||||
use codex_analytics::TurnSteerResult;
|
||||
use codex_analytics::build_track_events_context;
|
||||
use codex_async_utils::OrCancelExt;
|
||||
@@ -3967,8 +3981,44 @@ impl Session {
|
||||
input: Vec<UserInput>,
|
||||
expected_turn_id: Option<&str>,
|
||||
) -> Result<String, SteerInputError> {
|
||||
let created_at = SystemTime::now()
|
||||
.duration_since(UNIX_EPOCH)
|
||||
.unwrap_or_default()
|
||||
.as_secs();
|
||||
let thread_id = self.conversation_id.to_string();
|
||||
let fallback_tracking = || {
|
||||
build_track_events_context(
|
||||
String::new(),
|
||||
thread_id.clone(),
|
||||
expected_turn_id.unwrap_or_default().to_string(),
|
||||
)
|
||||
};
|
||||
let track_rejection = |tracking: TrackEventsContext,
|
||||
expected_turn_id: Option<String>,
|
||||
rejection_reason: TurnSteerRejectionReason,
|
||||
num_input_images: usize| {
|
||||
self.services.analytics_events_client.track_turn_steer(
|
||||
tracking,
|
||||
CodexTurnSteerEvent {
|
||||
expected_turn_id,
|
||||
accepted_turn_id: None,
|
||||
num_input_images,
|
||||
result: TurnSteerResult::Rejected,
|
||||
rejection_reason: Some(rejection_reason),
|
||||
created_at,
|
||||
},
|
||||
);
|
||||
};
|
||||
|
||||
if input.is_empty() {
|
||||
return Err(SteerInputError::EmptyInput);
|
||||
let err = SteerInputError::EmptyInput;
|
||||
track_rejection(
|
||||
fallback_tracking(),
|
||||
expected_turn_id.map(str::to_string),
|
||||
err.to_turn_steer_rejection_reason(),
|
||||
0,
|
||||
);
|
||||
return Err(err);
|
||||
}
|
||||
|
||||
let num_input_images = input
|
||||
@@ -3978,41 +4028,89 @@ impl Session {
|
||||
|
||||
let mut active = self.active_turn.lock().await;
|
||||
let Some(active_turn) = active.as_mut() else {
|
||||
return Err(SteerInputError::NoActiveTurn(input));
|
||||
let err = SteerInputError::NoActiveTurn(input);
|
||||
track_rejection(
|
||||
fallback_tracking(),
|
||||
expected_turn_id.map(str::to_string),
|
||||
err.to_turn_steer_rejection_reason(),
|
||||
num_input_images,
|
||||
);
|
||||
return Err(err);
|
||||
};
|
||||
|
||||
let Some((active_turn_id, task)) = active_turn.tasks.first() else {
|
||||
return Err(SteerInputError::NoActiveTurn(input));
|
||||
let err = SteerInputError::NoActiveTurn(input);
|
||||
track_rejection(
|
||||
fallback_tracking(),
|
||||
expected_turn_id.map(str::to_string),
|
||||
err.to_turn_steer_rejection_reason(),
|
||||
num_input_images,
|
||||
);
|
||||
return Err(err);
|
||||
};
|
||||
let active_turn_id = active_turn_id.clone();
|
||||
let tracking = build_track_events_context(
|
||||
task.turn_context.model_info.slug.clone(),
|
||||
self.conversation_id.to_string(),
|
||||
thread_id,
|
||||
task.turn_context.sub_id.clone(),
|
||||
);
|
||||
|
||||
if let Some(expected_turn_id) = expected_turn_id
|
||||
&& expected_turn_id != active_turn_id
|
||||
{
|
||||
return Err(SteerInputError::ExpectedTurnMismatch {
|
||||
let err = SteerInputError::ExpectedTurnMismatch {
|
||||
expected: expected_turn_id.to_string(),
|
||||
actual: active_turn_id.clone(),
|
||||
});
|
||||
};
|
||||
track_rejection(
|
||||
tracking.clone(),
|
||||
Some(expected_turn_id.to_string()),
|
||||
err.to_turn_steer_rejection_reason(),
|
||||
num_input_images,
|
||||
);
|
||||
return Err(err);
|
||||
}
|
||||
|
||||
match active_turn.tasks.first().map(|(_, task)| task.kind) {
|
||||
Some(crate::state::TaskKind::Regular) => {}
|
||||
Some(crate::state::TaskKind::Review) => {
|
||||
return Err(SteerInputError::ActiveTurnNotSteerable {
|
||||
let err = SteerInputError::ActiveTurnNotSteerable {
|
||||
turn_kind: NonSteerableTurnKind::Review,
|
||||
});
|
||||
};
|
||||
track_rejection(
|
||||
tracking.clone(),
|
||||
expected_turn_id
|
||||
.map(str::to_string)
|
||||
.or_else(|| Some(active_turn_id.clone())),
|
||||
err.to_turn_steer_rejection_reason(),
|
||||
num_input_images,
|
||||
);
|
||||
return Err(err);
|
||||
}
|
||||
Some(crate::state::TaskKind::Compact) => {
|
||||
return Err(SteerInputError::ActiveTurnNotSteerable {
|
||||
let err = SteerInputError::ActiveTurnNotSteerable {
|
||||
turn_kind: NonSteerableTurnKind::Compact,
|
||||
});
|
||||
};
|
||||
track_rejection(
|
||||
tracking.clone(),
|
||||
expected_turn_id
|
||||
.map(str::to_string)
|
||||
.or_else(|| Some(active_turn_id.clone())),
|
||||
err.to_turn_steer_rejection_reason(),
|
||||
num_input_images,
|
||||
);
|
||||
return Err(err);
|
||||
}
|
||||
None => {
|
||||
let err = SteerInputError::NoActiveTurn(input);
|
||||
track_rejection(
|
||||
fallback_tracking(),
|
||||
expected_turn_id.map(str::to_string),
|
||||
err.to_turn_steer_rejection_reason(),
|
||||
num_input_images,
|
||||
);
|
||||
return Err(err);
|
||||
}
|
||||
None => return Err(SteerInputError::NoActiveTurn(input)),
|
||||
}
|
||||
|
||||
let mut turn_state = active_turn.turn_state.lock().await;
|
||||
@@ -4025,15 +4123,12 @@ impl Session {
|
||||
self.services.analytics_events_client.track_turn_steer(
|
||||
tracking,
|
||||
CodexTurnSteerEvent {
|
||||
expected_turn_id,
|
||||
expected_turn_id: Some(expected_turn_id),
|
||||
accepted_turn_id: Some(active_turn_id.clone()),
|
||||
num_input_images,
|
||||
result: TurnSteerResult::Accepted,
|
||||
rejection_reason: None,
|
||||
created_at: SystemTime::now()
|
||||
.duration_since(UNIX_EPOCH)
|
||||
.unwrap_or_default()
|
||||
.as_secs(),
|
||||
created_at,
|
||||
},
|
||||
);
|
||||
Ok(active_turn_id)
|
||||
|
||||
@@ -1,6 +1,8 @@
|
||||
use anyhow::Result;
|
||||
use codex_core::CodexAuth;
|
||||
use codex_core::SteerInputError;
|
||||
use codex_core::config::Constrained;
|
||||
use codex_protocol::user_input::UserInput;
|
||||
use core_test_support::responses::start_mock_server;
|
||||
use core_test_support::test_codex::test_codex;
|
||||
use std::time::Duration;
|
||||
@@ -76,3 +78,87 @@ async fn thread_initialization_tracks_thread_initialized_analytics() -> Result<(
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn rejected_steer_tracks_turn_steer_analytics() -> Result<()> {
|
||||
let server = start_mock_server().await;
|
||||
let chatgpt_base_url = server.uri();
|
||||
|
||||
let test = test_codex()
|
||||
.with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing())
|
||||
.with_config(move |config| {
|
||||
config.chatgpt_base_url = chatgpt_base_url;
|
||||
config.model = Some("gpt-5".to_string());
|
||||
config.permissions.sandbox_policy = Constrained::allow_any(
|
||||
codex_protocol::protocol::SandboxPolicy::new_workspace_write_policy(),
|
||||
);
|
||||
config.ephemeral = true;
|
||||
})
|
||||
.build(&server)
|
||||
.await?;
|
||||
|
||||
let err = test
|
||||
.codex
|
||||
.steer_input(
|
||||
vec![UserInput::Text {
|
||||
text: "reject this steer".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
None,
|
||||
)
|
||||
.await
|
||||
.expect_err("steer should be rejected without an active turn");
|
||||
|
||||
assert!(matches!(err, SteerInputError::NoActiveTurn(_)));
|
||||
|
||||
let deadline = Instant::now() + Duration::from_secs(10);
|
||||
let event = loop {
|
||||
let requests = server.received_requests().await.unwrap_or_default();
|
||||
if let Some(event) = requests
|
||||
.into_iter()
|
||||
.filter(|request| request.url.path() == "/codex/analytics-events/events")
|
||||
.filter_map(|request| serde_json::from_slice::<serde_json::Value>(&request.body).ok())
|
||||
.flat_map(|payload| {
|
||||
payload["events"]
|
||||
.as_array()
|
||||
.cloned()
|
||||
.unwrap_or_default()
|
||||
.into_iter()
|
||||
})
|
||||
.find(|event| event["event_type"] == "codex_turn_steer_event")
|
||||
{
|
||||
break event;
|
||||
}
|
||||
if Instant::now() >= deadline {
|
||||
panic!("timed out waiting for steer analytics event");
|
||||
}
|
||||
tokio::time::sleep(Duration::from_millis(50)).await;
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
event["event_params"]["thread_id"],
|
||||
serde_json::json!(test.session_configured.session_id.to_string())
|
||||
);
|
||||
assert_eq!(
|
||||
event["event_params"]["expected_turn_id"],
|
||||
serde_json::Value::Null
|
||||
);
|
||||
assert_eq!(
|
||||
event["event_params"]["accepted_turn_id"],
|
||||
serde_json::Value::Null
|
||||
);
|
||||
assert_eq!(
|
||||
event["event_params"]["product_client_id"],
|
||||
serde_json::json!(codex_core::default_client::originator().value)
|
||||
);
|
||||
assert_eq!(event["event_params"]["num_input_images"], 0);
|
||||
assert_eq!(event["event_params"]["result"], "rejected");
|
||||
assert_eq!(event["event_params"]["rejection_reason"], "no_active_turn");
|
||||
assert!(
|
||||
event["event_params"]["created_at"]
|
||||
.as_u64()
|
||||
.is_some_and(|timestamp| timestamp > 0)
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user