Merge bd312d17c3 into sapling-pr-archive-rhan-oai

This commit is contained in:
rhan-oai
2026-04-15 18:23:10 -07:00
committed by GitHub
22 changed files with 559 additions and 159 deletions
@@ -1332,6 +1332,7 @@ mod tests {
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
}),
];
@@ -1406,6 +1407,7 @@ mod tests {
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
})),
];
@@ -1729,6 +1731,7 @@ mod tests {
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
}),
];
@@ -2235,6 +2238,7 @@ mod tests {
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
}),
EventMsg::TurnStarted(TurnStartedEvent {
turn_id: "turn-b".into(),
@@ -2272,6 +2276,7 @@ mod tests {
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
}),
];
@@ -2324,6 +2329,7 @@ mod tests {
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
}),
EventMsg::TurnStarted(TurnStartedEvent {
turn_id: "turn-b".into(),
@@ -2361,6 +2367,7 @@ mod tests {
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
}),
];
@@ -2535,6 +2542,7 @@ mod tests {
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
}),
EventMsg::TurnStarted(TurnStartedEvent {
turn_id: "turn-b".into(),
@@ -2553,6 +2561,7 @@ mod tests {
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
}),
EventMsg::AgentMessage(AgentMessageEvent {
message: "still in b".into(),
@@ -2564,6 +2573,7 @@ mod tests {
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
}),
];
@@ -2598,6 +2608,7 @@ mod tests {
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
}),
EventMsg::TurnStarted(TurnStartedEvent {
turn_id: "turn-b".into(),
@@ -2654,6 +2665,7 @@ mod tests {
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
})),
];
@@ -2899,6 +2911,7 @@ mod tests {
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
}),
EventMsg::Error(ErrorEvent {
message: "request-level failure".into(),
@@ -2958,6 +2971,7 @@ mod tests {
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
}),
];
@@ -3009,6 +3023,7 @@ mod tests {
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
})),
];
@@ -3057,6 +3072,7 @@ mod tests {
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
})),
];
@@ -3042,6 +3042,7 @@ mod tests {
last_agent_message: None,
completed_at: Some(TEST_TURN_COMPLETED_AT),
duration_ms: Some(TEST_TURN_DURATION_MS),
time_to_first_token_ms: None,
}
}
+3
View File
@@ -277,6 +277,7 @@ async fn on_event_updates_status_from_task_complete() {
last_agent_message: Some("done".to_string()),
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
}));
let expected = AgentStatus::Completed(Some("done".to_string()));
assert_eq!(status, Some(expected));
@@ -1227,6 +1228,7 @@ async fn multi_agent_v2_completion_ignores_dead_direct_parent() {
last_agent_message: Some("done".to_string()),
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
}),
)
.await;
@@ -1313,6 +1315,7 @@ async fn multi_agent_v2_completion_queues_message_for_direct_parent() {
last_agent_message: Some("done".to_string()),
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
}),
)
.await;
@@ -148,6 +148,7 @@ async fn record_initial_history_resumed_hydrates_previous_turn_settings_from_lif
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
},
)),
];
@@ -215,6 +216,7 @@ async fn reconstruct_history_rollback_keeps_history_and_metadata_in_sync_for_com
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
},
)),
RolloutItem::EventMsg(EventMsg::TurnStarted(
@@ -242,6 +244,7 @@ async fn reconstruct_history_rollback_keeps_history_and_metadata_in_sync_for_com
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
},
)),
RolloutItem::EventMsg(EventMsg::ThreadRolledBack(
@@ -311,6 +314,7 @@ async fn reconstruct_history_rollback_keeps_history_and_metadata_in_sync_for_inc
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
},
)),
RolloutItem::EventMsg(EventMsg::TurnStarted(
@@ -400,6 +404,7 @@ async fn reconstruct_history_rollback_skips_non_user_turns_for_history_and_metad
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
},
)),
RolloutItem::EventMsg(EventMsg::TurnStarted(
@@ -426,6 +431,7 @@ async fn reconstruct_history_rollback_skips_non_user_turns_for_history_and_metad
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
},
)),
RolloutItem::EventMsg(EventMsg::TurnStarted(
@@ -443,6 +449,7 @@ async fn reconstruct_history_rollback_skips_non_user_turns_for_history_and_metad
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
},
)),
RolloutItem::EventMsg(EventMsg::ThreadRolledBack(
@@ -515,6 +522,7 @@ async fn reconstruct_history_rollback_counts_inter_agent_assistant_turns() {
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
},
)),
RolloutItem::EventMsg(EventMsg::TurnStarted(
@@ -534,6 +542,7 @@ async fn reconstruct_history_rollback_counts_inter_agent_assistant_turns() {
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
},
)),
RolloutItem::EventMsg(EventMsg::ThreadRolledBack(
@@ -601,6 +610,7 @@ async fn reconstruct_history_rollback_clears_history_and_metadata_when_exceeding
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
},
)),
RolloutItem::EventMsg(EventMsg::ThreadRolledBack(
@@ -650,6 +660,7 @@ async fn record_initial_history_resumed_rollback_skips_only_user_turns() {
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
},
)),
// Standalone task turn (no UserMessage) should not consume rollback skips.
@@ -667,6 +678,7 @@ async fn record_initial_history_resumed_rollback_skips_only_user_turns() {
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
},
)),
RolloutItem::EventMsg(EventMsg::ThreadRolledBack(
@@ -720,6 +732,7 @@ async fn record_initial_history_resumed_rollback_drops_incomplete_user_turn_comp
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
},
)),
RolloutItem::EventMsg(EventMsg::TurnStarted(
@@ -876,6 +889,7 @@ async fn reconstruct_history_legacy_compaction_without_replacement_history_clear
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
},
)),
];
@@ -945,6 +959,7 @@ async fn record_initial_history_resumed_turn_context_after_compaction_reestablis
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
},
)),
];
@@ -1046,6 +1061,7 @@ async fn record_initial_history_resumed_aborted_turn_without_id_clears_active_tu
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
},
)),
RolloutItem::EventMsg(EventMsg::TurnStarted(
@@ -1153,6 +1169,7 @@ async fn record_initial_history_resumed_unmatched_abort_preserves_active_turn_fo
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
},
)),
RolloutItem::EventMsg(EventMsg::TurnStarted(
@@ -1186,6 +1203,7 @@ async fn record_initial_history_resumed_unmatched_abort_preserves_active_turn_fo
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
},
)),
];
@@ -1268,6 +1286,7 @@ async fn record_initial_history_resumed_trailing_incomplete_turn_compaction_clea
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
},
)),
RolloutItem::EventMsg(EventMsg::TurnStarted(
@@ -1418,6 +1437,7 @@ async fn record_initial_history_resumed_replaced_incomplete_compacted_turn_clear
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
},
)),
RolloutItem::EventMsg(EventMsg::TurnStarted(
+10
View File
@@ -1472,6 +1472,7 @@ async fn record_initial_history_forked_hydrates_previous_turn_settings() {
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
},
)),
];
@@ -1657,6 +1658,7 @@ async fn thread_rollback_recomputes_previous_turn_settings_and_reference_context
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
})),
RolloutItem::EventMsg(EventMsg::TurnStarted(
codex_protocol::protocol::TurnStartedEvent {
@@ -1682,6 +1684,7 @@ async fn thread_rollback_recomputes_previous_turn_settings_and_reference_context
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
})),
])
.await;
@@ -1759,6 +1762,7 @@ async fn thread_rollback_restores_cleared_reference_context_item_after_compactio
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
})),
RolloutItem::EventMsg(EventMsg::TurnStarted(
codex_protocol::protocol::TurnStartedEvent {
@@ -1777,6 +1781,7 @@ async fn thread_rollback_restores_cleared_reference_context_item_after_compactio
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
})),
RolloutItem::EventMsg(EventMsg::TurnStarted(
codex_protocol::protocol::TurnStartedEvent {
@@ -1804,6 +1809,7 @@ async fn thread_rollback_restores_cleared_reference_context_item_after_compactio
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
})),
])
.await;
@@ -1850,6 +1856,7 @@ async fn thread_rollback_persists_marker_and_replays_cumulatively() {
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
})),
RolloutItem::EventMsg(EventMsg::TurnStarted(
codex_protocol::protocol::TurnStartedEvent {
@@ -1873,6 +1880,7 @@ async fn thread_rollback_persists_marker_and_replays_cumulatively() {
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
})),
RolloutItem::EventMsg(EventMsg::TurnStarted(
codex_protocol::protocol::TurnStartedEvent {
@@ -1896,6 +1904,7 @@ async fn thread_rollback_persists_marker_and_replays_cumulatively() {
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
})),
])
.await;
@@ -4928,6 +4937,7 @@ async fn task_finish_emits_turn_item_lifecycle_for_leftover_pending_user_input()
EventMsg::TurnComplete(TurnCompleteEvent {
turn_id,
last_agent_message: None,
time_to_first_token_ms: None,
..
}) if turn_id == tc.sub_id
));
+43 -23
View File
@@ -9,7 +9,7 @@ use serde::Serialize;
use serde_json::Value;
use super::GUARDIAN_MAX_ACTION_STRING_TOKENS;
use super::prompt::guardian_truncate_text;
use super::prompt::guardian_truncate_text_with_metadata;
#[derive(Debug, Clone, PartialEq)]
pub(crate) enum GuardianApprovalRequest {
@@ -167,32 +167,49 @@ fn guardian_command_source_tool_name(source: GuardianCommandSource) -> &'static
}
}
fn truncate_guardian_action_value(value: Value) -> Value {
fn truncate_guardian_action_value(value: Value) -> (Value, bool) {
match value {
Value::String(text) => Value::String(guardian_truncate_text(
&text,
GUARDIAN_MAX_ACTION_STRING_TOKENS,
)),
Value::Array(values) => Value::Array(
values
Value::String(text) => {
let (text, truncated) =
guardian_truncate_text_with_metadata(&text, GUARDIAN_MAX_ACTION_STRING_TOKENS);
(Value::String(text), truncated)
}
Value::Array(values) => {
let mut truncated = false;
let values = values
.into_iter()
.map(truncate_guardian_action_value)
.collect::<Vec<_>>(),
),
.map(|value| {
let (value, value_truncated) = truncate_guardian_action_value(value);
truncated |= value_truncated;
value
})
.collect::<Vec<_>>();
(Value::Array(values), truncated)
}
Value::Object(values) => {
let mut entries = values.into_iter().collect::<Vec<_>>();
entries.sort_by(|(left, _), (right, _)| left.cmp(right));
Value::Object(
entries
.into_iter()
.map(|(key, value)| (key, truncate_guardian_action_value(value)))
.collect(),
)
let mut truncated = false;
let values = entries
.into_iter()
.map(|(key, value)| {
let (value, value_truncated) = truncate_guardian_action_value(value);
truncated |= value_truncated;
(key, value)
})
.collect();
(Value::Object(values), truncated)
}
other => other,
other => (other, false),
}
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub(crate) struct FormattedGuardianAction {
pub(crate) text: String,
pub(crate) truncated: bool,
}
pub(crate) fn guardian_approval_request_to_json(
action: &GuardianApprovalRequest,
) -> serde_json::Result<Value> {
@@ -382,10 +399,13 @@ pub(crate) fn guardian_request_turn_id<'a>(
}
}
pub(crate) fn format_guardian_action_pretty(
pub(crate) fn format_guardian_action_pretty_with_truncation(
action: &GuardianApprovalRequest,
) -> serde_json::Result<String> {
let mut value = guardian_approval_request_to_json(action)?;
value = truncate_guardian_action_value(value);
serde_json::to_string_pretty(&value)
) -> serde_json::Result<FormattedGuardianAction> {
let value = guardian_approval_request_to_json(action)?;
let (value, truncated) = truncate_guardian_action_value(value);
Ok(FormattedGuardianAction {
text: serde_json::to_string_pretty(&value)?,
truncated,
})
}
+3 -1
View File
@@ -69,7 +69,7 @@ pub(crate) struct GuardianRejection {
}
#[cfg(test)]
use approval_request::format_guardian_action_pretty;
use approval_request::format_guardian_action_pretty_with_truncation;
#[cfg(test)]
use approval_request::guardian_assessment_action;
#[cfg(test)]
@@ -101,6 +101,8 @@ use prompt::render_guardian_transcript_entries;
#[cfg(test)]
use review::GuardianReviewOutcome;
#[cfg(test)]
use review::GuardianReviewSessionReport;
#[cfg(test)]
use review::run_guardian_review_session as run_guardian_review_session_for_test;
#[cfg(test)]
use review_session::build_guardian_review_session_config as build_guardian_review_session_config_for_test;
+17 -8
View File
@@ -19,7 +19,7 @@ use super::GUARDIAN_RECENT_ENTRY_LIMIT;
use super::GuardianApprovalRequest;
use super::GuardianAssessment;
use super::TRUNCATION_TAG;
use super::approval_request::format_guardian_action_pretty;
use super::approval_request::format_guardian_action_pretty_with_truncation;
/// Transcript entry retained for guardian review after filtering.
#[derive(Debug, PartialEq, Eq)]
@@ -56,6 +56,7 @@ impl GuardianTranscriptEntryKind {
pub(crate) struct GuardianPromptItems {
pub(crate) items: Vec<UserInput>,
pub(crate) transcript_cursor: GuardianTranscriptCursor,
pub(crate) reviewed_action_truncated: bool,
}
/// Points to the end of the transcript that the guardian has already reviewed.
@@ -91,7 +92,7 @@ pub(crate) async fn build_guardian_prompt_items(
parent_history_version: history.history_version(),
transcript_entry_count: transcript_entries.len(),
};
let planned_action_json = format_guardian_action_pretty(&request)?;
let planned_action = format_guardian_action_pretty_with_truncation(&request)?;
let prompt_shape = match mode {
GuardianPromptMode::Full => GuardianPromptShape::Full,
@@ -176,11 +177,12 @@ pub(crate) async fn build_guardian_prompt_items(
.to_string(),
);
push_text("Planned action JSON:\n".to_string());
push_text(format!("{planned_action_json}\n"));
push_text(format!("{}\n", planned_action.text));
push_text(">>> APPROVAL REQUEST END\n".to_string());
Ok(GuardianPromptItems {
items,
transcript_cursor,
reviewed_action_truncated: planned_action.truncated,
})
}
@@ -420,20 +422,23 @@ pub(crate) fn collect_guardian_transcript_entries(
entries
}
pub(crate) fn guardian_truncate_text(content: &str, token_cap: usize) -> String {
pub(crate) fn guardian_truncate_text_with_metadata(
content: &str,
token_cap: usize,
) -> (String, bool) {
if content.is_empty() {
return String::new();
return (String::new(), false);
}
let max_bytes = approx_bytes_for_tokens(token_cap);
if content.len() <= max_bytes {
return content.to_string();
return (content.to_string(), false);
}
let omitted_tokens = approx_tokens_from_byte_count(content.len().saturating_sub(max_bytes));
let marker = format!("<{TRUNCATION_TAG} omitted_approx_tokens=\"{omitted_tokens}\" />");
if max_bytes <= marker.len() {
return marker;
return (marker, true);
}
let available_bytes = max_bytes.saturating_sub(marker.len());
@@ -441,7 +446,11 @@ pub(crate) fn guardian_truncate_text(content: &str, token_cap: usize) -> String
let suffix_budget = available_bytes.saturating_sub(prefix_budget);
let (prefix, suffix) = split_guardian_truncation_bounds(content, prefix_budget, suffix_budget);
format!("{prefix}{marker}{suffix}")
(format!("{prefix}{marker}{suffix}"), true)
}
pub(crate) fn guardian_truncate_text(content: &str, token_cap: usize) -> String {
guardian_truncate_text_with_metadata(content, token_cap).0
}
fn split_guardian_truncation_bounds(
+137 -87
View File
@@ -37,6 +37,7 @@ use super::approval_request::guardian_request_target_item_id;
use super::approval_request::guardian_request_turn_id;
use super::prompt::guardian_output_schema;
use super::prompt::parse_guardian_assessment;
use super::review_session::GuardianReviewSessionMetadata;
use super::review_session::GuardianReviewSessionOutcome;
use super::review_session::GuardianReviewSessionParams;
use super::review_session::build_guardian_review_session_config;
@@ -84,14 +85,56 @@ pub(crate) fn guardian_timeout_message() -> String {
GUARDIAN_TIMEOUT_INSTRUCTIONS.to_string()
}
#[derive(Debug)]
pub(super) struct GuardianReviewSessionReport {
pub(super) outcome: GuardianReviewOutcome,
pub(super) metadata: Option<GuardianReviewSessionMetadata>,
}
#[derive(Debug)]
pub(super) enum GuardianReviewOutcome {
Completed(anyhow::Result<GuardianAssessment>),
Completed(GuardianAssessment),
Failed(GuardianReviewFailure),
TimedOut,
Aborted,
}
impl GuardianReviewSessionReport {
fn completed(
assessment: GuardianAssessment,
metadata: Option<GuardianReviewSessionMetadata>,
) -> Self {
Self {
outcome: GuardianReviewOutcome::Completed(assessment),
metadata,
}
}
fn failed(
failure: GuardianReviewFailure,
metadata: Option<GuardianReviewSessionMetadata>,
) -> Self {
Self {
outcome: GuardianReviewOutcome::Failed(failure),
metadata,
}
}
fn timed_out(metadata: Option<GuardianReviewSessionMetadata>) -> Self {
Self {
outcome: GuardianReviewOutcome::TimedOut,
metadata,
}
}
fn aborted(metadata: Option<GuardianReviewSessionMetadata>) -> Self {
Self {
outcome: GuardianReviewOutcome::Aborted,
metadata,
}
}
}
#[derive(Debug)]
pub(super) enum GuardianReviewFailure {
PromptBuild(anyhow::Error),
@@ -208,37 +251,41 @@ struct GuardianReviewAnalyticsResult {
completed_at: u64,
}
#[derive(Default)]
struct GuardianReviewMetadataFields {
guardian_thread_id: Option<String>,
guardian_session_kind: Option<GuardianReviewSessionKind>,
guardian_model: Option<String>,
guardian_reasoning_effort: Option<String>,
had_prior_review_context: Option<bool>,
reviewed_action_truncated: bool,
token_usage: Option<TokenUsage>,
time_to_first_token_ms: Option<u64>,
}
impl GuardianReviewAnalyticsResult {
fn from_metadata(metadata: GuardianReviewMetadataFields, completed_at: u64) -> Self {
Self {
fn from_session_metadata(
metadata: Option<GuardianReviewSessionMetadata>,
completed_at: u64,
) -> Self {
let mut result = Self {
decision: GuardianReviewDecision::Denied,
terminal_status: GuardianReviewTerminalStatus::FailedClosed,
failure_reason: None,
risk_level: None,
user_authorization: None,
outcome: None,
guardian_thread_id: metadata.guardian_thread_id,
guardian_session_kind: metadata.guardian_session_kind,
guardian_model: metadata.guardian_model,
guardian_reasoning_effort: metadata.guardian_reasoning_effort,
had_prior_review_context: metadata.had_prior_review_context,
reviewed_action_truncated: metadata.reviewed_action_truncated,
token_usage: metadata.token_usage,
time_to_first_token_ms: metadata.time_to_first_token_ms,
guardian_thread_id: None,
guardian_session_kind: None,
guardian_model: None,
guardian_reasoning_effort: None,
had_prior_review_context: None,
reviewed_action_truncated: false,
token_usage: None,
time_to_first_token_ms: None,
completed_at,
};
if let Some(metadata) = metadata {
result.guardian_thread_id = Some(metadata.guardian_thread_id);
result.guardian_session_kind = Some(metadata.guardian_session_kind);
result.guardian_model = Some(metadata.guardian_model);
result.guardian_reasoning_effort = metadata.guardian_reasoning_effort;
result.had_prior_review_context = Some(metadata.had_prior_review_context);
result.reviewed_action_truncated = metadata.reviewed_action_truncated;
result.token_usage = metadata.token_usage;
result.time_to_first_token_ms = metadata.time_to_first_token_ms;
}
result
}
}
@@ -365,8 +412,8 @@ async fn run_guardian_review(
decision: GuardianReviewDecision::Aborted,
terminal_status: GuardianReviewTerminalStatus::Aborted,
failure_reason: Some(GuardianReviewFailureReason::Cancelled),
..GuardianReviewAnalyticsResult::from_metadata(
GuardianReviewMetadataFields::default(),
..GuardianReviewAnalyticsResult::from_session_metadata(
/*metadata*/ None,
now_unix_seconds(),
)
},
@@ -392,7 +439,7 @@ async fn run_guardian_review(
let schema = guardian_output_schema();
let terminal_action = action_summary.clone();
let outcome = Box::pin(run_guardian_review_session(
let GuardianReviewSessionReport { outcome, metadata } = Box::pin(run_guardian_review_session(
session.clone(),
turn.clone(),
request,
@@ -403,14 +450,10 @@ async fn run_guardian_review(
.await;
let completed_at = now_unix_seconds();
let result = || {
GuardianReviewAnalyticsResult::from_metadata(
GuardianReviewMetadataFields::default(),
completed_at,
)
};
let result =
|metadata| GuardianReviewAnalyticsResult::from_session_metadata(metadata, completed_at);
let assessment = match outcome {
GuardianReviewOutcome::Completed(Ok(assessment)) => {
GuardianReviewOutcome::Completed(assessment) => {
let approved = matches!(assessment.outcome, GuardianAssessmentOutcome::Allow);
analytics_context.track(
session.as_ref(),
@@ -430,30 +473,11 @@ async fn run_guardian_review(
risk_level: Some(assessment.risk_level),
user_authorization: Some(assessment.user_authorization),
outcome: Some(assessment.outcome),
..result()
..result(metadata)
},
);
assessment
}
GuardianReviewOutcome::Completed(Err(err)) => {
let rationale = format!("Automatic approval review failed: {err}");
analytics_context.track(
session.as_ref(),
turn.as_ref(),
GuardianReviewAnalyticsResult {
decision: GuardianReviewDecision::Denied,
terminal_status: GuardianReviewTerminalStatus::FailedClosed,
failure_reason: Some(GuardianReviewFailureReason::SessionError),
..result()
},
);
GuardianAssessment {
risk_level: GuardianRiskLevel::High,
user_authorization: GuardianUserAuthorization::Unknown,
outcome: GuardianAssessmentOutcome::Deny,
rationale,
}
}
GuardianReviewOutcome::Failed(failure) => {
let rationale = format!("Automatic approval review failed: {}", failure.error());
analytics_context.track(
@@ -463,7 +487,7 @@ async fn run_guardian_review(
decision: GuardianReviewDecision::Denied,
terminal_status: GuardianReviewTerminalStatus::FailedClosed,
failure_reason: Some(failure.reason()),
..result()
..result(metadata)
},
);
GuardianAssessment {
@@ -484,7 +508,7 @@ async fn run_guardian_review(
decision: GuardianReviewDecision::Denied,
terminal_status: GuardianReviewTerminalStatus::TimedOut,
failure_reason: Some(GuardianReviewFailureReason::Timeout),
..result()
..result(metadata)
},
);
session
@@ -521,7 +545,7 @@ async fn run_guardian_review(
decision: GuardianReviewDecision::Aborted,
terminal_status: GuardianReviewTerminalStatus::Aborted,
failure_reason: Some(GuardianReviewFailureReason::Cancelled),
..result()
..result(metadata)
},
);
session
@@ -671,12 +695,15 @@ pub(super) async fn run_guardian_review_session(
retry_reason: Option<String>,
schema: serde_json::Value,
external_cancel: Option<CancellationToken>,
) -> GuardianReviewOutcome {
) -> GuardianReviewSessionReport {
let live_network_config = match session.services.network_proxy.as_ref() {
Some(network_proxy) => match network_proxy.proxy().current_cfg().await {
Ok(config) => Some(config),
Err(err) => {
return GuardianReviewOutcome::Failed(GuardianReviewFailure::PromptBuild(err));
return GuardianReviewSessionReport::failed(
GuardianReviewFailure::PromptBuild(err),
/*metadata*/ None,
);
}
},
None => None,
@@ -727,45 +754,68 @@ pub(super) async fn run_guardian_review_session(
);
let guardian_config = match guardian_config {
Ok(config) => config,
Err(err) => return GuardianReviewOutcome::Failed(GuardianReviewFailure::PromptBuild(err)),
Err(err) => {
return GuardianReviewSessionReport::failed(
GuardianReviewFailure::PromptBuild(err),
/*metadata*/ None,
);
}
};
match Box::pin(
session
.guardian_review_session
.run_review(GuardianReviewSessionParams {
parent_session: Arc::clone(&session),
parent_turn: turn.clone(),
spawn_config: guardian_config,
request,
retry_reason,
schema,
model: guardian_model,
reasoning_effort: guardian_reasoning_effort,
reasoning_summary: turn.reasoning_summary,
personality: turn.personality,
external_cancel,
}),
)
.await
{
let (session_outcome, session_metadata) = Box::pin(session.guardian_review_session.run_review(
GuardianReviewSessionParams {
parent_session: Arc::clone(&session),
parent_turn: turn.clone(),
spawn_config: guardian_config,
request,
retry_reason,
schema,
model: guardian_model,
reasoning_effort: guardian_reasoning_effort,
reasoning_summary: turn.reasoning_summary,
personality: turn.personality,
external_cancel,
},
))
.await;
match session_outcome {
GuardianReviewSessionOutcome::Completed(Ok(last_agent_message)) => match last_agent_message
{
Some(last_agent_message) => {
match parse_guardian_assessment(Some(&last_agent_message)) {
Ok(assessment) => GuardianReviewOutcome::Completed(Ok(assessment)),
Err(err) => GuardianReviewOutcome::Failed(GuardianReviewFailure::Parse(err)),
Ok(assessment) => {
GuardianReviewSessionReport::completed(assessment, session_metadata)
}
Err(err) => GuardianReviewSessionReport::failed(
GuardianReviewFailure::Parse(err),
session_metadata,
),
}
}
None => GuardianReviewOutcome::Failed(GuardianReviewFailure::Session(anyhow::anyhow!(
"guardian review completed without an assessment payload"
))),
None => GuardianReviewSessionReport::failed(
GuardianReviewFailure::Session(anyhow::anyhow!(
"guardian review completed without an assessment payload"
)),
session_metadata,
),
},
GuardianReviewSessionOutcome::Completed(Err(err)) => {
GuardianReviewOutcome::Failed(GuardianReviewFailure::Session(err))
GuardianReviewSessionOutcome::Completed(Err(err)) => GuardianReviewSessionReport::failed(
GuardianReviewFailure::Session(err),
session_metadata,
),
GuardianReviewSessionOutcome::PromptBuildFailed(err) => {
GuardianReviewSessionReport::failed(
GuardianReviewFailure::PromptBuild(err),
session_metadata,
)
}
GuardianReviewSessionOutcome::TimedOut => {
GuardianReviewSessionReport::timed_out(session_metadata)
}
GuardianReviewSessionOutcome::Aborted => {
GuardianReviewSessionReport::aborted(session_metadata)
}
GuardianReviewSessionOutcome::TimedOut => GuardianReviewOutcome::TimedOut,
GuardianReviewSessionOutcome::Aborted => GuardianReviewOutcome::Aborted,
}
}
+173 -32
View File
@@ -5,6 +5,7 @@ use std::sync::Arc;
use std::time::Duration;
use anyhow::anyhow;
use codex_analytics::GuardianReviewSessionKind;
use codex_protocol::config_types::Personality;
use codex_protocol::config_types::ReasoningSummary as ReasoningSummaryConfig;
use codex_protocol::models::DeveloperInstructions;
@@ -17,6 +18,7 @@ use codex_protocol::protocol::Op;
use codex_protocol::protocol::RolloutItem;
use codex_protocol::protocol::SandboxPolicy;
use codex_protocol::protocol::SubAgentSource;
use codex_protocol::protocol::TokenUsage;
use serde_json::Value;
use tokio::sync::Mutex;
use tokio_util::sync::CancellationToken;
@@ -58,10 +60,23 @@ const GUARDIAN_FOLLOWUP_REVIEW_REMINDER: &str = concat!(
#[derive(Debug)]
pub(crate) enum GuardianReviewSessionOutcome {
Completed(anyhow::Result<Option<String>>),
PromptBuildFailed(anyhow::Error),
TimedOut,
Aborted,
}
#[derive(Debug, Clone)]
pub(crate) struct GuardianReviewSessionMetadata {
pub(crate) guardian_thread_id: String,
pub(crate) guardian_session_kind: GuardianReviewSessionKind,
pub(crate) guardian_model: String,
pub(crate) guardian_reasoning_effort: Option<String>,
pub(crate) had_prior_review_context: bool,
pub(crate) reviewed_action_truncated: bool,
pub(crate) token_usage: Option<TokenUsage>,
pub(crate) time_to_first_token_ms: Option<u64>,
}
pub(crate) struct GuardianReviewSessionParams {
pub(crate) parent_session: Arc<Session>,
pub(crate) parent_turn: Arc<TurnContext>,
@@ -101,6 +116,21 @@ struct GuardianReviewState {
last_committed_fork_snapshot: Option<GuardianReviewForkSnapshot>,
}
fn had_prior_review_context(prompt_mode: &GuardianPromptMode) -> bool {
matches!(prompt_mode, GuardianPromptMode::Delta { .. })
}
fn token_usage_delta(start: &TokenUsage, end: &TokenUsage) -> TokenUsage {
TokenUsage {
input_tokens: (end.input_tokens - start.input_tokens).max(0),
cached_input_tokens: (end.cached_input_tokens - start.cached_input_tokens).max(0),
output_tokens: (end.output_tokens - start.output_tokens).max(0),
reasoning_output_tokens: (end.reasoning_output_tokens - start.reasoning_output_tokens)
.max(0),
total_tokens: (end.total_tokens - start.total_tokens).max(0),
}
}
struct EphemeralReviewCleanup {
state: Arc<Mutex<GuardianReviewSessionState>>,
review_session: Option<Arc<GuardianReviewSession>>,
@@ -267,10 +297,14 @@ impl GuardianReviewSessionManager {
pub(crate) async fn run_review(
&self,
params: GuardianReviewSessionParams,
) -> GuardianReviewSessionOutcome {
) -> (
GuardianReviewSessionOutcome,
Option<GuardianReviewSessionMetadata>,
) {
let deadline = tokio::time::Instant::now() + GUARDIAN_REVIEW_TIMEOUT;
let next_reuse_key = GuardianReviewSessionReuseKey::from_spawn_config(&params.spawn_config);
let mut stale_trunk_to_shutdown = None;
let mut spawned_trunk = false;
let trunk_candidate = match run_before_review_deadline(
deadline,
params.external_cancel.as_ref(),
@@ -304,16 +338,17 @@ impl GuardianReviewSessionManager {
{
Ok(Ok(review_session)) => Arc::new(review_session),
Ok(Err(err)) => {
return GuardianReviewSessionOutcome::Completed(Err(err));
return (GuardianReviewSessionOutcome::PromptBuildFailed(err), None);
}
Err(outcome) => return outcome,
Err(outcome) => return (outcome, None),
};
state.trunk = Some(Arc::clone(&review_session));
spawned_trunk = true;
}
state.trunk.as_ref().cloned()
}
Err(outcome) => return outcome,
Err(outcome) => return (outcome, None),
};
if let Some(review_session) = stale_trunk_to_shutdown {
@@ -321,9 +356,12 @@ impl GuardianReviewSessionManager {
}
let Some(trunk) = trunk_candidate else {
return GuardianReviewSessionOutcome::Completed(Err(anyhow!(
"guardian review session was not available after spawn"
)));
return (
GuardianReviewSessionOutcome::Completed(Err(anyhow!(
"guardian review session was not available after spawn"
))),
None,
);
};
if trunk.reuse_key != next_reuse_key {
@@ -349,20 +387,30 @@ impl GuardianReviewSessionManager {
}
};
let (outcome, keep_review_session) =
Box::pin(run_review_on_session(trunk.as_ref(), &params, deadline)).await;
let guardian_session_kind = if spawned_trunk {
GuardianReviewSessionKind::TrunkNew
} else {
GuardianReviewSessionKind::TrunkReused
};
let (outcome, keep_review_session, metadata) = Box::pin(run_review_on_session(
trunk.as_ref(),
&params,
guardian_session_kind,
deadline,
))
.await;
if keep_review_session && matches!(outcome, GuardianReviewSessionOutcome::Completed(_)) {
trunk.refresh_last_committed_fork_snapshot().await;
}
drop(trunk_guard);
if keep_review_session {
outcome
(outcome, Some(metadata))
} else {
if let Some(review_session) = self.remove_trunk_if_current(&trunk).await {
review_session.shutdown_in_background();
}
outcome
(outcome, Some(metadata))
}
}
@@ -459,7 +507,10 @@ impl GuardianReviewSessionManager {
reuse_key: GuardianReviewSessionReuseKey,
deadline: tokio::time::Instant,
fork_snapshot: Option<GuardianReviewForkSnapshot>,
) -> GuardianReviewSessionOutcome {
) -> (
GuardianReviewSessionOutcome,
Option<GuardianReviewSessionMetadata>,
) {
let spawn_cancel_token = CancellationToken::new();
let mut fork_config = params.spawn_config.clone();
fork_config.ephemeral = true;
@@ -478,17 +529,18 @@ impl GuardianReviewSessionManager {
.await
{
Ok(Ok(review_session)) => Arc::new(review_session),
Ok(Err(err)) => return GuardianReviewSessionOutcome::Completed(Err(err)),
Err(outcome) => return outcome,
Ok(Err(err)) => return (GuardianReviewSessionOutcome::PromptBuildFailed(err), None),
Err(outcome) => return (outcome, None),
};
self.register_active_ephemeral(Arc::clone(&review_session))
.await;
let mut cleanup =
EphemeralReviewCleanup::new(Arc::clone(&self.state), Arc::clone(&review_session));
let (outcome, _) = Box::pin(run_review_on_session(
let (outcome, _, metadata) = Box::pin(run_review_on_session(
review_session.as_ref(),
&params,
GuardianReviewSessionKind::EphemeralForked,
deadline,
))
.await;
@@ -496,7 +548,7 @@ impl GuardianReviewSessionManager {
cleanup.disarm();
review_session.shutdown_in_background();
}
outcome
(outcome, Some(metadata))
}
}
@@ -543,8 +595,13 @@ async fn spawn_guardian_review_session(
async fn run_review_on_session(
review_session: &GuardianReviewSession,
params: &GuardianReviewSessionParams,
guardian_session_kind: GuardianReviewSessionKind,
deadline: tokio::time::Instant,
) -> (GuardianReviewSessionOutcome, bool) {
) -> (
GuardianReviewSessionOutcome,
bool,
GuardianReviewSessionMetadata,
) {
let (send_followup_reminder, prompt_mode) = {
let state = review_session.state.lock().await;
@@ -559,6 +616,16 @@ async fn run_review_on_session(
(send_followup_reminder, prompt_mode)
};
let mut guardian_metadata = GuardianReviewSessionMetadata {
guardian_thread_id: review_session.codex.session.conversation_id.to_string(),
guardian_session_kind,
guardian_model: params.model.clone(),
guardian_reasoning_effort: params.reasoning_effort.map(|effort| effort.to_string()),
had_prior_review_context: had_prior_review_context(&prompt_mode),
reviewed_action_truncated: false,
token_usage: None,
time_to_first_token_ms: None,
};
if send_followup_reminder {
append_guardian_followup_reminder(review_session).await;
}
@@ -583,6 +650,9 @@ async fn run_review_on_session(
prompt_mode,
)
.await?;
let reviewed_action_truncated = prompt_items.reviewed_action_truncated;
let token_usage_at_review_start =
review_session.codex.session.total_token_usage().await;
review_session
.codex
@@ -602,29 +672,53 @@ async fn run_review_on_session(
})
.await?;
Ok::<GuardianTranscriptCursor, anyhow::Error>(prompt_items.transcript_cursor)
Ok::<(GuardianTranscriptCursor, bool, Option<TokenUsage>), anyhow::Error>((
prompt_items.transcript_cursor,
reviewed_action_truncated,
token_usage_at_review_start,
))
}),
)
.await;
let submit_result = match submit_result {
Ok(submit_result) => submit_result,
Err(outcome) => return (outcome, false),
};
let transcript_cursor = match submit_result {
Ok(transcript_cursor) => transcript_cursor,
Err(err) => {
return (GuardianReviewSessionOutcome::Completed(Err(err)), false);
}
Err(outcome) => return (outcome, false, guardian_metadata),
};
let (transcript_cursor, reviewed_action_truncated, token_usage_at_review_start) =
match submit_result {
Ok(submit_result) => submit_result,
Err(err) => {
return (
GuardianReviewSessionOutcome::PromptBuildFailed(err),
false,
guardian_metadata,
);
}
};
guardian_metadata.reviewed_action_truncated = reviewed_action_truncated;
let outcome =
wait_for_guardian_review(review_session, deadline, params.external_cancel.as_ref()).await;
let outcome = wait_for_guardian_review(
review_session,
deadline,
params.external_cancel.as_ref(),
&mut guardian_metadata,
)
.await;
if matches!(outcome.0, GuardianReviewSessionOutcome::Completed(_)) {
if outcome.2
&& let Some(token_usage_at_review_start) = token_usage_at_review_start
&& let Some(total_token_usage) = review_session.codex.session.total_token_usage().await
{
guardian_metadata.token_usage = Some(token_usage_delta(
&token_usage_at_review_start,
&total_token_usage,
));
}
let mut state = review_session.state.lock().await;
state.prior_review_count = state.prior_review_count.saturating_add(1);
state.last_reviewed_transcript_cursor = Some(transcript_cursor);
}
outcome
(outcome.0, outcome.1, guardian_metadata)
}
async fn append_guardian_followup_reminder(review_session: &GuardianReviewSession) {
@@ -653,7 +747,8 @@ async fn wait_for_guardian_review(
review_session: &GuardianReviewSession,
deadline: tokio::time::Instant,
external_cancel: Option<&CancellationToken>,
) -> (GuardianReviewSessionOutcome, bool) {
metadata: &mut GuardianReviewSessionMetadata,
) -> (GuardianReviewSessionOutcome, bool, bool) {
let timeout = tokio::time::sleep_until(deadline);
tokio::pin!(timeout);
let mut last_error_message: Option<String> = None;
@@ -662,7 +757,7 @@ async fn wait_for_guardian_review(
tokio::select! {
_ = &mut timeout => {
let keep_review_session = interrupt_and_drain_turn(&review_session.codex).await.is_ok();
return (GuardianReviewSessionOutcome::TimedOut, keep_review_session);
return (GuardianReviewSessionOutcome::TimedOut, keep_review_session, false);
}
_ = async {
if let Some(cancel_token) = external_cancel {
@@ -672,30 +767,35 @@ async fn wait_for_guardian_review(
}
} => {
let keep_review_session = interrupt_and_drain_turn(&review_session.codex).await.is_ok();
return (GuardianReviewSessionOutcome::Aborted, keep_review_session);
return (GuardianReviewSessionOutcome::Aborted, keep_review_session, false);
}
event = review_session.codex.next_event() => {
match event {
Ok(event) => match event.msg {
EventMsg::TurnComplete(turn_complete) => {
metadata.time_to_first_token_ms = turn_complete
.time_to_first_token_ms
.and_then(|ms| u64::try_from(ms).ok());
if turn_complete.last_agent_message.is_none()
&& let Some(error_message) = last_error_message
{
return (
GuardianReviewSessionOutcome::Completed(Err(anyhow!(error_message))),
true,
true,
);
}
return (
GuardianReviewSessionOutcome::Completed(Ok(turn_complete.last_agent_message)),
true,
true,
);
}
EventMsg::Error(error) => {
last_error_message = Some(error.message);
}
EventMsg::TurnAborted(_) => {
return (GuardianReviewSessionOutcome::Aborted, true);
return (GuardianReviewSessionOutcome::Aborted, true, false);
}
_ => {}
},
@@ -703,6 +803,7 @@ async fn wait_for_guardian_review(
return (
GuardianReviewSessionOutcome::Completed(Err(err.into())),
false,
false,
);
}
}
@@ -974,4 +1075,44 @@ mod tests {
assert_eq!(outcome.unwrap(), 42);
assert!(!cancel_token.is_cancelled());
}
#[test]
fn had_prior_review_context_tracks_prompt_mode() {
assert!(!had_prior_review_context(&GuardianPromptMode::Full));
assert!(had_prior_review_context(&GuardianPromptMode::Delta {
cursor: GuardianTranscriptCursor {
parent_history_version: 7,
transcript_entry_count: 42,
}
}));
}
#[test]
fn token_usage_delta_never_reports_negative_usage() {
let start = TokenUsage {
input_tokens: 10,
cached_input_tokens: 8,
output_tokens: 6,
reasoning_output_tokens: 4,
total_tokens: 28,
};
let end = TokenUsage {
input_tokens: 15,
cached_input_tokens: 7,
output_tokens: 10,
reasoning_output_tokens: 2,
total_tokens: 34,
};
assert_eq!(
token_usage_delta(&start, &end),
TokenUsage {
input_tokens: 5,
cached_input_tokens: 0,
output_tokens: 4,
reasoning_output_tokens: 0,
total_tokens: 6,
}
);
}
}
+92 -8
View File
@@ -585,11 +585,29 @@ fn format_guardian_action_pretty_truncates_large_string_fields() -> serde_json::
patch: patch.clone(),
};
let rendered = format_guardian_action_pretty(&action)?;
let rendered = format_guardian_action_pretty_with_truncation(&action)?;
assert!(rendered.contains("\"tool\": \"apply_patch\""));
assert!(rendered.contains("<truncated omitted_approx_tokens="));
assert!(rendered.len() < patch.len());
assert!(rendered.text.contains("\"tool\": \"apply_patch\""));
assert!(rendered.text.contains("<truncated omitted_approx_tokens="));
assert!(rendered.text.len() < patch.len());
assert!(rendered.truncated);
Ok(())
}
#[test]
fn format_guardian_action_pretty_reports_no_truncation_for_small_payload() -> serde_json::Result<()>
{
let action = GuardianApprovalRequest::ApplyPatch {
id: "patch-1".to_string(),
cwd: test_path_buf("/tmp").abs(),
files: Vec::new(),
patch: "line\n".to_string(),
};
let rendered = format_guardian_action_pretty_with_truncation(&action)?;
assert!(rendered.text.contains("\"tool\": \"apply_patch\""));
assert!(!rendered.truncated);
Ok(())
}
@@ -918,10 +936,32 @@ async fn guardian_review_request_layout_matches_model_visible_request_snapshot()
/*external_cancel*/ None,
)
.await;
let GuardianReviewOutcome::Completed(Ok(assessment)) = outcome else {
let GuardianReviewSessionReport {
outcome: GuardianReviewOutcome::Completed(assessment),
metadata,
} = outcome
else {
panic!("expected guardian assessment");
};
let metadata = metadata.expect("guardian session metadata");
assert_eq!(assessment.outcome, GuardianAssessmentOutcome::Allow);
assert_ne!(
metadata.guardian_thread_id,
session.conversation_id.to_string()
);
ThreadId::from_string(&metadata.guardian_thread_id)
.expect("guardian thread id should be a valid UUID");
assert!(matches!(
metadata.guardian_session_kind,
codex_analytics::GuardianReviewSessionKind::TrunkNew
));
assert_eq!(metadata.guardian_model, "gpt-5.4");
assert_eq!(metadata.guardian_reasoning_effort.as_deref(), Some("low"));
assert!(!metadata.had_prior_review_context);
assert!(
metadata.time_to_first_token_ms.is_some(),
"guardian review metadata should capture TTFT when the nested turn completes"
);
let request = request_log.single_request();
let mut settings = Settings::clone_current();
@@ -1127,18 +1167,62 @@ async fn guardian_reuses_prompt_cache_key_and_appends_prior_reviews() -> anyhow:
)
.await;
let GuardianReviewOutcome::Completed(Ok(first_assessment)) = first_outcome else {
let GuardianReviewSessionReport {
outcome: GuardianReviewOutcome::Completed(first_assessment),
metadata: first_metadata,
} = first_outcome
else {
panic!("expected first guardian assessment");
};
let GuardianReviewOutcome::Completed(Ok(second_assessment)) = second_outcome else {
let first_metadata = first_metadata.expect("first guardian session metadata");
let GuardianReviewSessionReport {
outcome: GuardianReviewOutcome::Completed(second_assessment),
metadata: second_metadata,
} = second_outcome
else {
panic!("expected second guardian assessment");
};
let GuardianReviewOutcome::Completed(Ok(third_assessment)) = third_outcome else {
let second_metadata = second_metadata.expect("second guardian session metadata");
let GuardianReviewSessionReport {
outcome: GuardianReviewOutcome::Completed(third_assessment),
metadata: third_metadata,
} = third_outcome
else {
panic!("expected third guardian assessment");
};
let third_metadata = third_metadata.expect("third guardian session metadata");
assert_eq!(first_assessment.outcome, GuardianAssessmentOutcome::Allow);
assert_eq!(second_assessment.outcome, GuardianAssessmentOutcome::Allow);
assert_eq!(third_assessment.outcome, GuardianAssessmentOutcome::Allow);
assert!(matches!(
first_metadata.guardian_session_kind,
codex_analytics::GuardianReviewSessionKind::TrunkNew
));
assert!(matches!(
second_metadata.guardian_session_kind,
codex_analytics::GuardianReviewSessionKind::TrunkReused
));
assert!(matches!(
third_metadata.guardian_session_kind,
codex_analytics::GuardianReviewSessionKind::TrunkReused
));
ThreadId::from_string(&first_metadata.guardian_thread_id)
.expect("first guardian thread id should be a valid UUID");
ThreadId::from_string(&second_metadata.guardian_thread_id)
.expect("second guardian thread id should be a valid UUID");
ThreadId::from_string(&third_metadata.guardian_thread_id)
.expect("third guardian thread id should be a valid UUID");
assert!(!first_metadata.had_prior_review_context);
assert!(second_metadata.had_prior_review_context);
assert!(third_metadata.had_prior_review_context);
assert_eq!(
first_metadata.guardian_thread_id,
second_metadata.guardian_thread_id
);
assert_eq!(
second_metadata.guardian_thread_id,
third_metadata.guardian_thread_id
);
let requests = request_log.requests();
assert_eq!(requests.len(), 3);
+5
View File
@@ -535,11 +535,16 @@ impl Session {
.turn_timing_state
.completed_at_and_duration_ms()
.await;
let time_to_first_token_ms = turn_context
.turn_timing_state
.time_to_first_token_ms()
.await;
let event = EventMsg::TurnComplete(TurnCompleteEvent {
turn_id: turn_context.sub_id.clone(),
last_agent_message,
completed_at,
duration_ms,
time_to_first_token_ms,
});
self.send_event(turn_context.as_ref(), event).await;
@@ -1154,6 +1154,7 @@ async fn multi_agent_v2_list_agents_returns_completed_status_and_last_task_messa
last_agent_message: Some("done".to_string()),
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
}),
)
.await;
@@ -1632,6 +1633,7 @@ async fn multi_agent_v2_followup_task_completion_notifies_parent_on_every_turn()
last_agent_message: Some("first done".to_string()),
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
}),
)
.await;
@@ -1660,6 +1662,7 @@ async fn multi_agent_v2_followup_task_completion_notifies_parent_on_every_turn()
last_agent_message: Some("second done".to_string()),
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
}),
)
.await;
+10
View File
@@ -74,6 +74,16 @@ impl TurnTimingState {
(completed_at, duration_ms)
}
pub(crate) async fn time_to_first_token_ms(&self) -> Option<i64> {
let state = self.state.lock().await;
let started_at = state.started_at?;
let first_token_at = state.first_token_at?;
Some(
i64::try_from(first_token_at.duration_since(started_at).as_millis())
.unwrap_or(i64::MAX),
)
}
pub(crate) async fn record_ttft_for_response_event(
&self,
event: &ResponseEvent,
@@ -69,6 +69,7 @@ fn resume_history(
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
})),
],
rollout_path: rollout_path.to_path_buf(),
+4
View File
@@ -2056,6 +2056,10 @@ pub struct TurnCompleteEvent {
#[serde(default, skip_serializing_if = "Option::is_none")]
#[ts(type = "number | null", optional)]
pub duration_ms: Option<i64>,
/// Duration between turn start and the first model token in milliseconds, if known.
#[serde(default, skip_serializing_if = "Option::is_none")]
#[ts(type = "number | null", optional)]
pub time_to_first_token_ms: Option<i64>,
}
#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)]
@@ -766,6 +766,7 @@ fn append_terminal_turn_events(events: &mut Vec<Event>, turn: &Turn, include_fai
last_agent_message: None,
completed_at: turn.completed_at,
duration_ms: turn.duration_ms,
time_to_first_token_ms: None,
}),
}),
TurnStatus::Interrupted => events.push(Event {
@@ -797,6 +798,7 @@ fn append_terminal_turn_events(events: &mut Vec<Event>, turn: &Turn, include_fai
last_agent_message: None,
completed_at: turn.completed_at,
duration_ms: turn.duration_ms,
time_to_first_token_ms: None,
}),
});
}
@@ -654,6 +654,7 @@ async fn unified_exec_wait_after_final_agent_message_snapshot() {
last_agent_message: Some("Final response.".into()),
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
}),
});
@@ -699,6 +700,7 @@ async fn unified_exec_wait_before_streamed_agent_message_snapshot() {
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
}),
});
@@ -766,6 +768,7 @@ async fn unified_exec_waiting_multiple_empty_snapshots() {
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
}),
});
@@ -846,6 +849,7 @@ async fn unified_exec_non_empty_then_empty_snapshots() {
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
}),
});
@@ -1350,6 +1354,7 @@ async fn turn_complete_keeps_unified_exec_processes() {
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
}),
});
@@ -546,6 +546,7 @@ async fn plan_implementation_popup_skips_replayed_turn_complete() {
last_agent_message: Some("Plan details".to_string()),
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
})]);
let popup = render_bottom_popup(&chat, /*width*/ 80);
@@ -572,6 +573,7 @@ async fn plan_implementation_popup_shows_once_when_replay_precedes_live_turn_com
last_agent_message: Some("Plan details".to_string()),
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
})]);
let replay_popup = render_bottom_popup(&chat, /*width*/ 80);
assert!(
@@ -586,6 +588,7 @@ async fn plan_implementation_popup_shows_once_when_replay_precedes_live_turn_com
last_agent_message: Some("Plan details".to_string()),
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
}),
});
@@ -609,6 +612,7 @@ async fn plan_implementation_popup_shows_once_when_replay_precedes_live_turn_com
last_agent_message: Some("Plan details".to_string()),
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
}),
});
let duplicate_popup = render_bottom_popup(&chat, /*width*/ 80);
@@ -234,6 +234,7 @@ async fn steer_rejection_queues_review_follow_up_before_existing_queued_messages
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
}),
});
@@ -255,6 +256,7 @@ async fn steer_rejection_queues_review_follow_up_before_existing_queued_messages
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
}),
});
@@ -225,6 +225,7 @@ async fn slash_copy_state_tracks_turn_complete_final_reply() {
last_agent_message: Some("Final reply **markdown**".to_string()),
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
}),
});
@@ -257,6 +258,7 @@ async fn slash_copy_state_tracks_plan_item_completion() {
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
}),
});
@@ -343,6 +345,7 @@ async fn slash_copy_state_is_preserved_during_running_task() {
last_agent_message: Some("Previous completed reply".to_string()),
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
}),
});
chat.on_task_started();
@@ -373,6 +376,7 @@ async fn slash_copy_tracks_replayed_legacy_agent_message_when_turn_complete_omit
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
}),
});
let _ = drain_insert_history(&mut rx);
@@ -410,6 +414,7 @@ async fn slash_copy_uses_agent_message_item_when_turn_complete_omits_final_text(
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
}),
});
let _ = drain_insert_history(&mut rx);
@@ -961,6 +961,7 @@ async fn status_line_branch_refreshes_after_turn_complete() {
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
}),
});
@@ -1311,6 +1312,7 @@ async fn multiple_agent_messages_in_single_turn_emit_multiple_headers() {
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
}),
});
@@ -2251,6 +2253,7 @@ printf 'fenced within fenced\n'
last_agent_message: None,
completed_at: None,
duration_ms: None,
time_to_first_token_ms: None,
}),
});
for lines in drain_insert_history(&mut rx) {