cargo-rail: fixed some basic perf stuff; added docs/architecture.md

Author: loadingalias

.gitignore

@@ -26,6 +26,7 @@ docs/src
 rules.md
 cmd_cfg.md
 v1_precheck.md
+ARCHITECTURE.md
 
 # Cargo-Rail (Testing)
 *rail.toml

README.md

@@ -57,7 +57,7 @@ cargo rail test --explain
 **After:**
 
 ```yaml
-- run: cargo rail test --nextest  # Tests only affected crates (1 minute)
+- run: cargo rail test            # Tests only affected crates (1 minute)
 ```
 
 ### 2. Dependency Unification (replaces cargo-hakari)
@@ -213,7 +213,7 @@ Run `cargo rail init --check` to preview generated config.
 cargo rail affected                    # Show affected crates
 cargo rail affected --since main       # Compare against branch
 cargo rail test                        # Test affected crates
-cargo rail test --nextest --watch      # Watch mode with nextest
+cargo rail test --explain              # Explain why each crate is tested
 ```
 
 ### Dependency Unification
@@ -228,7 +228,6 @@ cargo rail unify                       # Apply unification
 ```bash
 cargo rail split my-crate              # Split to standalone repo
 cargo rail sync my-crate               # Bidirectional sync
-cargo rail status                      # Show sync status
 ```
 
 ### Release Automation (replaces cargo-release + git-cliff)
@@ -249,18 +248,6 @@ cargo rail release --all --bump patch --execute
 - GitHub release creation via `gh` CLI (optional)
 - GPG/SSH tag signing support
 
-### Watch Mode (bacon/cargo-watch integration)
-
-```bash
-# Auto-detect best watcher and run smart tests
-cargo rail test --watch
-
-# Explicit watcher selection
-cargo rail test --watch --watch-mode bacon
-```
-
----
-
 ## Comparison
 
 ### vs Dependency Management
@@ -303,7 +290,6 @@ cargo rail test --watch --watch-mode bacon
 | Change detection | ✅ Graph-aware | ⚠️ Path-based | ✅ Graph-aware |
 | Rust-native | ✅ | ⚠️ Shell | ❌ JS ecosystem |
 | Nextest integration | ✅ Auto-detect | Manual | ❌ |
-| Watch mode | ✅ bacon/cargo-watch | Manual | ✅ |
 | Dependency resolution | ✅ Cargo's resolver | ❌ | ❌ |
 
 ---

src/cargo/cargo_transform.rs

@@ -7,8 +7,9 @@
 use crate::cargo::manifest_ops;
 use crate::error::{RailError, RailResult};
 use cargo_metadata::Metadata;
+use std::cell::RefCell;
 use std::path::PathBuf;
-use toml_edit::{DocumentMut, Item};
+use toml_edit::{DocumentMut, Item, Table};
 
 /// Context for Cargo.toml transformations
 pub struct TransformContext {
@@ -19,26 +20,71 @@ pub struct TransformContext {
 }
 
 /// Cargo.toml transformer for split/sync operations
+///
+/// Caches the workspace document to avoid repeated I/O when transforming multiple manifests.
+/// Uses interior mutability (`RefCell`) so the public API remains `&self`.
 pub struct CargoTransform {
   metadata: Metadata,
+  /// Cached workspace document (loaded lazily via RefCell for interior mutability)
+  cached_workspace_doc: RefCell<Option<DocumentMut>>,
+  /// Workspace root for lazy loading
+  workspace_root: PathBuf,
 }
 
 impl CargoTransform {
   /// Create a new transformer with workspace metadata
   pub fn new(metadata: Metadata) -> Self {
-    Self { metadata }
+    let workspace_root = metadata.workspace_root.as_std_path().to_path_buf();
+    Self {
+      metadata,
+      cached_workspace_doc: RefCell::new(None),
+      workspace_root,
+    }
+  }
+
+  /// Get workspace.package table, loading and caching the workspace doc if needed
+  ///
+  /// Uses `RefCell` for interior mutability - loads once, caches for reuse.
+  fn get_workspace_package(&self) -> RailResult<Option<Table>> {
+    // Check if already cached
+    {
+      let cache = self.cached_workspace_doc.borrow();
+      if let Some(ref doc) = *cache {
+        return Ok(
+          doc
+            .get("workspace")
+            .and_then(|w| w.as_table())
+            .and_then(|w| w.get("package"))
+            .and_then(|p| p.as_table())
+            .cloned(),
+        );
+      }
+    }
+
+    // Load and cache
+    let workspace_toml_path = self.workspace_root.join("Cargo.toml");
+    let doc = manifest_ops::read_toml_file(&workspace_toml_path)?;
+    let result = doc
+      .get("workspace")
+      .and_then(|w| w.as_table())
+      .and_then(|w| w.get("package"))
+      .and_then(|p| p.as_table())
+      .cloned();
+
+    *self.cached_workspace_doc.borrow_mut() = Some(doc);
+    Ok(result)
   }
 
   /// Transform a Cargo.toml from workspace format to split (standalone) format
   ///
   /// This replaces workspace dependency references with concrete version requirements.
-  pub fn transform_to_split(&self, content: &str, context: &TransformContext) -> RailResult<String> {
+  pub fn transform_to_split(&self, content: &str, _context: &TransformContext) -> RailResult<String> {
     let mut doc: DocumentMut = content
       .parse()
       .map_err(|e| RailError::message(format!("Failed to parse Cargo.toml: {}", e)))?;
 
     // Remove workspace inheritance markers and resolve to actual values
-    self.resolve_workspace_inheritance(&mut doc, &context.workspace_root)?;
+    self.resolve_workspace_inheritance(&mut doc)?;
 
     // Transform workspace dependencies to standalone format
     self.transform_dependencies_to_standalone(&mut doc)?;
@@ -57,21 +103,12 @@ impl CargoTransform {
   }
 
   /// Resolve workspace inheritance (workspace = true fields) to actual values
-  fn resolve_workspace_inheritance(&self, doc: &mut DocumentMut, workspace_root: &std::path::Path) -> RailResult<()> {
-    // Load workspace Cargo.toml to get [workspace.package] values
-    let workspace_toml_path = workspace_root.join("Cargo.toml");
-    let workspace_doc = manifest_ops::read_toml_file(&workspace_toml_path)?;
-
-    // Get workspace.package table if it exists
-    let workspace_package = workspace_doc
-      .get("workspace")
-      .and_then(|w| w.as_table())
-      .and_then(|w| w.get("package"))
-      .and_then(|p| p.as_table());
-
-    // Use manifest_ops to resolve package inheritance
-    if let Some(workspace_pkg) = workspace_package {
-      manifest_ops::resolve_package_workspace_inheritance(doc, workspace_pkg)?;
+  ///
+  /// Uses cached workspace document to avoid repeated I/O.
+  fn resolve_workspace_inheritance(&self, doc: &mut DocumentMut) -> RailResult<()> {
+    // Get workspace.package table from cache (loads workspace doc if needed)
+    if let Some(workspace_pkg) = self.get_workspace_package()? {
+      manifest_ops::resolve_package_workspace_inheritance(doc, &workspace_pkg)?;
     }
 
     Ok(())

src/cargo/manifest_analyzer.rs

@@ -177,6 +177,9 @@ pub struct ManifestAnalyzer {
   usage_index: HashMap<DepKey, Vec<DepUsage>>,
   /// Pre-computed usage counts for O(1) lookup
   usage_counts: HashMap<DepKey, usize>,
+  /// Package name index for O(1) lookup of dep keys by package name
+  /// Maps package_name -> list of DepKeys that refer to that package
+  package_index: HashMap<String, Vec<DepKey>>,
 }
 
 impl ManifestAnalyzer {
@@ -218,10 +221,20 @@ impl ManifestAnalyzer {
       })
       .collect();
 
+    // Build package name index for O(1) lookup of dep keys by package name
+    let mut package_index: HashMap<String, Vec<DepKey>> = HashMap::new();
+    for dep_key in usage_index.keys() {
+      package_index
+        .entry(dep_key.name.clone())
+        .or_default()
+        .push(dep_key.clone());
+    }
+
     Ok(Self {
       members: parsed_members,
       usage_index,
       usage_counts,
+      package_index,
     })
   }
 
@@ -622,11 +635,15 @@ impl ManifestAnalyzer {
   /// Count how many crates use a package (aggregated across renamed and non-renamed deps)
   ///
   /// Issue #6: When include_renamed = true, count all usages of the package
+  /// Uses package_index for O(1) key lookup instead of O(n) scan.
   pub fn package_usage_count(&self, package_name: &str) -> usize {
-    let mut unique_users: HashSet<&String> = HashSet::new();
+    let Some(dep_keys) = self.package_index.get(package_name) else {
+      return 0;
+    };
 
-    for (dep_key, usages) in &self.usage_index {
-      if dep_key.name == package_name {
+    let mut unique_users: HashSet<&String> = HashSet::new();
+    for dep_key in dep_keys {
+      if let Some(usages) = self.usage_index.get(dep_key) {
         for usage in usages {
           unique_users.insert(&usage.used_by);
         }
@@ -639,18 +656,27 @@ impl ManifestAnalyzer {
   /// Get all dep keys that refer to a specific package (including renamed)
   ///
   /// Issue #6: Used to find all renamed variants of a package
+  /// Uses package_index for O(1) lookup instead of O(n) scan.
   pub fn dep_keys_for_package(&self, package_name: &str) -> Vec<&DepKey> {
-    self.usage_index.keys().filter(|k| k.name == package_name).collect()
+    self
+      .package_index
+      .get(package_name)
+      .map(|keys| keys.iter().collect())
+      .unwrap_or_default()
   }
 
   /// Get aggregated usage sites for a package (all renamed and non-renamed usages)
   ///
   /// Issue #6: Used when include_renamed = true to merge features across all usages
+  /// Uses package_index for O(1) key lookup instead of O(n) scan.
   pub fn get_package_usage_sites(&self, package_name: &str) -> Vec<&DepUsage> {
-    let mut all_usages = Vec::new();
+    let Some(dep_keys) = self.package_index.get(package_name) else {
+      return Vec::new();
+    };
 
-    for (dep_key, usages) in &self.usage_index {
-      if dep_key.name == package_name {
+    let mut all_usages = Vec::new();
+    for dep_key in dep_keys {
+      if let Some(usages) = self.usage_index.get(dep_key) {
         all_usages.extend(usages.iter());
       }
     }

src/cargo/manifest_writer.rs

@@ -39,28 +39,24 @@ impl ManifestWriter {
     // Ensure [workspace] section exists
     manifest_ops::ensure_section(&mut doc, "workspace").context("Failed to create [workspace] section")?;
 
-    // Get or create [workspace.dependencies] - DO NOT CLEAR IT
-    // We merge new deps with existing ones, not replace them
-    let deps_table = manifest_ops::get_or_create_table(&mut doc, "workspace.dependencies")
-      .context("Failed to create [workspace.dependencies]")?;
-    // NOTE: Removed deps_table.clear() - BUG FIX: preserve existing deps
-
-    // Group dependencies by target
-    let (regular_deps, target_deps) = self.group_dependencies(deps);
-
-    // Write regular dependencies
-    for dep in regular_deps {
-      let entry = manifest_ops::build_dep_entry(&dep);
-      manifest_ops::insert_dependency(deps_table, &dep.name, entry).context("Failed to insert regular dependency")?;
+    // First pass: write regular dependencies (no target)
+    // This scope ensures deps_table reference is released before we handle target deps
+    {
+      let deps_table = manifest_ops::get_or_create_table(&mut doc, "workspace.dependencies")
+        .context("Failed to create [workspace.dependencies]")?;
+
+      for dep in deps.iter().filter(|d| d.target.is_none()) {
+        let entry = manifest_ops::build_dep_entry(dep);
+        manifest_ops::insert_dependency(deps_table, &dep.name, entry).context("Failed to insert regular dependency")?;
+      }
     }
 
-    // Write target-specific dependencies
-    for (target, deps) in target_deps {
-      for dep in deps {
-        let entry = manifest_ops::build_dep_entry(&dep);
-        manifest_ops::insert_target_dependency(&mut doc, &target, "dependencies", &dep.name, entry)
-          .context("Failed to insert target dependency")?;
-      }
+    // Second pass: write target-specific dependencies
+    for dep in deps.iter().filter(|d| d.target.is_some()) {
+      let entry = manifest_ops::build_dep_entry(dep);
+      let target = dep.target.as_ref().unwrap();
+      manifest_ops::insert_target_dependency(&mut doc, target, "dependencies", &dep.name, entry)
+        .context("Failed to insert target dependency")?;
     }
 
     // Format and write
@@ -70,25 +66,6 @@ impl ManifestWriter {
     Ok(())
   }
 
-  /// Group dependencies into regular and target-specific
-  fn group_dependencies(
-    &self,
-    deps: &[UnifiedDep],
-  ) -> (Vec<UnifiedDep>, std::collections::HashMap<String, Vec<UnifiedDep>>) {
-    let mut regular_deps = Vec::new();
-    let mut target_deps: std::collections::HashMap<String, Vec<UnifiedDep>> = std::collections::HashMap::new();
-
-    for dep in deps {
-      if let Some(ref target) = dep.target {
-        target_deps.entry(target.clone()).or_default().push(dep.clone());
-      } else {
-        regular_deps.push(dep.clone());
-      }
-    }
-
-    (regular_deps, target_deps)
-  }
-
   /// Update a member's Cargo.toml to use workspace inheritance
   ///
   /// # Arguments