From eab49f4d5283345cc8c4b9c3172d4ca5916fde39 Mon Sep 17 00:00:00 2001 From: David Hagerty Date: Mon, 9 Feb 2026 10:38:19 -0500 Subject: [PATCH] =?UTF-8?q?fix:=20address=20Phase=201c=20re-review=20?= =?UTF-8?q?=E2=80=94=20dry=5Frun=20guard,=20clippy,=20fmt?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Move dry_run check BEFORE import_goal call to prevent database writes - Remove redundant closures in session row-mapping (clippy) - Remove unused imports from tests - Run cargo fmt for consistent formatting Co-Authored-By: Claude Opus 4.6 --- src/graph/session.rs | 4 ++-- src/main.rs | 49 +++++++++++++++++++-------------------- tests/interchange_test.rs | 14 ++++++----- 3 files changed, 34 insertions(+), 33 deletions(-) diff --git a/src/graph/session.rs b/src/graph/session.rs index c25f129..696f6b8 100644 --- a/src/graph/session.rs +++ b/src/graph/session.rs @@ -124,7 +124,7 @@ impl SessionStore { )?; let session: Option = stmt - .query_row([&session_id_owned], |row| map_session_row(row)) + .query_row([&session_id_owned], map_session_row) .optional()?; Ok(session) @@ -150,7 +150,7 @@ impl SessionStore { )?; let session: Option = stmt - .query_row([&goal_id_owned], |row| map_session_row(row)) + .query_row([&goal_id_owned], map_session_row) .optional()?; Ok(session) diff --git a/src/main.rs b/src/main.rs index 4532d22..d1d2db8 100644 --- a/src/main.rs +++ b/src/main.rs @@ -691,15 +691,28 @@ async fn main() -> anyhow::Result<()> { rustagent::graph::interchange::ImportStrategy::Merge }; - match rustagent::graph::interchange::import_goal( - &graph_store, - &content, - strategy, - ) - .await - { - Ok(result) => { - if !dry_run { + if dry_run { + // Parse the TOML and show what would be imported without writing + match toml::from_str::( + &content, + ) { + Ok(goal_file) => { + println!("[DRY RUN] Changes that would be applied:"); + println!(" Nodes to process: {}", goal_file.nodes.len()); + println!(" Edges to process: {}", goal_file.edges.len()); + println!(" Import strategy: {:?}", strategy); + } + Err(e) => println!("Failed to parse TOML: {}", e), + } + } else { + match rustagent::graph::interchange::import_goal( + &graph_store, + &content, + strategy, + ) + .await + { + Ok(result) => { println!(" Added nodes: {}", result.added_nodes); println!(" Added edges: {}", result.added_edges); println!(" Unchanged: {}", result.unchanged); @@ -709,30 +722,16 @@ async fn main() -> anyhow::Result<()> { if !result.skipped_edges.is_empty() { println!(" Skipped edges: {}", result.skipped_edges.len()); } - } else { - // Parse the TOML and show what would be imported - match toml::from_str::( - &content, - ) { - Ok(goal_file) => { - println!("[DRY RUN] Changes that would be applied:"); - println!(" Nodes to process: {}", goal_file.nodes.len()); - println!(" Edges to process: {}", goal_file.edges.len()); - println!(" Import strategy: {:?}", strategy); - } - Err(e) => println!("Failed to parse TOML: {}", e), - } } + Err(e) => println!("Import failed: {}", e), } - Err(e) => println!("Import failed: {}", e), } } Err(e) => println!("Failed to read file: {}", e), }, GraphAction::Diff { path } => match std::fs::read_to_string(&path) { Ok(content) => { - match rustagent::graph::interchange::diff_goal(&graph_store, &content) - .await + match rustagent::graph::interchange::diff_goal(&graph_store, &content).await { Ok(result) => { println!("Diff results for {}:", path); diff --git a/tests/interchange_test.rs b/tests/interchange_test.rs index 5ae25bd..650eef5 100644 --- a/tests/interchange_test.rs +++ b/tests/interchange_test.rs @@ -1,10 +1,8 @@ use anyhow::Result; use chrono::Utc; use rustagent::graph::interchange::{ImportStrategy, diff_goal, export_goal, import_goal}; -use rustagent::graph::store::{GraphStore, SqliteGraphStore}; +use rustagent::graph::store::GraphStore; use rustagent::graph::*; -use std::collections::HashMap; - mod common; use common::*; @@ -251,7 +249,7 @@ async fn test_import_skips_edges_with_missing_nodes() -> Result<()> { let toml_str = export_goal(&graph_store, "ra-test", "test-project").await?; // Modify the TOML to add a new node that doesn't exist and an edge to it - let mut toml_content = toml_str.clone(); + let _toml_content = toml_str.clone(); // Parse and modify let mut parsed: toml::Value = toml::from_str(&toml_str)?; @@ -378,8 +376,12 @@ async fn test_round_trip_export_import() -> Result<()> { let parsed_export2: toml::Value = toml::from_str(&export2)?; // Verify nodes are identical between exports (at minimum the counts should match) - let nodes1 = parsed_export1["nodes"].as_table().expect("Export should have nodes"); - let nodes2 = parsed_export2["nodes"].as_table().expect("Import export should have nodes"); + let nodes1 = parsed_export1["nodes"] + .as_table() + .expect("Export should have nodes"); + let nodes2 = parsed_export2["nodes"] + .as_table() + .expect("Import export should have nodes"); // After round-trip, we should have at least the goal node and ideally all original nodes // Verify goal exists in both -- 2.51.2