feat: add config validation and dependency cycle detection

- Config::validate() checks project.root, language, scan.include,
  python.src_roots, caching.max_cache_age, and scan.max_file_size
- Add parse_duration() and parse_file_size() helper functions
- Implement DFS-based cycle detection in cycle_detector.rs
- Wire cycle detection into renderer critical points section
- Add comprehensive unit tests for all new functionality

archdoc-core/src/config.rs

@@ -423,6 +423,71 @@ fn default_max_cache_age() -> String {
 }
 
 impl Config {
+    /// Validate the configuration for correctness.
+    ///
+    /// Checks that paths exist, values are parseable, and settings are sensible.
+    pub fn validate(&self) -> Result<(), ArchDocError> {
+        // Check project.root exists and is a directory
+        let root = Path::new(&self.project.root);
+        if !root.exists() {
+            return Err(ArchDocError::ConfigError(format!(
+                "project.root '{}' does not exist",
+                self.project.root
+            )));
+        }
+        if !root.is_dir() {
+            return Err(ArchDocError::ConfigError(format!(
+                "project.root '{}' is not a directory",
+                self.project.root
+            )));
+        }
+
+        // Check language is python
+        if self.project.language != "python" {
+            return Err(ArchDocError::ConfigError(format!(
+                "project.language '{}' is not supported. Only 'python' is currently supported",
+                self.project.language
+            )));
+        }
+
+        // Check scan.include is not empty
+        if self.scan.include.is_empty() {
+            return Err(ArchDocError::ConfigError(
+                "scan.include must not be empty — at least one directory must be specified".to_string(),
+            ));
+        }
+
+        // Check python.src_roots exist relative to project.root
+        for src_root in &self.python.src_roots {
+            let path = root.join(src_root);
+            if !path.exists() {
+                return Err(ArchDocError::ConfigError(format!(
+                    "python.src_roots entry '{}' does not exist (resolved to '{}')",
+                    src_root,
+                    path.display()
+                )));
+            }
+        }
+
+        // Parse max_cache_age
+        parse_duration(&self.caching.max_cache_age).map_err(|e| {
+            ArchDocError::ConfigError(format!(
+                "caching.max_cache_age '{}' is not valid: {}. Use formats like '24h', '7d', '30m'",
+                self.caching.max_cache_age, e
+            ))
+        })?;
+
+        // Parse max_file_size
+        parse_file_size(&self.scan.max_file_size).map_err(|e| {
+            ArchDocError::ConfigError(format!(
+                "scan.max_file_size '{}' is not valid: {}. Use formats like '10MB', '1GB', '500KB'",
+                self.scan.max_file_size, e
+            ))
+        })?;
+
+        Ok(())
+    }
+
     /// Load configuration from a TOML file
     pub fn load_from_file(path: &Path) -> Result<Self, ArchDocError> {
         let content = std::fs::read_to_string(path)
@@ -441,3 +506,130 @@ impl Config {
             .map_err(|e| ArchDocError::ConfigError(format!("Failed to write config file: {}", e)))
     }
 }
+
+/// Parse a duration string like "24h", "7d", "30m" into seconds.
+pub fn parse_duration(s: &str) -> Result<u64, String> {
+    let s = s.trim();
+    if s.is_empty() {
+        return Err("empty duration string".to_string());
+    }
+
+    let (num_str, suffix) = split_numeric_suffix(s)?;
+    let value: u64 = num_str
+        .parse()
+        .map_err(|_| format!("'{}' is not a valid number", num_str))?;
+
+    match suffix {
+        "s" => Ok(value),
+        "m" => Ok(value * 60),
+        "h" => Ok(value * 3600),
+        "d" => Ok(value * 86400),
+        "w" => Ok(value * 604800),
+        _ => Err(format!("unknown duration suffix '{}'. Use s, m, h, d, or w", suffix)),
+    }
+}
+
+/// Parse a file size string like "10MB", "1GB", "500KB" into bytes.
+pub fn parse_file_size(s: &str) -> Result<u64, String> {
+    let s = s.trim();
+    if s.is_empty() {
+        return Err("empty file size string".to_string());
+    }
+
+    let (num_str, suffix) = split_numeric_suffix(s)?;
+    let value: u64 = num_str
+        .parse()
+        .map_err(|_| format!("'{}' is not a valid number", num_str))?;
+
+    let suffix_upper = suffix.to_uppercase();
+    match suffix_upper.as_str() {
+        "B" => Ok(value),
+        "KB" | "K" => Ok(value * 1024),
+        "MB" | "M" => Ok(value * 1024 * 1024),
+        "GB" | "G" => Ok(value * 1024 * 1024 * 1024),
+        _ => Err(format!("unknown size suffix '{}'. Use B, KB, MB, or GB", suffix)),
+    }
+}
+
+fn split_numeric_suffix(s: &str) -> Result<(&str, &str), String> {
+    let pos = s
+        .find(|c: char| !c.is_ascii_digit())
+        .ok_or_else(|| format!("no unit suffix found in '{}'", s))?;
+    if pos == 0 {
+        return Err(format!("no numeric value found in '{}'", s));
+    }
+    Ok((&s[..pos], &s[pos..]))
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_parse_duration() {
+        assert_eq!(parse_duration("24h").unwrap(), 86400);
+        assert_eq!(parse_duration("7d").unwrap(), 604800);
+        assert_eq!(parse_duration("30m").unwrap(), 1800);
+        assert_eq!(parse_duration("60s").unwrap(), 60);
+        assert!(parse_duration("abc").is_err());
+        assert!(parse_duration("").is_err());
+        assert!(parse_duration("10x").is_err());
+    }
+
+    #[test]
+    fn test_parse_file_size() {
+        assert_eq!(parse_file_size("10MB").unwrap(), 10 * 1024 * 1024);
+        assert_eq!(parse_file_size("1GB").unwrap(), 1024 * 1024 * 1024);
+        assert_eq!(parse_file_size("500KB").unwrap(), 500 * 1024);
+        assert!(parse_file_size("abc").is_err());
+        assert!(parse_file_size("").is_err());
+    }
+
+    #[test]
+    fn test_validate_default_config() {
+        // Default config with "." as root should validate if we're in a valid dir
+        let config = Config::default();
+        // This should work since "." exists and is a directory
+        assert!(config.validate().is_ok());
+    }
+
+    #[test]
+    fn test_validate_bad_language() {
+        let mut config = Config::default();
+        config.project.language = "java".to_string();
+        let err = config.validate().unwrap_err();
+        assert!(err.to_string().contains("not supported"));
+    }
+
+    #[test]
+    fn test_validate_empty_include() {
+        let mut config = Config::default();
+        config.scan.include = vec![];
+        let err = config.validate().unwrap_err();
+        assert!(err.to_string().contains("must not be empty"));
+    }
+
+    #[test]
+    fn test_validate_bad_root() {
+        let mut config = Config::default();
+        config.project.root = "/nonexistent/path/xyz".to_string();
+        let err = config.validate().unwrap_err();
+        assert!(err.to_string().contains("does not exist"));
+    }
+
+    #[test]
+    fn test_validate_bad_cache_age() {
+        let mut config = Config::default();
+        config.caching.max_cache_age = "invalid".to_string();
+        let err = config.validate().unwrap_err();
+        assert!(err.to_string().contains("not valid"));
+    }
+
+    #[test]
+    fn test_validate_bad_file_size() {
+        let mut config = Config::default();
+        config.scan.max_file_size = "notasize".to_string();
+        let err = config.validate().unwrap_err();
+        assert!(err.to_string().contains("not valid"));
+    }
+}

archdoc-core/src/cycle_detector.rs

@@ -0,0 +1,183 @@
+//! Dependency cycle detection for module graphs.
+//!
+//! Uses DFS-based cycle detection to find circular dependencies
+//! in the module dependency graph.
+
+use crate::model::ProjectModel;
+use std::collections::{HashMap, HashSet};
+
+/// Detect cycles in the module dependency graph.
+///
+/// Returns a list of cycles, where each cycle is a list of module IDs
+/// forming a circular dependency chain.
+pub fn detect_cycles(model: &ProjectModel) -> Vec<Vec<String>> {
+    let mut visited = HashSet::new();
+    let mut rec_stack = HashSet::new();
+    let mut path = Vec::new();
+    let mut cycles = Vec::new();
+
+    // Build adjacency list from model
+    let adj = build_adjacency_list(model);
+
+    for module_id in model.modules.keys() {
+        if !visited.contains(module_id.as_str()) {
+            dfs(
+                module_id,
+                &adj,
+                &mut visited,
+                &mut rec_stack,
+                &mut path,
+                &mut cycles,
+            );
+        }
+    }
+
+    // Deduplicate cycles (normalize by rotating to smallest element first)
+    deduplicate_cycles(cycles)
+}
+
+fn build_adjacency_list(model: &ProjectModel) -> HashMap<String, Vec<String>> {
+    let mut adj: HashMap<String, Vec<String>> = HashMap::new();
+
+    for (module_id, module) in &model.modules {
+        let neighbors: Vec<String> = module
+            .outbound_modules
+            .iter()
+            .filter(|target| model.modules.contains_key(*target))
+            .cloned()
+            .collect();
+        adj.insert(module_id.clone(), neighbors);
+    }
+
+    adj
+}
+
+fn dfs(
+    node: &str,
+    adj: &HashMap<String, Vec<String>>,
+    visited: &mut HashSet<String>,
+    rec_stack: &mut HashSet<String>,
+    path: &mut Vec<String>,
+    cycles: &mut Vec<Vec<String>>,
+) {
+    visited.insert(node.to_string());
+    rec_stack.insert(node.to_string());
+    path.push(node.to_string());
+
+    if let Some(neighbors) = adj.get(node) {
+        for neighbor in neighbors {
+            if !visited.contains(neighbor.as_str()) {
+                dfs(neighbor, adj, visited, rec_stack, path, cycles);
+            } else if rec_stack.contains(neighbor.as_str()) {
+                // Found a cycle: extract it from path
+                if let Some(start_idx) = path.iter().position(|n| n == neighbor) {
+                    let cycle: Vec<String> = path[start_idx..].to_vec();
+                    cycles.push(cycle);
+                }
+            }
+        }
+    }
+
+    path.pop();
+    rec_stack.remove(node);
+}
+
+fn deduplicate_cycles(cycles: Vec<Vec<String>>) -> Vec<Vec<String>> {
+    let mut seen: HashSet<Vec<String>> = HashSet::new();
+    let mut unique = Vec::new();
+
+    for cycle in cycles {
+        if cycle.is_empty() {
+            continue;
+        }
+        // Normalize: rotate so the lexicographically smallest element is first
+        let min_idx = cycle
+            .iter()
+            .enumerate()
+            .min_by_key(|(_, v)| v.as_str())
+            .map(|(i, _)| i)
+            .unwrap_or(0);
+
+        let mut normalized = Vec::with_capacity(cycle.len());
+        for i in 0..cycle.len() {
+            normalized.push(cycle[(min_idx + i) % cycle.len()].clone());
+        }
+
+        if seen.insert(normalized.clone()) {
+            unique.push(normalized);
+        }
+    }
+
+    unique
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::model::{Edges, Module, ProjectModel};
+    use std::collections::HashMap;
+
+    fn make_module(id: &str, outbound: Vec<&str>) -> Module {
+        Module {
+            id: id.to_string(),
+            path: format!("{}.py", id),
+            files: vec![],
+            doc_summary: None,
+            outbound_modules: outbound.into_iter().map(String::from).collect(),
+            inbound_modules: vec![],
+            symbols: vec![],
+        }
+    }
+
+    #[test]
+    fn test_no_cycles() {
+        let mut model = ProjectModel::new();
+        model.modules.insert("a".into(), make_module("a", vec!["b"]));
+        model.modules.insert("b".into(), make_module("b", vec!["c"]));
+        model.modules.insert("c".into(), make_module("c", vec![]));
+
+        let cycles = detect_cycles(&model);
+        assert!(cycles.is_empty());
+    }
+
+    #[test]
+    fn test_simple_cycle() {
+        let mut model = ProjectModel::new();
+        model.modules.insert("a".into(), make_module("a", vec!["b"]));
+        model.modules.insert("b".into(), make_module("b", vec!["a"]));
+
+        let cycles = detect_cycles(&model);
+        assert_eq!(cycles.len(), 1);
+        assert!(cycles[0].contains(&"a".to_string()));
+        assert!(cycles[0].contains(&"b".to_string()));
+    }
+
+    #[test]
+    fn test_three_node_cycle() {
+        let mut model = ProjectModel::new();
+        model.modules.insert("a".into(), make_module("a", vec!["b"]));
+        model.modules.insert("b".into(), make_module("b", vec!["c"]));
+        model.modules.insert("c".into(), make_module("c", vec!["a"]));
+
+        let cycles = detect_cycles(&model);
+        assert_eq!(cycles.len(), 1);
+        assert_eq!(cycles[0].len(), 3);
+    }
+
+    #[test]
+    fn test_empty_graph() {
+        let model = ProjectModel::new();
+        let cycles = detect_cycles(&model);
+        assert!(cycles.is_empty());
+    }
+
+    #[test]
+    fn test_self_cycle() {
+        let mut model = ProjectModel::new();
+        model.modules.insert("a".into(), make_module("a", vec!["a"]));
+
+        let cycles = detect_cycles(&model);
+        assert_eq!(cycles.len(), 1);
+        assert_eq!(cycles[0], vec!["a".to_string()]);
+    }
+}

archdoc-core/src/lib.rs

@@ -12,6 +12,7 @@ pub mod python_analyzer;
 pub mod renderer;
 pub mod writer;
 pub mod cache;
+pub mod cycle_detector;
 
 // Re-export commonly used types
 pub use errors::ArchDocError;

archdoc-core/src/renderer.rs

@@ -3,6 +3,7 @@
 //! This module handles generating Markdown documentation from the project model
 //! using templates.
 
+use crate::cycle_detector;
 use crate::model::ProjectModel;
 use handlebars::Handlebars;
 
@@ -493,7 +494,14 @@ impl Renderer {
         let data = serde_json::json!({
             "high_fan_in": high_fan_in,
             "high_fan_out": high_fan_out,
-            "cycles": Vec::<String>::new(), // TODO: Implement cycle detection
+            "cycles": cycle_detector::detect_cycles(model)
+                .iter()
+                .map(|cycle| {
+                    serde_json::json!({
+                        "cycle_path": format!("{} → {}", cycle.join(" → "), cycle.first().unwrap_or(&String::new()))
+                    })
+                })
+                .collect::<Vec<_>>(),
         });
         
         // Create a smaller template just for the critical points section