Fix clippy warnings in fabro-workflow

- run_dump: take &Path instead of PathBuf by value in path helpers
- test_support: remove empty no-op persist_run_artifacts_for_tests
- agent.rs: use u32::try_from instead of as u32 cast
- retro.rs: remove unnecessary let binding
- pull_request.rs: use NodeState::default() instead of Default::default()
- execute/tests.rs: replace bool::then in filter_map with filter+map

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Bryan Helmkamp 2026-04-03 15:52:49 -07:00
parent bb70face17
commit 79e48ddaea
6 changed files with 29 additions and 36 deletions

View file

@ -673,7 +673,8 @@ mod tests {
) -> Result<CodergenResult, FabroError> {
emitter.emit(&crate::event::WorkflowRunEvent::Agent {
stage: node.id.clone(),
visit: crate::run_dir::visit_from_context(context) as u32,
visit: u32::try_from(crate::run_dir::visit_from_context(context))
.unwrap_or(u32::MAX),
event: fabro_agent::AgentEvent::SessionStarted {
provider: Some("openai".to_string()),
model: Some("gpt-5.4".to_string()),

View file

@ -936,10 +936,8 @@ async fn retry_emits_stage_started_per_attempt() {
let collected = events.lock().unwrap();
let work_started: Vec<_> = collected
.iter()
.filter_map(|event| {
(event.event == "stage.started" && event.node_id.as_deref() == Some("work"))
.then(|| event.properties["attempt"].as_u64().unwrap())
})
.filter(|event| event.event == "stage.started" && event.node_id.as_deref() == Some("work"))
.map(|event| event.properties["attempt"].as_u64().unwrap())
.collect();
assert_eq!(work_started, vec![1, 2]);
}
@ -1060,16 +1058,15 @@ async fn git_checkpoint_skips_start_node() {
let collected = events.lock().unwrap();
let checkpoint_node_ids: Vec<&str> = collected
.iter()
.filter_map(|event| {
(event.event == "checkpoint.completed"
.filter(|event| {
event.event == "checkpoint.completed"
&& event
.properties
.get("git_commit_sha")
.and_then(|value| value.as_str())
.is_some())
.then(|| event.node_id.as_deref())
.flatten()
.is_some()
})
.filter_map(|event| event.node_id.as_deref())
.collect();
assert!(!checkpoint_node_ids.contains(&"start"));
assert!(checkpoint_node_ids.contains(&"work"));

View file

@ -990,9 +990,10 @@ mod tests {
#[test]
fn read_plan_text_not_found() {
let mut state = RunState::default();
state
.nodes
.insert(("implement".to_string(), 1), Default::default());
state.nodes.insert(
("implement".to_string(), 1),
fabro_store::NodeState::default(),
);
let result = read_plan_text(&state);
assert_eq!(result, None);

View file

@ -191,7 +191,7 @@ mod tests {
context.set("response.work", serde_json::json!("done"));
let mut outcomes = HashMap::new();
outcomes.insert("work".to_string(), crate::outcome::Outcome::success());
let checkpoint = Checkpoint::from_context(
Checkpoint::from_context(
&context,
"work",
vec!["work".to_string()],
@ -201,8 +201,7 @@ mod tests {
HashMap::new(),
HashMap::new(),
HashMap::new(),
);
checkpoint
)
}
fn test_store() -> Arc<SlateStore> {

View file

@ -66,7 +66,7 @@ impl RunDump {
if let Some(status) = node.status.as_ref() {
push_json_entry_path(
&mut entries,
metadata_node_file_path(node_id, *visit, "status.json").into(),
&PathBuf::from(metadata_node_file_path(node_id, *visit, "status.json")),
status,
);
}
@ -153,28 +153,28 @@ impl RunDump {
if let Some(prompt) = node.prompt.as_ref() {
entries.push(RunDumpEntry::text_path(
base.join("prompt.md"),
&base.join("prompt.md"),
prompt.clone(),
));
}
if let Some(response) = node.response.as_ref() {
entries.push(RunDumpEntry::text_path(
base.join("response.md"),
&base.join("response.md"),
response.clone(),
));
}
if let Some(status) = node.status.as_ref() {
push_json_entry_path(&mut entries, base.join("status.json"), status);
push_json_entry_path(&mut entries, &base.join("status.json"), status);
}
if let Some(stdout) = node.stdout.as_ref() {
entries.push(RunDumpEntry::text_path(
base.join("stdout.log"),
&base.join("stdout.log"),
stdout.clone(),
));
}
if let Some(stderr) = node.stderr.as_ref() {
entries.push(RunDumpEntry::text_path(
base.join("stderr.log"),
&base.join("stderr.log"),
stderr.clone(),
));
}
@ -197,7 +197,7 @@ impl RunDump {
for (seq, checkpoint) in &state.checkpoints {
push_json_entry_path(
&mut entries,
PathBuf::from("checkpoints").join(format!("{seq:04}.json")),
&PathBuf::from("checkpoints").join(format!("{seq:04}.json")),
checkpoint,
);
}
@ -211,7 +211,7 @@ impl RunDump {
format!("artifact value {artifact_id:?} is missing from the store")
})?;
entries.push(RunDumpEntry::json_path(
PathBuf::from("artifacts")
&PathBuf::from("artifacts")
.join("values")
.join(format!("{}.json", artifact_id_segment.display())),
value,
@ -234,7 +234,7 @@ impl RunDump {
)
})?;
entries.push(RunDumpEntry::bytes_path(
PathBuf::from("artifacts")
&PathBuf::from("artifacts")
.join("nodes")
.join(node_id_segment)
.join(format!("visit-{visit}"))
@ -293,7 +293,7 @@ impl RunDumpEntry {
}
}
fn text_path(path: PathBuf, contents: String) -> Self {
fn text_path(path: &Path, contents: String) -> Self {
Self {
path: path_to_string(path),
contents: RunDumpContents::Text(contents),
@ -307,7 +307,7 @@ impl RunDumpEntry {
}
}
fn json_path(path: PathBuf, contents: serde_json::Value) -> Self {
fn json_path(path: &Path, contents: serde_json::Value) -> Self {
Self {
path: path_to_string(path),
contents: RunDumpContents::Json(contents),
@ -321,7 +321,7 @@ impl RunDumpEntry {
}
}
fn bytes_path(path: PathBuf, contents: Vec<u8>) -> Self {
fn bytes_path(path: &Path, contents: Vec<u8>) -> Self {
Self {
path: path_to_string(path),
contents: RunDumpContents::Bytes(contents),
@ -357,7 +357,7 @@ where
}
}
fn push_json_entry_path<T>(entries: &mut Vec<RunDumpEntry>, path: PathBuf, value: &T)
fn push_json_entry_path<T>(entries: &mut Vec<RunDumpEntry>, path: &Path, value: &T)
where
T: serde::Serialize,
{
@ -374,7 +374,7 @@ fn metadata_node_file_path(node_id: &str, visit: u32, filename: &str) -> String
}
}
fn path_to_string(path: PathBuf) -> String {
fn path_to_string(path: &Path) -> String {
path.to_string_lossy().into_owned()
}

View file

@ -6,7 +6,7 @@ use std::time::Duration;
use chrono::Utc;
use fabro_agent::Sandbox;
use fabro_graphviz::graph::Graph as GvGraph;
use fabro_store::{SlateRunStore, SlateStore};
use fabro_store::SlateStore;
use object_store::memory::InMemory;
use crate::error::Result;
@ -127,7 +127,6 @@ pub async fn run_graph(
)
.await;
let executed = pipeline::execute(initialized).await;
persist_run_artifacts_for_tests(executed.run_store.as_ref(), &run_options.run_dir).await;
executed.outcome
}
@ -154,7 +153,6 @@ pub async fn run_graph_with_hooks(
)
.await;
let executed = pipeline::execute(initialized).await;
persist_run_artifacts_for_tests(executed.run_store.as_ref(), &run_options.run_dir).await;
executed.outcome
}
@ -180,12 +178,9 @@ pub async fn run_graph_from_checkpoint(
)
.await;
let executed = pipeline::execute(initialized).await;
persist_run_artifacts_for_tests(executed.run_store.as_ref(), &run_options.run_dir).await;
executed.outcome
}
async fn persist_run_artifacts_for_tests(_run_store: &SlateRunStore, _run_dir: &std::path::Path) {}
pub struct WorkflowRunner {
registry: std::sync::Mutex<Option<HandlerRegistry>>,
emitter: Arc<EventEmitter>,