openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-20 17:38:45 +00:00
Code Issues Packages Projects Releases Wiki Activity

Distinguish unsupported evidence in Decisions comparison metrics (#54057)

Browse Source 
## Why

Unsupported evidence currently shares a single Decisions comparison reason, making it difficult to distinguish why a comparison was skipped.

## What changed

Report separate reasons for unsupported developer messages, other non-user messages, trusted skills, images, and audio. Keep these outcomes classified as `skipped` and retain the generic reason for other unsupported evidence. Expose `TrustedSkills::KIND` for identifying trusted skill fragments.

## Testing

Expand adapter tests for the distinct rejection reasons and add a lifecycle case verifying that trusted skill evidence skips the Decisions request, reports `trusted_skills`, and preserves the baseline score.

GitOrigin-RevId: 98f076d1aa6771d0209e7f5910eed0dcc40c6263
This commit is contained in:
swastik-oai
2026-10-14 17:57:29 +00:00
committed by copyberry
parent 80ad49e3b7
commit 1f7fe9b0af
5 changed files with 102 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/ext/guardian-v2/src/async_scorer/decisions.rs
+23 -4
View File
@@ -7,6 +7,7 @@
use super::sampler::LunaSampler;
use super::sampler::LunaSamplingRequest;
use codex_context_fragments::RenderedFragment;
use codex_guardian_context::TrustedSkills;
use codex_guardian_context::TrustedTool;
use codex_history::ResponseItemEnvelope;
use codex_http_client::HttpClient;
@@ -44,6 +45,16 @@ pub(super) enum DecisionsError {
ClientSetup,
#[error("Decisions cannot represent the complete classifier evidence")]
UnsupportedEvidence,
#[error("Decisions cannot represent this developer message")]
UnsupportedDeveloperMessage,
#[error("Decisions cannot represent this non-user message")]
UnsupportedNonUserMessage,
#[error("Decisions cannot represent trusted skill evidence")]
UnsupportedTrustedSkills,
#[error("Decisions cannot represent this image evidence")]
UnsupportedImage,
#[error("Decisions cannot represent audio evidence")]
UnsupportedAudio,
#[error("Decisions input exceeds the request limit")]
InputTooLarge,
#[error("Decisions request timed out")]
@@ -247,7 +258,11 @@ fn request_body(
continue;
}
("user", _, _) => {}
_ => return Err(DecisionsError::UnsupportedEvidence),
("developer", _, Some([kind])) if kind.0 == TrustedSkills::KIND => {
return Err(DecisionsError::UnsupportedTrustedSkills);
}
("developer", _, _) => return Err(DecisionsError::UnsupportedDeveloperMessage),
_ => return Err(DecisionsError::UnsupportedNonUserMessage),
}
let mut parts = Vec::new();
for part in content {
@@ -268,9 +283,13 @@ fn request_body(
json!({"type": "input_image", "image_url": image_url, "detail": null}),
);
}
ContentItem::InputImage { .. }
| ContentItem::InputAudio { .. }
| ContentItem::OutputText { .. } => {
ContentItem::InputImage { .. } => {
return Err(DecisionsError::UnsupportedImage);
}
ContentItem::InputAudio { .. } => {
return Err(DecisionsError::UnsupportedAudio);
}
ContentItem::OutputText { .. } => {
return Err(DecisionsError::UnsupportedEvidence);
}
}
codex-rs/ext/guardian-v2/src/async_scorer/decisions_lifecycle_tests.rs
+11 -2
View File
@@ -13,6 +13,7 @@ async fn decisions_disagreement_failures_and_disabled_flag_preserve_baseline() -
("cannot_escalate", true, 200, "high", "low", 0.0),
("cannot_lower", true, 200, "low", "high", 1.0),
("candidate_failure", true, 403, "low", "low", 0.0),
("unsupported_skills", true, 200, "low", "low", 0.0),
("baseline_failure", true, 200, "low", "invalid", 1.0),
] {
let server = responses::start_mock_server().await;
@@ -69,7 +70,9 @@ async fn decisions_disagreement_failures_and_disabled_flag_preserve_baseline() -
"choice":choice}]
})),
)
.expect(/*r*/ u64::from(enabled && case != "not_initialized"))
.expect(/*r*/ u64::from(
enabled && !matches!(case, "not_initialized" | "unsupported_skills"),
))
.mount(&decisions_server)
.await;
// Inject only the transport, using the existing mock HTTP server instead of credentials.
@@ -84,6 +87,11 @@ async fn decisions_disagreement_failures_and_disabled_flag_preserve_baseline() -
} else {
assert!(store.get::<DecisionsSampler>().is_none());
}
if case == "unsupported_skills" {
store
.get_or_init(codex_core::context::GuardianReviewEvidence::default)
.record_trusted_skill("turn-1", "/skills/example/SKILL.md".into());
}
metrics.0.lock().unwrap().clear();
fixture.score_tool(ToolName::plain("read_file")).await;
let score = tokio::time::timeout(ASYNC_TEST_TIMEOUT, async {
@@ -135,6 +143,7 @@ async fn decisions_disagreement_failures_and_disabled_flag_preserve_baseline() -
}
let (outcome, reason) = match case {
"not_initialized" => ("skipped", "not_initialized"),
"unsupported_skills" => ("skipped", "trusted_skills"),
"candidate_failure" => ("failure", "http_403"),
_ => ("success", "none"),
};
@@ -153,7 +162,7 @@ async fn decisions_disagreement_failures_and_disabled_flag_preserve_baseline() -
)],
"{case}"
);
if case == "not_initialized" {
if matches!(case, "not_initialized" | "unsupported_skills") {
let comparisons = metrics.0.lock().unwrap().iter().filter(|sample| matches!(sample,
RecordedMetric::Counter(name, _, _) if name == "codex.guardian_v2.decisions_comparison.comparison"))
.cloned().collect::<Vec<_>>();
codex-rs/ext/guardian-v2/src/async_scorer/decisions_tests.rs
+48 -10
View File
@@ -41,21 +41,46 @@ fn adapter_rejects_unsupported_evidence() {
&[user.clone(), trusted],
/*parent_compaction*/ None
),
Err(DecisionsError::UnsupportedEvidence)
Err(DecisionsError::UnsupportedDeveloperMessage)
);
for role in ["assistant", "system", "unknown"] {
let item = serde_json::from_value(json!({
"type":"message", "role":role,
"content":[{"type":"input_text", "text":"evidence"}]
}))
.unwrap();
assert_eq!(
request_body(&rubric, &[item], /*parent_compaction*/ None),
Err(DecisionsError::UnsupportedNonUserMessage)
);
}
assert_eq!(
request_body(&rubric, std::slice::from_ref(&user), Some(&user)),
Err(DecisionsError::UnsupportedEvidence)
);
let image: ResponseItem =
serde_json::from_value(json!({"type":"message", "role":"user", "content":[
{"type":"input_image", "image_url":"https://example.com/image.png"}
]}))
for (content, expected_error) in [
(
json!({"type":"input_image", "image_url":"https://example.com/image.png"}),
DecisionsError::UnsupportedImage,
),
(
json!({"type":"input_audio", "audio_url":"data:audio/wav;base64,AA=="}),
DecisionsError::UnsupportedAudio,
),
(
json!({"type":"output_text", "text":"text"}),
DecisionsError::UnsupportedEvidence,
),
] {
let evidence: ResponseItem = serde_json::from_value(json!({
"type":"message", "role":"user", "content":[content]
}))
.unwrap();
assert_eq!(
request_body(&rubric, &[image], /*parent_compaction*/ None),
Err(DecisionsError::UnsupportedEvidence)
);
assert_eq!(
request_body(&rubric, &[evidence], /*parent_compaction*/ None),
Err(expected_error)
);
}
}
#[test]
@@ -107,6 +132,10 @@ fn adapter_preserves_trusted_tool_authority_and_scope() {
"internal_chat_message_metadata_passthrough",
json!({"content_item_kinds": [TrustedTool::KIND, TrustedTool::KIND]}),
),
(
"internal_chat_message_metadata_passthrough",
json!({"content_item_kinds": [TrustedTool::KIND, TrustedSkills::KIND]}),
),
(
"content",
json!([
@@ -119,12 +148,21 @@ fn adapter_preserves_trusted_tool_authority_and_scope() {
json!([{"type": "input_image", "image_url": "data:image/png;base64,AA=="}]),
),
] {
let expected_error = if field == "internal_chat_message_metadata_passthrough"
&& replacement["content_item_kinds"][0] == TrustedSkills::KIND
{
DecisionsError::UnsupportedTrustedSkills
} else if field == "role" {
DecisionsError::UnsupportedNonUserMessage
} else {
DecisionsError::UnsupportedDeveloperMessage
};
let mut unsupported = trusted.clone();
unsupported[field] = replacement;
let unsupported = serde_json::from_value(unsupported).unwrap();
assert_eq!(
request_body(&rubric, &[unsupported], /*parent_compaction*/ None),
Err(DecisionsError::UnsupportedEvidence)
Err(expected_error)
);
}
}
codex-rs/ext/guardian-v2/src/async_scorer/metrics.rs
+14 -3
View File
@@ -187,6 +187,11 @@ pub(super) fn decisions_failure_reason(error: DecisionsError) -> &'static str {
DecisionsError::Credentials => "credentials_unavailable",
DecisionsError::ClientSetup => "request_build_error",
DecisionsError::UnsupportedEvidence => "decisions_unsupported_evidence",
DecisionsError::UnsupportedDeveloperMessage => "developer_message",
DecisionsError::UnsupportedNonUserMessage => "non_user_message",
DecisionsError::UnsupportedTrustedSkills => "trusted_skills",
DecisionsError::UnsupportedImage => "image",
DecisionsError::UnsupportedAudio => "audio",
DecisionsError::InputTooLarge => "input_too_large",
DecisionsError::Timeout => "transport_timeout",
// The adapter erases transport details; do not claim a specific network failure.
@@ -214,9 +219,15 @@ pub(super) fn record_decisions_comparison(
Ok((result, duration)) => {
let outcome = match &result {
Ok(_) => "success",
Err(DecisionsError::UnsupportedEvidence | DecisionsError::InputTooLarge) => {
"skipped"
}
Err(
DecisionsError::UnsupportedEvidence
| DecisionsError::UnsupportedDeveloperMessage
| DecisionsError::UnsupportedNonUserMessage
| DecisionsError::UnsupportedTrustedSkills
| DecisionsError::UnsupportedImage
| DecisionsError::UnsupportedAudio
| DecisionsError::InputTooLarge,
) => "skipped",
Err(_) => "failure",
};
record_decisions_comparison_outcome(
codex-rs/guardian-context/src/trusted_skills.rs
+6 -1
View File
@@ -20,9 +20,14 @@ pub struct TrustedSkills {
pub paths: Vec<String>,
}
impl TrustedSkills {
/// Classification attached to this fragment's rendered text.
pub const KIND: &str = "guardian.trusted_skills";
}
impl ContextualUserFragment for TrustedSkills {
fn content_kind(&self) -> ContentItemKind {
ContentItemKind("guardian.trusted_skills".to_owned())
ContentItemKind(Self::KIND.to_owned())
}
fn role(&self) -> &'static str {