cargo-rail: a tolerable cleanup to the readme.md; started working throught the performance fixes: batching, parallelism, efficiencies/caches.

Author: loadingalias

TODO.md

@@ -16,64 +16,42 @@
 
 All command flag support: ✅ COMPLETE (--dry-run, --json, --apply across all commands)
 
-Build status: ✅ 113 tests passing, zero warnings, cargo deny/audit passing
+Build status: ✅ 115 tests passing, zero warnings, cargo deny/audit passing
 
 ---
 
 ## Week 1: Performance + Foundation (10 hours)
 
 ### Critical: Unified Domain Model (NEW - 2 hours)
 
-- [ ] Create `WorkspaceContext` struct - *1 hour*
-  - Unifies `WorkspaceMetadata`, `WorkspaceGraph`, `RailConfig`, `root`
-  - Build once in `main.rs`, pass `&WorkspaceContext` to all commands
-  - Eliminates redundant metadata loads (currently: split.rs:111, sync.rs:362-364)
-  - Foundation for visibility/tier annotations
-
-- [ ] Wire `WorkspaceContext` into all commands - *1 hour*
-  - Refactor: `split`, `sync`, `graph`, `release`, `lint`, `doctor`
-  - Remove individual metadata/config loading
-  - Pass context by reference
+- [x] Create `WorkspaceContext` struct + wire into graph commands
+- [x] Made WorkspaceGraph thread-safe (RefCell → RwLock)
 
 ### Critical: Performance Fixes (6 hours)
 
 #### Subprocess N+1 Patterns (CRITICAL - 5-10s each)
 
-- [ ] Cache HEAD commit in sync loops (sync.rs:514, 613) - *15 min*
-  - Currently calls `head_commit()` subprocess inside commit loop
-  - Update cached head after each commit instead
+- [x] Cache HEAD commit in sync loops
 
 #### Bulk Operations Not Integrated (CRITICAL - 50-250s)
 
-- [ ] Use `read_files_bulk()` in apply_mono_commit_to_remote (sync.rs:462-503) - *60 min*
-- [ ] Use `read_files_bulk()` in apply_remote_commit_to_mono (sync.rs:550-602) - *60 min*
-- [ ] Use `read_files_bulk()` in resolve_conflicts_for_commit (sync.rs:715, 724) - *30 min*
-  - Bulk API exists (system_git_ops.rs:627) but not used in hot paths
-  - Sequential file reads = 500-50,000 subprocess calls vs. <500ms batch
+- [x] Use `read_files_bulk()` in apply_mono_commit_to_remote
+- [x] Use `read_files_bulk()` in apply_remote_commit_to_mono
+- [x] Use `read_files_bulk()` in resolve_conflicts_for_commit
 
 #### Algorithm O(n) → O(1) (CRITICAL - 100-500ms)
 
-- [ ] Build reverse mapping index in MappingStore - *30 min*
-  - sync.rs:296, 432 scan all values with `.values().any()`
-  - Add `reverse_mappings: HashMap<String, String>` for O(1) lookup
+- [x] Build reverse mapping index in MappingStore
 
 #### Quadratic → Linear (HIGH - 100ms-1s)
 
-- [ ] Cache conflict detection in resolve_conflicts_for_commit (sync.rs:665-781) - *60 min*
-  - Currently: `get_commits_touching_path()` per file (20 files = 20 subprocess calls)
-  - Solution: Get all mono changes once, build HashSet for O(1) lookup
+- [x] Cache conflict detection in resolve_conflicts_for_commit
 
 #### Caching (HIGH - 10-50ms)
 
-- [ ] Cache `get_changed_files()` results (sync.rs:446, 541, 673) - *30 min*
-  - Same commit parsed 3x (resolve → apply → check)
-  - Pass through call chain or cache on CommitInfo
-
-- [ ] Wrap config.security in Arc<> for parallel execution (sync.rs:363-366) - *20 min*
-  - Currently deep clones per parallel task
-
-- [ ] Cache mapping loads in SyncEngine (sync.rs:136, 263, 686, 794, 818) - *20 min*
-  - 5 subprocess calls × 10ms = 50ms per sync
+- [x] Cache `get_changed_files()` results
+- [x] Wrap config.security in Arc<> for parallel execution
+- [x] Cache mapping loads in SyncEngine
 
 #### Minor Optimizations (MEDIUM - 20-50ms)
 
@@ -120,7 +98,14 @@ Build status: ✅ 113 tests passing, zero warnings, cargo deny/audit passing
 - [ ] Address release.rs:66 skip_sync TODO - *15 min*
   - Implement or document deferral
 
-**Week 1 total**: ~10 hours
+**Week 1 Performance Complete**: All critical performance optimizations done
+- Reverse mapping index: O(1) lookups (100-500ms saved per sync)
+- Conflict detection cache: Single git call vs N calls (100ms-1s saved)
+- get_changed_files() cache: Eliminated duplicate parsing (10-50ms saved)
+- Arc<SecurityConfig>: Cheap clones in parallel (eliminates N deep clones)
+- Mapping load cache: Eliminated redundant subprocess calls (50ms saved)
+- Type complexity warning: Fixed with ConflictResolutionResult type alias
+- Dead code warning: Fixed with proper documentation + #[allow(dead_code)]
 
 ---
 

src/commands/affected.rs

@@ -5,10 +5,10 @@
 //! - Which crates transitively depend on those changed crates
 //! - The minimal set of crates that need testing/building
 
+use crate::core::context::WorkspaceContext;
 use crate::core::error::{RailError, RailResult};
 use crate::core::vcs::SystemGit;
-use crate::graph::{AffectedAnalysis, WorkspaceGraph};
-use std::env;
+use crate::graph::AffectedAnalysis;
 use std::path::{Path, PathBuf};
 
 /// Output format for affected command
@@ -35,20 +35,17 @@ impl OutputFormat {
 
 /// Run the affected command
 pub fn run_affected(
+  ctx: &WorkspaceContext,
   since: String,
   from: Option<String>,
   to: Option<String>,
   format: String,
   dry_run: bool,
 ) -> RailResult<()> {
-  let workspace_root = env::current_dir()?;
   let output_format = OutputFormat::from_str(&format)?;
 
-  // Load workspace graph
-  let graph = WorkspaceGraph::load(&workspace_root)?;
-
   // Get changed files from git
-  let changed_files = get_changed_files(&workspace_root, &since, from.as_deref(), to.as_deref())?;
+  let changed_files = get_changed_files(ctx.workspace_root(), &since, from.as_deref(), to.as_deref())?;
 
   if dry_run {
     println!("DRY RUN: Would analyze {} changed files", changed_files.len());
@@ -59,7 +56,7 @@ pub fn run_affected(
   }
 
   // Analyze affected crates
-  let analysis = crate::graph::affected::analyze(&graph, &changed_files)?;
+  let analysis = crate::graph::affected::analyze(&ctx.graph, &changed_files)?;
 
   // Output results
   display_results(&analysis, output_format)?;

src/commands/check.rs

@@ -9,25 +9,29 @@
 //! - `--workspace` to override and check all workspace crates
 //! - `--dry-run` to show the plan without executing
 
+use crate::core::context::WorkspaceContext;
 use crate::core::error::{RailError, RailResult};
 use crate::core::vcs::SystemGit;
-use crate::graph::{AffectedAnalysis, WorkspaceGraph};
-use std::env;
+use crate::graph::AffectedAnalysis;
 use std::path::{Path, PathBuf};
 use std::process::Command;
 
 /// Run the check command
-pub fn run_check(since: Option<String>, workspace: bool, dry_run: bool, cargo_args: Vec<String>) -> RailResult<()> {
-  let workspace_root = env::current_dir()?;
-
+pub fn run_check(
+  ctx: &WorkspaceContext,
+  since: Option<String>,
+  workspace: bool,
+  dry_run: bool,
+  cargo_args: Vec<String>,
+) -> RailResult<()> {
   if workspace {
     // Run check for entire workspace
-    return run_workspace_check(&workspace_root, dry_run, &cargo_args);
+    return run_workspace_check(ctx.workspace_root(), dry_run, &cargo_args);
   }
 
   // Default: run targeted check based on affected analysis
   let since_ref = since.unwrap_or_else(|| "origin/main".to_string());
-  run_affected_check(&workspace_root, &since_ref, dry_run, &cargo_args)
+  run_affected_check(ctx, &since_ref, dry_run, &cargo_args)
 }
 
 /// Run check for the entire workspace
@@ -64,12 +68,9 @@ fn run_workspace_check(workspace_root: &Path, dry_run: bool, cargo_args: &[Strin
 }
 
 /// Run check for affected crates only
-fn run_affected_check(workspace_root: &Path, since: &str, dry_run: bool, cargo_args: &[String]) -> RailResult<()> {
-  // Load workspace graph
-  let graph = WorkspaceGraph::load(workspace_root)?;
-
+fn run_affected_check(ctx: &WorkspaceContext, since: &str, dry_run: bool, cargo_args: &[String]) -> RailResult<()> {
   // Get changed files from git
-  let changed_files = get_changed_files(workspace_root, since)?;
+  let changed_files = get_changed_files(ctx.workspace_root(), since)?;
 
   if changed_files.is_empty() {
     println!("✅ No changes detected since {}", since);
@@ -78,7 +79,7 @@ fn run_affected_check(workspace_root: &Path, since: &str, dry_run: bool, cargo_a
   }
 
   // Analyze affected crates
-  let analysis = crate::graph::affected::analyze(&graph, &changed_files)?;
+  let analysis = crate::graph::affected::analyze(&ctx.graph, &changed_files)?;
 
   if analysis.impact.test_targets.is_empty() {
     println!("✅ Changes detected but no workspace crates affected");
@@ -100,7 +101,7 @@ fn run_affected_check(workspace_root: &Path, since: &str, dry_run: bool, cargo_a
 
   // Execute check for each affected crate
   println!("\nExecuting checks...\n");
-  execute_checks(&analysis.impact.test_targets, workspace_root, cargo_args)?;
+  execute_checks(&analysis.impact.test_targets, ctx.workspace_root(), cargo_args)?;
 
   println!("\n✅ All checks completed successfully");
   Ok(())

src/commands/clippy.rs

@@ -9,25 +9,29 @@
 //! - `--workspace` to override and lint all workspace crates
 //! - `--dry-run` to show the plan without executing
 
+use crate::core::context::WorkspaceContext;
 use crate::core::error::{RailError, RailResult};
 use crate::core::vcs::SystemGit;
-use crate::graph::{AffectedAnalysis, WorkspaceGraph};
-use std::env;
+use crate::graph::AffectedAnalysis;
 use std::path::{Path, PathBuf};
 use std::process::Command;
 
 /// Run the clippy command
-pub fn run_clippy(since: Option<String>, workspace: bool, dry_run: bool, cargo_args: Vec<String>) -> RailResult<()> {
-  let workspace_root = env::current_dir()?;
-
+pub fn run_clippy(
+  ctx: &WorkspaceContext,
+  since: Option<String>,
+  workspace: bool,
+  dry_run: bool,
+  cargo_args: Vec<String>,
+) -> RailResult<()> {
   if workspace {
     // Run clippy for entire workspace
-    return run_workspace_clippy(&workspace_root, dry_run, &cargo_args);
+    return run_workspace_clippy(ctx.workspace_root(), dry_run, &cargo_args);
   }
 
   // Default: run targeted clippy based on affected analysis
   let since_ref = since.unwrap_or_else(|| "origin/main".to_string());
-  run_affected_clippy(&workspace_root, &since_ref, dry_run, &cargo_args)
+  run_affected_clippy(ctx, &since_ref, dry_run, &cargo_args)
 }
 
 /// Run clippy for the entire workspace
@@ -64,12 +68,11 @@ fn run_workspace_clippy(workspace_root: &Path, dry_run: bool, cargo_args: &[Stri
 }
 
 /// Run clippy for affected crates only
-fn run_affected_clippy(workspace_root: &Path, since: &str, dry_run: bool, cargo_args: &[String]) -> RailResult<()> {
+fn run_affected_clippy(ctx: &WorkspaceContext, since: &str, dry_run: bool, cargo_args: &[String]) -> RailResult<()> {
   // Load workspace graph
-  let graph = WorkspaceGraph::load(workspace_root)?;
 
   // Get changed files from git
-  let changed_files = get_changed_files(workspace_root, since)?;
+  let changed_files = get_changed_files(ctx.workspace_root(), since)?;
 
   if changed_files.is_empty() {
     println!("✅ No changes detected since {}", since);
@@ -78,7 +81,7 @@ fn run_affected_clippy(workspace_root: &Path, since: &str, dry_run: bool, cargo_
   }
 
   // Analyze affected crates
-  let analysis = crate::graph::affected::analyze(&graph, &changed_files)?;
+  let analysis = crate::graph::affected::analyze(&ctx.graph, &changed_files)?;
 
   if analysis.impact.test_targets.is_empty() {
     println!("✅ Changes detected but no workspace crates affected");
@@ -100,7 +103,7 @@ fn run_affected_clippy(workspace_root: &Path, since: &str, dry_run: bool, cargo_
 
   // Execute clippy for each affected crate
   println!("\nExecuting clippy...\n");
-  execute_clippy(&analysis.impact.test_targets, workspace_root, cargo_args)?;
+  execute_clippy(&analysis.impact.test_targets, ctx.workspace_root(), cargo_args)?;
 
   println!("\n✅ All clippy checks completed successfully");
   Ok(())

src/commands/sync.rs

@@ -345,6 +345,9 @@ pub fn run_sync(
       .map(|(split_config, _, _, _, _, _)| multi_progress.add_bar(1, format!("Syncing {}", split_config.name)))
       .collect();
 
+    // Wrap security config in Arc for cheap sharing across threads (avoid deep clones)
+    let security_config = std::sync::Arc::new(config.security.clone());
+
     let results: Vec<RailResult<SyncResult>> = plans
       .into_par_iter()
       .enumerate()
@@ -358,11 +361,11 @@ pub fn run_sync(
           remote_url: split_config.remote.clone(),
         };
 
-        // Create sync engine for this thread
+        // Create sync engine for this thread (security_config Arc clone is cheap)
         let mut engine = SyncEngine::new(
           config.workspace.root.clone(),
           sync_config,
-          config.security.clone(),
+          security_config.clone(),
           strategy,
         )?;
 
@@ -425,7 +428,7 @@ pub fn run_sync(
       let mut engine = SyncEngine::new(
         config.workspace.root.clone(),
         sync_config,
-        config.security.clone(),
+        std::sync::Arc::new(config.security.clone()),
         strategy,
       )?;
 

src/commands/test.rs

@@ -10,25 +10,29 @@
 //! - `--dry-run` to show the plan without executing
 //! - Parallel execution via rayon
 
+use crate::core::context::WorkspaceContext;
 use crate::core::error::{RailError, RailResult};
 use crate::core::vcs::SystemGit;
-use crate::graph::{AffectedAnalysis, WorkspaceGraph};
-use std::env;
+use crate::graph::AffectedAnalysis;
 use std::path::{Path, PathBuf};
 use std::process::Command;
 
 /// Run the test command
-pub fn run_test(since: Option<String>, workspace: bool, dry_run: bool, cargo_args: Vec<String>) -> RailResult<()> {
-  let workspace_root = env::current_dir()?;
-
+pub fn run_test(
+  ctx: &WorkspaceContext,
+  since: Option<String>,
+  workspace: bool,
+  dry_run: bool,
+  cargo_args: Vec<String>,
+) -> RailResult<()> {
   if workspace {
     // Run tests for entire workspace
-    return run_workspace_tests(&workspace_root, dry_run, &cargo_args);
+    return run_workspace_tests(ctx.workspace_root(), dry_run, &cargo_args);
   }
 
   // Default: run targeted tests based on affected analysis
   let since_ref = since.unwrap_or_else(|| "origin/main".to_string());
-  run_affected_tests(&workspace_root, &since_ref, dry_run, &cargo_args)
+  run_affected_tests(ctx, &since_ref, dry_run, &cargo_args)
 }
 
 /// Run tests for the entire workspace
@@ -65,12 +69,9 @@ fn run_workspace_tests(workspace_root: &Path, dry_run: bool, cargo_args: &[Strin
 }
 
 /// Run tests for affected crates only
-fn run_affected_tests(workspace_root: &Path, since: &str, dry_run: bool, cargo_args: &[String]) -> RailResult<()> {
-  // Load workspace graph
-  let graph = WorkspaceGraph::load(workspace_root)?;
-
+fn run_affected_tests(ctx: &WorkspaceContext, since: &str, dry_run: bool, cargo_args: &[String]) -> RailResult<()> {
   // Get changed files from git
-  let changed_files = get_changed_files(workspace_root, since)?;
+  let changed_files = get_changed_files(ctx.workspace_root(), since)?;
 
   if changed_files.is_empty() {
     println!("✅ No changes detected since {}", since);
@@ -79,7 +80,7 @@ fn run_affected_tests(workspace_root: &Path, since: &str, dry_run: bool, cargo_a
   }
 
   // Analyze affected crates
-  let analysis = crate::graph::affected::analyze(&graph, &changed_files)?;
+  let analysis = crate::graph::affected::analyze(&ctx.graph, &changed_files)?;
 
   if analysis.impact.test_targets.is_empty() {
     println!("✅ Changes detected but no workspace crates affected");
@@ -101,7 +102,7 @@ fn run_affected_tests(workspace_root: &Path, since: &str, dry_run: bool, cargo_a
 
   // Execute tests for each affected crate
   println!("\nExecuting tests...\n");
-  execute_tests(&analysis.impact.test_targets, workspace_root, cargo_args)?;
+  execute_tests(&analysis.impact.test_targets, ctx.workspace_root(), cargo_args)?;
 
   println!("\n✅ All tests completed successfully");
   Ok(())