From 517f5faed3b9063826cca888bedfdb787501bcfb Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Sun, 1 Mar 2026 13:46:48 -0500 Subject: [PATCH] Add alias normalization for FailureClass parsing LLM-authored output can set failure_class to non-canonical strings like "retryable", "transient", or "permanent". Expand FromStr to accept 30+ aliases with case-insensitive trimmed matching, matching Kilroy's normalizedFailureClass(). Unknown values fail-closed to Deterministic instead of returning Err. Co-Authored-By: Claude Opus 4.6 --- crates/arc-workflows/src/engine.rs | 11 +-- crates/arc-workflows/src/error.rs | 142 ++++++++++++++++++++++++++--- 2 files changed, 136 insertions(+), 17 deletions(-) diff --git a/crates/arc-workflows/src/engine.rs b/crates/arc-workflows/src/engine.rs index 536b67a54..ac111f9ca 100644 --- a/crates/arc-workflows/src/engine.rs +++ b/crates/arc-workflows/src/engine.rs @@ -44,9 +44,8 @@ fn classify_outcome(outcome: &Outcome) -> Option { // Check handler hint in context_updates if let Some(hint) = outcome.context_updates.get("failure_class") { if let Some(s) = hint.as_str() { - if let Ok(fc) = s.parse::() { - return Some(fc); - } + let fc: FailureClass = s.parse().unwrap(); + return Some(fc); } } @@ -3328,16 +3327,16 @@ mod tests { } #[test] - fn classify_outcome_ignores_invalid_handler_hint() { + fn classify_outcome_unknown_hint_defaults_to_deterministic() { let mut outcome = Outcome::fail("timeout occurred"); outcome.context_updates.insert( "failure_class".to_string(), serde_json::json!("not_a_valid_class"), ); - // Falls through to string heuristics on failure_reason + // Unknown hint normalizes to Deterministic (fail-closed), taking priority over heuristics assert_eq!( classify_outcome(&outcome), - Some(FailureClass::TransientInfra) + Some(FailureClass::Deterministic) ); } diff --git a/crates/arc-workflows/src/error.rs b/crates/arc-workflows/src/error.rs index 37e39332b..f9311b056 100644 --- a/crates/arc-workflows/src/error.rs +++ b/crates/arc-workflows/src/error.rs @@ -41,18 +41,47 @@ impl fmt::Display for FailureClass { } impl FromStr for FailureClass { - type Err = String; + type Err = std::convert::Infallible; fn from_str(s: &str) -> std::result::Result { - match s { - "transient_infra" => Ok(Self::TransientInfra), - "deterministic" => Ok(Self::Deterministic), - "budget_exhausted" => Ok(Self::BudgetExhausted), - "compilation_loop" => Ok(Self::CompilationLoop), - "canceled" => Ok(Self::Canceled), - "structural" => Ok(Self::Structural), - other => Err(format!("unknown failure class: {other}")), - } + let normalized = s.trim().to_lowercase(); + Ok(match normalized.as_str() { + // Canonical names + "transient_infra" => Self::TransientInfra, + "deterministic" => Self::Deterministic, + "budget_exhausted" => Self::BudgetExhausted, + "compilation_loop" => Self::CompilationLoop, + "canceled" => Self::Canceled, + "structural" => Self::Structural, + + // Aliases: transient_infra + "transient" | "transient-infra" | "infra_transient" | "transient infra" + | "infrastructure_transient" | "retryable" | "toolchain_workspace_io" + | "toolchain-workspace-io" | "toolchain_or_dependency_registry_unavailable" + | "toolchain-dependency-registry-unavailable" => Self::TransientInfra, + + // Aliases: deterministic + "non_transient" | "non-transient" | "permanent" | "logic" | "product" => { + Self::Deterministic + } + + // Aliases: canceled + "cancelled" => Self::Canceled, + + // Aliases: budget_exhausted + "budget-exhausted" | "budget exhausted" | "budget" => Self::BudgetExhausted, + + // Aliases: compilation_loop + "compilation-loop" | "compilation loop" | "compile_loop" | "compile-loop" => { + Self::CompilationLoop + } + + // Aliases: structural + "structure" | "scope_violation" | "write_scope_violation" => Self::Structural, + + // Unknown → fail-closed to Deterministic + _ => Self::Deterministic, + }) } } @@ -390,7 +419,98 @@ mod tests { #[test] fn failure_class_from_str_invalid() { - assert!("unknown".parse::().is_err()); + assert_eq!( + "unknown".parse::().unwrap(), + FailureClass::Deterministic + ); + } + + #[test] + fn failure_class_from_str_alias_retryable() { + assert_eq!( + "retryable".parse::().unwrap(), + FailureClass::TransientInfra + ); + } + + #[test] + fn failure_class_from_str_alias_transient() { + assert_eq!( + "transient".parse::().unwrap(), + FailureClass::TransientInfra + ); + } + + #[test] + fn failure_class_from_str_alias_permanent() { + assert_eq!( + "permanent".parse::().unwrap(), + FailureClass::Deterministic + ); + } + + #[test] + fn failure_class_from_str_alias_cancelled_british() { + assert_eq!( + "cancelled".parse::().unwrap(), + FailureClass::Canceled + ); + } + + #[test] + fn failure_class_from_str_alias_budget() { + assert_eq!( + "budget".parse::().unwrap(), + FailureClass::BudgetExhausted + ); + } + + #[test] + fn failure_class_from_str_alias_compile_loop() { + assert_eq!( + "compile_loop".parse::().unwrap(), + FailureClass::CompilationLoop + ); + } + + #[test] + fn failure_class_from_str_alias_scope_violation() { + assert_eq!( + "scope_violation".parse::().unwrap(), + FailureClass::Structural + ); + } + + #[test] + fn failure_class_from_str_unknown_defaults_deterministic() { + assert_eq!( + "garbage_xyz".parse::().unwrap(), + FailureClass::Deterministic + ); + } + + #[test] + fn failure_class_from_str_case_insensitive() { + assert_eq!( + "TRANSIENT_INFRA".parse::().unwrap(), + FailureClass::TransientInfra + ); + } + + #[test] + fn failure_class_from_str_trims_whitespace() { + assert_eq!( + " transient_infra ".parse::().unwrap(), + FailureClass::TransientInfra + ); + } + + #[test] + fn failure_class_from_str_empty_defaults_deterministic() { + assert_eq!( + "".parse::().unwrap(), + FailureClass::Deterministic + ); } #[test]