Fix code review issues: node ID validation, grid_states ordering, and remove sample nodes

Co-authored-by: Steake <530040+Steake@users.noreply.github.com>

Author: Copilot

crates/bitcell-admin/src/api/nodes.rs

@@ -32,6 +32,19 @@ pub struct StartNodeRequest {
     pub config: Option<serde_json::Value>,
 }
 
+/// Validate node ID format (alphanumeric, hyphens, and underscores only)
+fn validate_node_id(id: &str) -> Result<(), (StatusCode, Json<ErrorResponse>)> {
+    if !id.chars().all(|c| c.is_alphanumeric() || c == '-' || c == '_') {
+        return Err((
+            StatusCode::BAD_REQUEST,
+            Json(ErrorResponse {
+                error: "Invalid node ID format".to_string(),
+            }),
+        ));
+    }
+    Ok(())
+}
+
 /// List all registered nodes
 pub async fn list_nodes(
     State(state): State<Arc<AppState>>,
@@ -47,15 +60,7 @@ pub async fn get_node(
     State(state): State<Arc<AppState>>,
     Path(id): Path<String>,
 ) -> Result<Json<NodeResponse>, (StatusCode, Json<ErrorResponse>)> {
-    // Validate node ID format (alphanumeric and hyphens only)
-    if !id.chars().all(|c| c.is_alphanumeric() || c == '-' || c == '_') {
-        return Err((
-            StatusCode::BAD_REQUEST,
-            Json(ErrorResponse {
-                error: "Invalid node ID format".to_string(),
-            }),
-        ));
-    }
+    validate_node_id(&id)?;
 
     match state.process.get_node(&id) {
         Some(node) => Ok(Json(NodeResponse { node })),
@@ -74,15 +79,7 @@ pub async fn start_node(
     Path(id): Path<String>,
     Json(req): Json<StartNodeRequest>,
 ) -> Result<Json<NodeResponse>, (StatusCode, Json<ErrorResponse>)> {
-    // Validate node ID format (alphanumeric and hyphens only)
-    if !id.chars().all(|c| c.is_alphanumeric() || c == '-' || c == '_') {
-        return Err((
-            StatusCode::BAD_REQUEST,
-            Json(ErrorResponse {
-                error: "Invalid node ID format".to_string(),
-            }),
-        ));
-    }
+    validate_node_id(&id)?;
 
     // Config is not supported yet
     if req.config.is_some() {
@@ -113,15 +110,7 @@ pub async fn stop_node(
     State(state): State<Arc<AppState>>,
     Path(id): Path<String>,
 ) -> Result<Json<NodeResponse>, (StatusCode, Json<ErrorResponse>)> {
-    // Validate node ID format (alphanumeric and hyphens only)
-    if !id.chars().all(|c| c.is_alphanumeric() || c == '-' || c == '_') {
-        return Err((
-            StatusCode::BAD_REQUEST,
-            Json(ErrorResponse {
-                error: "Invalid node ID format".to_string(),
-            }),
-        ));
-    }
+    validate_node_id(&id)?;
 
     match state.process.stop_node(&id) {
         Ok(node) => {

crates/bitcell-admin/src/main.rs

@@ -1,6 +1,6 @@
 //! BitCell Admin Console - Main Entry Point
 
-use bitcell_admin::{AdminConsole, process::{ProcessManager, NodeConfig}, api::NodeType};
+use bitcell_admin::AdminConsole;
 use tracing_subscriber::{layer::SubscriberExt, util::SubscriberInitExt};
 
 #[tokio::main]
@@ -23,47 +23,11 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
         .parse()?;
 
     let console = AdminConsole::new(addr);
-    let process_mgr = console.process_manager();
 
-    // Register some sample nodes for demonstration
-    register_sample_nodes(&process_mgr);
-
-    tracing::info!("Admin console ready - registered {} nodes", process_mgr.list_nodes().len());
+    tracing::info!("Admin console ready");
     tracing::info!("Dashboard available at http://{}", addr);
 
     console.serve().await?;
 
     Ok(())
 }
-
-fn register_sample_nodes(process: &ProcessManager) {
-    // Register sample validator nodes
-    for i in 1..=3 {
-        let config = NodeConfig {
-            node_type: NodeType::Validator,
-            data_dir: format!("/tmp/bitcell/validator-{}", i),
-            port: 9000 + i as u16,
-            rpc_port: 10000 + i as u16,
-            log_level: "info".to_string(),
-            network: "testnet".to_string(),
-        };
-
-        process.register_node(format!("validator-{}", i), config);
-        tracing::info!("Registered validator-{}", i);
-    }
-
-    // Register sample miner nodes
-    for i in 1..=2 {
-        let config = NodeConfig {
-            node_type: NodeType::Miner,
-            data_dir: format!("/tmp/bitcell/miner-{}", i),
-            port: 9100 + i as u16,
-            rpc_port: 10100 + i as u16,
-            log_level: "info".to_string(),
-            network: "testnet".to_string(),
-        };
-
-        process.register_node(format!("miner-{}", i), config);
-        tracing::info!("Registered miner-{}", i);
-    }
-}

crates/bitcell-ca/src/battle.rs

@@ -131,35 +131,41 @@ impl Battle {
 
     /// Get grid states at specific steps for visualization.
     /// 
-    /// Returns a vector of grids at the requested step intervals.
+    /// Returns a vector of grids at the requested step intervals in the same order
+    /// as the input `sample_steps` array.
     /// Steps that exceed `self.steps` are silently skipped.
-    /// For efficiency, steps should be provided in ascending order.
     /// 
     /// # Performance Note
-    /// This implementation evolves incrementally from one step to the next,
-    /// which is more efficient than evolving from scratch for each step.
+    /// This implementation sorts steps internally for incremental evolution efficiency,
+    /// but returns grids in the original order requested.
     pub fn grid_states(&self, sample_steps: &[usize]) -> Vec<Grid> {
-        let mut grids = Vec::new();
         let initial = self.setup_grid();
 
-        // Sort sample_steps to ensure incremental evolution
-        let mut sorted_steps: Vec<usize> = sample_steps.iter()
-            .filter(|&&step| step <= self.steps)
-            .copied()
+        // Filter and create (index, step) pairs to preserve original order
+        let mut indexed_steps: Vec<(usize, usize)> = sample_steps.iter()
+            .enumerate()
+            .filter(|(_, &step)| step <= self.steps)
+            .map(|(idx, &step)| (idx, step))
             .collect();
-        sorted_steps.sort_unstable();
 
+        // Sort by step for efficient incremental evolution
+        indexed_steps.sort_unstable_by_key(|(_, step)| *step);
+
+        // Evolve grids in sorted order
+        let mut evolved_grids = Vec::with_capacity(indexed_steps.len());
         let mut current_grid = initial;
         let mut prev_step = 0;
 
-        for step in sorted_steps {
+        for (original_idx, step) in &indexed_steps {
             let steps_to_evolve = step - prev_step;
             current_grid = evolve_n_steps(&current_grid, steps_to_evolve);
-            grids.push(current_grid.clone());
-            prev_step = step;
+            evolved_grids.push((*original_idx, current_grid.clone()));
+            prev_step = *step;
         }
 
-        grids
+        // Sort back to original order and extract grids
+        evolved_grids.sort_unstable_by_key(|(idx, _)| *idx);
+        evolved_grids.into_iter().map(|(_, grid)| grid).collect()
     }
 }
 

crates/bitcell-ca/src/grid.rs

@@ -156,7 +156,10 @@ impl Grid {
     /// 
     /// # Note
     /// When `GRID_SIZE` is not evenly divisible by `target_size`, some cells near
-    /// the edges may not be sampled. This is acceptable for visualization purposes.
+    /// the edges may not be sampled. For example, with `GRID_SIZE=1024` and
+    /// `target_size=100`, `block_size=10`, so only cells from indices 0-999 are
+    /// sampled, leaving rows/columns 1000-1023 unsampled. This is acceptable for
+    /// visualization purposes where approximate representation is sufficient.
     pub fn downsample(&self, target_size: usize) -> Vec<Vec<u8>> {
         if target_size == 0 || target_size > GRID_SIZE {
             panic!("target_size must be between 1 and {}", GRID_SIZE);