[codex-analytics] guardian review truncation

Author: rhan-oai

codex-rs/core/src/guardian/approval_request.rs

@@ -184,32 +184,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(&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> {
@@ -482,8 +499,11 @@ pub(crate) fn guardian_request_turn_id<'a>(
 
 pub(crate) fn format_guardian_action_pretty(
     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,
+    })
 }

codex-rs/core/src/guardian/prompt.rs

@@ -59,6 +59,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.
@@ -179,11 +180,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_json.text));
     push_text(">>> APPROVAL REQUEST END\n".to_string());
     Ok(GuardianPromptItems {
         items,
         transcript_cursor,
+        reviewed_action_truncated: planned_action_json.truncated,
     })
 }
 
@@ -243,7 +245,7 @@ fn render_guardian_transcript_entries_with_offset(
             } else {
                 GUARDIAN_MAX_MESSAGE_ENTRY_TOKENS
             };
-            let text = guardian_truncate_text(&entry.text, token_cap);
+            let (text, _) = guardian_truncate_text(&entry.text, token_cap);
             let rendered = format!(
                 "[{}] {}: {}",
                 index + entry_number_offset + 1,
@@ -423,28 +425,28 @@ 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(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());
     let prefix_budget = available_bytes / 2;
     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)
 }
 
 fn split_guardian_truncation_bounds(

codex-rs/core/src/guardian/review_session.rs

@@ -672,6 +672,7 @@ async fn run_review_on_session(
             );
         }
     };
+    let reviewed_action_truncated = prompt_items.reviewed_action_truncated;
     let transcript_cursor = prompt_items.transcript_cursor;
     let token_usage_at_review_start = review_session
         .codex
@@ -711,6 +712,7 @@ async fn run_review_on_session(
         }
         Err(outcome) => return (outcome, false, analytics_result),
     }
+    analytics_result.reviewed_action_truncated = reviewed_action_truncated;
 
     let outcome =
         wait_for_guardian_review(review_session, deadline, params.external_cancel.as_ref()).await;

codex-rs/core/src/guardian/tests.rs

@@ -574,11 +574,12 @@ fn collect_guardian_transcript_entries_includes_recent_tool_calls_and_output() {
 fn guardian_truncate_text_keeps_prefix_suffix_and_xml_marker() {
     let content = "prefix ".repeat(200) + &" suffix".repeat(200);
 
-    let truncated = guardian_truncate_text(&content, /*token_cap*/ 20);
+    let (truncated, was_truncated) = guardian_truncate_text(&content, /*token_cap*/ 20);
 
     assert!(truncated.starts_with("prefix"));
     assert!(truncated.contains("<truncated omitted_approx_tokens=\""));
     assert!(truncated.ends_with("suffix"));
+    assert!(was_truncated);
 }
 
 #[test]
@@ -593,9 +594,27 @@ fn format_guardian_action_pretty_truncates_large_string_fields() -> serde_json::
 
     let rendered = format_guardian_action_pretty(&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(&action)?;
+
+    assert!(rendered.text.contains("\"tool\": \"apply_patch\""));
+    assert!(!rendered.truncated);
     Ok(())
 }
 
@@ -996,11 +1015,20 @@ async fn guardian_review_request_layout_matches_model_visible_request_snapshot()
         /*external_cancel*/ None,
     )
     .await;
-    let (GuardianReviewOutcome::Completed(assessment), _) = outcome else {
+    let (GuardianReviewOutcome::Completed(assessment), metadata) = outcome else {
         panic!("expected guardian assessment");
     };
+    let guardian_thread_id = metadata
+        .guardian_thread_id
+        .as_deref()
+        .expect("guardian thread id");
     assert_eq!(assessment.outcome, GuardianAssessmentOutcome::Allow);
-
+    assert_ne!(guardian_thread_id, session.conversation_id.to_string());
+    ThreadId::from_string(guardian_thread_id).expect("guardian thread id should be a valid UUID");
+    assert!(matches!(
+        metadata.guardian_session_kind,
+        Some(codex_analytics::GuardianReviewSessionKind::TrunkNew)
+    ));
     let request = request_log.single_request();
     let request_body = request.body_json();
     assert_eq!(
@@ -1032,6 +1060,21 @@ async fn guardian_review_request_layout_matches_model_visible_request_snapshot()
             "required": ["outcome"]
         }))
     );
+    let request_model = request_body
+        .get("model")
+        .and_then(|value| value.as_str())
+        .expect("guardian request should include a model");
+    let request_reasoning_effort = request_body
+        .get("reasoning")
+        .and_then(|reasoning| reasoning.get("effort"))
+        .and_then(|value| value.as_str());
+    assert_eq!(metadata.guardian_model.as_deref(), Some(request_model));
+    assert_eq!(
+        metadata.guardian_reasoning_effort.as_deref(),
+        request_reasoning_effort
+    );
+    assert_eq!(metadata.had_prior_review_context, Some(false));
+
     let mut settings = Settings::clone_current();
     settings.set_snapshot_path("snapshots");
     settings.set_prepend_module_to_snapshot(false);
@@ -1235,18 +1278,63 @@ async fn guardian_reuses_prompt_cache_key_and_appends_prior_reviews() -> anyhow:
     )
     .await;
 
-    let (GuardianReviewOutcome::Completed(first_assessment), _) = first_outcome else {
+    let (GuardianReviewOutcome::Completed(first_assessment), first_metadata) = first_outcome else {
         panic!("expected first guardian assessment");
     };
-    let (GuardianReviewOutcome::Completed(second_assessment), _) = second_outcome else {
+    let (GuardianReviewOutcome::Completed(second_assessment), second_metadata) = second_outcome
+    else {
         panic!("expected second guardian assessment");
     };
-    let (GuardianReviewOutcome::Completed(third_assessment), _) = third_outcome else {
+    let (GuardianReviewOutcome::Completed(third_assessment), third_metadata) = third_outcome else {
         panic!("expected third guardian assessment");
     };
     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,
+        Some(codex_analytics::GuardianReviewSessionKind::TrunkNew)
+    ));
+    assert!(matches!(
+        second_metadata.guardian_session_kind,
+        Some(codex_analytics::GuardianReviewSessionKind::TrunkReused)
+    ));
+    assert!(matches!(
+        third_metadata.guardian_session_kind,
+        Some(codex_analytics::GuardianReviewSessionKind::TrunkReused)
+    ));
+    ThreadId::from_string(
+        first_metadata
+            .guardian_thread_id
+            .as_deref()
+            .expect("first guardian thread id"),
+    )
+    .expect("first guardian thread id should be a valid UUID");
+    ThreadId::from_string(
+        second_metadata
+            .guardian_thread_id
+            .as_deref()
+            .expect("second guardian thread id"),
+    )
+    .expect("second guardian thread id should be a valid UUID");
+    ThreadId::from_string(
+        third_metadata
+            .guardian_thread_id
+            .as_deref()
+            .expect("third guardian thread id"),
+    )
+    .expect("third guardian thread id should be a valid UUID");
+    assert_eq!(first_metadata.had_prior_review_context, Some(false));
+    assert_eq!(second_metadata.had_prior_review_context, Some(true));
+    assert_eq!(third_metadata.had_prior_review_context, Some(true));
+    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);