github/codex
4
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-20 00:18:49 +00:00
Code Issues Packages Projects Releases Wiki Activity

Record Guardian context mode in review and classification telemetry (#49294)

Browse Source 
## Why

Guardian review and classification telemetry lacks the context mode needed to distinguish `legacy`, `independent`, and `thread_owned` reviews.

## What changed

- Add `guardian_context_mode` to review and classification analytics and `context_mode` to their metrics.
- Capture the mode from the conversation history snapshot before review preparation or classification setup so failure metrics retain it, including when review preparation or reviewer startup fails.
- Use `unknown` for review metric tags when no context mode is available.

## Testing

Update metric assertions to cover `thread_owned` classification tags and the `unknown` review fallback.

GitOrigin-RevId: 79f7d11b3e4ccfc53681890d75fbc7c54e1ce7de
This commit is contained in:
felixxia-oai
2026-09-29 16:09:48 +00:00
committed by copyberry
parent 4773a132c3
commit d79a95bdf8
10 changed files with 58 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/analytics/src/events.rs
+4
View File
@@ -400,6 +400,7 @@ pub enum GuardianReviewedAction {
#[derive(Clone, Serialize)]
pub struct GuardianReviewEventParams {
pub guardian_context_mode: Option<&'static str>,
pub thread_id: String,
pub turn_id: String,
pub review_id: String,
@@ -479,6 +480,7 @@ impl GuardianReviewTrackContext {
completed_at_ms: u64,
) -> GuardianReviewEventParams {
GuardianReviewEventParams {
guardian_context_mode: result.guardian_context_mode,
thread_id: self.thread_id.clone(),
turn_id: self.turn_id.clone(),
review_id: self.review_id.clone(),
@@ -531,6 +533,7 @@ impl GuardianReviewTrackContext {
#[derive(Debug)]
pub struct GuardianReviewAnalyticsResult {
pub guardian_context_mode: Option<&'static str>,
pub decision: GuardianReviewDecision,
pub terminal_status: GuardianReviewTerminalStatus,
pub failure_reason: Option<GuardianReviewFailureReason>,
@@ -556,6 +559,7 @@ pub struct GuardianReviewAnalyticsResult {
impl GuardianReviewAnalyticsResult {
pub fn without_session() -> Self {
Self {
guardian_context_mode: None,
decision: GuardianReviewDecision::Denied,
terminal_status: GuardianReviewTerminalStatus::FailedClosed,
failure_reason: None,
codex-rs/analytics/src/guardian_v2.rs
+1
View File
@@ -21,6 +21,7 @@ pub struct GuardianV2Event {
#[serde(untagged)]
pub enum GuardianV2EventKind {
Classification {
guardian_context_mode: &'static str,
outcome: &'static str,
risk_level: Option<&'static str>,
duration_ms: u64,
codex-rs/analytics/src/tests/suite/approvals_tests.rs
+1
View File
@@ -292,6 +292,7 @@ async fn guardian_review_event_ingests_custom_fact_with_optional_target_item() {
.ingest(
AnalyticsFact::Custom(CustomAnalyticsFact::GuardianReview(Box::new(
GuardianReviewEventParams {
guardian_context_mode: None,
thread_id: "thread-guardian".to_string(),
turn_id: "turn-guardian".to_string(),
review_id: "review-guardian".to_string(),
codex-rs/core/src/context/guardian_context_mode.rs
+9
View File
@@ -15,6 +15,15 @@ pub enum GuardianContextMode {
}
impl GuardianContextMode {
/// Stable, bounded label for the context actually selected for a review.
pub fn as_str(self) -> &'static str {
match self {
Self::Legacy => "legacy",
Self::Independent => "independent",
Self::ThreadOwned => "thread_owned",
}
}
/// Read reviewer policy from the same snapshot as its evidence, including delayed reviews.
pub fn from_history(history: &dyn ConversationHistorySnapshot) -> Self {
if history.uses_independent_review_history() {
codex-rs/core/src/guardian/review_session_setup.rs
+11 -2
View File
@@ -207,13 +207,22 @@ pub(crate) async fn run_guardian_review_session(
pool: Arc<ReviewerPool<GuardianReviewSession>>,
params: GuardianReviewSessionParams,
) -> (GuardianReviewSessionOutcome, GuardianReviewAnalyticsResult) {
match prepare_review(params).await {
let context_mode = GuardianContextMode::from_history(
params
.parent_history
.conversation_history_snapshot()
.as_ref(),
);
let (outcome, mut analytics) = match prepare_review(params).await {
Ok(prepared) => pool.review(prepared).await,
Err(error) => (
GuardianReviewSessionOutcome::PromptBuildFailed(error),
GuardianReviewAnalyticsResult::without_session(),
),
}
};
// Keep the captured mode even when preparation or reviewer startup fails.
analytics.guardian_context_mode = Some(context_mode.as_str());
(outcome, analytics)
}
pub(super) async fn prepare_review(
codex-rs/ext/guardian-reviewer/src/metrics.rs
+8
View File
@@ -106,6 +106,13 @@ fn guardian_review_metric_tags(
reviewed_action: &GuardianReviewedAction,
) -> Vec<(&'static str, String)> {
vec![
(
"context_mode",
result
.guardian_context_mode
.unwrap_or("unknown")
.to_string(),
),
("decision", decision_tag(result.decision).to_string()),
(
"terminal_status",
@@ -436,6 +443,7 @@ mod tests {
let (attrs, value) = counter_point(&snapshot, GUARDIAN_REVIEW_COUNT_METRIC);
let expected_base_tags = BTreeMap::from([
("context_mode".to_string(), "unknown".to_string()),
("action".to_string(), expected_action.to_string()),
(
"approval_request_source".to_string(),
codex-rs/ext/guardian-v2/src/async_scorer/classification.rs
+3
View File
@@ -211,6 +211,7 @@ impl Classification {
score_progress.fail_closed(sampled_at);
record_classification(
metrics.as_deref(),
context_mode,
classification_started_at.elapsed(),
"failure",
Some("context_build_error"),
@@ -364,6 +365,7 @@ impl Classification {
};
record_classification(
metrics.as_deref(),
context_mode,
duration,
outcome,
result.is_err().then_some(failure_reason),
@@ -376,6 +378,7 @@ impl Classification {
model: parent_model.as_ref().map(|model| model.slug.clone()),
occurred_at_ms: codex_analytics::now_unix_millis(),
kind: GuardianV2EventKind::Classification {
guardian_context_mode: context_mode.as_str(),
outcome,
risk_level: classification_risk,
duration_ms: u64::try_from(duration.as_millis()).unwrap_or(u64::MAX),
codex-rs/ext/guardian-v2/src/async_scorer/extension_tests.rs
+8 -2
View File
@@ -1675,12 +1675,18 @@ max_recent_non_user_entries = 8
RecordedMetric::Counter(
CLASSIFICATION_METRIC.to_owned(),
1,
vec![("outcome".to_owned(), "success".to_owned())],
vec![
("outcome".to_owned(), "success".to_owned()),
("context_mode".to_owned(), "thread_owned".to_owned())
],
),
RecordedMetric::Histogram(
CLASSIFICATION_DURATION_METRIC.to_owned(),
classification_duration_ms,
vec![("outcome".to_owned(), "success".to_owned())],
vec![
("outcome".to_owned(), "success".to_owned()),
("context_mode".to_owned(), "thread_owned".to_owned())
],
),
])
.chain([
codex-rs/ext/guardian-v2/src/async_scorer/metrics.rs
+6 -1
View File
@@ -4,6 +4,7 @@ use std::time::Duration;
use codex_api::ApiError;
use codex_api::TransportError;
use codex_core::context::GuardianContextMode;
use codex_extension_api::ExtensionMetrics;
use super::sampler::LunaSamplerError;
@@ -61,6 +62,7 @@ pub(super) fn sampler_failure_reason(error: &LunaSamplerError) -> &'static str {
pub(super) fn record_classification(
metrics: Option<&dyn ExtensionMetrics>,
context_mode: GuardianContextMode,
duration: Duration,
outcome: &str,
failure_reason: Option<&str>,
@@ -68,7 +70,10 @@ pub(super) fn record_classification(
let Some(metrics) = metrics else {
return;
};
let mut tags = vec![("outcome", outcome)];
let mut tags = vec![
("outcome", outcome),
("context_mode", context_mode.as_str()),
];
if let Some(reason) = failure_reason {
tags.push(("failure_reason", reason));
}
codex-rs/ext/guardian-v2/src/async_scorer/observation.rs
+7 -1
View File
@@ -118,6 +118,7 @@ impl GuardianV2Extension {
score_progress.observe_js_execution();
}
let metrics = score_progress.metrics.clone();
let context_mode = GuardianContextMode::from_history(input.conversation_history.as_ref());
let analytics = input.session_store.get::<AnalyticsEventsClient>();
let sampled_at = SystemTime::now();
let tool_call_index = score_progress.observe(&input);
@@ -150,6 +151,7 @@ impl GuardianV2Extension {
score_progress.invalidate(tool_call_index);
record_classification(
metrics.as_deref(),
context_mode,
classification_started_at.elapsed(),
"failure",
Some("thread_context_error"),
@@ -194,6 +196,7 @@ impl GuardianV2Extension {
score_progress.fail_closed(sampled_at);
record_classification(
metrics.as_deref(),
context_mode,
classification_started_at.elapsed(),
"failure",
Some("configuration_error"),
@@ -216,7 +219,6 @@ impl GuardianV2Extension {
let guardian_evidence = input
.thread_store
.get_or_init(GuardianReviewEvidence::default);
let context_mode = GuardianContextMode::from_history(input.conversation_history.as_ref());
let selected_compaction = match select_parent_compaction(
context_mode,
&guardian_config,
@@ -237,6 +239,7 @@ impl GuardianV2Extension {
score_progress.fail_closed(sampled_at);
record_classification(
metrics.as_deref(),
context_mode,
classification_started_at.elapsed(),
outcome,
failure_reason,
@@ -259,6 +262,7 @@ impl GuardianV2Extension {
score_progress.fail_closed(sampled_at);
record_classification(
metrics.as_deref(),
context_mode,
classification_started_at.elapsed(),
"failure",
Some("input_too_large"),
@@ -270,6 +274,7 @@ impl GuardianV2Extension {
score_progress.fail_closed(sampled_at);
record_classification(
metrics.as_deref(),
context_mode,
classification_started_at.elapsed(),
"failure",
Some("action_serialization_error"),
@@ -314,6 +319,7 @@ impl GuardianV2Extension {
score_progress.fail_closed(sampled_at);
record_classification(
metrics.as_deref(),
context_mode,
classification_started_at.elapsed(),
"failure",
Some("permission_resolution_error"),