mirror of
https://github.com/openai/codex.git
synced 2026-11-01 19:33:35 +00:00
Preserve root turn attribution in turn-start events (#44611)
## Why Persisted turn-start events lack the originating root turn ID, so they cannot directly associate child turns with the root turn that initiated them. ## What changed - Add optional `root_turn_id` to `TurnStartedEvent`. Use the inherited root turn ID when available, otherwise the turn's own ID. - Centralize turn-start emission for regular turns, shell commands, and manual compaction so they record attribution consistently. - Carry attribution into `ThreadHistoryTurnChange` when processing turn-start events and retain it through completion in the history builder. - Keep older records compatible by defaulting missing attribution to `None`. ## Testing Add coverage for root and child turn attribution, persistence when tool collisions fail a turn before sampling, history change propagation, and deserialization of older turn-start records. GitOrigin-RevId: 54a80b828ca7d1c4c19f13d468c01d052e1b9130
This commit is contained in:
@@ -109,6 +109,7 @@ fn guardian_stdin_reviews_preserve_parent_command_history() {
|
||||
let start_turn = |turn_id: &str| {
|
||||
RolloutItem::EventMsg(EventMsg::TurnStarted(TurnStartedEvent {
|
||||
turn_id: turn_id.into(),
|
||||
root_turn_id: None,
|
||||
trace_id: None,
|
||||
started_at: None,
|
||||
model_context_window: None,
|
||||
|
||||
@@ -104,6 +104,7 @@ pub struct ThreadHistoryItemChange {
|
||||
#[derive(Debug, Clone, PartialEq)]
|
||||
pub struct ThreadHistoryTurnChange {
|
||||
pub turn_id: String,
|
||||
pub root_turn_id: Option<String>,
|
||||
pub status: TurnStatus,
|
||||
pub error: Option<TurnError>,
|
||||
pub started_at: Option<i64>,
|
||||
@@ -131,6 +132,7 @@ impl ThreadHistoryTurnChange {
|
||||
fn from_pending_turn(turn: &PendingTurn) -> Self {
|
||||
Self {
|
||||
turn_id: turn.id.clone(),
|
||||
root_turn_id: turn.root_turn_id.clone(),
|
||||
status: turn.status.clone(),
|
||||
error: turn.error.clone(),
|
||||
started_at: turn.started_at,
|
||||
@@ -1242,11 +1244,12 @@ impl ThreadHistoryBuilder {
|
||||
|
||||
fn handle_turn_started(&mut self, payload: &TurnStartedEvent) {
|
||||
self.finish_current_turn();
|
||||
let turn = self
|
||||
let mut turn = self
|
||||
.new_turn(Some(payload.turn_id.clone()))
|
||||
.with_status(TurnStatus::InProgress)
|
||||
.with_started_at(payload.started_at)
|
||||
.opened_explicitly();
|
||||
turn.root_turn_id = payload.root_turn_id.clone();
|
||||
self.record_changed_pending_turn(&turn);
|
||||
self.current_turn = Some(turn);
|
||||
}
|
||||
@@ -1362,6 +1365,7 @@ impl ThreadHistoryBuilder {
|
||||
});
|
||||
PendingTurn {
|
||||
id,
|
||||
root_turn_id: None,
|
||||
items: Vec::new(),
|
||||
item_index: TurnItemIndex::default(),
|
||||
error: None,
|
||||
@@ -1593,6 +1597,7 @@ impl TurnItemIndex {
|
||||
|
||||
struct PendingTurn {
|
||||
id: String,
|
||||
root_turn_id: Option<String>,
|
||||
items: Vec<ThreadItem>,
|
||||
item_index: TurnItemIndex,
|
||||
error: Option<TurnError>,
|
||||
@@ -2033,6 +2038,7 @@ mod tests {
|
||||
let events = vec![
|
||||
EventMsg::TurnStarted(TurnStartedEvent {
|
||||
turn_id: turn_id.to_string(),
|
||||
root_turn_id: None,
|
||||
trace_id: None,
|
||||
started_at: None,
|
||||
model_context_window: None,
|
||||
@@ -2098,6 +2104,7 @@ mod tests {
|
||||
let events = vec![
|
||||
EventMsg::TurnStarted(TurnStartedEvent {
|
||||
turn_id: turn_id.to_string(),
|
||||
root_turn_id: None,
|
||||
trace_id: None,
|
||||
started_at: None,
|
||||
model_context_window: None,
|
||||
@@ -2166,6 +2173,7 @@ mod tests {
|
||||
let events = vec![
|
||||
EventMsg::TurnStarted(TurnStartedEvent {
|
||||
turn_id: turn_id.to_string(),
|
||||
root_turn_id: None,
|
||||
trace_id: None,
|
||||
started_at: None,
|
||||
model_context_window: None,
|
||||
@@ -2259,6 +2267,7 @@ mod tests {
|
||||
let events = vec![
|
||||
EventMsg::TurnStarted(TurnStartedEvent {
|
||||
turn_id: turn_id.to_string(),
|
||||
root_turn_id: None,
|
||||
trace_id: None,
|
||||
started_at: None,
|
||||
model_context_window: None,
|
||||
@@ -2376,6 +2385,7 @@ mod tests {
|
||||
let events = vec![
|
||||
EventMsg::TurnStarted(TurnStartedEvent {
|
||||
turn_id: turn_id.to_string(),
|
||||
root_turn_id: None,
|
||||
trace_id: None,
|
||||
started_at: None,
|
||||
model_context_window: None,
|
||||
@@ -2476,6 +2486,7 @@ mod tests {
|
||||
let items = vec![
|
||||
RolloutItem::EventMsg(EventMsg::TurnStarted(TurnStartedEvent {
|
||||
turn_id: "turn-image".into(),
|
||||
root_turn_id: None,
|
||||
trace_id: None,
|
||||
started_at: None,
|
||||
model_context_window: None,
|
||||
@@ -2869,6 +2880,7 @@ mod tests {
|
||||
let events = vec![
|
||||
EventMsg::TurnStarted(TurnStartedEvent {
|
||||
turn_id: "turn-a".into(),
|
||||
root_turn_id: None,
|
||||
trace_id: None,
|
||||
started_at: None,
|
||||
model_context_window: None,
|
||||
@@ -2936,6 +2948,7 @@ mod tests {
|
||||
let events = vec![
|
||||
EventMsg::TurnStarted(TurnStartedEvent {
|
||||
turn_id: "turn-1".into(),
|
||||
root_turn_id: None,
|
||||
trace_id: None,
|
||||
started_at: None,
|
||||
model_context_window: None,
|
||||
@@ -3071,6 +3084,7 @@ mod tests {
|
||||
let events = vec![
|
||||
EventMsg::TurnStarted(TurnStartedEvent {
|
||||
turn_id: "turn-1".into(),
|
||||
root_turn_id: None,
|
||||
trace_id: None,
|
||||
started_at: None,
|
||||
model_context_window: None,
|
||||
@@ -3150,6 +3164,7 @@ mod tests {
|
||||
let events = vec![
|
||||
EventMsg::TurnStarted(TurnStartedEvent {
|
||||
turn_id: "turn-1".into(),
|
||||
root_turn_id: None,
|
||||
trace_id: None,
|
||||
started_at: None,
|
||||
model_context_window: None,
|
||||
@@ -3234,6 +3249,7 @@ mod tests {
|
||||
let events = vec![
|
||||
EventMsg::TurnStarted(TurnStartedEvent {
|
||||
turn_id: "turn-1".into(),
|
||||
root_turn_id: None,
|
||||
trace_id: None,
|
||||
started_at: None,
|
||||
model_context_window: None,
|
||||
@@ -3330,6 +3346,7 @@ mod tests {
|
||||
let events = vec![
|
||||
EventMsg::TurnStarted(TurnStartedEvent {
|
||||
turn_id: "turn-1".into(),
|
||||
root_turn_id: None,
|
||||
trace_id: None,
|
||||
started_at: None,
|
||||
model_context_window: None,
|
||||
@@ -3424,6 +3441,7 @@ mod tests {
|
||||
let events = vec![
|
||||
EventMsg::TurnStarted(TurnStartedEvent {
|
||||
turn_id: "turn-1".into(),
|
||||
root_turn_id: None,
|
||||
trace_id: None,
|
||||
started_at: None,
|
||||
model_context_window: None,
|
||||
@@ -3495,6 +3513,7 @@ mod tests {
|
||||
let events = vec![
|
||||
EventMsg::TurnStarted(TurnStartedEvent {
|
||||
turn_id: "turn-a".into(),
|
||||
root_turn_id: None,
|
||||
trace_id: None,
|
||||
started_at: None,
|
||||
model_context_window: None,
|
||||
@@ -3519,6 +3538,7 @@ mod tests {
|
||||
}),
|
||||
EventMsg::TurnStarted(TurnStartedEvent {
|
||||
turn_id: "turn-b".into(),
|
||||
root_turn_id: None,
|
||||
trace_id: None,
|
||||
started_at: None,
|
||||
model_context_window: None,
|
||||
@@ -3601,6 +3621,7 @@ mod tests {
|
||||
let events = vec![
|
||||
EventMsg::TurnStarted(TurnStartedEvent {
|
||||
turn_id: "turn-a".into(),
|
||||
root_turn_id: None,
|
||||
trace_id: None,
|
||||
started_at: None,
|
||||
model_context_window: None,
|
||||
@@ -3625,6 +3646,7 @@ mod tests {
|
||||
}),
|
||||
EventMsg::TurnStarted(TurnStartedEvent {
|
||||
turn_id: "turn-b".into(),
|
||||
root_turn_id: None,
|
||||
trace_id: None,
|
||||
started_at: None,
|
||||
model_context_window: None,
|
||||
@@ -3701,6 +3723,7 @@ mod tests {
|
||||
let events = vec![
|
||||
EventMsg::TurnStarted(TurnStartedEvent {
|
||||
turn_id: turn_id.to_string(),
|
||||
root_turn_id: None,
|
||||
trace_id: None,
|
||||
started_at: None,
|
||||
model_context_window: None,
|
||||
@@ -3769,6 +3792,7 @@ mod tests {
|
||||
let events = vec![
|
||||
EventMsg::TurnStarted(TurnStartedEvent {
|
||||
turn_id: turn_id.to_string(),
|
||||
root_turn_id: None,
|
||||
trace_id: None,
|
||||
started_at: None,
|
||||
model_context_window: None,
|
||||
@@ -3837,6 +3861,7 @@ mod tests {
|
||||
let events = vec![
|
||||
EventMsg::TurnStarted(TurnStartedEvent {
|
||||
turn_id: "turn-a".into(),
|
||||
root_turn_id: None,
|
||||
trace_id: None,
|
||||
started_at: None,
|
||||
model_context_window: None,
|
||||
@@ -3861,6 +3886,7 @@ mod tests {
|
||||
}),
|
||||
EventMsg::TurnStarted(TurnStartedEvent {
|
||||
turn_id: "turn-b".into(),
|
||||
root_turn_id: None,
|
||||
trace_id: None,
|
||||
started_at: None,
|
||||
model_context_window: None,
|
||||
@@ -3917,6 +3943,7 @@ mod tests {
|
||||
let events = vec![
|
||||
EventMsg::TurnStarted(TurnStartedEvent {
|
||||
turn_id: "turn-a".into(),
|
||||
root_turn_id: None,
|
||||
trace_id: None,
|
||||
started_at: Some(10),
|
||||
model_context_window: None,
|
||||
@@ -3932,6 +3959,7 @@ mod tests {
|
||||
}),
|
||||
EventMsg::TurnStarted(TurnStartedEvent {
|
||||
turn_id: "turn-b".into(),
|
||||
root_turn_id: None,
|
||||
trace_id: None,
|
||||
started_at: Some(30),
|
||||
model_context_window: None,
|
||||
@@ -4019,6 +4047,7 @@ mod tests {
|
||||
let events = vec![
|
||||
EventMsg::TurnStarted(TurnStartedEvent {
|
||||
turn_id: "turn-a".into(),
|
||||
root_turn_id: None,
|
||||
trace_id: None,
|
||||
started_at: None,
|
||||
model_context_window: None,
|
||||
@@ -4043,6 +4072,7 @@ mod tests {
|
||||
}),
|
||||
EventMsg::TurnStarted(TurnStartedEvent {
|
||||
turn_id: "turn-b".into(),
|
||||
root_turn_id: None,
|
||||
trace_id: None,
|
||||
started_at: None,
|
||||
model_context_window: None,
|
||||
@@ -4089,6 +4119,7 @@ mod tests {
|
||||
let items = vec![
|
||||
RolloutItem::EventMsg(EventMsg::TurnStarted(TurnStartedEvent {
|
||||
turn_id: "turn-compact".into(),
|
||||
root_turn_id: None,
|
||||
trace_id: None,
|
||||
started_at: None,
|
||||
model_context_window: None,
|
||||
@@ -4361,6 +4392,7 @@ mod tests {
|
||||
let events = vec![
|
||||
EventMsg::TurnStarted(TurnStartedEvent {
|
||||
turn_id: "turn-a".into(),
|
||||
root_turn_id: None,
|
||||
trace_id: None,
|
||||
started_at: None,
|
||||
model_context_window: None,
|
||||
@@ -4423,6 +4455,7 @@ mod tests {
|
||||
let events = vec![
|
||||
EventMsg::TurnStarted(TurnStartedEvent {
|
||||
turn_id: "turn-a".into(),
|
||||
root_turn_id: None,
|
||||
trace_id: None,
|
||||
started_at: None,
|
||||
model_context_window: None,
|
||||
@@ -4482,6 +4515,7 @@ mod tests {
|
||||
let events = vec![
|
||||
EventMsg::TurnStarted(TurnStartedEvent {
|
||||
turn_id: "turn-a".into(),
|
||||
root_turn_id: None,
|
||||
trace_id: None,
|
||||
started_at: Some(10),
|
||||
model_context_window: None,
|
||||
@@ -4552,6 +4586,7 @@ mod tests {
|
||||
let items = vec![
|
||||
RolloutItem::EventMsg(EventMsg::TurnStarted(TurnStartedEvent {
|
||||
turn_id: "turn-a".into(),
|
||||
root_turn_id: None,
|
||||
trace_id: None,
|
||||
started_at: None,
|
||||
model_context_window: None,
|
||||
@@ -4612,6 +4647,7 @@ mod tests {
|
||||
let mut builder = ThreadHistoryBuilder::new();
|
||||
builder.handle_event(&EventMsg::TurnStarted(TurnStartedEvent {
|
||||
turn_id: "turn-a".into(),
|
||||
root_turn_id: None,
|
||||
trace_id: None,
|
||||
started_at: None,
|
||||
model_context_window: None,
|
||||
@@ -4640,6 +4676,7 @@ mod tests {
|
||||
let items = vec![
|
||||
RolloutItem::EventMsg(EventMsg::TurnStarted(TurnStartedEvent {
|
||||
turn_id: "turn-a".into(),
|
||||
root_turn_id: None,
|
||||
trace_id: None,
|
||||
started_at: None,
|
||||
model_context_window: None,
|
||||
@@ -4687,6 +4724,7 @@ mod tests {
|
||||
let items = vec![
|
||||
RolloutItem::EventMsg(EventMsg::TurnStarted(TurnStartedEvent {
|
||||
turn_id: "turn-a".into(),
|
||||
root_turn_id: None,
|
||||
trace_id: None,
|
||||
started_at: None,
|
||||
model_context_window: None,
|
||||
@@ -4752,6 +4790,7 @@ mod tests {
|
||||
}],
|
||||
changed_turns: vec![ThreadHistoryTurnChange {
|
||||
turn_id: "rollout-0".into(),
|
||||
root_turn_id: None,
|
||||
status: TurnStatus::Completed,
|
||||
error: None,
|
||||
started_at: None,
|
||||
@@ -4846,6 +4885,7 @@ mod tests {
|
||||
let start_changes = builder.handle_rollout_item_with_changes(&RolloutItem::EventMsg(
|
||||
EventMsg::TurnStarted(TurnStartedEvent {
|
||||
turn_id: "turn-a".into(),
|
||||
root_turn_id: Some("root-turn".into()),
|
||||
trace_id: None,
|
||||
started_at: Some(10),
|
||||
model_context_window: None,
|
||||
@@ -4858,6 +4898,7 @@ mod tests {
|
||||
changed_items: Vec::new(),
|
||||
changed_turns: vec![ThreadHistoryTurnChange {
|
||||
turn_id: "turn-a".into(),
|
||||
root_turn_id: Some("root-turn".into()),
|
||||
status: TurnStatus::InProgress,
|
||||
error: None,
|
||||
started_at: Some(10),
|
||||
@@ -4896,6 +4937,7 @@ mod tests {
|
||||
changed_items: Vec::new(),
|
||||
changed_turns: vec![ThreadHistoryTurnChange {
|
||||
turn_id: "turn-a".into(),
|
||||
root_turn_id: Some("root-turn".into()),
|
||||
status: TurnStatus::Completed,
|
||||
error: None,
|
||||
started_at: Some(10),
|
||||
@@ -4943,6 +4985,7 @@ mod tests {
|
||||
}],
|
||||
changed_turns: vec![ThreadHistoryTurnChange {
|
||||
turn_id: "rollout-0".into(),
|
||||
root_turn_id: None,
|
||||
status: TurnStatus::Completed,
|
||||
error: None,
|
||||
started_at: None,
|
||||
@@ -4960,6 +5003,7 @@ mod tests {
|
||||
let changes = builder.handle_rollout_items_with_changes(&[
|
||||
RolloutItem::EventMsg(EventMsg::TurnStarted(TurnStartedEvent {
|
||||
turn_id: "turn-a".into(),
|
||||
root_turn_id: Some("root-turn".into()),
|
||||
trace_id: None,
|
||||
started_at: Some(10),
|
||||
model_context_window: None,
|
||||
@@ -4982,6 +5026,7 @@ mod tests {
|
||||
changed_items: Vec::new(),
|
||||
changed_turns: vec![ThreadHistoryTurnChange {
|
||||
turn_id: "turn-a".into(),
|
||||
root_turn_id: Some("root-turn".into()),
|
||||
status: TurnStatus::Completed,
|
||||
error: None,
|
||||
started_at: Some(10),
|
||||
@@ -4999,6 +5044,7 @@ mod tests {
|
||||
let changes = builder.handle_rollout_items_with_changes(&[
|
||||
RolloutItem::EventMsg(EventMsg::TurnStarted(TurnStartedEvent {
|
||||
turn_id: "turn-a".into(),
|
||||
root_turn_id: None,
|
||||
trace_id: None,
|
||||
started_at: None,
|
||||
model_context_window: None,
|
||||
|
||||
@@ -23,6 +23,7 @@ pub fn project_rollout_line(line: &RolloutLine) -> ThreadHistoryChangeSet {
|
||||
RolloutItem::EventMsg(EventMsg::TurnStarted(event)) => ThreadHistoryChangeSet {
|
||||
changed_turns: vec![ThreadHistoryTurnChange {
|
||||
turn_id: event.turn_id.clone(),
|
||||
root_turn_id: event.root_turn_id.clone(),
|
||||
status: TurnStatus::InProgress,
|
||||
error: None,
|
||||
started_at: event.started_at,
|
||||
@@ -34,6 +35,7 @@ pub fn project_rollout_line(line: &RolloutLine) -> ThreadHistoryChangeSet {
|
||||
RolloutItem::EventMsg(EventMsg::TurnComplete(event)) => ThreadHistoryChangeSet {
|
||||
changed_turns: vec![ThreadHistoryTurnChange {
|
||||
turn_id: event.turn_id.clone(),
|
||||
root_turn_id: None,
|
||||
status: if event.error.is_some() {
|
||||
TurnStatus::Failed
|
||||
} else {
|
||||
@@ -58,6 +60,7 @@ pub fn project_rollout_line(line: &RolloutLine) -> ThreadHistoryChangeSet {
|
||||
ThreadHistoryChangeSet {
|
||||
changed_turns: vec![ThreadHistoryTurnChange {
|
||||
turn_id: turn_id.clone(),
|
||||
root_turn_id: None,
|
||||
status: TurnStatus::Interrupted,
|
||||
error: None,
|
||||
started_at: event.started_at,
|
||||
|
||||
@@ -27,6 +27,7 @@ fn projects_turn_lifecycle_without_prior_builder_state() {
|
||||
let started = project(RolloutItem::EventMsg(EventMsg::TurnStarted(
|
||||
TurnStartedEvent {
|
||||
turn_id: "turn-1".to_string(),
|
||||
root_turn_id: Some("root-turn".into()),
|
||||
trace_id: None,
|
||||
started_at: Some(10),
|
||||
model_context_window: None,
|
||||
@@ -49,11 +50,16 @@ fn projects_turn_lifecycle_without_prior_builder_state() {
|
||||
assert_eq!(started.changed_turns[0].turn_id, "turn-1");
|
||||
assert_eq!(started.changed_turns[0].status, TurnStatus::InProgress);
|
||||
assert_eq!(started.changed_turns[0].started_at, Some(10));
|
||||
assert_eq!(
|
||||
started.changed_turns[0].root_turn_id.as_deref(),
|
||||
Some("root-turn")
|
||||
);
|
||||
assert_eq!(
|
||||
completed,
|
||||
ThreadHistoryChangeSet {
|
||||
changed_turns: vec![ThreadHistoryTurnChange {
|
||||
turn_id: "turn-1".to_string(),
|
||||
root_turn_id: None,
|
||||
status: TurnStatus::Completed,
|
||||
error: None,
|
||||
started_at: Some(10),
|
||||
@@ -90,6 +96,7 @@ fn projects_failed_turn_completion_as_snapshot() {
|
||||
ThreadHistoryChangeSet {
|
||||
changed_turns: vec![ThreadHistoryTurnChange {
|
||||
turn_id: "turn-1".to_string(),
|
||||
root_turn_id: None,
|
||||
status: TurnStatus::Failed,
|
||||
error: Some(TurnError {
|
||||
misalignment: None,
|
||||
@@ -240,6 +247,7 @@ fn projects_identified_turn_aborts() {
|
||||
ThreadHistoryChangeSet {
|
||||
changed_turns: vec![ThreadHistoryTurnChange {
|
||||
turn_id: "turn-1".to_string(),
|
||||
root_turn_id: None,
|
||||
status: TurnStatus::Interrupted,
|
||||
error: None,
|
||||
started_at: Some(10),
|
||||
|
||||
Reference in New Issue
Block a user