From f7e2391535d15dedc209bc9f20b5192cdc65903b Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Tue, 24 Mar 2026 09:24:36 -0400 Subject: [PATCH] Unify Outcome types between fabro-core and fabro-workflows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make fabro-core's Outcome generic over a usage/metadata type parameter (OutcomeMeta trait), allowing fabro-workflows to use core's types directly via a type alias instead of maintaining duplicate Outcome, StageStatus, and FailureDetail types with bidirectional conversions. Key changes: - Add FailureCategory enum to fabro-core (moved from fabro-workflows' FailureClass), with Display/FromStr/is_signature_tracked - Add OutcomeMeta supertrait + blanket impl for the generic parameter - Make Outcome, NodeResult, RunState, NodeDecision generic with default type parameter M=() - Add Graph::Meta associated type - Update FailureDetail with serde renames (category→"failure_class", signature→"failure_signature") for checkpoint backward compat - Replace fabro-workflows' Outcome with type alias to fabro_core::Outcome> - Add OutcomeExt extension trait for wf-specific factory methods (fail_classify, fail_deterministic, retry_classify, simulated, etc.) - Delete core_adapter/outcome.rs (~170 lines of conversion functions) - Replace FailureClass with FailureCategory throughout fabro-workflows - Fix timeout handler to use TransientInfra category, panic handler to use Deterministic category Net: -144 lines, zero-cost type unification with no runtime conversions. Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/crates/fabro-cli/src/commands/run.rs | 2 +- lib/crates/fabro-core/src/error.rs | 15 +- lib/crates/fabro-core/src/executor.rs | 10 +- lib/crates/fabro-core/src/graph.rs | 7 +- lib/crates/fabro-core/src/handler.rs | 13 +- lib/crates/fabro-core/src/lib.rs | 2 +- lib/crates/fabro-core/src/lifecycle.rs | 71 +-- lib/crates/fabro-core/src/outcome.rs | 301 +++++++++-- lib/crates/fabro-core/src/state.rs | 28 +- lib/crates/fabro-core/src/test_fixtures.rs | 1 + lib/crates/fabro-workflows/src/checkpoint.rs | 13 +- .../fabro-workflows/src/core_adapter/graph.rs | 18 +- .../src/core_adapter/handler.rs | 35 +- .../src/core_adapter/lifecycle.rs | 131 +++-- .../fabro-workflows/src/core_adapter/mod.rs | 1 - .../src/core_adapter/outcome.rs | 173 ------- lib/crates/fabro-workflows/src/engine.rs | 55 +- lib/crates/fabro-workflows/src/error.rs | 477 ++++++++---------- lib/crates/fabro-workflows/src/event.rs | 16 +- .../fabro-workflows/src/handler/agent.rs | 4 +- .../fabro-workflows/src/handler/command.rs | 2 +- .../fabro-workflows/src/handler/fan_in.rs | 2 +- .../fabro-workflows/src/handler/human.rs | 2 +- .../src/handler/manager_loop.rs | 2 +- lib/crates/fabro-workflows/src/handler/mod.rs | 2 +- .../fabro-workflows/src/handler/parallel.rs | 4 +- lib/crates/fabro-workflows/src/lib.rs | 2 +- lib/crates/fabro-workflows/src/outcome.rs | 284 +++-------- lib/crates/fabro-workflows/src/preamble.rs | 1 + .../tests/daytona_integration.rs | 2 +- .../fabro-workflows/tests/integration.rs | 34 +- 31 files changed, 783 insertions(+), 927 deletions(-) delete mode 100644 lib/crates/fabro-workflows/src/core_adapter/outcome.rs diff --git a/lib/crates/fabro-cli/src/commands/run.rs b/lib/crates/fabro-cli/src/commands/run.rs index 404ace8bb..b5002a068 100644 --- a/lib/crates/fabro-cli/src/commands/run.rs +++ b/lib/crates/fabro-cli/src/commands/run.rs @@ -26,7 +26,7 @@ use fabro_workflows::event::{EventEmitter, RunNoticeLevel, WorkflowRunEvent}; use fabro_workflows::git::GitSyncStatus; use fabro_workflows::handler::default_registry; use fabro_workflows::manifest::Manifest; -use fabro_workflows::outcome::{Outcome, StageStatus}; +use fabro_workflows::outcome::{Outcome, OutcomeExt, StageStatus}; use fabro_workflows::run_status::{RunStatus, StatusReason}; use fabro_workflows::sandbox_provider::SandboxProvider; use fabro_workflows::workflow::WorkflowBuilder; diff --git a/lib/crates/fabro-core/src/error.rs b/lib/crates/fabro-core/src/error.rs index 61bf7b25c..07146daa3 100644 --- a/lib/crates/fabro-core/src/error.rs +++ b/lib/crates/fabro-core/src/error.rs @@ -1,6 +1,6 @@ use std::fmt; -use crate::outcome::{FailureDetail, Outcome, StageStatus}; +use crate::outcome::{FailureCategory, FailureDetail, Outcome, OutcomeMeta, StageStatus}; /// Structured failure data on handler errors. Maps to FabroError's /// is_retryable(), failure_class(), failure_signature_hint(), to_fail_outcome(). @@ -8,7 +8,7 @@ use crate::outcome::{FailureDetail, Outcome, StageStatus}; pub struct HandlerErrorDetail { pub message: String, pub retryable: bool, - pub category: Option, + pub category: Option, pub signature: Option, } @@ -57,13 +57,13 @@ impl CoreError { matches!(self, Self::Handler { detail } if detail.retryable) } - pub fn to_fail_outcome(&self) -> Outcome { + pub fn to_fail_outcome(&self) -> Outcome { match self { Self::Handler { detail } => Outcome { status: StageStatus::Fail, failure: Some(FailureDetail { message: detail.message.clone(), - category: detail.category.clone(), + category: detail.category.unwrap_or(FailureCategory::Deterministic), signature: detail.signature.clone(), }), ..Outcome::default() @@ -140,17 +140,18 @@ mod tests { #[test] fn core_error_handler_to_fail_outcome() { + use crate::outcome::FailureCategory; let err = CoreError::handler(HandlerErrorDetail { message: "api down".into(), retryable: true, - category: Some("transient".into()), + category: Some(FailureCategory::TransientInfra), signature: Some("sig123".into()), }); - let outcome = err.to_fail_outcome(); + let outcome: crate::outcome::Outcome = err.to_fail_outcome(); assert_eq!(outcome.status, StageStatus::Fail); let failure = outcome.failure.unwrap(); assert_eq!(failure.message, "api down"); - assert_eq!(failure.category.as_deref(), Some("transient")); + assert_eq!(failure.category, FailureCategory::TransientInfra); assert_eq!(failure.signature.as_deref(), Some("sig123")); } diff --git a/lib/crates/fabro-core/src/executor.rs b/lib/crates/fabro-core/src/executor.rs index 7e9a6e6c5..ea2774c84 100644 --- a/lib/crates/fabro-core/src/executor.rs +++ b/lib/crates/fabro-core/src/executor.rs @@ -80,7 +80,7 @@ impl ExecutorBuilder { } impl Executor { - pub async fn run(&self, graph: &G, mut state: RunState) -> Result { + pub async fn run(&self, graph: &G, mut state: RunState) -> Result> { self.lifecycle.on_run_start(graph, &state).await?; loop { @@ -229,9 +229,9 @@ impl Executor { async fn execute_with_retry( &self, node: &G::Node, - state: &RunState, + state: &RunState, graph: &G, - ) -> Result { + ) -> Result> { let policy = self.handler.retry_policy(node, graph); let start = Instant::now(); @@ -330,8 +330,8 @@ impl Executor { async fn resolve_next_step( &self, node: &G::Node, - outcome: &Outcome, - state: &RunState, + outcome: &Outcome, + state: &RunState, graph: &G, ) -> Result { // Jump takes priority diff --git a/lib/crates/fabro-core/src/graph.rs b/lib/crates/fabro-core/src/graph.rs index 19502f1bb..0cb1744be 100644 --- a/lib/crates/fabro-core/src/graph.rs +++ b/lib/crates/fabro-core/src/graph.rs @@ -2,7 +2,7 @@ use std::collections::HashMap; use crate::context::Context; use crate::error::Result; -use crate::outcome::Outcome; +use crate::outcome::{Outcome, OutcomeMeta}; pub trait NodeSpec: Send + Sync + Clone { fn id(&self) -> &str; @@ -24,6 +24,7 @@ pub struct EdgeSelection { pub trait Graph: Send + Sync { type Node: NodeSpec + Clone; type Edge: EdgeSpec + Clone; + type Meta: OutcomeMeta; fn get_node(&self, id: &str) -> Option; fn find_start_node(&self) -> Result; @@ -31,12 +32,12 @@ pub trait Graph: Send + Sync { fn select_edge( &self, node: &Self::Node, - outcome: &Outcome, + outcome: &Outcome, context: &Context, ) -> Option>; fn check_goal_gates( &self, - outcomes: &HashMap, + outcomes: &HashMap>, ) -> std::result::Result<(), String>; fn get_retry_target(&self, failed_node_id: &str) -> Option; } diff --git a/lib/crates/fabro-core/src/handler.rs b/lib/crates/fabro-core/src/handler.rs index ecad071eb..e0a371adb 100644 --- a/lib/crates/fabro-core/src/handler.rs +++ b/lib/crates/fabro-core/src/handler.rs @@ -8,13 +8,22 @@ use crate::retry::RetryPolicy; #[async_trait] pub trait NodeHandler: Send + Sync { - async fn execute(&self, node: &G::Node, context: &Context, graph: &G) -> Result; + async fn execute( + &self, + node: &G::Node, + context: &Context, + graph: &G, + ) -> Result>; fn retry_policy(&self, _node: &G::Node, _graph: &G) -> RetryPolicy { RetryPolicy::none() } - fn on_retries_exhausted(&self, _node: &G::Node, _last_outcome: Outcome) -> Outcome { + fn on_retries_exhausted( + &self, + _node: &G::Node, + _last_outcome: Outcome, + ) -> Outcome { Outcome::fail("max retries exceeded") } } diff --git a/lib/crates/fabro-core/src/lib.rs b/lib/crates/fabro-core/src/lib.rs index 0c288b87f..70c227e4d 100644 --- a/lib/crates/fabro-core/src/lib.rs +++ b/lib/crates/fabro-core/src/lib.rs @@ -21,7 +21,7 @@ pub use lifecycle::{ AttemptContext, AttemptResultContext, CompositeLifecycle, EdgeContext, EdgeDecision, NodeDecision, NoopLifecycle, RunLifecycle, }; -pub use outcome::{FailureDetail, NodeResult, Outcome, StageStatus}; +pub use outcome::{FailureCategory, FailureDetail, NodeResult, Outcome, OutcomeMeta, StageStatus}; pub use retry::{BackoffPolicy, RetryPolicy}; pub use stall::{ActivityMonitor, StallGuard, StallWatchdog}; pub use state::RunState; diff --git a/lib/crates/fabro-core/src/lifecycle.rs b/lib/crates/fabro-core/src/lifecycle.rs index 71fff12da..8ee859e6e 100644 --- a/lib/crates/fabro-core/src/lifecycle.rs +++ b/lib/crates/fabro-core/src/lifecycle.rs @@ -4,13 +4,13 @@ use async_trait::async_trait; use crate::error::Result; use crate::graph::Graph; -use crate::outcome::{NodeResult, Outcome}; +use crate::outcome::{NodeResult, Outcome, OutcomeMeta}; use crate::state::RunState; #[derive(Debug, Clone)] -pub enum NodeDecision { +pub enum NodeDecision { Continue, - Skip(Box), + Skip(Box>), Block(String), } @@ -29,7 +29,7 @@ pub struct AttemptContext<'a, G: Graph> { pub struct AttemptResultContext<'a, G: Graph> { pub node: &'a G::Node, - pub result: &'a NodeResult, + pub result: &'a NodeResult, pub attempt: u32, pub will_retry: bool, pub backoff_delay: Option, @@ -40,13 +40,13 @@ pub struct EdgeContext<'a, G: Graph> { pub to: &'a str, pub edge: Option, pub is_jump: bool, - pub outcome: &'a Outcome, + pub outcome: &'a Outcome, pub reason: &'a str, } #[async_trait] pub trait RunLifecycle: Send + Sync { - async fn on_run_start(&self, _graph: &G, _state: &RunState) -> Result<()> { + async fn on_run_start(&self, _graph: &G, _state: &RunState) -> Result<()> { Ok(()) } @@ -54,26 +54,30 @@ pub trait RunLifecycle: Send + Sync { &self, _node: &G::Node, _goal_gates_passed: bool, - _state: &RunState, + _state: &RunState, ) { } - async fn before_node(&self, _node: &G::Node, _state: &RunState) -> Result { + async fn before_node( + &self, + _node: &G::Node, + _state: &RunState, + ) -> Result> { Ok(NodeDecision::Continue) } async fn before_attempt( &self, _ctx: &AttemptContext<'_, G>, - _state: &RunState, - ) -> Result { + _state: &RunState, + ) -> Result> { Ok(NodeDecision::Continue) } async fn after_attempt( &self, _ctx: &AttemptResultContext<'_, G>, - _state: &RunState, + _state: &RunState, ) -> Result<()> { Ok(()) } @@ -81,8 +85,8 @@ pub trait RunLifecycle: Send + Sync { async fn after_node( &self, _node: &G::Node, - _result: &mut NodeResult, - _state: &RunState, + _result: &mut NodeResult, + _state: &RunState, ) -> Result<()> { Ok(()) } @@ -90,7 +94,7 @@ pub trait RunLifecycle: Send + Sync { async fn on_edge_selected( &self, _ctx: &EdgeContext<'_, G>, - _state: &RunState, + _state: &RunState, ) -> Result { Ok(EdgeDecision::Continue) } @@ -98,14 +102,14 @@ pub trait RunLifecycle: Send + Sync { async fn on_checkpoint( &self, _node: &G::Node, - _result: &NodeResult, + _result: &NodeResult, _next_node_id: Option<&str>, - _state: &RunState, + _state: &RunState, ) -> Result<()> { Ok(()) } - async fn on_run_end(&self, _outcome: &Outcome, _state: &RunState) {} + async fn on_run_end(&self, _outcome: &Outcome, _state: &RunState) {} } /// No-op lifecycle that passes through everything. @@ -128,14 +132,19 @@ impl CompositeLifecycle { #[async_trait] impl RunLifecycle for CompositeLifecycle { - async fn on_run_start(&self, graph: &G, state: &RunState) -> Result<()> { + async fn on_run_start(&self, graph: &G, state: &RunState) -> Result<()> { for child in &self.children { child.on_run_start(graph, state).await?; } Ok(()) } - async fn on_terminal_reached(&self, node: &G::Node, goal_gates_passed: bool, state: &RunState) { + async fn on_terminal_reached( + &self, + node: &G::Node, + goal_gates_passed: bool, + state: &RunState, + ) { for child in &self.children { child .on_terminal_reached(node, goal_gates_passed, state) @@ -143,7 +152,11 @@ impl RunLifecycle for CompositeLifecycle { } } - async fn before_node(&self, node: &G::Node, state: &RunState) -> Result { + async fn before_node( + &self, + node: &G::Node, + state: &RunState, + ) -> Result> { for child in &self.children { match child.before_node(node, state).await? { NodeDecision::Continue => {} @@ -156,8 +169,8 @@ impl RunLifecycle for CompositeLifecycle { async fn before_attempt( &self, ctx: &AttemptContext<'_, G>, - state: &RunState, - ) -> Result { + state: &RunState, + ) -> Result> { for child in &self.children { match child.before_attempt(ctx, state).await? { NodeDecision::Continue => {} @@ -170,7 +183,7 @@ impl RunLifecycle for CompositeLifecycle { async fn after_attempt( &self, ctx: &AttemptResultContext<'_, G>, - state: &RunState, + state: &RunState, ) -> Result<()> { for child in &self.children { child.after_attempt(ctx, state).await?; @@ -181,8 +194,8 @@ impl RunLifecycle for CompositeLifecycle { async fn after_node( &self, node: &G::Node, - result: &mut NodeResult, - state: &RunState, + result: &mut NodeResult, + state: &RunState, ) -> Result<()> { for child in &self.children { child.after_node(node, result, state).await?; @@ -193,7 +206,7 @@ impl RunLifecycle for CompositeLifecycle { async fn on_edge_selected( &self, ctx: &EdgeContext<'_, G>, - state: &RunState, + state: &RunState, ) -> Result { for child in &self.children { match child.on_edge_selected(ctx, state).await? { @@ -207,9 +220,9 @@ impl RunLifecycle for CompositeLifecycle { async fn on_checkpoint( &self, node: &G::Node, - result: &NodeResult, + result: &NodeResult, next_node_id: Option<&str>, - state: &RunState, + state: &RunState, ) -> Result<()> { for child in &self.children { child @@ -219,7 +232,7 @@ impl RunLifecycle for CompositeLifecycle { Ok(()) } - async fn on_run_end(&self, outcome: &Outcome, state: &RunState) { + async fn on_run_end(&self, outcome: &Outcome, state: &RunState) { for child in &self.children { child.on_run_end(outcome, state).await; } diff --git a/lib/crates/fabro-core/src/outcome.rs b/lib/crates/fabro-core/src/outcome.rs index 4df16780d..400cddd60 100644 --- a/lib/crates/fabro-core/src/outcome.rs +++ b/lib/crates/fabro-core/src/outcome.rs @@ -3,9 +3,21 @@ use std::fmt; use std::str::FromStr; use std::time::Duration; +use serde::de::DeserializeOwned; use serde::{Deserialize, Serialize}; use serde_json::Value; +/// Supertrait for the generic usage/metadata type parameter on `Outcome`. +pub trait OutcomeMeta: + Default + Clone + Send + Sync + fmt::Debug + Serialize + DeserializeOwned + 'static +{ +} + +impl OutcomeMeta for T where + T: Default + Clone + Send + Sync + fmt::Debug + Serialize + DeserializeOwned + 'static +{ +} + #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] pub enum StageStatus { @@ -43,26 +55,149 @@ impl FromStr for StageStatus { } } -#[derive(Debug, Clone)] +/// Classification of failure modes. +/// +/// Pipeline authors can write edge conditions like `context.failure_class=budget_exhausted` +/// to route execution based on the nature of the failure. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum FailureCategory { + /// Temporary infrastructure failure (rate limit, timeout, network, 5xx). + TransientInfra, + /// Permanent failure (auth, bad config, code bug). + Deterministic, + /// Context length, token/turn limit, quota exceeded. + BudgetExhausted, + /// Reserved for future loop detection. + CompilationLoop, + /// User/system cancellation. + Canceled, + /// Reserved for future scope enforcement. + Structural, +} + +impl fmt::Display for FailureCategory { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let s = match self { + Self::TransientInfra => "transient_infra", + Self::Deterministic => "deterministic", + Self::BudgetExhausted => "budget_exhausted", + Self::CompilationLoop => "compilation_loop", + Self::Canceled => "canceled", + Self::Structural => "structural", + }; + write!(f, "{s}") + } +} + +impl FromStr for FailureCategory { + type Err = std::convert::Infallible; + + fn from_str(s: &str) -> std::result::Result { + 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, + }) + } +} + +impl FailureCategory { + /// Whether this failure category should be tracked by the cycle breaker. + pub fn is_signature_tracked(self) -> bool { + matches!(self, Self::Deterministic | Self::Structural) + } +} + +/// Structured failure information. +#[derive(Debug, Clone, Serialize, Deserialize)] pub struct FailureDetail { pub message: String, - pub category: Option, + #[serde(rename = "failure_class")] + pub category: FailureCategory, + #[serde( + rename = "failure_signature", + default, + skip_serializing_if = "Option::is_none" + )] pub signature: Option, } -#[derive(Debug, Clone)] -pub struct Outcome { - pub status: StageStatus, - pub preferred_label: Option, - pub suggested_next_ids: Vec, - pub context_updates: HashMap, - pub jump_to_node: Option, - pub notes: Option, - pub failure: Option, - pub metadata: HashMap, +impl FailureDetail { + pub fn new(message: impl Into, category: FailureCategory) -> Self { + Self { + message: message.into(), + category, + signature: None, + } + } } -impl Default for Outcome { +/// The result of executing a node handler. +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(bound = "M: OutcomeMeta")] +pub struct Outcome { + pub status: StageStatus, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub preferred_label: Option, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub suggested_next_ids: Vec, + #[serde(default, skip_serializing_if = "HashMap::is_empty")] + pub context_updates: HashMap, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub jump_to_node: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub notes: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub failure: Option, + #[serde(default)] + pub usage: M, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub files_touched: Vec, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub duration_ms: Option, +} + +impl Default for Outcome { fn default() -> Self { Self { status: StageStatus::Success, @@ -72,12 +207,14 @@ impl Default for Outcome { jump_to_node: None, notes: None, failure: None, - metadata: HashMap::new(), + usage: M::default(), + files_touched: Vec::new(), + duration_ms: None, } } } -impl Outcome { +impl Outcome { pub fn success() -> Self { Self::default() } @@ -87,7 +224,7 @@ impl Outcome { status: StageStatus::Fail, failure: Some(FailureDetail { message: message.to_string(), - category: None, + category: FailureCategory::Deterministic, signature: None, }), ..Self::default() @@ -104,15 +241,15 @@ impl Outcome { } #[derive(Debug, Clone)] -pub struct NodeResult { - pub outcome: Outcome, +pub struct NodeResult { + pub outcome: Outcome, pub duration: Duration, pub attempts: u32, pub max_attempts: u32, } -impl NodeResult { - pub fn new(outcome: Outcome, duration: Duration, attempts: u32, max_attempts: u32) -> Self { +impl NodeResult { + pub fn new(outcome: Outcome, duration: Duration, attempts: u32, max_attempts: u32) -> Self { Self { outcome, duration, @@ -135,7 +272,7 @@ impl NodeResult { } } - pub fn from_skip(outcome: Outcome) -> Self { + pub fn from_skip(outcome: Outcome) -> Self { Self { outcome, duration: Duration::ZERO, @@ -181,9 +318,55 @@ mod tests { } } + #[test] + fn failure_category_display_roundtrip() { + let categories = [ + FailureCategory::TransientInfra, + FailureCategory::Deterministic, + FailureCategory::BudgetExhausted, + FailureCategory::CompilationLoop, + FailureCategory::Canceled, + FailureCategory::Structural, + ]; + for cat in &categories { + let s = cat.to_string(); + let parsed: FailureCategory = s.parse().unwrap(); + assert_eq!(&parsed, cat); + } + } + + #[test] + fn failure_category_aliases() { + assert_eq!( + "transient".parse::().unwrap(), + FailureCategory::TransientInfra + ); + assert_eq!( + "cancelled".parse::().unwrap(), + FailureCategory::Canceled + ); + assert_eq!( + "permanent".parse::().unwrap(), + FailureCategory::Deterministic + ); + assert_eq!( + "budget".parse::().unwrap(), + FailureCategory::BudgetExhausted + ); + } + + #[test] + fn failure_category_is_signature_tracked() { + assert!(FailureCategory::Deterministic.is_signature_tracked()); + assert!(FailureCategory::Structural.is_signature_tracked()); + assert!(!FailureCategory::TransientInfra.is_signature_tracked()); + assert!(!FailureCategory::BudgetExhausted.is_signature_tracked()); + assert!(!FailureCategory::Canceled.is_signature_tracked()); + } + #[test] fn outcome_success_factory() { - let o = Outcome::success(); + let o: Outcome = Outcome::success(); assert_eq!(o.status, StageStatus::Success); assert!(o.failure.is_none()); assert!(o.notes.is_none()); @@ -191,24 +374,24 @@ mod tests { #[test] fn outcome_fail_factory() { - let o = Outcome::fail("broken"); + let o: Outcome = Outcome::fail("broken"); assert_eq!(o.status, StageStatus::Fail); let f = o.failure.unwrap(); assert_eq!(f.message, "broken"); - assert!(f.category.is_none()); + assert_eq!(f.category, FailureCategory::Deterministic); assert!(f.signature.is_none()); } #[test] fn outcome_skipped_factory() { - let o = Outcome::skipped("not needed"); + let o: Outcome = Outcome::skipped("not needed"); assert_eq!(o.status, StageStatus::Skipped); assert_eq!(o.notes.as_deref(), Some("not needed")); } #[test] fn outcome_with_context_updates() { - let mut o = Outcome::success(); + let mut o: Outcome = Outcome::success(); o.context_updates .insert("key".into(), serde_json::json!("value")); assert_eq!(o.context_updates["key"], serde_json::json!("value")); @@ -216,37 +399,71 @@ mod tests { #[test] fn outcome_with_jump() { - let mut o = Outcome::success(); + let mut o: Outcome = Outcome::success(); o.jump_to_node = Some("target".into()); assert_eq!(o.jump_to_node.as_deref(), Some("target")); } #[test] fn outcome_serde_roundtrip() { - // Test that metadata (the serde-friendly field) roundtrips - let mut o = Outcome::success(); - o.metadata - .insert("usage".into(), serde_json::json!({"tokens": 100})); - let json = serde_json::to_value(&o.metadata).unwrap(); - let parsed: HashMap = serde_json::from_value(json).unwrap(); - assert_eq!(parsed["usage"]["tokens"], 100); + let mut o: Outcome = Outcome::success(); + o.notes = Some("done".to_string()); + o.context_updates + .insert("key".into(), serde_json::json!("val")); + let json = serde_json::to_string(&o).unwrap(); + let parsed: Outcome = serde_json::from_str(&json).unwrap(); + assert_eq!(parsed.status, StageStatus::Success); + assert_eq!(parsed.notes.as_deref(), Some("done")); + assert_eq!( + parsed.context_updates.get("key"), + Some(&serde_json::json!("val")) + ); + } + + #[test] + fn outcome_deserialize_without_usage_key() { + // Old checkpoints may not have "usage" key — serde(default) handles this + let json = r#"{"status":"success"}"#; + let o: Outcome = serde_json::from_str(json).unwrap(); + assert_eq!(o.status, StageStatus::Success); } #[test] fn failure_detail_construction() { - let f = FailureDetail { - message: "timeout".into(), - category: Some("transient".into()), - signature: Some("sig".into()), - }; + let f = FailureDetail::new("timeout", FailureCategory::TransientInfra); assert_eq!(f.message, "timeout"); - assert_eq!(f.category.as_deref(), Some("transient")); - assert_eq!(f.signature.as_deref(), Some("sig")); + assert_eq!(f.category, FailureCategory::TransientInfra); + assert!(f.signature.is_none()); + } + + #[test] + fn failure_detail_serde_uses_renamed_keys() { + let f = FailureDetail::new("timeout", FailureCategory::TransientInfra); + let json = serde_json::to_string(&f).unwrap(); + // category serializes as "failure_class" + assert!(json.contains("\"failure_class\"")); + assert!(!json.contains("\"category\"")); + // signature omitted when None + assert!(!json.contains("failure_signature")); + } + + #[test] + fn failure_detail_serde_roundtrip() { + let f = FailureDetail { + message: "api down".into(), + category: FailureCategory::TransientInfra, + signature: Some("sig123".into()), + }; + let json = serde_json::to_string(&f).unwrap(); + let parsed: FailureDetail = serde_json::from_str(&json).unwrap(); + assert_eq!(parsed.message, "api down"); + assert_eq!(parsed.category, FailureCategory::TransientInfra); + assert_eq!(parsed.signature.as_deref(), Some("sig123")); } #[test] fn node_result_from_outcome() { - let o = Outcome::success(); + let o: Outcome = Outcome::success(); let r = NodeResult::new(o, Duration::from_millis(100), 1, 3); assert_eq!(r.outcome.status, StageStatus::Success); assert_eq!(r.duration, Duration::from_millis(100)); diff --git a/lib/crates/fabro-core/src/state.rs b/lib/crates/fabro-core/src/state.rs index ac0de7471..cd2f1e8d3 100644 --- a/lib/crates/fabro-core/src/state.rs +++ b/lib/crates/fabro-core/src/state.rs @@ -3,13 +3,13 @@ use std::collections::HashMap; use crate::context::Context; use crate::error::Result; use crate::graph::{Graph, NodeSpec}; -use crate::outcome::{NodeResult, Outcome}; +use crate::outcome::{NodeResult, Outcome, OutcomeMeta}; -pub struct RunState { +pub struct RunState { pub context: Context, pub current_node_id: String, pub completed_nodes: Vec, - pub node_outcomes: HashMap, + pub node_outcomes: HashMap>, pub node_retries: HashMap, pub node_visits: HashMap, pub stage_index: usize, @@ -17,7 +17,7 @@ pub struct RunState { pub cancelled: bool, } -impl RunState { +impl RunState { pub fn new(graph: &G) -> Result { let start = graph.find_start_node()?; Ok(Self { @@ -33,7 +33,7 @@ impl RunState { }) } - pub fn record(&mut self, node_id: &str, result: &NodeResult) { + pub fn record(&mut self, node_id: &str, result: &NodeResult) { self.completed_nodes.push(node_id.to_string()); self.node_outcomes .insert(node_id.to_string(), result.outcome.clone()); @@ -87,7 +87,7 @@ mod tests { #[test] fn run_state_new_from_graph() { let g = linear_graph(&["start", "work", "end"]); - let state = RunState::new(&g).unwrap(); + let state = RunState::<()>::new(&g).unwrap(); assert_eq!(state.current_node_id, "start"); assert!(state.completed_nodes.is_empty()); assert!(state.node_outcomes.is_empty()); @@ -98,7 +98,7 @@ mod tests { #[test] fn run_state_record_updates_all_fields() { let g = linear_graph(&["start", "end"]); - let mut state = RunState::new(&g).unwrap(); + let mut state = RunState::<()>::new(&g).unwrap(); let result = NodeResult::new(Outcome::success(), Duration::from_millis(50), 2, 3); state.record("start", &result); @@ -111,7 +111,7 @@ mod tests { #[test] fn run_state_record_applies_context_updates() { let g = linear_graph(&["start", "end"]); - let mut state = RunState::new(&g).unwrap(); + let mut state = RunState::<()>::new(&g).unwrap(); let mut outcome = Outcome::success(); outcome.context_updates.insert("key".into(), json!("value")); let result = NodeResult::new(outcome, Duration::ZERO, 1, 1); @@ -122,7 +122,7 @@ mod tests { #[test] fn run_state_advance_updates_current_and_previous() { let g = linear_graph(&["start", "mid", "end"]); - let mut state = RunState::new(&g).unwrap(); + let mut state = RunState::<()>::new(&g).unwrap(); assert_eq!(state.current_node_id, "start"); assert!(state.previous_node_id.is_none()); @@ -138,7 +138,7 @@ mod tests { #[test] fn run_state_restart_clears_progress_keeps_visits() { let g = linear_graph(&["start", "work", "end"]); - let mut state = RunState::new(&g).unwrap(); + let mut state = RunState::<()>::new(&g).unwrap(); state.increment_visits("start"); state.increment_visits("work"); state.record( @@ -163,7 +163,7 @@ mod tests { #[test] fn run_state_current_node_from_graph() { let g = linear_graph(&["start", "end"]); - let state = RunState::new(&g).unwrap(); + let state = RunState::<()>::new(&g).unwrap(); let node = state.current_node(&g).unwrap(); assert_eq!(node.id(), "start"); } @@ -171,7 +171,7 @@ mod tests { #[test] fn run_state_increment_visits() { let g = linear_graph(&["start", "end"]); - let mut state = RunState::new(&g).unwrap(); + let mut state = RunState::<()>::new(&g).unwrap(); assert_eq!(state.increment_visits("start"), 1); assert_eq!(state.increment_visits("start"), 2); assert_eq!(state.increment_visits("other"), 1); @@ -180,7 +180,7 @@ mod tests { #[test] fn run_state_restart_with_new_context() { let g = linear_graph(&["start", "end"]); - let mut state = RunState::new(&g).unwrap(); + let mut state = RunState::<()>::new(&g).unwrap(); state.context.set("key", json!("old_value")); state.increment_visits("start"); @@ -199,7 +199,7 @@ mod tests { #[test] fn run_state_restart_without_context_preserves() { let g = linear_graph(&["start", "end"]); - let mut state = RunState::new(&g).unwrap(); + let mut state = RunState::<()>::new(&g).unwrap(); state.context.set("key", json!("value")); state.restart("start", None); diff --git a/lib/crates/fabro-core/src/test_fixtures.rs b/lib/crates/fabro-core/src/test_fixtures.rs index 6fcf0f675..79ef70b5b 100644 --- a/lib/crates/fabro-core/src/test_fixtures.rs +++ b/lib/crates/fabro-core/src/test_fixtures.rs @@ -139,6 +139,7 @@ impl TestGraph { impl Graph for TestGraph { type Node = TestNode; type Edge = TestEdge; + type Meta = (); fn get_node(&self, id: &str) -> Option { self.nodes.iter().find(|n| n.id == id).cloned() diff --git a/lib/crates/fabro-workflows/src/checkpoint.rs b/lib/crates/fabro-workflows/src/checkpoint.rs index c4d741712..f35464047 100644 --- a/lib/crates/fabro-workflows/src/checkpoint.rs +++ b/lib/crates/fabro-workflows/src/checkpoint.rs @@ -207,7 +207,7 @@ mod tests { #[test] fn signature_maps_roundtrip() { - use crate::error::{FailureClass, FailureSignature}; + use crate::error::{FailureCategory, FailureSignature}; let dir = tempfile::tempdir().unwrap(); let path = dir.path().join("checkpoint.json"); @@ -217,7 +217,7 @@ mod tests { loop_sigs.insert( FailureSignature::new( "verify", - FailureClass::Deterministic, + FailureCategory::Deterministic, None, Some("test failed"), ), @@ -225,7 +225,12 @@ mod tests { ); let mut restart_sigs = HashMap::new(); restart_sigs.insert( - FailureSignature::new("build", FailureClass::Structural, None, Some("scope error")), + FailureSignature::new( + "build", + FailureCategory::Structural, + None, + Some("scope error"), + ), 1, ); @@ -247,7 +252,7 @@ mod tests { assert_eq!(loaded.restart_failure_signatures.len(), 1); let sig = FailureSignature::new( "verify", - FailureClass::Deterministic, + FailureCategory::Deterministic, None, Some("test failed"), ); diff --git a/lib/crates/fabro-workflows/src/core_adapter/graph.rs b/lib/crates/fabro-workflows/src/core_adapter/graph.rs index 448debc41..dcc02c7a7 100644 --- a/lib/crates/fabro-workflows/src/core_adapter/graph.rs +++ b/lib/crates/fabro-workflows/src/core_adapter/graph.rs @@ -4,10 +4,10 @@ use std::sync::Arc; use fabro_core::context::Context as CoreContext; use fabro_core::error::{CoreError, Result as CoreResult}; use fabro_core::graph::{EdgeSelection, EdgeSpec, Graph, NodeSpec}; -use fabro_core::outcome::Outcome as CoreOutcome; use fabro_graphviz::graph::types::{Edge as GvEdge, Graph as GvGraph, Node as GvNode}; use crate::engine; +use crate::outcome::{Outcome, StageUsage}; // ---- WorkflowNode ---- @@ -73,6 +73,7 @@ impl WorkflowGraph { impl Graph for WorkflowGraph { type Node = WorkflowNode; type Edge = WorkflowEdge; + type Meta = Option; fn get_node(&self, id: &str) -> Option { self.0 @@ -99,15 +100,14 @@ impl Graph for WorkflowGraph { fn select_edge( &self, node: &Self::Node, - outcome: &CoreOutcome, + outcome: &Outcome, _context: &CoreContext, ) -> Option> { - // Convert core outcome to workflow outcome for edge selection - let wf_outcome = super::outcome::core_to_wf_outcome(outcome); + // Outcome is now the wf type directly — no conversion needed let wf_context = crate::context::Context::new(); let selection = engine::select_edge( node.inner(), - &wf_outcome, + outcome, &wf_context, self.inner(), node.inner().selection(), @@ -120,13 +120,9 @@ impl Graph for WorkflowGraph { fn check_goal_gates( &self, - outcomes: &HashMap, + outcomes: &HashMap, ) -> std::result::Result<(), String> { - let wf_outcomes: HashMap = outcomes - .iter() - .map(|(k, v)| (k.clone(), super::outcome::core_to_wf_outcome(v))) - .collect(); - engine::check_goal_gates(self.inner(), &wf_outcomes) + engine::check_goal_gates(self.inner(), outcomes) } fn get_retry_target(&self, failed_node_id: &str) -> Option { diff --git a/lib/crates/fabro-workflows/src/core_adapter/handler.rs b/lib/crates/fabro-workflows/src/core_adapter/handler.rs index 1f3960653..81e9b37e8 100644 --- a/lib/crates/fabro-workflows/src/core_adapter/handler.rs +++ b/lib/crates/fabro-workflows/src/core_adapter/handler.rs @@ -8,15 +8,14 @@ use futures::FutureExt; use fabro_core::context::Context as CoreContext; use fabro_core::error::{CoreError, HandlerErrorDetail, Result as CoreResult}; use fabro_core::handler::NodeHandler; -use fabro_core::outcome::Outcome as CoreOutcome; +use fabro_core::outcome::FailureCategory; use fabro_core::retry::RetryPolicy as CoreRetryPolicy; use super::graph::WorkflowGraph; -use super::outcome::{wf_to_core_outcome, wf_to_core_status}; use super::WorkflowNode; use crate::engine; use crate::handler::EngineServices; -use crate::outcome::StageStatus as WfStatus; +use crate::outcome::{Outcome, StageStatus}; /// Production node handler that bridges fabro-core's NodeHandler to the /// existing fabro-workflows Handler trait via EngineServices. @@ -32,14 +31,10 @@ impl NodeHandler for WorkflowNodeHandler { node: &WorkflowNode, _context: &CoreContext, _graph: &WorkflowGraph, - ) -> CoreResult { + ) -> CoreResult { let gv_node = node.inner(); let handler = self.services.registry.resolve(gv_node); - // Build a wf context from the core context's state — the lifecycle's - // before_node populates the shared context, so we reconstruct a wf::Context - // that reads from the same store. For now, use the context bridge. - // The actual wf::Context is shared via the bridge set up by the lifecycle. let wf_context = crate::context::Context::new(); let wf_graph = fabro_graphviz::graph::types::Graph::new("stub"); @@ -65,7 +60,7 @@ impl NodeHandler for WorkflowNodeHandler { return Err(CoreError::handler(HandlerErrorDetail { message: format!("handler timed out after {}ms", duration.as_millis()), retryable: true, - category: None, + category: Some(FailureCategory::TransientInfra), signature: None, })); } @@ -75,14 +70,13 @@ impl NodeHandler for WorkflowNodeHandler { }; match timed_result { - Ok(Ok(wf_outcome)) => Ok(wf_to_core_outcome(&wf_outcome)), + Ok(Ok(wf_outcome)) => Ok(wf_outcome), Ok(Err(fabro_err)) => { - // Use the handler's should_retry, not just is_retryable let retryable = handler.should_retry(&fabro_err); Err(CoreError::handler(HandlerErrorDetail { message: fabro_err.to_string(), retryable, - category: Some(fabro_err.failure_class().to_string()), + category: Some(fabro_err.failure_category()), signature: fabro_err.failure_signature_hint(), })) } @@ -97,7 +91,7 @@ impl NodeHandler for WorkflowNodeHandler { Err(CoreError::handler(HandlerErrorDetail { message: msg, retryable: false, - category: None, + category: Some(FailureCategory::Deterministic), signature: None, })) } @@ -114,17 +108,16 @@ impl NodeHandler for WorkflowNodeHandler { } } - fn on_retries_exhausted(&self, node: &WorkflowNode, last_outcome: CoreOutcome) -> CoreOutcome { + fn on_retries_exhausted(&self, node: &WorkflowNode, last_outcome: Outcome) -> Outcome { let gv_node = node.inner(); if gv_node.allow_partial() { - CoreOutcome { - status: fabro_core::outcome::StageStatus::PartialSuccess, + Outcome { + status: StageStatus::PartialSuccess, ..last_outcome } } else { - let status = wf_to_core_status(&WfStatus::Fail); - CoreOutcome { - status, + Outcome { + status: StageStatus::Fail, ..last_outcome } } @@ -155,8 +148,8 @@ mod tests { _node: &WorkflowNode, _context: &CoreContext, _graph: &WorkflowGraph, - ) -> CoreResult { - Ok(CoreOutcome::success()) + ) -> CoreResult { + Ok(Outcome::success()) } fn retry_policy(&self, _node: &WorkflowNode, _graph: &WorkflowGraph) -> CoreRetryPolicy { diff --git a/lib/crates/fabro-workflows/src/core_adapter/lifecycle.rs b/lib/crates/fabro-workflows/src/core_adapter/lifecycle.rs index 026761014..f0ccd2b60 100644 --- a/lib/crates/fabro-workflows/src/core_adapter/lifecycle.rs +++ b/lib/crates/fabro-workflows/src/core_adapter/lifecycle.rs @@ -10,17 +10,16 @@ use fabro_core::graph::NodeSpec; use fabro_core::lifecycle::{ AttemptContext, AttemptResultContext, EdgeContext, EdgeDecision, NodeDecision, RunLifecycle, }; -use fabro_core::outcome::{NodeResult, Outcome as CoreOutcome}; +use fabro_core::outcome::NodeResult; use fabro_core::state::RunState; use super::graph::WorkflowGraph; -use super::outcome::{core_to_wf_outcome, core_to_wf_status}; use super::WorkflowNode; use crate::checkpoint::Checkpoint; use crate::context::keys; -use crate::error::{FailureClass, FailureSignature}; +use crate::error::{FailureCategory, FailureSignature}; use crate::event::{EventEmitter, WorkflowRunEvent}; -use crate::outcome::StageStatus as WfStatus; +use crate::outcome::{FailureDetail, Outcome, StageStatus, StageUsage}; use fabro_hooks::{HookContext, HookDecision, HookEvent, HookRunner}; use fabro_sandbox::Sandbox; @@ -102,9 +101,13 @@ impl WorkflowLifecycle { } } +type WfRunState = RunState>; +type WfNodeResult = NodeResult>; +type WfNodeDecision = NodeDecision>; + #[async_trait] impl RunLifecycle for WorkflowLifecycle { - async fn on_run_start(&self, _graph: &WorkflowGraph, _state: &RunState) -> CoreResult<()> { + async fn on_run_start(&self, _graph: &WorkflowGraph, _state: &WfRunState) -> CoreResult<()> { // Clear incoming edge data (reset stale fidelity/thread from prior iteration) *self.incoming_edge_data.lock().unwrap() = None; @@ -137,7 +140,7 @@ impl RunLifecycle for WorkflowLifecycle { &self, node: &WorkflowNode, goal_gates_passed: bool, - state: &RunState, + state: &WfRunState, ) { if !goal_gates_passed { return; @@ -171,13 +174,16 @@ impl RunLifecycle for WorkflowLifecycle { }); } - async fn before_node(&self, node: &WorkflowNode, state: &RunState) -> CoreResult { + async fn before_node( + &self, + node: &WorkflowNode, + state: &WfRunState, + ) -> CoreResult { // Resolve fidelity from incoming edge data let incoming = self.incoming_edge_data.lock().unwrap().take(); let gv_node = node.inner(); // Set context keys for the current node - // Note: This operates on state.context which is the core context bridged to wf context let visits = state.node_visits.get(node.id()).copied().unwrap_or(0); state .context @@ -220,8 +226,8 @@ impl RunLifecycle for WorkflowLifecycle { async fn before_attempt( &self, ctx: &AttemptContext<'_, WorkflowGraph>, - state: &RunState, - ) -> CoreResult { + state: &WfRunState, + ) -> CoreResult { let gv = ctx.node.inner(); let stage_index = state.stage_index; @@ -235,7 +241,7 @@ impl RunLifecycle for WorkflowLifecycle { match decision { HookDecision::Skip { reason } => { let msg = reason.unwrap_or_else(|| "skipped by hook".into()); - return Ok(NodeDecision::Skip(Box::new(CoreOutcome::skipped(&msg)))); + return Ok(NodeDecision::Skip(Box::new(Outcome::skipped(&msg)))); } HookDecision::Block { reason } => { let msg = reason.unwrap_or_else(|| "blocked by StageStart hook".into()); @@ -261,11 +267,11 @@ impl RunLifecycle for WorkflowLifecycle { async fn after_attempt( &self, ctx: &AttemptResultContext<'_, WorkflowGraph>, - state: &RunState, + state: &WfRunState, ) -> CoreResult<()> { if ctx.will_retry { let gv = ctx.node.inner(); - let wf_outcome = core_to_wf_outcome(&ctx.result.outcome); + let outcome = &ctx.result.outcome; let stage_index = state.stage_index; // Emit StageFailed event @@ -273,11 +279,8 @@ impl RunLifecycle for WorkflowLifecycle { node_id: gv.id.clone(), name: gv.label().to_string(), index: stage_index, - failure: wf_outcome.failure.unwrap_or_else(|| { - crate::outcome::FailureDetail::new( - "handler failed", - FailureClass::TransientInfra, - ) + failure: outcome.failure.clone().unwrap_or_else(|| { + FailureDetail::new("handler failed", FailureCategory::TransientInfra) }), will_retry: true, }); @@ -298,42 +301,40 @@ impl RunLifecycle for WorkflowLifecycle { async fn after_node( &self, node: &WorkflowNode, - result: &mut NodeResult, - state: &RunState, + result: &mut WfNodeResult, + state: &WfRunState, ) -> CoreResult<()> { let gv = node.inner(); let stage_index = state.stage_index; - let mut wf_outcome = core_to_wf_outcome(&result.outcome); + let outcome = &mut result.outcome; // Auto-status override if gv.auto_status() - && wf_outcome.status != WfStatus::Success - && wf_outcome.status != WfStatus::Skipped + && outcome.status != StageStatus::Success + && outcome.status != StageStatus::Skipped { - wf_outcome.status = WfStatus::Success; - wf_outcome.notes = + outcome.status = StageStatus::Success; + outcome.notes = Some("auto-status: handler completed without writing status".to_string()); - result.outcome.status = fabro_core::outcome::StageStatus::Success; - result.outcome.notes = wf_outcome.notes.clone(); } // Circuit breaker: classify + track failure signatures - let outcome_failure_class = if wf_outcome.status == WfStatus::Fail { - wf_outcome.failure.as_ref().map(|f| f.failure_class) + let outcome_failure_category = if outcome.status == StageStatus::Fail { + outcome.failure.as_ref().map(|f| f.category) } else { None }; - if let Some(fc) = outcome_failure_class { - let sig_hint = wf_outcome + if let Some(fc) = outcome_failure_category { + let sig_hint = outcome .failure .as_ref() - .and_then(|f| f.failure_signature.as_deref()); + .and_then(|f| f.signature.as_deref()); let sig = FailureSignature::new( &gv.id, fc, sig_hint, - wf_outcome.failure.as_ref().map(|f| f.message.as_str()), + outcome.failure.as_ref().map(|f| f.message.as_str()), ); if fc.is_signature_tracked() { let mut sigs = self.loop_failure_signatures.lock().unwrap(); @@ -350,16 +351,13 @@ impl RunLifecycle for WorkflowLifecycle { // Emit StageCompleted or StageFailed event let duration_ms = result.duration.as_millis() as u64; - if wf_outcome.status == WfStatus::Fail { + if outcome.status == StageStatus::Fail { self.emitter.emit(&WorkflowRunEvent::StageFailed { node_id: gv.id.clone(), name: gv.label().to_string(), index: stage_index, - failure: wf_outcome.failure.clone().unwrap_or_else(|| { - crate::outcome::FailureDetail::new( - "handler failed", - FailureClass::Deterministic, - ) + failure: outcome.failure.clone().unwrap_or_else(|| { + FailureDetail::new("handler failed", FailureCategory::Deterministic) }), will_retry: false, }); @@ -369,34 +367,34 @@ impl RunLifecycle for WorkflowLifecycle { name: gv.label().to_string(), index: stage_index, duration_ms, - status: wf_outcome.status.to_string(), - preferred_label: wf_outcome.preferred_label.clone(), - suggested_next_ids: wf_outcome.suggested_next_ids.clone(), - usage: wf_outcome.usage.clone(), + status: outcome.status.to_string(), + preferred_label: outcome.preferred_label.clone(), + suggested_next_ids: outcome.suggested_next_ids.clone(), + usage: outcome.usage.clone(), failure: None, - notes: wf_outcome.notes.clone(), - files_touched: wf_outcome.files_touched.clone(), + notes: outcome.notes.clone(), + files_touched: outcome.files_touched.clone(), attempt: result.attempts as usize, max_attempts: result.max_attempts as usize, }); } // StageComplete/StageFailed hook (non-blocking) - let hook_event = if wf_outcome.status == WfStatus::Fail { + let hook_event = if outcome.status == StageStatus::Fail { HookEvent::StageFailed } else { HookEvent::StageComplete }; let mut hook_ctx = HookContext::new(hook_event, self.run_id.clone(), self.graph.name.clone()); - hook_ctx.status = Some(wf_outcome.status.to_string()); + hook_ctx.status = Some(outcome.status.to_string()); let _ = self.run_hook(&hook_ctx).await; // Write node status let status_dir = self.run_dir.join("stages").join(&gv.id); let _ = std::fs::create_dir_all(&status_dir); let status_path = status_dir.join("status.json"); - let _ = crate::save_json(&wf_outcome, &status_path, "node_status"); + let _ = crate::save_json(outcome, &status_path, "node_status"); Ok(()) } @@ -404,7 +402,7 @@ impl RunLifecycle for WorkflowLifecycle { async fn on_edge_selected( &self, ctx: &EdgeContext<'_, WorkflowGraph>, - _state: &RunState, + _state: &WfRunState, ) -> CoreResult { // Capture fidelity/thread from edge for next node if let Some(ref edge) = ctx.edge { @@ -416,8 +414,7 @@ impl RunLifecycle for WorkflowLifecycle { *self.incoming_edge_data.lock().unwrap() = Some(edge_data); } - // Compute outcome-derived fields for EdgeSelected event - let wf_outcome = core_to_wf_outcome(ctx.outcome); + let outcome = ctx.outcome; // Emit EdgeSelected event let label = ctx @@ -434,9 +431,9 @@ impl RunLifecycle for WorkflowLifecycle { label, condition, reason: ctx.reason.to_string(), - preferred_label: wf_outcome.preferred_label.clone(), - suggested_next_ids: wf_outcome.suggested_next_ids.clone(), - stage_status: wf_outcome.status.to_string(), + preferred_label: outcome.preferred_label.clone(), + suggested_next_ids: outcome.suggested_next_ids.clone(), + stage_status: outcome.status.to_string(), is_jump: ctx.is_jump, }); @@ -466,23 +463,18 @@ impl RunLifecycle for WorkflowLifecycle { async fn on_checkpoint( &self, node: &WorkflowNode, - result: &NodeResult, + result: &WfNodeResult, next_node_id: Option<&str>, - state: &RunState, + state: &WfRunState, ) -> CoreResult<()> { if !self.checkpoint_enabled { return Ok(()); } - // Build checkpoint from state - let wf_outcome = core_to_wf_outcome(&result.outcome); - let mut node_outcomes: HashMap = state - .node_outcomes - .iter() - .map(|(k, v)| (k.clone(), core_to_wf_outcome(v))) - .collect(); + // Build checkpoint from state — outcomes are already the wf type + let mut node_outcomes: HashMap = state.node_outcomes.clone(); // Include current node's outcome - node_outcomes.insert(node.id().to_string(), wf_outcome); + node_outcomes.insert(node.id().to_string(), result.outcome.clone()); let checkpoint = Checkpoint { timestamp: chrono::Utc::now(), @@ -508,7 +500,7 @@ impl RunLifecycle for WorkflowLifecycle { } // Emit CheckpointCompleted event - let status = core_to_wf_status(&result.outcome.status).to_string(); + let status = result.outcome.status.to_string(); self.emitter.emit(&WorkflowRunEvent::CheckpointCompleted { node_id: node.id().to_string(), status, @@ -518,21 +510,20 @@ impl RunLifecycle for WorkflowLifecycle { Ok(()) } - async fn on_run_end(&self, outcome: &CoreOutcome, state: &RunState) { + async fn on_run_end(&self, outcome: &Outcome, state: &WfRunState) { // If cancelled, skip all events/hooks if state.cancelled { return; } let duration_ms = self.run_start.elapsed().as_millis() as u64; - let wf_outcome = core_to_wf_outcome(outcome); - if wf_outcome.status == WfStatus::Success || wf_outcome.status == WfStatus::PartialSuccess { + if outcome.status == StageStatus::Success || outcome.status == StageStatus::PartialSuccess { // Success path self.emitter.emit(&WorkflowRunEvent::WorkflowRunCompleted { duration_ms, artifact_count: 0, - status: wf_outcome.status.to_string(), + status: outcome.status.to_string(), total_cost: None, final_git_commit_sha: None, usage: None, @@ -547,7 +538,7 @@ impl RunLifecycle for WorkflowLifecycle { let _ = self.run_hook(&hook_ctx).await; } else { // Failure path - let error_msg = wf_outcome + let error_msg = outcome .failure .as_ref() .map(|f| f.message.clone()) diff --git a/lib/crates/fabro-workflows/src/core_adapter/mod.rs b/lib/crates/fabro-workflows/src/core_adapter/mod.rs index 5c05f2d45..044e33fd6 100644 --- a/lib/crates/fabro-workflows/src/core_adapter/mod.rs +++ b/lib/crates/fabro-workflows/src/core_adapter/mod.rs @@ -2,7 +2,6 @@ pub mod context; pub mod graph; pub mod handler; pub mod lifecycle; -pub mod outcome; pub use context::{bridge_context, WorkflowContextExt}; pub use graph::{WorkflowEdge, WorkflowGraph, WorkflowNode}; diff --git a/lib/crates/fabro-workflows/src/core_adapter/outcome.rs b/lib/crates/fabro-workflows/src/core_adapter/outcome.rs deleted file mode 100644 index 4f07daeaa..000000000 --- a/lib/crates/fabro-workflows/src/core_adapter/outcome.rs +++ /dev/null @@ -1,173 +0,0 @@ -use fabro_core::outcome::{ - FailureDetail as CoreFailureDetail, Outcome as CoreOutcome, StageStatus as CoreStatus, -}; - -use crate::error::{classify_failure_reason, FailureClass}; -use crate::outcome::{ - FailureDetail as WfFailureDetail, Outcome as WfOutcome, StageStatus as WfStatus, -}; - -pub fn wf_to_core_status(s: &WfStatus) -> CoreStatus { - match s { - WfStatus::Success => CoreStatus::Success, - WfStatus::Fail => CoreStatus::Fail, - WfStatus::Skipped => CoreStatus::Skipped, - WfStatus::PartialSuccess => CoreStatus::PartialSuccess, - WfStatus::Retry => CoreStatus::Retry, - } -} - -pub fn core_to_wf_status(s: &CoreStatus) -> WfStatus { - match s { - CoreStatus::Success => WfStatus::Success, - CoreStatus::Fail => WfStatus::Fail, - CoreStatus::Skipped => WfStatus::Skipped, - CoreStatus::PartialSuccess => WfStatus::PartialSuccess, - CoreStatus::Retry => WfStatus::Retry, - } -} - -pub fn wf_to_core_outcome(wf: &WfOutcome) -> CoreOutcome { - CoreOutcome { - status: wf_to_core_status(&wf.status), - preferred_label: wf.preferred_label.clone(), - suggested_next_ids: wf.suggested_next_ids.clone(), - context_updates: wf.context_updates.clone(), - jump_to_node: wf.jump_to_node.clone(), - notes: wf.notes.clone(), - failure: wf.failure.as_ref().map(wf_to_core_failure), - metadata: Default::default(), - } -} - -pub fn core_to_wf_outcome(core: &CoreOutcome) -> WfOutcome { - WfOutcome { - status: core_to_wf_status(&core.status), - preferred_label: core.preferred_label.clone(), - suggested_next_ids: core.suggested_next_ids.clone(), - context_updates: core.context_updates.clone(), - jump_to_node: core.jump_to_node.clone(), - notes: core.notes.clone(), - failure: core.failure.as_ref().map(core_to_wf_failure), - usage: None, - files_touched: Vec::new(), - duration_ms: None, - } -} - -pub fn wf_to_core_failure(wf: &WfFailureDetail) -> CoreFailureDetail { - CoreFailureDetail { - message: wf.message.clone(), - category: Some(wf.failure_class.to_string()), - signature: wf.failure_signature.clone(), - } -} - -pub fn core_to_wf_failure(core: &CoreFailureDetail) -> WfFailureDetail { - let failure_class = core - .category - .as_deref() - .and_then(|c| c.parse::().ok()) - .unwrap_or_else(|| classify_failure_reason(&core.message)); - WfFailureDetail { - message: core.message.clone(), - failure_class, - failure_signature: core.signature.clone(), - } -} - -#[cfg(test)] -mod tests { - use super::*; - use serde_json::json; - - #[test] - fn status_roundtrip_all_variants() { - let wf_statuses = [ - WfStatus::Success, - WfStatus::Fail, - WfStatus::Skipped, - WfStatus::PartialSuccess, - WfStatus::Retry, - ]; - for wf in &wf_statuses { - let core = wf_to_core_status(wf); - let back = core_to_wf_status(&core); - assert_eq!(&back, wf, "roundtrip failed for {:?}", wf); - } - } - - #[test] - fn outcome_roundtrip_success() { - let wf = WfOutcome::success(); - let core = wf_to_core_outcome(&wf); - let back = core_to_wf_outcome(&core); - assert_eq!(back.status, WfStatus::Success); - assert!(back.failure.is_none()); - } - - #[test] - fn outcome_roundtrip_with_shared_fields() { - let mut wf = WfOutcome::success(); - wf.preferred_label = Some("next".into()); - wf.suggested_next_ids = vec!["a".into(), "b".into()]; - wf.context_updates.insert("key".into(), json!("val")); - wf.jump_to_node = Some("target".into()); - wf.notes = Some("hello".into()); - - let core = wf_to_core_outcome(&wf); - assert_eq!(core.preferred_label.as_deref(), Some("next")); - assert_eq!(core.suggested_next_ids, vec!["a", "b"]); - assert_eq!(core.context_updates.get("key"), Some(&json!("val"))); - assert_eq!(core.jump_to_node.as_deref(), Some("target")); - assert_eq!(core.notes.as_deref(), Some("hello")); - - let back = core_to_wf_outcome(&core); - assert_eq!(back.preferred_label, wf.preferred_label); - assert_eq!(back.suggested_next_ids, wf.suggested_next_ids); - assert_eq!(back.context_updates, wf.context_updates); - assert_eq!(back.jump_to_node, wf.jump_to_node); - assert_eq!(back.notes, wf.notes); - } - - #[test] - fn failure_roundtrip() { - let wf_failure = WfFailureDetail { - message: "api down".into(), - failure_class: FailureClass::TransientInfra, - failure_signature: Some("sig123".into()), - }; - let core = wf_to_core_failure(&wf_failure); - assert_eq!(core.message, "api down"); - assert_eq!(core.category.as_deref(), Some("transient_infra")); - assert_eq!(core.signature.as_deref(), Some("sig123")); - - let back = core_to_wf_failure(&core); - assert_eq!(back.message, "api down"); - assert_eq!(back.failure_class, FailureClass::TransientInfra); - assert_eq!(back.failure_signature.as_deref(), Some("sig123")); - } - - #[test] - fn outcome_roundtrip_fail_with_failure() { - let wf = WfOutcome::fail_classify("timeout talking to LLM"); - let core = wf_to_core_outcome(&wf); - let back = core_to_wf_outcome(&core); - assert_eq!(back.status, WfStatus::Fail); - let f = back.failure.unwrap(); - assert_eq!(f.message, "timeout talking to LLM"); - } - - #[test] - fn core_to_wf_failure_classifies_unknown_category() { - let core = CoreFailureDetail { - message: "something broke".into(), - category: None, - signature: None, - }; - let wf = core_to_wf_failure(&core); - // Should fall back to classify_failure_reason - assert_eq!(wf.message, "something broke"); - // The class should be some valid FailureClass (exact value depends on classifier) - } -} diff --git a/lib/crates/fabro-workflows/src/engine.rs b/lib/crates/fabro-workflows/src/engine.rs index db832dec5..2bf92ea28 100644 --- a/lib/crates/fabro-workflows/src/engine.rs +++ b/lib/crates/fabro-workflows/src/engine.rs @@ -20,11 +20,11 @@ use crate::checkpoint::Checkpoint; use crate::condition::evaluate_condition; use crate::context; use crate::context::Context; -use crate::error::{FabroError, FailureClass, FailureSignature, Result}; +use crate::error::{FabroError, FailureCategory, FailureSignature, Result}; use crate::event::{EventEmitter, WorkflowRunEvent}; use crate::handler::{EngineServices, HandlerRegistry}; use crate::millis_u64; -use crate::outcome::{Outcome, StageStatus}; +use crate::outcome::{Outcome, OutcomeExt, StageStatus}; use crate::preamble::build_preamble; use fabro_config::run::PullRequestConfig; use fabro_graphviz::graph::{Edge, Graph, Node}; @@ -46,12 +46,12 @@ pub(crate) fn set_hook_node(ctx: &mut HookContext, node: &Node) { /// 2. String heuristics on `failure_reason` /// 3. Default to `Deterministic` #[must_use] -fn classify_outcome(outcome: &Outcome) -> Option { +fn classify_outcome(outcome: &Outcome) -> Option { match outcome.status { StageStatus::Success | StageStatus::PartialSuccess | StageStatus::Skipped => None, StageStatus::Fail | StageStatus::Retry => outcome - .failure_class() - .or(Some(FailureClass::Deterministic)), + .failure_category() + .or(Some(FailureCategory::Deterministic)), } } @@ -991,9 +991,9 @@ impl WorkflowRunEngine { let decision = self.run_hooks(&hook_ctx, hook_work_dir).await; match decision { HookDecision::Skip { reason } => { - let mut outcome = Outcome::skipped(); - outcome.notes = - Some(reason.unwrap_or_else(|| "skipped by StageStart hook".into())); + let msg = reason.unwrap_or_else(|| "skipped by StageStart hook".into()); + let mut outcome = Outcome::skipped(&msg); + outcome.notes = Some(msg); return Ok((outcome, attempt)); } HookDecision::Block { reason } => { @@ -1115,8 +1115,8 @@ impl WorkflowRunEngine { index: stage_index, failure: crate::outcome::FailureDetail { message: e.to_string(), - failure_class: e.failure_class(), - failure_signature: e.failure_signature_hint(), + category: e.failure_category(), + signature: e.failure_signature_hint(), }, will_retry: true, }); @@ -1482,10 +1482,7 @@ impl WorkflowRunEngine { s.node_visits = cp.node_visits.clone(); // Restore node outcomes for (k, v) in &cp.node_outcomes { - s.node_outcomes.insert( - k.clone(), - crate::core_adapter::outcome::wf_to_core_outcome(v), - ); + s.node_outcomes.insert(k.clone(), v.clone()); } // Set start node to the checkpoint's next_node_id if let Some(ref next) = cp.next_node_id { @@ -1587,10 +1584,9 @@ impl WorkflowRunEngine { // Convert result match result { Ok(core_outcome) => { - let wf_outcome = crate::core_adapter::outcome::core_to_wf_outcome(&core_outcome); - // Return outcome + a fresh context (lifecycle manages the real context) + // Outcome is now the wf type directly — no conversion needed let ctx = Context::new(); - Ok((wf_outcome, ctx)) + Ok((core_outcome, ctx)) } Err(fabro_core::CoreError::StallTimeout { node_id }) => { let stall_timeout = graph.stall_timeout().unwrap_or_default(); @@ -2066,7 +2062,7 @@ impl WorkflowRunEngine { let sig_hint = outcome .failure .as_ref() - .and_then(|f| f.failure_signature.as_deref()); + .and_then(|f| f.signature.as_deref()); let sig = FailureSignature::new(&node.id, fc, sig_hint, outcome.failure_reason()); if fc.is_signature_tracked() { let count = loop_state @@ -2095,7 +2091,10 @@ impl WorkflowRunEngine { name: node.label().to_string(), index: stage_index, failure: outcome.failure.clone().unwrap_or_else(|| { - crate::outcome::FailureDetail::new("unknown", FailureClass::Deterministic) + crate::outcome::FailureDetail::new( + "unknown", + FailureCategory::Deterministic, + ) }), will_retry: false, }); @@ -2428,7 +2427,7 @@ impl WorkflowRunEngine { "git checkpoint commit failed for node '{}': {e}", node.id ), - failure_class: FailureClass::Deterministic, + failure_class: FailureCategory::Deterministic, }); } } @@ -2502,7 +2501,7 @@ impl WorkflowRunEngine { if edge.loop_restart() { // Guard: only transient_infra failures may loop_restart (matches Kilroy) if let Some(fc) = outcome_failure_class { - if fc != FailureClass::TransientInfra { + if fc != FailureCategory::TransientInfra { return Err(FabroError::engine(format!( "loop_restart blocked: failure_class={fc} (requires transient_infra), node={}, failure_reason={}", node.id, @@ -4927,7 +4926,7 @@ mod tests { #[test] fn classify_outcome_returns_none_for_skipped() { - assert!(classify_outcome(&Outcome::skipped()).is_none()); + assert!(classify_outcome(&Outcome::skipped("")).is_none()); } #[test] @@ -4943,10 +4942,10 @@ mod tests { fn classify_outcome_reads_failure_detail() { let mut outcome = Outcome::fail_classify("some error"); // Override the FailureDetail's class directly - outcome.failure.as_mut().unwrap().failure_class = FailureClass::BudgetExhausted; + outcome.failure.as_mut().unwrap().category = FailureCategory::BudgetExhausted; assert_eq!( classify_outcome(&outcome), - Some(FailureClass::BudgetExhausted) + Some(FailureCategory::BudgetExhausted) ); } @@ -4955,7 +4954,7 @@ mod tests { let outcome = Outcome::fail_classify("rate limited by provider"); assert_eq!( classify_outcome(&outcome), - Some(FailureClass::TransientInfra) + Some(FailureCategory::TransientInfra) ); } @@ -4964,7 +4963,7 @@ mod tests { let outcome = Outcome::fail_classify("something went wrong"); assert_eq!( classify_outcome(&outcome), - Some(FailureClass::Deterministic) + Some(FailureCategory::Deterministic) ); } @@ -4977,7 +4976,7 @@ mod tests { }; assert_eq!( classify_outcome(&outcome), - Some(FailureClass::Deterministic) + Some(FailureCategory::Deterministic) ); } @@ -4986,7 +4985,7 @@ mod tests { let outcome = Outcome::retry_classify("connection refused"); assert_eq!( classify_outcome(&outcome), - Some(FailureClass::TransientInfra) + Some(FailureCategory::TransientInfra) ); } diff --git a/lib/crates/fabro-workflows/src/error.rs b/lib/crates/fabro-workflows/src/error.rs index 101874166..e54e38b55 100644 --- a/lib/crates/fabro-workflows/src/error.rs +++ b/lib/crates/fabro-workflows/src/error.rs @@ -1,121 +1,36 @@ use std::fmt; -use std::str::FromStr; use fabro_llm::error::{ProviderErrorKind, SdkError}; use serde::{Deserialize, Serialize}; use thiserror::Error; -/// Classification of failure modes for pipeline edge conditions. -/// -/// Pipeline authors can write edge conditions like `context.failure_class=budget_exhausted` -/// to route execution based on the nature of the failure. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] -#[serde(rename_all = "snake_case")] -pub enum FailureClass { - /// Temporary infrastructure failure (rate limit, timeout, network, 5xx). - TransientInfra, - /// Permanent failure (auth, bad config, code bug). - Deterministic, - /// Context length, token/turn limit, quota exceeded. - BudgetExhausted, - /// Reserved for future loop detection. - CompilationLoop, - /// User/system cancellation. - Canceled, - /// Reserved for future scope enforcement. - Structural, -} +pub use fabro_core::outcome::FailureCategory; -impl fmt::Display for FailureClass { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let s = match self { - Self::TransientInfra => "transient_infra", - Self::Deterministic => "deterministic", - Self::BudgetExhausted => "budget_exhausted", - Self::CompilationLoop => "compilation_loop", - Self::Canceled => "canceled", - Self::Structural => "structural", - }; - write!(f, "{s}") - } -} - -impl FromStr for FailureClass { - type Err = std::convert::Infallible; - - fn from_str(s: &str) -> std::result::Result { - 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, - }) - } -} - -/// Classify an `SdkError` into a `FailureClass` based on its structure. +/// Classify an `SdkError` into a `FailureCategory` based on its structure. #[must_use] -pub fn classify_sdk_error(err: &SdkError) -> FailureClass { +pub fn classify_sdk_error(err: &SdkError) -> FailureCategory { match err { SdkError::Provider { kind, .. } => match kind { ProviderErrorKind::RateLimit | ProviderErrorKind::Server => { - FailureClass::TransientInfra + FailureCategory::TransientInfra } ProviderErrorKind::ContextLength | ProviderErrorKind::QuotaExceeded => { - FailureClass::BudgetExhausted + FailureCategory::BudgetExhausted } ProviderErrorKind::Authentication | ProviderErrorKind::AccessDenied | ProviderErrorKind::NotFound | ProviderErrorKind::InvalidRequest - | ProviderErrorKind::ContentFilter => FailureClass::Deterministic, + | ProviderErrorKind::ContentFilter => FailureCategory::Deterministic, }, SdkError::RequestTimeout { .. } | SdkError::Network { .. } | SdkError::Stream { .. } => { - FailureClass::TransientInfra + FailureCategory::TransientInfra } - SdkError::Abort { .. } => FailureClass::Canceled, + SdkError::Abort { .. } => FailureCategory::Canceled, SdkError::InvalidToolCall { .. } | SdkError::NoObjectGenerated { .. } | SdkError::Configuration { .. } - | SdkError::UnsupportedToolChoice { .. } => FailureClass::Deterministic, + | SdkError::UnsupportedToolChoice { .. } => FailureCategory::Deterministic, } } @@ -186,32 +101,32 @@ const STRUCTURAL_HINTS: &[&str] = &[ /// This is the fallback when structured error information is not available /// (e.g. for `Handler(String)` or `Engine(String)` errors). #[must_use] -pub fn classify_failure_reason(reason: &str) -> FailureClass { +pub fn classify_failure_reason(reason: &str) -> FailureCategory { let lower = reason.to_lowercase(); if lower.contains("cancel") || lower.contains("abort") { - return FailureClass::Canceled; + return FailureCategory::Canceled; } if TRANSIENT_INFRA_HINTS .iter() .any(|hint| lower.contains(hint)) { - return FailureClass::TransientInfra; + return FailureCategory::TransientInfra; } if BUDGET_EXHAUSTED_HINTS .iter() .any(|hint| lower.contains(hint)) { - return FailureClass::BudgetExhausted; + return FailureCategory::BudgetExhausted; } if STRUCTURAL_HINTS.iter().any(|hint| lower.contains(hint)) { - return FailureClass::Structural; + return FailureCategory::Structural; } - FailureClass::Deterministic + FailureCategory::Deterministic } /// Normalize a failure reason for stable signature grouping. @@ -261,7 +176,7 @@ impl FailureSignature { /// grouping keys. pub fn new( node_id: &str, - failure_class: FailureClass, + failure_class: FailureCategory, signature_hint: Option<&str>, failure_reason: Option<&str>, ) -> Self { @@ -281,13 +196,6 @@ impl fmt::Display for FailureSignature { } } -impl FailureClass { - /// Whether this failure class should be tracked by the cycle breaker. - pub fn is_signature_tracked(self) -> bool { - matches!(self, Self::Deterministic | Self::Structural) - } -} - #[derive(Error, Debug, Clone, Serialize, Deserialize)] #[serde(tag = "type", content = "data", rename_all = "snake_case")] pub enum FabroError { @@ -300,13 +208,13 @@ pub enum FabroError { #[error("Engine error: {message}")] Engine { message: String, - failure_class: FailureClass, + failure_class: FailureCategory, }, #[error("Handler error: {message}")] Handler { message: String, - failure_class: FailureClass, + failure_class: FailureCategory, }, #[error("LLM error: {0}")] @@ -365,15 +273,15 @@ impl FabroError { } } - /// Classify this error into a `FailureClass`. + /// Classify this error into a `FailureCategory`. #[must_use] - pub fn failure_class(&self) -> FailureClass { + pub fn failure_category(&self) -> FailureCategory { match self { - Self::Cancelled => FailureClass::Canceled, + Self::Cancelled => FailureCategory::Canceled, Self::Llm(sdk_err) => classify_sdk_error(sdk_err), - Self::Io(_) => FailureClass::TransientInfra, + Self::Io(_) => FailureCategory::TransientInfra, Self::Parse(_) | Self::Validation(_) | Self::Stylesheet(_) | Self::Checkpoint(_) => { - FailureClass::Deterministic + FailureCategory::Deterministic } Self::Handler { failure_class, .. } | Self::Engine { failure_class, .. } => { *failure_class @@ -394,8 +302,8 @@ impl FabroError { pub fn to_fail_outcome(&self) -> crate::outcome::Outcome { let failure = crate::outcome::FailureDetail { message: self.to_string(), - failure_class: self.failure_class(), - failure_signature: self.failure_signature_hint(), + category: self.failure_category(), + signature: self.failure_signature_hint(), }; crate::outcome::Outcome { status: crate::outcome::StageStatus::Fail, @@ -437,6 +345,7 @@ pub type Result = std::result::Result; #[cfg(test)] mod tests { use super::*; + use crate::outcome::OutcomeExt; use fabro_llm::error::ProviderErrorDetail; #[test] @@ -518,161 +427,164 @@ mod tests { assert!(FabroError::Io("connection reset".to_string()).is_retryable()); } - // --- FailureClass Display/FromStr/serde tests --- + // --- FailureCategory Display/FromStr/serde tests --- #[test] fn failure_class_display_all_values() { - assert_eq!(FailureClass::TransientInfra.to_string(), "transient_infra"); - assert_eq!(FailureClass::Deterministic.to_string(), "deterministic"); assert_eq!( - FailureClass::BudgetExhausted.to_string(), + FailureCategory::TransientInfra.to_string(), + "transient_infra" + ); + assert_eq!(FailureCategory::Deterministic.to_string(), "deterministic"); + assert_eq!( + FailureCategory::BudgetExhausted.to_string(), "budget_exhausted" ); assert_eq!( - FailureClass::CompilationLoop.to_string(), + FailureCategory::CompilationLoop.to_string(), "compilation_loop" ); - assert_eq!(FailureClass::Canceled.to_string(), "canceled"); - assert_eq!(FailureClass::Structural.to_string(), "structural"); + assert_eq!(FailureCategory::Canceled.to_string(), "canceled"); + assert_eq!(FailureCategory::Structural.to_string(), "structural"); } #[test] fn failure_class_from_str_all_values() { assert_eq!( - "transient_infra".parse::().unwrap(), - FailureClass::TransientInfra + "transient_infra".parse::().unwrap(), + FailureCategory::TransientInfra ); assert_eq!( - "deterministic".parse::().unwrap(), - FailureClass::Deterministic + "deterministic".parse::().unwrap(), + FailureCategory::Deterministic ); assert_eq!( - "budget_exhausted".parse::().unwrap(), - FailureClass::BudgetExhausted + "budget_exhausted".parse::().unwrap(), + FailureCategory::BudgetExhausted ); assert_eq!( - "compilation_loop".parse::().unwrap(), - FailureClass::CompilationLoop + "compilation_loop".parse::().unwrap(), + FailureCategory::CompilationLoop ); assert_eq!( - "canceled".parse::().unwrap(), - FailureClass::Canceled + "canceled".parse::().unwrap(), + FailureCategory::Canceled ); assert_eq!( - "structural".parse::().unwrap(), - FailureClass::Structural + "structural".parse::().unwrap(), + FailureCategory::Structural ); } #[test] fn failure_class_from_str_invalid() { assert_eq!( - "unknown".parse::().unwrap(), - FailureClass::Deterministic + "unknown".parse::().unwrap(), + FailureCategory::Deterministic ); } #[test] fn failure_class_from_str_alias_retryable() { assert_eq!( - "retryable".parse::().unwrap(), - FailureClass::TransientInfra + "retryable".parse::().unwrap(), + FailureCategory::TransientInfra ); } #[test] fn failure_class_from_str_alias_transient() { assert_eq!( - "transient".parse::().unwrap(), - FailureClass::TransientInfra + "transient".parse::().unwrap(), + FailureCategory::TransientInfra ); } #[test] fn failure_class_from_str_alias_permanent() { assert_eq!( - "permanent".parse::().unwrap(), - FailureClass::Deterministic + "permanent".parse::().unwrap(), + FailureCategory::Deterministic ); } #[test] fn failure_class_from_str_alias_cancelled_british() { assert_eq!( - "cancelled".parse::().unwrap(), - FailureClass::Canceled + "cancelled".parse::().unwrap(), + FailureCategory::Canceled ); } #[test] fn failure_class_from_str_alias_budget() { assert_eq!( - "budget".parse::().unwrap(), - FailureClass::BudgetExhausted + "budget".parse::().unwrap(), + FailureCategory::BudgetExhausted ); } #[test] fn failure_class_from_str_alias_compile_loop() { assert_eq!( - "compile_loop".parse::().unwrap(), - FailureClass::CompilationLoop + "compile_loop".parse::().unwrap(), + FailureCategory::CompilationLoop ); } #[test] fn failure_class_from_str_alias_scope_violation() { assert_eq!( - "scope_violation".parse::().unwrap(), - FailureClass::Structural + "scope_violation".parse::().unwrap(), + FailureCategory::Structural ); } #[test] fn failure_class_from_str_unknown_defaults_deterministic() { assert_eq!( - "garbage_xyz".parse::().unwrap(), - FailureClass::Deterministic + "garbage_xyz".parse::().unwrap(), + FailureCategory::Deterministic ); } #[test] fn failure_class_from_str_case_insensitive() { assert_eq!( - "TRANSIENT_INFRA".parse::().unwrap(), - FailureClass::TransientInfra + "TRANSIENT_INFRA".parse::().unwrap(), + FailureCategory::TransientInfra ); } #[test] fn failure_class_from_str_trims_whitespace() { assert_eq!( - " transient_infra ".parse::().unwrap(), - FailureClass::TransientInfra + " transient_infra ".parse::().unwrap(), + FailureCategory::TransientInfra ); } #[test] fn failure_class_from_str_empty_defaults_deterministic() { assert_eq!( - "".parse::().unwrap(), - FailureClass::Deterministic + "".parse::().unwrap(), + FailureCategory::Deterministic ); } #[test] fn failure_class_serde_roundtrip() { let values = [ - FailureClass::TransientInfra, - FailureClass::Deterministic, - FailureClass::BudgetExhausted, - FailureClass::CompilationLoop, - FailureClass::Canceled, - FailureClass::Structural, + FailureCategory::TransientInfra, + FailureCategory::Deterministic, + FailureCategory::BudgetExhausted, + FailureCategory::CompilationLoop, + FailureCategory::Canceled, + FailureCategory::Structural, ]; for fc in values { let json = serde_json::to_string(&fc).unwrap(); - let parsed: FailureClass = serde_json::from_str(&json).unwrap(); + let parsed: FailureCategory = serde_json::from_str(&json).unwrap(); assert_eq!(parsed, fc); } } @@ -722,40 +634,40 @@ mod tests { #[test] fn failure_class_cancelled() { assert_eq!( - FabroError::Cancelled.failure_class(), - FailureClass::Canceled + FabroError::Cancelled.failure_category(), + FailureCategory::Canceled ); } #[test] fn failure_class_io() { assert_eq!( - FabroError::Io("disk full".into()).failure_class(), - FailureClass::TransientInfra + FabroError::Io("disk full".into()).failure_category(), + FailureCategory::TransientInfra ); } #[test] fn failure_class_parse() { assert_eq!( - FabroError::Parse("bad syntax".into()).failure_class(), - FailureClass::Deterministic + FabroError::Parse("bad syntax".into()).failure_category(), + FailureCategory::Deterministic ); } #[test] fn failure_class_handler_with_timeout() { assert_eq!( - FabroError::handler("request timed out").failure_class(), - FailureClass::TransientInfra + FabroError::handler("request timed out").failure_category(), + FailureCategory::TransientInfra ); } #[test] fn failure_class_handler_deterministic() { assert_eq!( - FabroError::handler("invalid configuration").failure_class(), - FailureClass::Deterministic + FabroError::handler("invalid configuration").failure_category(), + FailureCategory::Deterministic ); } @@ -765,7 +677,7 @@ mod tests { kind: ProviderErrorKind::RateLimit, detail: Box::new(ProviderErrorDetail::new("too fast", "openai")), }); - assert_eq!(err.failure_class(), FailureClass::TransientInfra); + assert_eq!(err.failure_category(), FailureCategory::TransientInfra); } #[test] @@ -774,7 +686,7 @@ mod tests { kind: ProviderErrorKind::ContextLength, detail: Box::new(ProviderErrorDetail::new("too long", "openai")), }); - assert_eq!(err.failure_class(), FailureClass::BudgetExhausted); + assert_eq!(err.failure_category(), FailureCategory::BudgetExhausted); } #[test] @@ -783,7 +695,7 @@ mod tests { kind: ProviderErrorKind::Authentication, detail: Box::new(ProviderErrorDetail::new("bad key", "openai")), }); - assert_eq!(err.failure_class(), FailureClass::Deterministic); + assert_eq!(err.failure_category(), FailureCategory::Deterministic); } #[test] @@ -791,7 +703,7 @@ mod tests { let err = FabroError::Llm(SdkError::Abort { message: "user cancelled".into(), }); - assert_eq!(err.failure_class(), FailureClass::Canceled); + assert_eq!(err.failure_category(), FailureCategory::Canceled); } #[test] @@ -800,7 +712,7 @@ mod tests { message: "timed out".into(), source: None, }); - assert_eq!(err.failure_class(), FailureClass::TransientInfra); + assert_eq!(err.failure_category(), FailureCategory::TransientInfra); } // --- classify_sdk_error tests --- @@ -811,7 +723,7 @@ mod tests { kind: ProviderErrorKind::RateLimit, detail: Box::new(ProviderErrorDetail::new("too fast", "openai")), }; - assert_eq!(classify_sdk_error(&err), FailureClass::TransientInfra); + assert_eq!(classify_sdk_error(&err), FailureCategory::TransientInfra); } #[test] @@ -820,7 +732,7 @@ mod tests { kind: ProviderErrorKind::Server, detail: Box::new(ProviderErrorDetail::new("500", "openai")), }; - assert_eq!(classify_sdk_error(&err), FailureClass::TransientInfra); + assert_eq!(classify_sdk_error(&err), FailureCategory::TransientInfra); } #[test] @@ -829,7 +741,7 @@ mod tests { kind: ProviderErrorKind::ContextLength, detail: Box::new(ProviderErrorDetail::new("too long", "openai")), }; - assert_eq!(classify_sdk_error(&err), FailureClass::BudgetExhausted); + assert_eq!(classify_sdk_error(&err), FailureCategory::BudgetExhausted); } #[test] @@ -838,7 +750,7 @@ mod tests { kind: ProviderErrorKind::QuotaExceeded, detail: Box::new(ProviderErrorDetail::new("out of quota", "openai")), }; - assert_eq!(classify_sdk_error(&err), FailureClass::BudgetExhausted); + assert_eq!(classify_sdk_error(&err), FailureCategory::BudgetExhausted); } #[test] @@ -847,7 +759,7 @@ mod tests { kind: ProviderErrorKind::Authentication, detail: Box::new(ProviderErrorDetail::new("bad key", "openai")), }; - assert_eq!(classify_sdk_error(&err), FailureClass::Deterministic); + assert_eq!(classify_sdk_error(&err), FailureCategory::Deterministic); } #[test] @@ -856,7 +768,7 @@ mod tests { message: "timed out".into(), source: None, }; - assert_eq!(classify_sdk_error(&err), FailureClass::TransientInfra); + assert_eq!(classify_sdk_error(&err), FailureCategory::TransientInfra); } #[test] @@ -864,7 +776,7 @@ mod tests { let err = SdkError::Abort { message: "cancelled".into(), }; - assert_eq!(classify_sdk_error(&err), FailureClass::Canceled); + assert_eq!(classify_sdk_error(&err), FailureCategory::Canceled); } #[test] @@ -872,7 +784,7 @@ mod tests { let err = SdkError::InvalidToolCall { message: "bad tool".into(), }; - assert_eq!(classify_sdk_error(&err), FailureClass::Deterministic); + assert_eq!(classify_sdk_error(&err), FailureCategory::Deterministic); } // --- hints count guards --- @@ -900,7 +812,7 @@ mod tests { fn classify_reason_cancel() { assert_eq!( classify_failure_reason("operation cancelled by user"), - FailureClass::Canceled + FailureCategory::Canceled ); } @@ -908,7 +820,7 @@ mod tests { fn classify_reason_abort() { assert_eq!( classify_failure_reason("aborted by signal"), - FailureClass::Canceled + FailureCategory::Canceled ); } @@ -918,7 +830,7 @@ mod tests { fn classify_reason_turn_limit() { assert_eq!( classify_failure_reason("exceeded turn limit of 10"), - FailureClass::BudgetExhausted + FailureCategory::BudgetExhausted ); } @@ -926,7 +838,7 @@ mod tests { fn classify_reason_token_limit() { assert_eq!( classify_failure_reason("token limit reached"), - FailureClass::BudgetExhausted + FailureCategory::BudgetExhausted ); } @@ -934,7 +846,7 @@ mod tests { fn classify_reason_context_length() { assert_eq!( classify_failure_reason("context length exceeded"), - FailureClass::BudgetExhausted + FailureCategory::BudgetExhausted ); } @@ -942,7 +854,7 @@ mod tests { fn classify_reason_budget() { assert_eq!( classify_failure_reason("budget exceeded for run"), - FailureClass::BudgetExhausted + FailureCategory::BudgetExhausted ); } @@ -950,7 +862,7 @@ mod tests { fn classify_reason_quota_exceeded() { assert_eq!( classify_failure_reason("quota exceeded"), - FailureClass::BudgetExhausted + FailureCategory::BudgetExhausted ); } @@ -958,7 +870,7 @@ mod tests { fn classify_reason_max_turns() { assert_eq!( classify_failure_reason("hit max_turns limit"), - FailureClass::BudgetExhausted + FailureCategory::BudgetExhausted ); } @@ -966,7 +878,7 @@ mod tests { fn classify_reason_max_turns_space() { assert_eq!( classify_failure_reason("max turns reached"), - FailureClass::BudgetExhausted + FailureCategory::BudgetExhausted ); } @@ -974,7 +886,7 @@ mod tests { fn classify_reason_max_tokens() { assert_eq!( classify_failure_reason("max_tokens exceeded"), - FailureClass::BudgetExhausted + FailureCategory::BudgetExhausted ); } @@ -982,7 +894,7 @@ mod tests { fn classify_reason_max_tokens_space() { assert_eq!( classify_failure_reason("max tokens reached"), - FailureClass::BudgetExhausted + FailureCategory::BudgetExhausted ); } @@ -990,7 +902,7 @@ mod tests { fn classify_reason_context_window_exceeded() { assert_eq!( classify_failure_reason("context window exceeded"), - FailureClass::BudgetExhausted + FailureCategory::BudgetExhausted ); } @@ -998,7 +910,7 @@ mod tests { fn classify_reason_budget_exhausted() { assert_eq!( classify_failure_reason("budget exhausted for this session"), - FailureClass::BudgetExhausted + FailureCategory::BudgetExhausted ); } @@ -1006,7 +918,7 @@ mod tests { fn classify_reason_token_limit_exceeded() { assert_eq!( classify_failure_reason("token limit exceeded"), - FailureClass::BudgetExhausted + FailureCategory::BudgetExhausted ); } @@ -1016,7 +928,7 @@ mod tests { fn classify_reason_scope_violation() { assert_eq!( classify_failure_reason("scope violation detected"), - FailureClass::Structural + FailureCategory::Structural ); } @@ -1026,7 +938,7 @@ mod tests { fn classify_reason_timeout() { assert_eq!( classify_failure_reason("request timed out after 30s"), - FailureClass::TransientInfra + FailureCategory::TransientInfra ); } @@ -1034,7 +946,7 @@ mod tests { fn classify_reason_rate_limit() { assert_eq!( classify_failure_reason("rate limited by provider"), - FailureClass::TransientInfra + FailureCategory::TransientInfra ); } @@ -1042,7 +954,7 @@ mod tests { fn classify_reason_connection_refused() { assert_eq!( classify_failure_reason("connection refused"), - FailureClass::TransientInfra + FailureCategory::TransientInfra ); } @@ -1050,7 +962,7 @@ mod tests { fn classify_reason_connection_reset() { assert_eq!( classify_failure_reason("connection reset by peer"), - FailureClass::TransientInfra + FailureCategory::TransientInfra ); } @@ -1058,7 +970,7 @@ mod tests { fn classify_reason_500() { assert_eq!( classify_failure_reason("HTTP 500 Internal Server Error"), - FailureClass::TransientInfra + FailureCategory::TransientInfra ); } @@ -1066,7 +978,7 @@ mod tests { fn classify_reason_502() { assert_eq!( classify_failure_reason("HTTP 502 Bad Gateway"), - FailureClass::TransientInfra + FailureCategory::TransientInfra ); } @@ -1074,7 +986,7 @@ mod tests { fn classify_reason_503() { assert_eq!( classify_failure_reason("HTTP 503 Service Unavailable"), - FailureClass::TransientInfra + FailureCategory::TransientInfra ); } @@ -1082,7 +994,7 @@ mod tests { fn classify_reason_504() { assert_eq!( classify_failure_reason("HTTP 504 Gateway Timeout"), - FailureClass::TransientInfra + FailureCategory::TransientInfra ); } @@ -1090,7 +1002,7 @@ mod tests { fn classify_reason_context_deadline_exceeded() { assert_eq!( classify_failure_reason("context deadline exceeded"), - FailureClass::TransientInfra + FailureCategory::TransientInfra ); } @@ -1098,7 +1010,7 @@ mod tests { fn classify_reason_could_not_resolve_host() { assert_eq!( classify_failure_reason("could not resolve host api.example.com"), - FailureClass::TransientInfra + FailureCategory::TransientInfra ); } @@ -1106,7 +1018,7 @@ mod tests { fn classify_reason_could_not_resolve_hostname() { assert_eq!( classify_failure_reason("could not resolve hostname"), - FailureClass::TransientInfra + FailureCategory::TransientInfra ); } @@ -1114,7 +1026,7 @@ mod tests { fn classify_reason_temporary_failure() { assert_eq!( classify_failure_reason("temporary failure"), - FailureClass::TransientInfra + FailureCategory::TransientInfra ); } @@ -1122,7 +1034,7 @@ mod tests { fn classify_reason_temporary_failure_in_name_resolution() { assert_eq!( classify_failure_reason("temporary failure in name resolution"), - FailureClass::TransientInfra + FailureCategory::TransientInfra ); } @@ -1130,7 +1042,7 @@ mod tests { fn classify_reason_network_is_unreachable() { assert_eq!( classify_failure_reason("network is unreachable"), - FailureClass::TransientInfra + FailureCategory::TransientInfra ); } @@ -1138,7 +1050,7 @@ mod tests { fn classify_reason_broken_pipe() { assert_eq!( classify_failure_reason("broken pipe"), - FailureClass::TransientInfra + FailureCategory::TransientInfra ); } @@ -1146,7 +1058,7 @@ mod tests { fn classify_reason_tls_handshake_timeout() { assert_eq!( classify_failure_reason("tls handshake timeout"), - FailureClass::TransientInfra + FailureCategory::TransientInfra ); } @@ -1154,7 +1066,7 @@ mod tests { fn classify_reason_io_timeout() { assert_eq!( classify_failure_reason("i/o timeout"), - FailureClass::TransientInfra + FailureCategory::TransientInfra ); } @@ -1162,7 +1074,7 @@ mod tests { fn classify_reason_no_route_to_host() { assert_eq!( classify_failure_reason("no route to host"), - FailureClass::TransientInfra + FailureCategory::TransientInfra ); } @@ -1170,7 +1082,7 @@ mod tests { fn classify_reason_temporarily_unavailable() { assert_eq!( classify_failure_reason("resource temporarily unavailable"), - FailureClass::TransientInfra + FailureCategory::TransientInfra ); } @@ -1178,7 +1090,7 @@ mod tests { fn classify_reason_try_again() { assert_eq!( classify_failure_reason("try again later"), - FailureClass::TransientInfra + FailureCategory::TransientInfra ); } @@ -1186,7 +1098,7 @@ mod tests { fn classify_reason_too_many_requests() { assert_eq!( classify_failure_reason("too many requests"), - FailureClass::TransientInfra + FailureCategory::TransientInfra ); } @@ -1194,7 +1106,7 @@ mod tests { fn classify_reason_service_unavailable() { assert_eq!( classify_failure_reason("service unavailable"), - FailureClass::TransientInfra + FailureCategory::TransientInfra ); } @@ -1202,7 +1114,7 @@ mod tests { fn classify_reason_gateway_timeout() { assert_eq!( classify_failure_reason("gateway timeout"), - FailureClass::TransientInfra + FailureCategory::TransientInfra ); } @@ -1210,7 +1122,7 @@ mod tests { fn classify_reason_econnrefused() { assert_eq!( classify_failure_reason("ECONNREFUSED"), - FailureClass::TransientInfra + FailureCategory::TransientInfra ); } @@ -1218,7 +1130,7 @@ mod tests { fn classify_reason_econnreset() { assert_eq!( classify_failure_reason("ECONNRESET"), - FailureClass::TransientInfra + FailureCategory::TransientInfra ); } @@ -1226,7 +1138,7 @@ mod tests { fn classify_reason_dial_tcp() { assert_eq!( classify_failure_reason("dial tcp 10.0.0.1:443: connect: connection refused"), - FailureClass::TransientInfra + FailureCategory::TransientInfra ); } @@ -1234,7 +1146,7 @@ mod tests { fn classify_reason_transport_is_closing() { assert_eq!( classify_failure_reason("transport is closing"), - FailureClass::TransientInfra + FailureCategory::TransientInfra ); } @@ -1242,7 +1154,7 @@ mod tests { fn classify_reason_stream_disconnected() { assert_eq!( classify_failure_reason("stream disconnected"), - FailureClass::TransientInfra + FailureCategory::TransientInfra ); } @@ -1250,7 +1162,7 @@ mod tests { fn classify_reason_stream_closed_before() { assert_eq!( classify_failure_reason("stream closed before completion"), - FailureClass::TransientInfra + FailureCategory::TransientInfra ); } @@ -1258,7 +1170,7 @@ mod tests { fn classify_reason_index_crates_io() { assert_eq!( classify_failure_reason("failed to fetch index.crates.io"), - FailureClass::TransientInfra + FailureCategory::TransientInfra ); } @@ -1266,7 +1178,7 @@ mod tests { fn classify_reason_download_config_json_failed() { assert_eq!( classify_failure_reason("download of config.json failed"), - FailureClass::TransientInfra + FailureCategory::TransientInfra ); } @@ -1274,7 +1186,7 @@ mod tests { fn classify_reason_toolchain_registry_unavailable() { assert_eq!( classify_failure_reason("toolchain_or_dependency_registry_unavailable"), - FailureClass::TransientInfra + FailureCategory::TransientInfra ); } @@ -1282,7 +1194,7 @@ mod tests { fn classify_reason_toolchain_dependency_network() { assert_eq!( classify_failure_reason("toolchain dependency resolution blocked by network"), - FailureClass::TransientInfra + FailureCategory::TransientInfra ); } @@ -1290,7 +1202,7 @@ mod tests { fn classify_reason_toolchain_workspace_io() { assert_eq!( classify_failure_reason("toolchain_workspace_io"), - FailureClass::TransientInfra + FailureCategory::TransientInfra ); } @@ -1298,7 +1210,7 @@ mod tests { fn classify_reason_cross_device_link() { assert_eq!( classify_failure_reason("cross-device link"), - FailureClass::TransientInfra + FailureCategory::TransientInfra ); } @@ -1306,7 +1218,7 @@ mod tests { fn classify_reason_invalid_cross_device_link() { assert_eq!( classify_failure_reason("invalid cross-device link"), - FailureClass::TransientInfra + FailureCategory::TransientInfra ); } @@ -1314,7 +1226,7 @@ mod tests { fn classify_reason_os_error_18() { assert_eq!( classify_failure_reason("os error 18"), - FailureClass::TransientInfra + FailureCategory::TransientInfra ); } @@ -1324,7 +1236,7 @@ mod tests { fn classify_reason_write_scope_violation_underscore() { assert_eq!( classify_failure_reason("write_scope_violation detected"), - FailureClass::Structural + FailureCategory::Structural ); } @@ -1332,7 +1244,7 @@ mod tests { fn classify_reason_write_scope_violation_space() { assert_eq!( classify_failure_reason("write scope violation detected"), - FailureClass::Structural + FailureCategory::Structural ); } @@ -1342,7 +1254,7 @@ mod tests { fn classify_reason_default_deterministic() { assert_eq!( classify_failure_reason("invalid configuration parameter"), - FailureClass::Deterministic + FailureCategory::Deterministic ); } @@ -1422,7 +1334,7 @@ mod tests { fn failure_signature_format() { let sig = FailureSignature::new( "verify", - FailureClass::Deterministic, + FailureCategory::Deterministic, None, Some("test failed"), ); @@ -1433,7 +1345,7 @@ mod tests { fn failure_signature_display() { let sig = FailureSignature::new( "build", - FailureClass::Structural, + FailureCategory::Structural, None, Some("scope violation"), ); @@ -1444,7 +1356,7 @@ mod tests { fn failure_signature_hint_takes_priority() { let sig = FailureSignature::new( "verify", - FailureClass::Deterministic, + FailureCategory::Deterministic, Some("custom hint"), Some("raw reason"), ); @@ -1453,7 +1365,7 @@ mod tests { #[test] fn failure_signature_missing_reason_falls_back_to_unknown() { - let sig = FailureSignature::new("node", FailureClass::Deterministic, None, None); + let sig = FailureSignature::new("node", FailureCategory::Deterministic, None, None); assert_eq!(sig.to_string(), "node|deterministic|unknown"); } @@ -1461,13 +1373,13 @@ mod tests { fn failure_signature_equality_and_hash() { let sig1 = FailureSignature::new( "verify", - FailureClass::Deterministic, + FailureCategory::Deterministic, None, Some("test failed"), ); let sig2 = FailureSignature::new( "verify", - FailureClass::Deterministic, + FailureCategory::Deterministic, None, Some("test failed"), ); @@ -1482,16 +1394,16 @@ mod tests { #[test] fn is_signature_tracked_deterministic_and_structural() { - assert!(FailureClass::Deterministic.is_signature_tracked()); - assert!(FailureClass::Structural.is_signature_tracked()); + assert!(FailureCategory::Deterministic.is_signature_tracked()); + assert!(FailureCategory::Structural.is_signature_tracked()); } #[test] fn is_signature_tracked_false_for_others() { - assert!(!FailureClass::TransientInfra.is_signature_tracked()); - assert!(!FailureClass::BudgetExhausted.is_signature_tracked()); - assert!(!FailureClass::Canceled.is_signature_tracked()); - assert!(!FailureClass::CompilationLoop.is_signature_tracked()); + assert!(!FailureCategory::TransientInfra.is_signature_tracked()); + assert!(!FailureCategory::BudgetExhausted.is_signature_tracked()); + assert!(!FailureCategory::Canceled.is_signature_tracked()); + assert!(!FailureCategory::CompilationLoop.is_signature_tracked()); } // --- failure_signature_hint tests --- @@ -1531,9 +1443,9 @@ mod tests { let outcome = err.to_fail_outcome(); assert_eq!(outcome.status, crate::outcome::StageStatus::Fail); let failure = outcome.failure.as_ref().unwrap(); - assert_eq!(failure.failure_class, FailureClass::Deterministic); + assert_eq!(failure.category, FailureCategory::Deterministic); assert_eq!( - failure.failure_signature.as_deref(), + failure.signature.as_deref(), Some("api_deterministic|openai|authentication") ); } @@ -1544,8 +1456,8 @@ mod tests { let outcome = err.to_fail_outcome(); assert_eq!(outcome.status, crate::outcome::StageStatus::Fail); let failure = outcome.failure.as_ref().unwrap(); - assert_eq!(failure.failure_class, FailureClass::TransientInfra); - assert!(failure.failure_signature.is_none()); + assert_eq!(failure.category, FailureCategory::TransientInfra); + assert!(failure.signature.is_none()); } #[test] @@ -1576,7 +1488,7 @@ mod tests { #[test] fn handler_eager_classification() { let err = FabroError::handler("connection refused"); - assert_eq!(err.failure_class(), FailureClass::TransientInfra); + assert_eq!(err.failure_category(), FailureCategory::TransientInfra); } #[test] @@ -1584,7 +1496,10 @@ mod tests { let err = FabroError::handler("connection refused"); let json = serde_json::to_string(&err).unwrap(); let deserialized: FabroError = serde_json::from_str(&json).unwrap(); - assert_eq!(deserialized.failure_class(), FailureClass::TransientInfra); + assert_eq!( + deserialized.failure_category(), + FailureCategory::TransientInfra + ); } #[test] @@ -1596,7 +1511,7 @@ mod tests { #[test] fn engine_eager_classification() { let err = FabroError::engine("rate limit exceeded"); - assert_eq!(err.failure_class(), FailureClass::TransientInfra); + assert_eq!(err.failure_category(), FailureCategory::TransientInfra); } #[test] @@ -1651,7 +1566,7 @@ mod tests { ]; for msg in messages { assert_eq!( - FabroError::handler(msg).failure_class(), + FabroError::handler(msg).failure_category(), classify_failure_reason(msg), "mismatch for message: {msg}" ); @@ -1662,7 +1577,10 @@ mod tests { fn to_fail_outcome_preserves_class() { let err = FabroError::handler("timeout"); let outcome = err.to_fail_outcome(); - assert_eq!(outcome.failure_class(), Some(FailureClass::TransientInfra)); + assert_eq!( + outcome.failure_category(), + Some(FailureCategory::TransientInfra) + ); } // --- E2E error pipeline tests --- @@ -1677,11 +1595,14 @@ mod tests { detail: Box::new(ProviderErrorDetail::new("too fast", "openai")), }; let arc_err = FabroError::Llm(sdk_err); - assert_eq!(arc_err.failure_class(), FailureClass::TransientInfra); + assert_eq!(arc_err.failure_category(), FailureCategory::TransientInfra); // 2. FabroError → Outcome let outcome = arc_err.to_fail_outcome(); - assert_eq!(outcome.failure_class(), Some(FailureClass::TransientInfra)); + assert_eq!( + outcome.failure_category(), + Some(FailureCategory::TransientInfra) + ); // 3. Outcome → StageFailed event let failure = outcome.failure.clone().unwrap(); @@ -1696,7 +1617,7 @@ mod tests { // 4. Verify classification survived all the way through match &event { WorkflowRunEvent::StageFailed { failure, .. } => { - assert_eq!(failure.failure_class, FailureClass::TransientInfra); + assert_eq!(failure.category, FailureCategory::TransientInfra); } _ => panic!("expected StageFailed"), } @@ -1706,15 +1627,18 @@ mod tests { fn e2e_handler_error_classified_at_edge() { // handler smart constructor classifies eagerly let err = FabroError::handler("connection refused"); - assert_eq!(err.failure_class(), FailureClass::TransientInfra); + assert_eq!(err.failure_category(), FailureCategory::TransientInfra); // to_fail_outcome preserves let outcome = err.to_fail_outcome(); - assert_eq!(outcome.failure_class(), Some(FailureClass::TransientInfra)); + assert_eq!( + outcome.failure_category(), + Some(FailureCategory::TransientInfra) + ); // event preserves let failure = outcome.failure.unwrap(); - assert_eq!(failure.failure_class, FailureClass::TransientInfra); + assert_eq!(failure.category, FailureCategory::TransientInfra); } #[test] @@ -1739,7 +1663,10 @@ mod tests { // Round-trip let deserialized: FabroError = serde_json::from_str(&json).unwrap(); - assert_eq!(deserialized.failure_class(), FailureClass::TransientInfra); + assert_eq!( + deserialized.failure_category(), + FailureCategory::TransientInfra + ); } #[test] @@ -1770,9 +1697,9 @@ mod tests { let failure = deserialized.failure.unwrap(); assert_eq!(failure.message, "rate limit exceeded"); - assert_eq!(failure.failure_class, FailureClass::TransientInfra); + assert_eq!(failure.category, FailureCategory::TransientInfra); assert_eq!( - failure.failure_signature.as_deref(), + failure.signature.as_deref(), Some("api_transient|openai|rate_limited") ); } diff --git a/lib/crates/fabro-workflows/src/event.rs b/lib/crates/fabro-workflows/src/event.rs index 438a673e3..cab10bfc2 100644 --- a/lib/crates/fabro-workflows/src/event.rs +++ b/lib/crates/fabro-workflows/src/event.rs @@ -1294,8 +1294,7 @@ mod tests { #[test] fn stage_completed_event_serialization_with_new_fields() { - use crate::error::FailureClass; - use crate::outcome::FailureDetail; + use crate::outcome::{FailureCategory, FailureDetail}; let event = WorkflowRunEvent::StageCompleted { node_id: "plan".to_string(), @@ -1308,7 +1307,7 @@ mod tests { usage: None, failure: Some(FailureDetail::new( "lint errors remain", - FailureClass::Deterministic, + FailureCategory::Deterministic, )), notes: Some("fixed 3 of 5 issues".to_string()), files_touched: vec!["src/main.rs".to_string()], @@ -1343,8 +1342,7 @@ mod tests { #[test] fn stage_failed_event_serialization() { - use crate::error::FailureClass; - use crate::outcome::FailureDetail; + use crate::outcome::{FailureCategory, FailureDetail}; let event = WorkflowRunEvent::StageFailed { node_id: "plan".to_string(), @@ -1352,8 +1350,8 @@ mod tests { index: 0, failure: FailureDetail { message: "LLM request timed out".to_string(), - failure_class: FailureClass::TransientInfra, - failure_signature: None, + category: FailureCategory::TransientInfra, + signature: None, }, will_retry: true, }; @@ -1364,14 +1362,14 @@ mod tests { let deserialized: WorkflowRunEvent = serde_json::from_str(&json).unwrap(); assert!(matches!( deserialized, - WorkflowRunEvent::StageFailed { failure, .. } if failure.failure_class == FailureClass::TransientInfra + WorkflowRunEvent::StageFailed { failure, .. } if failure.category == FailureCategory::TransientInfra )); let event_terminal = WorkflowRunEvent::StageFailed { node_id: "plan".to_string(), name: "plan".to_string(), index: 0, - failure: FailureDetail::new("timeout", FailureClass::Deterministic), + failure: FailureDetail::new("timeout", FailureCategory::Deterministic), will_retry: false, }; let json_terminal = serde_json::to_string(&event_terminal).unwrap(); diff --git a/lib/crates/fabro-workflows/src/handler/agent.rs b/lib/crates/fabro-workflows/src/handler/agent.rs index 6f2736a61..73f9d9876 100644 --- a/lib/crates/fabro-workflows/src/handler/agent.rs +++ b/lib/crates/fabro-workflows/src/handler/agent.rs @@ -9,7 +9,7 @@ use crate::context::keys; use crate::context::Context; use crate::error::FabroError; use crate::event::EventEmitter; -use crate::outcome::{Outcome, StageUsage}; +use crate::outcome::{Outcome, OutcomeExt, StageUsage}; use fabro_graphviz::graph::{Graph, Node}; use super::{EngineServices, Handler}; @@ -170,7 +170,7 @@ pub(crate) fn extract_status_fields(text: &str, outcome: &mut Outcome) -> bool { if let Some(reason) = obj.get("failure_reason").and_then(|v| v.as_str()) { outcome.failure = Some(crate::outcome::FailureDetail::new( reason, - crate::error::FailureClass::Deterministic, + crate::outcome::FailureCategory::Deterministic, )); } } diff --git a/lib/crates/fabro-workflows/src/handler/command.rs b/lib/crates/fabro-workflows/src/handler/command.rs index c4368922f..5a422d36f 100644 --- a/lib/crates/fabro-workflows/src/handler/command.rs +++ b/lib/crates/fabro-workflows/src/handler/command.rs @@ -5,7 +5,7 @@ use async_trait::async_trait; use crate::context::keys; use crate::context::Context; use crate::error::FabroError; -use crate::outcome::Outcome; +use crate::outcome::{Outcome, OutcomeExt}; use fabro_graphviz::graph::{Graph, Node}; use super::{EngineServices, Handler}; diff --git a/lib/crates/fabro-workflows/src/handler/fan_in.rs b/lib/crates/fabro-workflows/src/handler/fan_in.rs index 5d1bc8f0f..4db24d306 100644 --- a/lib/crates/fabro-workflows/src/handler/fan_in.rs +++ b/lib/crates/fabro-workflows/src/handler/fan_in.rs @@ -8,7 +8,7 @@ use crate::context::keys; use crate::context::Context; use crate::error::FabroError; use crate::event::EventEmitter; -use crate::outcome::Outcome; +use crate::outcome::{Outcome, OutcomeExt}; use fabro_graphviz::graph::{Graph, Node}; use super::agent::{CodergenBackend, CodergenResult}; diff --git a/lib/crates/fabro-workflows/src/handler/human.rs b/lib/crates/fabro-workflows/src/handler/human.rs index 3ab3ef554..9f9947475 100644 --- a/lib/crates/fabro-workflows/src/handler/human.rs +++ b/lib/crates/fabro-workflows/src/handler/human.rs @@ -9,7 +9,7 @@ use crate::context::Context; use crate::error::FabroError; use crate::event::{EventEmitter, WorkflowRunEvent}; use crate::millis_u64; -use crate::outcome::Outcome; +use crate::outcome::{Outcome, OutcomeExt}; use fabro_graphviz::graph::{Graph, Node}; use fabro_interview::{Answer, AnswerValue, Interviewer, Question, QuestionOption, QuestionType}; diff --git a/lib/crates/fabro-workflows/src/handler/manager_loop.rs b/lib/crates/fabro-workflows/src/handler/manager_loop.rs index 9d19570d4..203c3d0da 100644 --- a/lib/crates/fabro-workflows/src/handler/manager_loop.rs +++ b/lib/crates/fabro-workflows/src/handler/manager_loop.rs @@ -11,7 +11,7 @@ use crate::context::keys; use crate::context::Context; use crate::engine::{RunConfig, WorkflowRunEngine}; use crate::error::FabroError; -use crate::outcome::{Outcome, StageStatus}; +use crate::outcome::{Outcome, OutcomeExt, StageStatus}; use crate::workflow::{prepare_from_file, prepare_from_source}; use fabro_graphviz::graph::{Graph, Node}; diff --git a/lib/crates/fabro-workflows/src/handler/mod.rs b/lib/crates/fabro-workflows/src/handler/mod.rs index 7f1b503dd..e704951c7 100644 --- a/lib/crates/fabro-workflows/src/handler/mod.rs +++ b/lib/crates/fabro-workflows/src/handler/mod.rs @@ -21,7 +21,7 @@ use crate::context::Context; use crate::engine::GitState; use crate::error::FabroError; use crate::event::EventEmitter; -use crate::outcome::Outcome; +use crate::outcome::{Outcome, OutcomeExt}; use fabro_graphviz::graph::{shape_to_handler_type, Graph, Node}; use fabro_hooks::{HookContext, HookDecision, HookRunner}; use fabro_interview::Interviewer; diff --git a/lib/crates/fabro-workflows/src/handler/parallel.rs b/lib/crates/fabro-workflows/src/handler/parallel.rs index 1eb3f87d7..5d67803b4 100644 --- a/lib/crates/fabro-workflows/src/handler/parallel.rs +++ b/lib/crates/fabro-workflows/src/handler/parallel.rs @@ -12,7 +12,7 @@ use crate::engine::set_hook_node; use crate::error::FabroError; use crate::event::WorkflowRunEvent; use crate::millis_u64; -use crate::outcome::{Outcome, StageStatus}; +use crate::outcome::{Outcome, OutcomeExt, StageStatus}; use fabro_graphviz::graph::{Graph, Node}; use fabro_hooks::{HookContext, HookEvent}; @@ -516,7 +516,7 @@ impl Handler for ParallelHandler { failure: if is_fail { Some(crate::outcome::FailureDetail::new( format!("Join policy not satisfied: {success_count}/{total} succeeded"), - crate::error::FailureClass::Deterministic, + crate::outcome::FailureCategory::Deterministic, )) } else { None diff --git a/lib/crates/fabro-workflows/src/lib.rs b/lib/crates/fabro-workflows/src/lib.rs index 7b3471b1c..2106fd36f 100644 --- a/lib/crates/fabro-workflows/src/lib.rs +++ b/lib/crates/fabro-workflows/src/lib.rs @@ -31,7 +31,7 @@ pub fn build_completed_stages( cp: &checkpoint::Checkpoint, run_failed: bool, ) -> Vec { - use outcome::StageStatus; + use outcome::{OutcomeExt, StageStatus}; let mut stages = Vec::new(); let mut any_stage_failed = false; diff --git a/lib/crates/fabro-workflows/src/outcome.rs b/lib/crates/fabro-workflows/src/outcome.rs index bc6313e09..fc46cce04 100644 --- a/lib/crates/fabro-workflows/src/outcome.rs +++ b/lib/crates/fabro-workflows/src/outcome.rs @@ -1,49 +1,8 @@ -use std::collections::HashMap; -use std::fmt; -use std::str::FromStr; - use serde::{Deserialize, Serialize}; -use crate::error::{classify_failure_reason, FailureClass}; +pub use fabro_core::outcome::{FailureCategory, FailureDetail, OutcomeMeta, StageStatus}; -/// Status of a pipeline stage execution. -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -#[serde(rename_all = "snake_case")] -pub enum StageStatus { - Success, - Fail, - PartialSuccess, - Retry, - Skipped, -} - -impl fmt::Display for StageStatus { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let s = match self { - Self::Success => "success", - Self::Fail => "fail", - Self::PartialSuccess => "partial_success", - Self::Retry => "retry", - Self::Skipped => "skipped", - }; - write!(f, "{s}") - } -} - -impl FromStr for StageStatus { - type Err = String; - - fn from_str(s: &str) -> std::result::Result { - match s { - "success" => Ok(Self::Success), - "fail" => Ok(Self::Fail), - "partial_success" => Ok(Self::PartialSuccess), - "retry" => Ok(Self::Retry), - "skipped" => Ok(Self::Skipped), - other => Err(format!("unknown stage status: {other}")), - } - } -} +use crate::error::classify_failure_reason; /// Token usage from a single pipeline stage. #[derive(Debug, Clone, Serialize, Deserialize)] @@ -78,163 +37,82 @@ impl From<&StageUsage> for fabro_llm::types::Usage { } } -/// Structured failure information carried through the pipeline. -#[derive(Debug, Clone, Serialize, Deserialize)] -pub struct FailureDetail { - pub message: String, - pub failure_class: FailureClass, - #[serde(default, skip_serializing_if = "Option::is_none")] - pub failure_signature: Option, -} +/// The workflow-specific Outcome type, parameterized with optional stage usage. +pub type Outcome = fabro_core::Outcome>; -impl FailureDetail { - pub fn new(message: impl Into, failure_class: FailureClass) -> Self { - Self { - message: message.into(), - failure_class, - failure_signature: None, - } - } -} +/// Extension trait for workflow-specific Outcome factory methods and accessors. +pub trait OutcomeExt: Sized { + /// Create a failed outcome with a deterministic failure category. + fn fail_deterministic(reason: impl Into) -> Self; -/// The result of executing a node handler. -#[derive(Debug, Clone, Serialize, Deserialize)] -pub struct Outcome { - pub status: StageStatus, - #[serde(default, skip_serializing_if = "Option::is_none")] - pub preferred_label: Option, - #[serde(default, skip_serializing_if = "Vec::is_empty")] - pub suggested_next_ids: Vec, - #[serde(default, skip_serializing_if = "HashMap::is_empty")] - pub context_updates: HashMap, - #[serde(default, skip_serializing_if = "Option::is_none")] - pub notes: Option, - #[serde(default, skip_serializing_if = "Option::is_none")] - pub failure: Option, - #[serde(default, skip_serializing_if = "Option::is_none")] - pub usage: Option, - #[serde(default, skip_serializing_if = "Vec::is_empty")] - pub files_touched: Vec, - /// When set, the engine bypasses edge selection and jumps directly to this node. - /// Used by the parallel handler to skip re-executing branch nodes. - #[serde(default, skip_serializing_if = "Option::is_none")] - pub jump_to_node: Option, - /// Wall-clock duration of the stage execution in milliseconds. - #[serde(default, skip_serializing_if = "Option::is_none")] - pub duration_ms: Option, -} + /// Create a failed outcome with the failure category inferred from the message via heuristics. + fn fail_classify(reason: impl Into) -> Self; -impl Outcome { - #[must_use] - pub fn success() -> Self { - Self { - status: StageStatus::Success, - preferred_label: None, - suggested_next_ids: Vec::new(), - context_updates: HashMap::new(), - notes: None, - failure: None, - usage: None, - files_touched: Vec::new(), - jump_to_node: None, - duration_ms: None, - } - } - - /// Create a failed outcome with a deterministic failure class. - pub fn fail_deterministic(reason: impl Into) -> Self { - Self { - status: StageStatus::Fail, - preferred_label: None, - suggested_next_ids: Vec::new(), - context_updates: HashMap::new(), - notes: None, - failure: Some(FailureDetail::new(reason, FailureClass::Deterministic)), - usage: None, - files_touched: Vec::new(), - jump_to_node: None, - duration_ms: None, - } - } - - /// Create a failed outcome with the failure class inferred from the message via heuristics. - pub fn fail_classify(reason: impl Into) -> Self { - let reason = reason.into(); - let failure_class = classify_failure_reason(&reason); - Self { - status: StageStatus::Fail, - preferred_label: None, - suggested_next_ids: Vec::new(), - context_updates: HashMap::new(), - notes: None, - failure: Some(FailureDetail::new(reason, failure_class)), - usage: None, - files_touched: Vec::new(), - jump_to_node: None, - duration_ms: None, - } - } - - /// Create a retry outcome with the failure class inferred from the message via heuristics. - pub fn retry_classify(reason: impl Into) -> Self { - let reason = reason.into(); - let failure_class = classify_failure_reason(&reason); - Self { - status: StageStatus::Retry, - preferred_label: None, - suggested_next_ids: Vec::new(), - context_updates: HashMap::new(), - notes: None, - failure: Some(FailureDetail::new(reason, failure_class)), - usage: None, - files_touched: Vec::new(), - jump_to_node: None, - duration_ms: None, - } - } - - /// Set the failure signature on this outcome. Returns self for chaining. - #[must_use] - pub fn with_signature(mut self, sig: Option>) -> Self { - if let Some(ref mut f) = self.failure { - f.failure_signature = sig.map(Into::into); - } - self - } - - #[must_use] - pub fn skipped() -> Self { - Self { - status: StageStatus::Skipped, - preferred_label: None, - suggested_next_ids: Vec::new(), - context_updates: HashMap::new(), - notes: None, - failure: None, - usage: None, - files_touched: Vec::new(), - jump_to_node: None, - duration_ms: None, - } - } + /// Create a retry outcome with the failure category inferred from the message via heuristics. + fn retry_classify(reason: impl Into) -> Self; /// Create a simulated success outcome for dry-run mode. - #[must_use] - pub fn simulated(node_id: &str) -> Self { + fn simulated(node_id: &str) -> Self; + + /// Set the failure signature on this outcome. Returns self for chaining. + fn with_signature(self, sig: Option>) -> Self; + + /// Get the failure reason message, if any. + fn failure_reason(&self) -> Option<&str>; + + /// Get the failure category, if this is a failed outcome. + fn failure_category(&self) -> Option; +} + +impl OutcomeExt for Outcome { + fn fail_deterministic(reason: impl Into) -> Self { + Self { + status: StageStatus::Fail, + failure: Some(FailureDetail::new(reason, FailureCategory::Deterministic)), + ..Self::default() + } + } + + fn fail_classify(reason: impl Into) -> Self { + let reason = reason.into(); + let category = classify_failure_reason(&reason); + Self { + status: StageStatus::Fail, + failure: Some(FailureDetail::new(reason, category)), + ..Self::default() + } + } + + fn retry_classify(reason: impl Into) -> Self { + let reason = reason.into(); + let category = classify_failure_reason(&reason); + Self { + status: StageStatus::Retry, + failure: Some(FailureDetail::new(reason, category)), + ..Self::default() + } + } + + fn simulated(node_id: &str) -> Self { Self { notes: Some(format!("[Simulated] {node_id}")), ..Self::success() } } - /// Get the failure reason message, if any. - pub fn failure_reason(&self) -> Option<&str> { + fn with_signature(mut self, sig: Option>) -> Self { + if let Some(ref mut f) = self.failure { + f.signature = sig.map(Into::into); + } + self + } + + fn failure_reason(&self) -> Option<&str> { self.failure.as_ref().map(|f| f.message.as_str()) } - /// Get the failure class, if this is a failed outcome. - pub fn failure_class(&self) -> Option { - self.failure.as_ref().map(|f| f.failure_class) + fn failure_category(&self) -> Option { + self.failure.as_ref().map(|f| f.category) } } @@ -290,7 +168,7 @@ mod tests { let o = Outcome::fail_deterministic("something broke"); assert_eq!(o.status, StageStatus::Fail); assert_eq!(o.failure_reason(), Some("something broke")); - assert_eq!(o.failure_class(), Some(FailureClass::Deterministic)); + assert_eq!(o.failure_category(), Some(FailureCategory::Deterministic)); } #[test] @@ -298,7 +176,7 @@ mod tests { let o = Outcome::fail_classify("connection refused"); assert_eq!(o.status, StageStatus::Fail); assert_eq!(o.failure_reason(), Some("connection refused")); - assert_eq!(o.failure_class(), Some(FailureClass::TransientInfra)); + assert_eq!(o.failure_category(), Some(FailureCategory::TransientInfra)); } #[test] @@ -310,46 +188,46 @@ mod tests { #[test] fn outcome_skipped_factory() { - let o = Outcome::skipped(); + let o = Outcome::skipped(""); assert_eq!(o.status, StageStatus::Skipped); assert!(o.failure.is_none()); } #[test] fn failure_detail_construction() { - let fd = FailureDetail::new("timeout", FailureClass::TransientInfra); + let fd = FailureDetail::new("timeout", FailureCategory::TransientInfra); assert_eq!(fd.message, "timeout"); - assert_eq!(fd.failure_class, FailureClass::TransientInfra); - assert!(fd.failure_signature.is_none()); + assert_eq!(fd.category, FailureCategory::TransientInfra); + assert!(fd.signature.is_none()); } #[test] fn failure_detail_serde_roundtrip() { let fd = FailureDetail { message: "timeout".into(), - failure_class: FailureClass::TransientInfra, - failure_signature: Some("sig".into()), + category: FailureCategory::TransientInfra, + signature: Some("sig".into()), }; let json = serde_json::to_string(&fd).unwrap(); let deserialized: FailureDetail = serde_json::from_str(&json).unwrap(); assert_eq!(deserialized.message, "timeout"); - assert_eq!(deserialized.failure_class, FailureClass::TransientInfra); - assert_eq!(deserialized.failure_signature.as_deref(), Some("sig")); + assert_eq!(deserialized.category, FailureCategory::TransientInfra); + assert_eq!(deserialized.signature.as_deref(), Some("sig")); } #[test] fn fail_classify_known_patterns() { assert_eq!( - Outcome::fail_classify("timeout").failure_class(), - Some(FailureClass::TransientInfra) + Outcome::fail_classify("timeout").failure_category(), + Some(FailureCategory::TransientInfra) ); assert_eq!( - Outcome::fail_classify("context length exceeded").failure_class(), - Some(FailureClass::BudgetExhausted) + Outcome::fail_classify("context length exceeded").failure_category(), + Some(FailureCategory::BudgetExhausted) ); assert_eq!( - Outcome::fail_classify("cancel").failure_class(), - Some(FailureClass::Canceled) + Outcome::fail_classify("cancel").failure_category(), + Some(FailureCategory::Canceled) ); } @@ -367,7 +245,7 @@ mod tests { fn with_signature_builder() { let o = Outcome::fail_deterministic("x").with_signature(Some("sig")); assert_eq!( - o.failure.as_ref().unwrap().failure_signature.as_deref(), + o.failure.as_ref().unwrap().signature.as_deref(), Some("sig") ); } diff --git a/lib/crates/fabro-workflows/src/preamble.rs b/lib/crates/fabro-workflows/src/preamble.rs index f000da97f..d387de0f3 100644 --- a/lib/crates/fabro-workflows/src/preamble.rs +++ b/lib/crates/fabro-workflows/src/preamble.rs @@ -4,6 +4,7 @@ use crate::artifact::{artifact_path, format_artifact_reference}; use crate::context::keys; use crate::context::Context; use crate::outcome::Outcome; +use crate::outcome::OutcomeExt; use fabro_graphviz::graph::{is_llm_handler_type, Graph, Node}; const COMPACT_OUTPUT_MAX_LINES: usize = 25; diff --git a/lib/crates/fabro-workflows/tests/daytona_integration.rs b/lib/crates/fabro-workflows/tests/daytona_integration.rs index 726e07d56..fd8e8ed5c 100644 --- a/lib/crates/fabro-workflows/tests/daytona_integration.rs +++ b/lib/crates/fabro-workflows/tests/daytona_integration.rs @@ -20,7 +20,7 @@ use fabro_workflows::event::EventEmitter; use fabro_workflows::handler::exit::ExitHandler; use fabro_workflows::handler::start::StartHandler; use fabro_workflows::handler::{Handler, HandlerRegistry}; -use fabro_workflows::outcome::{Outcome, StageStatus}; +use fabro_workflows::outcome::{Outcome, OutcomeExt, StageStatus}; async fn create_env() -> DaytonaSandbox { let creds = load_github_app_credentials(); diff --git a/lib/crates/fabro-workflows/tests/integration.rs b/lib/crates/fabro-workflows/tests/integration.rs index dd7bbd32a..202ddb9f2 100644 --- a/lib/crates/fabro-workflows/tests/integration.rs +++ b/lib/crates/fabro-workflows/tests/integration.rs @@ -29,7 +29,7 @@ use fabro_workflows::handler::manager_loop::SubWorkflowHandler; use fabro_workflows::handler::start::StartHandler; use fabro_workflows::handler::wait::WaitHandler; use fabro_workflows::handler::{Handler, HandlerRegistry}; -use fabro_workflows::outcome::{Outcome, StageStatus}; +use fabro_workflows::outcome::{Outcome, OutcomeExt, StageStatus}; use fabro_workflows::stylesheet::{apply_stylesheet, parse_stylesheet}; use fabro_workflows::transform::{ StylesheetApplicationTransform, Transform, VariableExpansionTransform, @@ -11821,11 +11821,11 @@ fn e2e_normalize_failure_reason_strips_variable_data() { #[test] fn e2e_failure_signature_composite_key() { - use fabro_workflows::error::{FailureClass, FailureSignature}; + use fabro_workflows::error::{FailureCategory, FailureSignature}; let sig = FailureSignature::new( "verify", - FailureClass::Deterministic, + FailureCategory::Deterministic, None, Some("assertion failed at line 42"), ); @@ -11848,11 +11848,11 @@ fn e2e_failure_signature_composite_key() { #[test] fn e2e_failure_signature_hint_priority() { - use fabro_workflows::error::{FailureClass, FailureSignature}; + use fabro_workflows::error::{FailureCategory, FailureSignature}; let sig = FailureSignature::new( "build", - FailureClass::Deterministic, + FailureCategory::Deterministic, Some("custom-key-abc"), Some("raw error with line 123 and hash deadbeef"), ); @@ -11865,17 +11865,17 @@ fn e2e_failure_signature_hint_priority() { #[test] fn e2e_only_deterministic_and_structural_tracked() { - use fabro_workflows::error::FailureClass; + use fabro_workflows::error::FailureCategory; // These should be tracked - assert!(FailureClass::Deterministic.is_signature_tracked()); - assert!(FailureClass::Structural.is_signature_tracked()); + assert!(FailureCategory::Deterministic.is_signature_tracked()); + assert!(FailureCategory::Structural.is_signature_tracked()); // These should NOT be tracked (transient failures retry naturally) - assert!(!FailureClass::TransientInfra.is_signature_tracked()); - assert!(!FailureClass::BudgetExhausted.is_signature_tracked()); - assert!(!FailureClass::Canceled.is_signature_tracked()); - assert!(!FailureClass::CompilationLoop.is_signature_tracked()); + assert!(!FailureCategory::TransientInfra.is_signature_tracked()); + assert!(!FailureCategory::BudgetExhausted.is_signature_tracked()); + assert!(!FailureCategory::Canceled.is_signature_tracked()); + assert!(!FailureCategory::CompilationLoop.is_signature_tracked()); } // --- E2E Test: loop_restart_signature_limit graph attribute --- @@ -12373,7 +12373,7 @@ fn e2e_checkpoint_backward_compat_no_signatures() { #[test] fn e2e_checkpoint_signatures_roundtrip() { - use fabro_workflows::error::{FailureClass, FailureSignature}; + use fabro_workflows::error::{FailureCategory, FailureSignature}; let dir = tempfile::tempdir().unwrap(); let path = dir.path().join("cp.json"); @@ -12384,7 +12384,7 @@ fn e2e_checkpoint_signatures_roundtrip() { let mut loop_sigs = std::collections::HashMap::new(); let sig1 = FailureSignature::new( "verify", - FailureClass::Deterministic, + FailureCategory::Deterministic, None, Some("assertion failed"), ); @@ -12393,7 +12393,7 @@ fn e2e_checkpoint_signatures_roundtrip() { let mut restart_sigs = std::collections::HashMap::new(); let sig2 = FailureSignature::new( "build", - FailureClass::Structural, + FailureCategory::Structural, None, Some("scope violation"), ); @@ -12692,11 +12692,11 @@ impl Handler for ClassifiedFailHandler { if n >= self.succeed_on { return Ok(Outcome::success()); } - let failure_class: fabro_workflows::error::FailureClass = + let failure_class: fabro_workflows::error::FailureCategory = self.failure_class.parse().unwrap(); let mut outcome = Outcome::fail_classify("classified failure"); if let Some(ref mut f) = outcome.failure { - f.failure_class = failure_class; + f.category = failure_class; } Ok(outcome) }