Require assessment text in the Guardian parser (#52065)

All callers already check that an assessment payload is present before parsing it. Change `parse_guardian_assessment` to accept `&str` instead of `Option<&str>` and remove its redundant missing-payload check.

Update the callers and existing parser tests to pass text directly.

GitOrigin-RevId: e4fb4bf7aca5b20f25f5b27300ce9a6f22c97939
This commit is contained in:
ioanbudea-OAI
2026-10-08 11:01:28 +00:00
committed by copyberry
parent 48bd7667ca
commit be2e129ecd
5 changed files with 10 additions and 15 deletions
@@ -310,9 +310,9 @@ impl ReviewerRequest for PreparedReview {
// may follow a valid allow before publication; new guidance during capture must
// invalidate the attempt without overwriting the last committed review.
if let GuardianReviewSessionOutcome::Completed(Ok(Some(message))) = &result.outcome
&& codex_guardian_reviewer::parse_guardian_assessment(Some(message)).is_ok_and(
|assessment| assessment.outcome == super::super::GuardianAssessmentOutcome::Allow,
)
&& codex_guardian_reviewer::parse_guardian_assessment(message).is_ok_and(|assessment| {
assessment.outcome == super::super::GuardianAssessmentOutcome::Allow
})
&& let Some(authorization) = &self.params.authorization
&& let Some(outcome) = authorization
.invalidation(
@@ -21,10 +21,7 @@ pub struct GuardianAssessment {
/// wrapper so transient formatting drift fails less noisily during dogfooding.
/// Non-JSON output is still a review failure; this is only a thin recovery path
/// for cases where the model wrapped the JSON in extra prose.
pub fn parse_guardian_assessment(text: Option<&str>) -> anyhow::Result<GuardianAssessment> {
let Some(text) = text else {
anyhow::bail!("guardian review completed without an assessment payload");
};
pub fn parse_guardian_assessment(text: &str) -> anyhow::Result<GuardianAssessment> {
let parsed_payload =
if let Ok(payload) = serde_json::from_str::<GuardianAssessmentPayload>(text) {
payload
@@ -3,9 +3,9 @@ use pretty_assertions::assert_eq;
#[test]
fn parse_guardian_assessment_extracts_embedded_json() {
let parsed = parse_guardian_assessment(Some(
let parsed = parse_guardian_assessment(
"preface {\"risk_level\":\"medium\",\"user_authorization\":\"low\",\"outcome\":\"allow\",\"rationale\":\"ok\"}",
))
)
.expect("guardian assessment");
assert_eq!(
@@ -21,8 +21,7 @@ fn parse_guardian_assessment_extracts_embedded_json() {
#[test]
fn parse_guardian_assessment_treats_bare_allow_as_low_risk() {
let parsed =
parse_guardian_assessment(Some(r#"{"outcome":"allow"}"#)).expect("guardian assessment");
let parsed = parse_guardian_assessment(r#"{"outcome":"allow"}"#).expect("guardian assessment");
assert_eq!(
parsed,
@@ -37,8 +36,7 @@ fn parse_guardian_assessment_treats_bare_allow_as_low_risk() {
#[test]
fn parse_guardian_assessment_treats_bare_deny_as_high_risk() {
let parsed =
parse_guardian_assessment(Some(r#"{"outcome":"deny"}"#)).expect("guardian assessment");
let parsed = parse_guardian_assessment(r#"{"outcome":"deny"}"#).expect("guardian assessment");
assert_eq!(
parsed,
@@ -63,7 +63,7 @@ impl<'a> FailedReviewFeedback<'a> {
}
let (status, decision) = match outcome {
GuardianReviewSessionOutcome::Completed(Ok(Some(decision))) => {
match parse_guardian_assessment(Some(decision)) {
match parse_guardian_assessment(decision) {
Ok(assessment) if assessment.outcome == GuardianAssessmentOutcome::Allow => {
return None;
}
@@ -96,7 +96,7 @@ impl From<GuardianReviewSessionOutcome> for GuardianReviewOutcome {
fn from(outcome: GuardianReviewSessionOutcome) -> Self {
match outcome {
GuardianReviewSessionOutcome::Completed(Ok(Some(message))) => {
match crate::parse_guardian_assessment(Some(&message)) {
match crate::parse_guardian_assessment(&message) {
Ok(assessment) => Self::Completed(assessment),
Err(error) => Self::Error(GuardianReviewError::parse(error)),
}