mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-09-05 08:10:39 +00:00
Extract fabro-retro crate from fabro-workflows
Move retro.rs and retro_agent.rs into a new fabro-retro crate to reduce the size of fabro-workflows and clarify domain boundaries. Key design changes: - Add CompletedStage struct as a flat DTO that decouples retro derivation from Checkpoint/Outcome types in the workflow engine - derive_retro now takes Vec<CompletedStage> (owned) instead of &Checkpoint - run_retro_agent takes an event_callback closure instead of EventEmitter, pushing event filtering to the caller - Shared build_completed_stages() in fabro-workflows::lib converts Checkpoint → Vec<CompletedStage> for both run.rs and server.rs callers Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
49c7e329dd
commit
693f52de62
11 changed files with 260 additions and 206 deletions
17
Cargo.lock
generated
17
Cargo.lock
generated
|
|
@ -1286,6 +1286,7 @@ dependencies = [
|
|||
"fabro-github",
|
||||
"fabro-graphviz",
|
||||
"fabro-llm",
|
||||
"fabro-retro",
|
||||
"fabro-types",
|
||||
"fabro-util",
|
||||
"fabro-workflows",
|
||||
|
|
@ -1554,6 +1555,21 @@ dependencies = [
|
|||
"tracing",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "fabro-retro"
|
||||
version = "0.174.0"
|
||||
dependencies = [
|
||||
"anyhow",
|
||||
"chrono",
|
||||
"fabro-agent",
|
||||
"fabro-llm",
|
||||
"fabro-util",
|
||||
"serde",
|
||||
"serde_json",
|
||||
"tempfile",
|
||||
"tokio",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "fabro-slack"
|
||||
version = "0.174.0"
|
||||
|
|
@ -1684,6 +1700,7 @@ dependencies = [
|
|||
"fabro-graphviz",
|
||||
"fabro-llm",
|
||||
"fabro-mcp",
|
||||
"fabro-retro",
|
||||
"fabro-ssh",
|
||||
"fabro-util",
|
||||
"futures",
|
||||
|
|
|
|||
|
|
@ -15,6 +15,7 @@ fabro-workflows = { path = "../fabro-workflows", features = ["exedev"] }
|
|||
fabro-github = { path = "../fabro-github" }
|
||||
fabro-agent = { path = "../fabro-agent" }
|
||||
fabro-llm = { path = "../fabro-llm" }
|
||||
fabro-retro = { path = "../fabro-retro" }
|
||||
fabro-util = { path = "../fabro-util" }
|
||||
fabro-db = { path = "../fabro-db" }
|
||||
fabro-types = { path = "../fabro-types" }
|
||||
|
|
|
|||
|
|
@ -649,18 +649,14 @@ async fn execute_run(state: Arc<AppState>, run_id: String) {
|
|||
|
||||
// Auto-derive retro and accumulate aggregate usage
|
||||
if let Some(ref cp) = checkpoint {
|
||||
let (failed, failure_reason) = match &result {
|
||||
Ok(_) => (false, None),
|
||||
Err(e) => (true, Some(e.to_string())),
|
||||
};
|
||||
let stage_durations = fabro_workflows::retro::extract_stage_durations(&config.run_dir);
|
||||
let retro = fabro_workflows::retro::derive_retro(
|
||||
let failed = result.is_err();
|
||||
let completed_stages = fabro_workflows::build_completed_stages(cp, failed);
|
||||
let stage_durations = fabro_retro::retro::extract_stage_durations(&config.run_dir);
|
||||
let retro = fabro_retro::retro::derive_retro(
|
||||
&run_id,
|
||||
"workflow",
|
||||
"",
|
||||
cp,
|
||||
failed,
|
||||
failure_reason.as_deref(),
|
||||
completed_stages,
|
||||
0,
|
||||
&stage_durations,
|
||||
);
|
||||
|
|
@ -1425,7 +1421,7 @@ async fn get_retro(
|
|||
return (StatusCode::OK, Json(serde_json::json!(null))).into_response();
|
||||
};
|
||||
|
||||
match fabro_workflows::retro::Retro::load(&run_dir) {
|
||||
match fabro_retro::retro::Retro::load(&run_dir) {
|
||||
Ok(retro) => (StatusCode::OK, Json(retro)).into_response(),
|
||||
Err(_) => (StatusCode::OK, Json(serde_json::json!(null))).into_response(),
|
||||
}
|
||||
|
|
|
|||
23
lib/crates/fabro-retro/Cargo.toml
Normal file
23
lib/crates/fabro-retro/Cargo.toml
Normal file
|
|
@ -0,0 +1,23 @@
|
|||
[package]
|
||||
name = "fabro-retro"
|
||||
edition.workspace = true
|
||||
version.workspace = true
|
||||
license.workspace = true
|
||||
description = "Retrospective analysis for Fabro workflow runs"
|
||||
|
||||
[lib]
|
||||
doctest = false
|
||||
|
||||
[dependencies]
|
||||
anyhow = "1"
|
||||
chrono = { workspace = true, features = ["serde"] }
|
||||
fabro-agent = { path = "../fabro-agent" }
|
||||
fabro-llm = { path = "../fabro-llm" }
|
||||
fabro-util = { path = "../fabro-util" }
|
||||
serde.workspace = true
|
||||
serde_json.workspace = true
|
||||
tokio.workspace = true
|
||||
|
||||
[dev-dependencies]
|
||||
tokio = { workspace = true, features = ["test-util", "macros"] }
|
||||
tempfile = "3"
|
||||
2
lib/crates/fabro-retro/src/lib.rs
Normal file
2
lib/crates/fabro-retro/src/lib.rs
Normal file
|
|
@ -0,0 +1,2 @@
|
|||
pub mod retro;
|
||||
pub mod retro_agent;
|
||||
|
|
@ -5,9 +5,21 @@ use std::path::Path;
|
|||
use chrono::{DateTime, Utc};
|
||||
use serde::{Deserialize, Serialize};
|
||||
|
||||
use crate::checkpoint::Checkpoint;
|
||||
use crate::error::Result;
|
||||
use crate::outcome::StageStatus;
|
||||
/// Flat summary of a completed stage, built by callers from their own
|
||||
/// checkpoint/outcome types to decouple retro derivation from the workflow
|
||||
/// engine internals.
|
||||
#[derive(Debug, Clone)]
|
||||
pub struct CompletedStage {
|
||||
pub node_id: String,
|
||||
pub status: String,
|
||||
pub succeeded: bool,
|
||||
pub failed: bool,
|
||||
pub retries: u32,
|
||||
pub cost: Option<f64>,
|
||||
pub notes: Option<String>,
|
||||
pub failure_reason: Option<String>,
|
||||
pub files_touched: Vec<String>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
||||
#[serde(rename_all = "snake_case")]
|
||||
|
|
@ -169,13 +181,17 @@ impl Retro {
|
|||
}
|
||||
|
||||
/// Save the retro as JSON to `run_dir/retro.json`.
|
||||
pub fn save(&self, run_dir: &Path) -> Result<()> {
|
||||
crate::save_json(self, &run_dir.join("retro.json"), "retro")
|
||||
pub fn save(&self, run_dir: &Path) -> anyhow::Result<()> {
|
||||
let json = serde_json::to_string_pretty(self)
|
||||
.map_err(|e| anyhow::anyhow!("retro serialize failed: {e}"))?;
|
||||
std::fs::write(run_dir.join("retro.json"), json)?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Load a retro from `run_dir/retro.json`.
|
||||
pub fn load(run_dir: &Path) -> Result<Self> {
|
||||
crate::load_json(&run_dir.join("retro.json"), "retro")
|
||||
pub fn load(run_dir: &Path) -> anyhow::Result<Self> {
|
||||
let data = std::fs::read_to_string(run_dir.join("retro.json"))?;
|
||||
serde_json::from_str(&data).map_err(|e| anyhow::anyhow!("retro deserialize failed: {e}"))
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -204,17 +220,14 @@ pub fn extract_stage_durations(run_dir: &Path) -> HashMap<String, u64> {
|
|||
durations
|
||||
}
|
||||
|
||||
/// Build a `Retro` from checkpoint data and run metadata. All qualitative
|
||||
/// Build a `Retro` from completed stage data and run metadata. All qualitative
|
||||
/// fields (`smoothness`, `intent`, `outcome`, etc.) are left as `None` for
|
||||
/// the retro agent to fill in.
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
pub fn derive_retro(
|
||||
run_id: &str,
|
||||
workflow_name: &str,
|
||||
goal: &str,
|
||||
checkpoint: &Checkpoint,
|
||||
run_failed: bool,
|
||||
_run_error: Option<&str>,
|
||||
completed_stages: Vec<CompletedStage>,
|
||||
duration_ms: u64,
|
||||
stage_durations: &HashMap<String, u64>,
|
||||
) -> Retro {
|
||||
|
|
@ -225,51 +238,35 @@ pub fn derive_retro(
|
|||
let mut stages_completed: usize = 0;
|
||||
let mut stages_failed: usize = 0;
|
||||
|
||||
for node_id in &checkpoint.completed_nodes {
|
||||
let outcome = checkpoint.node_outcomes.get(node_id);
|
||||
// node_retries stores attempts_used (1-indexed), convert to retry count
|
||||
let retries = checkpoint
|
||||
.node_retries
|
||||
.get(node_id)
|
||||
.copied()
|
||||
.unwrap_or(1)
|
||||
.saturating_sub(1);
|
||||
total_retries += retries;
|
||||
for cs in completed_stages {
|
||||
total_retries += cs.retries;
|
||||
|
||||
let status = outcome
|
||||
.map(|o| o.status.to_string())
|
||||
.unwrap_or_else(|| "unknown".to_string());
|
||||
|
||||
match outcome.map(|o| &o.status) {
|
||||
Some(StageStatus::Success | StageStatus::PartialSuccess) => stages_completed += 1,
|
||||
Some(StageStatus::Fail) => stages_failed += 1,
|
||||
_ => {}
|
||||
if cs.succeeded {
|
||||
stages_completed += 1;
|
||||
}
|
||||
if cs.failed {
|
||||
stages_failed += 1;
|
||||
}
|
||||
|
||||
let cost = outcome.and_then(|o| o.usage.as_ref()).and_then(|u| u.cost);
|
||||
if let Some(c) = cost {
|
||||
if let Some(c) = cs.cost {
|
||||
*total_cost.get_or_insert(0.0) += c;
|
||||
}
|
||||
|
||||
let files = outcome.map(|o| o.files_touched.clone()).unwrap_or_default();
|
||||
all_files.extend(files.iter().cloned());
|
||||
let dur = stage_durations.get(&cs.node_id).copied().unwrap_or(0);
|
||||
|
||||
stages.push(StageRetro {
|
||||
stage_id: node_id.clone(),
|
||||
stage_label: node_id.clone(),
|
||||
status,
|
||||
duration_ms: stage_durations.get(node_id).copied().unwrap_or(0),
|
||||
retries,
|
||||
cost,
|
||||
notes: outcome.and_then(|o| o.notes.clone()),
|
||||
failure_reason: outcome.and_then(|o| o.failure_reason().map(String::from)),
|
||||
files_touched: files,
|
||||
stage_label: cs.node_id.clone(),
|
||||
duration_ms: dur,
|
||||
retries: cs.retries,
|
||||
cost: cs.cost,
|
||||
stage_id: cs.node_id,
|
||||
status: cs.status,
|
||||
notes: cs.notes,
|
||||
failure_reason: cs.failure_reason,
|
||||
files_touched: cs.files_touched,
|
||||
});
|
||||
}
|
||||
|
||||
// If run failed with an error not captured in stages, record it
|
||||
if run_failed && stages_failed == 0 {
|
||||
stages_failed = 1;
|
||||
all_files.extend(stages.last().unwrap().files_touched.iter().cloned());
|
||||
}
|
||||
|
||||
all_files.sort();
|
||||
|
|
@ -303,60 +300,37 @@ pub fn derive_retro(
|
|||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::outcome::Outcome;
|
||||
|
||||
fn make_checkpoint_with_stages() -> Checkpoint {
|
||||
let mut node_outcomes = HashMap::new();
|
||||
let mut outcome_a = Outcome::success();
|
||||
outcome_a.notes = Some("Planned the approach".to_string());
|
||||
outcome_a.files_touched = vec!["src/main.rs".to_string()];
|
||||
outcome_a.usage = Some(crate::outcome::StageUsage {
|
||||
model: "claude-opus-4-6".to_string(),
|
||||
input_tokens: 1000,
|
||||
output_tokens: 500,
|
||||
cache_read_tokens: None,
|
||||
cache_write_tokens: None,
|
||||
reasoning_tokens: None,
|
||||
cost: Some(0.05),
|
||||
});
|
||||
node_outcomes.insert("plan".to_string(), outcome_a);
|
||||
|
||||
let mut outcome_b = Outcome::success();
|
||||
outcome_b.files_touched = vec!["src/main.rs".to_string(), "src/lib.rs".to_string()];
|
||||
outcome_b.usage = Some(crate::outcome::StageUsage {
|
||||
model: "claude-opus-4-6".to_string(),
|
||||
input_tokens: 2000,
|
||||
output_tokens: 1000,
|
||||
cache_read_tokens: None,
|
||||
cache_write_tokens: None,
|
||||
reasoning_tokens: None,
|
||||
cost: Some(0.10),
|
||||
});
|
||||
node_outcomes.insert("code".to_string(), outcome_b);
|
||||
|
||||
let mut node_retries = HashMap::new();
|
||||
// 2 attempts_used = 1 actual retry
|
||||
node_retries.insert("code".to_string(), 2u32);
|
||||
|
||||
Checkpoint {
|
||||
timestamp: Utc::now(),
|
||||
current_node: "code".to_string(),
|
||||
completed_nodes: vec!["plan".to_string(), "code".to_string()],
|
||||
node_retries,
|
||||
context_values: HashMap::new(),
|
||||
logs: Vec::new(),
|
||||
node_outcomes,
|
||||
next_node_id: None,
|
||||
git_commit_sha: None,
|
||||
loop_failure_signatures: HashMap::new(),
|
||||
restart_failure_signatures: HashMap::new(),
|
||||
node_visits: HashMap::new(),
|
||||
}
|
||||
fn make_completed_stages() -> Vec<CompletedStage> {
|
||||
vec![
|
||||
CompletedStage {
|
||||
node_id: "plan".to_string(),
|
||||
status: "success".to_string(),
|
||||
succeeded: true,
|
||||
failed: false,
|
||||
retries: 0,
|
||||
cost: Some(0.05),
|
||||
notes: Some("Planned the approach".to_string()),
|
||||
failure_reason: None,
|
||||
files_touched: vec!["src/main.rs".to_string()],
|
||||
},
|
||||
CompletedStage {
|
||||
node_id: "code".to_string(),
|
||||
status: "success".to_string(),
|
||||
succeeded: true,
|
||||
failed: false,
|
||||
retries: 1,
|
||||
cost: Some(0.10),
|
||||
notes: None,
|
||||
failure_reason: None,
|
||||
files_touched: vec!["src/main.rs".to_string(), "src/lib.rs".to_string()],
|
||||
},
|
||||
]
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn derive_retro_builds_stages_from_checkpoint() {
|
||||
let cp = make_checkpoint_with_stages();
|
||||
fn derive_retro_builds_stages() {
|
||||
let stages = make_completed_stages();
|
||||
let durations: HashMap<String, u64> =
|
||||
[("plan".to_string(), 5000), ("code".to_string(), 15000)]
|
||||
.into_iter()
|
||||
|
|
@ -366,9 +340,7 @@ mod tests {
|
|||
"run-1",
|
||||
"my_pipeline",
|
||||
"Fix the bug",
|
||||
&cp,
|
||||
false,
|
||||
None,
|
||||
stages.clone(),
|
||||
20000,
|
||||
&durations,
|
||||
);
|
||||
|
|
@ -394,45 +366,36 @@ mod tests {
|
|||
}
|
||||
|
||||
#[test]
|
||||
fn derive_retro_handles_failed_run() {
|
||||
let cp = Checkpoint {
|
||||
timestamp: Utc::now(),
|
||||
current_node: "start".to_string(),
|
||||
completed_nodes: vec!["start".to_string()],
|
||||
node_retries: HashMap::new(),
|
||||
context_values: HashMap::new(),
|
||||
logs: Vec::new(),
|
||||
node_outcomes: {
|
||||
let mut m = HashMap::new();
|
||||
m.insert("start".to_string(), Outcome::success());
|
||||
m
|
||||
},
|
||||
next_node_id: None,
|
||||
git_commit_sha: None,
|
||||
loop_failure_signatures: HashMap::new(),
|
||||
restart_failure_signatures: HashMap::new(),
|
||||
node_visits: HashMap::new(),
|
||||
};
|
||||
fn derive_retro_handles_failed_stage() {
|
||||
let stages = vec![CompletedStage {
|
||||
node_id: "start".to_string(),
|
||||
status: "fail".to_string(),
|
||||
succeeded: false,
|
||||
failed: true,
|
||||
retries: 0,
|
||||
cost: None,
|
||||
notes: None,
|
||||
failure_reason: Some("boom".to_string()),
|
||||
files_touched: vec![],
|
||||
}];
|
||||
|
||||
let retro = derive_retro(
|
||||
"run-2",
|
||||
"pipe",
|
||||
"goal",
|
||||
&cp,
|
||||
true,
|
||||
Some("boom"),
|
||||
stages.clone(),
|
||||
5000,
|
||||
&HashMap::new(),
|
||||
);
|
||||
|
||||
assert_eq!(retro.stats.stages_failed, 1);
|
||||
assert_eq!(retro.stats.stages_completed, 1);
|
||||
assert_eq!(retro.stats.stages_completed, 0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn apply_narrative_merges_fields() {
|
||||
let cp = make_checkpoint_with_stages();
|
||||
let mut retro = derive_retro("r1", "p", "g", &cp, false, None, 1000, &HashMap::new());
|
||||
let stages = make_completed_stages();
|
||||
let mut retro = derive_retro("r1", "p", "g", stages.clone(), 1000, &HashMap::new());
|
||||
|
||||
let narrative = RetroNarrative {
|
||||
smoothness: SmoothnessRating::Smooth,
|
||||
|
|
@ -465,17 +428,8 @@ mod tests {
|
|||
#[test]
|
||||
fn save_and_load_roundtrip() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let cp = make_checkpoint_with_stages();
|
||||
let mut retro = derive_retro(
|
||||
"r1",
|
||||
"pipe",
|
||||
"goal",
|
||||
&cp,
|
||||
false,
|
||||
None,
|
||||
1000,
|
||||
&HashMap::new(),
|
||||
);
|
||||
let stages = make_completed_stages();
|
||||
let mut retro = derive_retro("r1", "pipe", "goal", stages.clone(), 1000, &HashMap::new());
|
||||
retro.smoothness = Some(SmoothnessRating::Bumpy);
|
||||
retro.intent = Some("Test intent".to_string());
|
||||
|
||||
|
|
@ -11,7 +11,6 @@ use fabro_llm::provider::Provider;
|
|||
use fabro_llm::types::ToolDefinition;
|
||||
use tokio::task::JoinHandle;
|
||||
|
||||
use crate::event::EventEmitter;
|
||||
use crate::retro::RetroNarrative;
|
||||
|
||||
const RETRO_SYSTEM_PROMPT: &str = r#"You are a workflow run retrospective analyst. Your job is to analyze a completed workflow run and generate a structured retrospective.
|
||||
|
|
@ -119,7 +118,7 @@ pub async fn run_retro_agent(
|
|||
llm_client: &Client,
|
||||
provider: Provider,
|
||||
model: &str,
|
||||
emitter: Option<Arc<EventEmitter>>,
|
||||
event_callback: Option<Arc<dyn Fn(SessionEvent) + Send + Sync>>,
|
||||
) -> anyhow::Result<RetroNarrative> {
|
||||
// Upload data files into sandbox (needed for Daytona; no-op effect for local
|
||||
// since the agent can also read from the original paths via tools).
|
||||
|
|
@ -172,10 +171,8 @@ pub async fn run_retro_agent(
|
|||
let rx = session.subscribe();
|
||||
let event_writer_handle = spawn_retro_event_writer(rx, retro_dir.join("retro_session.jsonl"));
|
||||
|
||||
// Optionally forward agent events to the workflow emitter for progress display
|
||||
let event_forwarder_handle = emitter
|
||||
.as_ref()
|
||||
.map(|em| spawn_retro_event_forwarder(&session, Arc::clone(em)));
|
||||
// Optionally forward agent events via the callback
|
||||
let event_forwarder_handle = event_callback.map(|cb| spawn_retro_event_forwarder(&session, cb));
|
||||
|
||||
session.initialize().await;
|
||||
|
||||
|
|
@ -313,34 +310,15 @@ fn spawn_retro_event_writer(
|
|||
})
|
||||
}
|
||||
|
||||
/// Spawn a background task that forwards non-streaming session events to
|
||||
/// the workflow `EventEmitter`, enabling `ProgressUI` to show tool call
|
||||
/// spinners under the retro stage.
|
||||
fn spawn_retro_event_forwarder(session: &Session, emitter: Arc<EventEmitter>) -> JoinHandle<()> {
|
||||
use crate::event::WorkflowRunEvent;
|
||||
use fabro_agent::AgentEvent;
|
||||
|
||||
/// Spawn a background task that forwards session events via the provided callback.
|
||||
fn spawn_retro_event_forwarder(
|
||||
session: &Session,
|
||||
callback: Arc<dyn Fn(SessionEvent) + Send + Sync>,
|
||||
) -> JoinHandle<()> {
|
||||
let mut rx = session.subscribe();
|
||||
tokio::spawn(async move {
|
||||
while let Ok(event) = rx.recv().await {
|
||||
emitter.touch();
|
||||
|
||||
if !matches!(
|
||||
&event.event,
|
||||
AgentEvent::SessionStarted
|
||||
| AgentEvent::SessionEnded
|
||||
| AgentEvent::AssistantTextStart
|
||||
| AgentEvent::AssistantOutputReplace { .. }
|
||||
| AgentEvent::TextDelta { .. }
|
||||
| AgentEvent::ReasoningDelta { .. }
|
||||
| AgentEvent::ToolCallOutputDelta { .. }
|
||||
| AgentEvent::SkillExpanded { .. }
|
||||
) {
|
||||
emitter.emit(&WorkflowRunEvent::Agent {
|
||||
stage: "retro".to_string(),
|
||||
event: event.event.clone(),
|
||||
});
|
||||
}
|
||||
callback(event);
|
||||
}
|
||||
})
|
||||
}
|
||||
|
|
@ -30,6 +30,7 @@ fabro-github = { path = "../fabro-github" }
|
|||
fabro-util = { path = "../fabro-util" }
|
||||
fabro-git-storage = { path = "../fabro-git-storage" }
|
||||
fabro-llm = { path = "../fabro-llm" }
|
||||
fabro-retro = { path = "../fabro-retro" }
|
||||
thiserror.workspace = true
|
||||
serde.workspace = true
|
||||
serde_json.workspace = true
|
||||
|
|
|
|||
|
|
@ -1242,7 +1242,7 @@ pub async fn run_command(
|
|||
|
||||
// Load checkpoint and stage durations to populate per-stage data
|
||||
let checkpoint = Checkpoint::load(&run_dir.join("checkpoint.json")).ok();
|
||||
let stage_durations = crate::retro::extract_stage_durations(&run_dir);
|
||||
let stage_durations = fabro_retro::retro::extract_stage_durations(&run_dir);
|
||||
|
||||
let (stages, total_cost, total_retries) = if let Some(ref cp) = checkpoint {
|
||||
let mut stages = Vec::new();
|
||||
|
|
@ -1293,12 +1293,9 @@ pub async fn run_command(
|
|||
|
||||
// Auto-derive retro (always, cheap) and optionally run retro agent
|
||||
if !args.no_retro && super::project_config::is_retro_enabled() {
|
||||
let (failed, failure_reason) = match &engine_result {
|
||||
Ok(ref o) => (
|
||||
o.status == StageStatus::Fail,
|
||||
o.failure_reason().map(String::from),
|
||||
),
|
||||
Err(e) => (true, Some(e.to_string())),
|
||||
let failed = match &engine_result {
|
||||
Ok(ref o) => o.status == StageStatus::Fail,
|
||||
Err(_) => true,
|
||||
};
|
||||
generate_retro(
|
||||
&config.run_id,
|
||||
|
|
@ -1306,7 +1303,6 @@ pub async fn run_command(
|
|||
graph.goal(),
|
||||
&run_dir,
|
||||
failed,
|
||||
failure_reason.as_deref(),
|
||||
run_duration_ms,
|
||||
dry_run_mode,
|
||||
llm_client.as_ref(),
|
||||
|
|
@ -1804,12 +1800,9 @@ async fn run_from_branch(
|
|||
|
||||
// Auto-derive retro
|
||||
if !args.no_retro && super::project_config::is_retro_enabled() {
|
||||
let (failed, failure_reason) = match &engine_result {
|
||||
Ok(ref o) => (
|
||||
o.status == StageStatus::Fail,
|
||||
o.failure_reason().map(String::from),
|
||||
),
|
||||
Err(e) => (true, Some(e.to_string())),
|
||||
let failed = match &engine_result {
|
||||
Ok(ref o) => o.status == StageStatus::Fail,
|
||||
Err(_) => true,
|
||||
};
|
||||
|
||||
let llm_client = if dry_run_mode {
|
||||
|
|
@ -1824,7 +1817,6 @@ async fn run_from_branch(
|
|||
graph.goal(),
|
||||
&run_dir,
|
||||
failed,
|
||||
failure_reason.as_deref(),
|
||||
run_duration_ms,
|
||||
dry_run_mode,
|
||||
llm_client.as_ref(),
|
||||
|
|
@ -2308,7 +2300,6 @@ async fn generate_retro(
|
|||
goal: &str,
|
||||
run_dir: &std::path::Path,
|
||||
failed: bool,
|
||||
failure_reason: Option<&str>,
|
||||
run_duration_ms: u64,
|
||||
dry_run_mode: bool,
|
||||
llm_client: Option<&fabro_llm::client::Client>,
|
||||
|
|
@ -2329,14 +2320,13 @@ async fn generate_retro(
|
|||
}
|
||||
};
|
||||
|
||||
let stage_durations = crate::retro::extract_stage_durations(run_dir);
|
||||
let mut retro = crate::retro::derive_retro(
|
||||
let completed_stages = crate::build_completed_stages(&cp, failed);
|
||||
let stage_durations = fabro_retro::retro::extract_stage_durations(run_dir);
|
||||
let mut retro = fabro_retro::retro::derive_retro(
|
||||
run_id,
|
||||
workflow_name,
|
||||
goal,
|
||||
&cp,
|
||||
failed,
|
||||
failure_reason,
|
||||
completed_stages,
|
||||
run_duration_ms,
|
||||
&stage_durations,
|
||||
);
|
||||
|
|
@ -2365,15 +2355,41 @@ async fn generate_retro(
|
|||
}
|
||||
|
||||
let narrative_result = if dry_run_mode {
|
||||
Ok(crate::retro_agent::dry_run_narrative())
|
||||
Ok(fabro_retro::retro_agent::dry_run_narrative())
|
||||
} else if let Some(client) = llm_client {
|
||||
crate::retro_agent::run_retro_agent(
|
||||
let emitter_clone = emitter.clone();
|
||||
let event_callback: Option<Arc<dyn Fn(fabro_agent::SessionEvent) + Send + Sync>> =
|
||||
emitter_clone.map(
|
||||
|em| -> Arc<dyn Fn(fabro_agent::SessionEvent) + Send + Sync> {
|
||||
Arc::new(move |event: fabro_agent::SessionEvent| {
|
||||
em.touch();
|
||||
|
||||
if !matches!(
|
||||
&event.event,
|
||||
fabro_agent::AgentEvent::SessionStarted
|
||||
| fabro_agent::AgentEvent::SessionEnded
|
||||
| fabro_agent::AgentEvent::AssistantTextStart
|
||||
| fabro_agent::AgentEvent::AssistantOutputReplace { .. }
|
||||
| fabro_agent::AgentEvent::TextDelta { .. }
|
||||
| fabro_agent::AgentEvent::ReasoningDelta { .. }
|
||||
| fabro_agent::AgentEvent::ToolCallOutputDelta { .. }
|
||||
| fabro_agent::AgentEvent::SkillExpanded { .. }
|
||||
) {
|
||||
em.emit(&crate::event::WorkflowRunEvent::Agent {
|
||||
stage: "retro".to_string(),
|
||||
event: event.event.clone(),
|
||||
});
|
||||
}
|
||||
})
|
||||
},
|
||||
);
|
||||
fabro_retro::retro_agent::run_retro_agent(
|
||||
sandbox,
|
||||
run_dir,
|
||||
client,
|
||||
provider_enum,
|
||||
model,
|
||||
emitter.clone(),
|
||||
event_callback,
|
||||
)
|
||||
.await
|
||||
} else {
|
||||
|
|
|
|||
|
|
@ -25,6 +25,74 @@ pub(crate) fn load_json<T: serde::de::DeserializeOwned>(
|
|||
.map_err(|e| error::FabroError::Checkpoint(format!("{label} deserialize failed: {e}")))
|
||||
}
|
||||
|
||||
/// Build `Vec<CompletedStage>` from a `Checkpoint`, mapping workflow-engine
|
||||
/// types into the flat struct expected by `fabro_retro::retro::derive_retro`.
|
||||
pub fn build_completed_stages(
|
||||
cp: &checkpoint::Checkpoint,
|
||||
run_failed: bool,
|
||||
) -> Vec<fabro_retro::retro::CompletedStage> {
|
||||
use outcome::StageStatus;
|
||||
|
||||
let mut stages = Vec::new();
|
||||
let mut any_stage_failed = false;
|
||||
|
||||
for node_id in &cp.completed_nodes {
|
||||
let outcome = cp.node_outcomes.get(node_id);
|
||||
let retries = cp
|
||||
.node_retries
|
||||
.get(node_id)
|
||||
.copied()
|
||||
.unwrap_or(1)
|
||||
.saturating_sub(1);
|
||||
|
||||
let status = outcome
|
||||
.map(|o| o.status.to_string())
|
||||
.unwrap_or_else(|| "unknown".to_string());
|
||||
|
||||
let succeeded = matches!(
|
||||
outcome.map(|o| &o.status),
|
||||
Some(StageStatus::Success | StageStatus::PartialSuccess)
|
||||
);
|
||||
let failed = matches!(outcome.map(|o| &o.status), Some(StageStatus::Fail));
|
||||
if failed {
|
||||
any_stage_failed = true;
|
||||
}
|
||||
|
||||
stages.push(fabro_retro::retro::CompletedStage {
|
||||
node_id: node_id.clone(),
|
||||
status,
|
||||
succeeded,
|
||||
failed,
|
||||
retries,
|
||||
cost: outcome.and_then(|o| o.usage.as_ref()).and_then(|u| u.cost),
|
||||
notes: outcome.and_then(|o| o.notes.clone()),
|
||||
failure_reason: outcome.and_then(|o| o.failure_reason().map(String::from)),
|
||||
files_touched: outcome.map(|o| o.files_touched.clone()).unwrap_or_default(),
|
||||
});
|
||||
}
|
||||
|
||||
// If run failed with an error not captured in stages, mark the last stage
|
||||
if run_failed && !any_stage_failed {
|
||||
if let Some(last) = stages.last_mut() {
|
||||
last.failed = true;
|
||||
} else {
|
||||
stages.push(fabro_retro::retro::CompletedStage {
|
||||
node_id: "unknown".to_string(),
|
||||
status: "fail".to_string(),
|
||||
succeeded: false,
|
||||
failed: true,
|
||||
retries: 0,
|
||||
cost: None,
|
||||
notes: None,
|
||||
failure_reason: None,
|
||||
files_touched: vec![],
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
stages
|
||||
}
|
||||
|
||||
pub mod artifact;
|
||||
pub mod asset_snapshot;
|
||||
pub mod checkpoint;
|
||||
|
|
@ -45,8 +113,6 @@ pub mod manifest;
|
|||
pub mod outcome;
|
||||
pub mod preamble;
|
||||
pub mod pull_request;
|
||||
pub mod retro;
|
||||
pub mod retro_agent;
|
||||
pub mod run_status;
|
||||
pub mod sandbox_record;
|
||||
pub mod stylesheet;
|
||||
|
|
|
|||
|
|
@ -6,7 +6,7 @@ use tracing::{debug, info};
|
|||
use fabro_github::{self as github_app, ssh_url_to_https, GitHubAppCredentials};
|
||||
|
||||
use crate::conclusion::Conclusion;
|
||||
use crate::retro::Retro;
|
||||
use fabro_retro::retro::Retro;
|
||||
|
||||
/// Record of a pull request created for a workflow run.
|
||||
#[derive(Debug, Clone, Serialize, Deserialize)]
|
||||
|
|
@ -428,10 +428,10 @@ pub async fn maybe_open_pull_request(
|
|||
mod tests {
|
||||
use super::*;
|
||||
use crate::conclusion::StageSummary;
|
||||
use crate::retro::{
|
||||
use chrono::Utc;
|
||||
use fabro_retro::retro::{
|
||||
AggregateStats, FrictionKind, FrictionPoint, OpenItem, OpenItemKind, StageRetro,
|
||||
};
|
||||
use chrono::Utc;
|
||||
|
||||
fn make_test_conclusion() -> Conclusion {
|
||||
Conclusion {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue