Merge pull request #20 from Eilodon/claude/calm-mcp-connection-9tk1f4

Add boundary globs, hotspot norm/bot signals, real preset filtering, …

Author: Eilodon

crates/calm-core/src/analysis/boundaries.rs

@@ -2,12 +2,13 @@ use rusqlite::Connection;
 use serde::{Deserialize, Serialize};
 
 /// One declared architecture rule: files under `from` must not import files
-/// under `to`. Matching is a simple path-prefix check on project-relative,
-/// forward-slash paths (the same format `import_edges.from_path`/`to_path`
-/// already use) — deliberately not glob/regex, so a rule reads exactly like
-/// the directory boundary it describes and there's no pattern syntax to get
-/// wrong. `from`/`to` are directory prefixes (e.g. `"crates/calm-core/src/indexer/"`),
-/// not exact file paths.
+/// under `to`. `from`/`to` are project-relative, forward-slash paths (the
+/// same format `import_edges.from_path`/`to_path` already use). A pattern
+/// containing a glob metacharacter (`*`, `?`, `[`) is matched with `globset`
+/// against the whole path (e.g. `"crates/*/src/indexer/**"`); otherwise it's
+/// treated as a plain directory prefix via `starts_with` (e.g.
+/// `"crates/calm-core/src/indexer/"`) — existing prefix-style rules keep
+/// working unchanged.
 #[derive(Debug, Clone, Deserialize, Serialize)]
 pub struct BoundaryRule {
     pub from: String,
@@ -30,6 +31,34 @@ pub struct BoundaryViolation {
 /// is reported per (edge, rule) pair — the same edge can violate more than
 /// one rule if rules overlap, which is surfaced rather than deduplicated so
 /// each rule's own `reason` is visible.
+enum PathMatcher {
+    Prefix(String),
+    Glob(globset::GlobMatcher),
+}
+
+impl PathMatcher {
+    /// A pattern containing a glob metacharacter is compiled with `globset`;
+    /// otherwise (including invalid glob syntax) it falls back to a plain
+    /// `starts_with` prefix check, so existing prefix-style rules are
+    /// unaffected and a typo'd glob degrades to its literal prefix rather
+    /// than silently matching nothing.
+    fn new(pattern: &str) -> Self {
+        if pattern.contains(['*', '?', '[']) {
+            if let Ok(glob) = globset::Glob::new(pattern) {
+                return PathMatcher::Glob(glob.compile_matcher());
+            }
+        }
+        PathMatcher::Prefix(pattern.to_string())
+    }
+
+    fn matches(&self, path: &str) -> bool {
+        match self {
+            PathMatcher::Prefix(prefix) => path.starts_with(prefix.as_str()),
+            PathMatcher::Glob(matcher) => matcher.is_match(path),
+        }
+    }
+}
+
 pub fn check_boundaries(
     conn: &Connection,
     rules: &[BoundaryRule],
@@ -46,10 +75,15 @@ pub fn check_boundaries(
         .filter_map(|r| r.ok())
         .collect();
 
+    let matchers: Vec<(PathMatcher, PathMatcher)> = rules
+        .iter()
+        .map(|rule| (PathMatcher::new(&rule.from), PathMatcher::new(&rule.to)))
+        .collect();
+
     let mut violations = Vec::new();
     for (from_path, to_path) in &edges {
-        for rule in rules {
-            if from_path.starts_with(&rule.from) && to_path.starts_with(&rule.to) {
+        for (rule, (from_matcher, to_matcher)) in rules.iter().zip(&matchers) {
+            if from_matcher.matches(from_path) && to_matcher.matches(to_path) {
                 violations.push(BoundaryViolation {
                     from_path: from_path.clone(),
                     to_path: to_path.clone(),
@@ -183,4 +217,43 @@ mod tests {
         let violations = check_boundaries(&conn, &rules).unwrap();
         assert_eq!(violations.len(), 2);
     }
+
+    #[test]
+    fn test_glob_rule_matches_nested_paths() {
+        let conn = test_conn();
+        insert_import(&conn, "crates/calm-core/src/indexer/foo.rs", "crates/calm-server/src/tools/orient.rs");
+        let rules = vec![BoundaryRule {
+            from: "crates/*/src/indexer/**".into(),
+            to: "crates/*/src/tools/**".into(),
+            reason: "indexer must not import server tools".into(),
+        }];
+        let violations = check_boundaries(&conn, &rules).unwrap();
+        assert_eq!(violations.len(), 1);
+    }
+
+    #[test]
+    fn test_glob_rule_does_not_match_unrelated_path() {
+        let conn = test_conn();
+        insert_import(&conn, "crates/calm-core/src/other/foo.rs", "crates/calm-server/src/tools/orient.rs");
+        let rules = vec![BoundaryRule {
+            from: "crates/*/src/indexer/**".into(),
+            to: "crates/*/src/tools/**".into(),
+            reason: "indexer must not import server tools".into(),
+        }];
+        let violations = check_boundaries(&conn, &rules).unwrap();
+        assert!(violations.is_empty());
+    }
+
+    #[test]
+    fn test_invalid_glob_falls_back_to_literal_prefix() {
+        let conn = test_conn();
+        insert_import(&conn, "a/[unclosed/x.py", "b/y.py");
+        let rules = vec![BoundaryRule {
+            from: "a/[unclosed".into(),
+            to: "b/".into(),
+            reason: "invalid glob syntax degrades to prefix match".into(),
+        }];
+        let violations = check_boundaries(&conn, &rules).unwrap();
+        assert_eq!(violations.len(), 1);
+    }
 }

crates/calm-core/src/analysis/coverage.rs

@@ -23,7 +23,7 @@ impl CoverageData {
     }
 }
 
-const COVERAGE_SEARCH_PATHS: &[(&str, &str)] = &[
+pub const COVERAGE_SEARCH_PATHS: &[(&str, &str)] = &[
     ("lcov.info", "lcov"),
     ("coverage/lcov.info", "lcov"),
     (".nyc_output/lcov.info", "lcov"),

crates/calm-core/src/analysis/hotspot.rs

@@ -8,10 +8,31 @@ use crate::config::HotspotsConfig;
 #[derive(Debug, Clone)]
 pub struct ChurnInfo {
     pub commit_count: i64,
+    /// Commits whose author looks like a bot account (see `is_bot_author`) —
+    /// counted separately from `commit_count` so `churn_source` can tell
+    /// human-driven churn from CI/dependency-bump noise.
+    pub bot_commit_count: i64,
     pub authors: HashSet<String>,
     pub last_changed: Option<String>,
 }
 
+impl ChurnInfo {
+    /// "unknown" with no churn data at all (e.g. git unavailable),
+    /// "human" when no commit came from a bot account, "bot_dominated" when
+    /// every commit did, "mixed" otherwise.
+    pub fn churn_source(&self) -> &'static str {
+        if self.commit_count == 0 {
+            "unknown"
+        } else if self.bot_commit_count == 0 {
+            "human"
+        } else if self.bot_commit_count >= self.commit_count {
+            "bot_dominated"
+        } else {
+            "mixed"
+        }
+    }
+}
+
 #[derive(Debug, Clone)]
 pub struct ComplexityInfo {
     pub symbol_count: i64,
@@ -41,6 +62,13 @@ pub struct HotspotEntry {
     pub language: String,
     pub churn: ChurnInfo,
     pub complexity: ComplexityInfo,
+    /// Churn share (0-1) of `hotspot_score`'s numerator, normalized against
+    /// the busiest candidate file this run — 0.0 when git is unavailable
+    /// (no churn signal at all, not "no churn").
+    pub norm_churn: f64,
+    /// Complexity share (0-1) of `hotspot_score`'s numerator, normalized
+    /// against the most complex candidate file this run.
+    pub norm_compl: f64,
     pub hotspot_score: f64,
     pub risk_level: String,
     pub top_symbols: Option<Vec<HotspotSymbol>>,
@@ -87,6 +115,7 @@ pub fn compute_hotspots(
                     path.clone(),
                     ChurnInfo {
                         commit_count: 0,
+                        bot_commit_count: 0,
                         authors: HashSet::new(),
                         last_changed: None,
                     },
@@ -141,10 +170,13 @@ pub fn compute_hotspots(
         .iter()
         .filter_map(|(path, churn)| {
             let cm = complexity_map.get(path)?;
+            // Computed unconditionally (not just when `git_available`) so
+            // callers can see the churn share even when it didn't factor
+            // into `hotspot_score` — 0.0 uniformly when git is unavailable,
+            // since every candidate's `commit_count` is 0 in that branch.
+            let norm_churn = churn_scores.get(path.as_str()).copied().unwrap_or(0.0) / max_churn;
             let norm_compl = compl_scores.get(path.as_str()).copied().unwrap_or(0.0) / max_compl;
             let score = if git_available {
-                let norm_churn =
-                    churn_scores.get(path.as_str()).copied().unwrap_or(0.0) / max_churn;
                 norm_churn * norm_compl
             } else {
                 norm_compl
@@ -165,6 +197,8 @@ pub fn compute_hotspots(
                 language: cm.language.clone(),
                 churn: churn.clone(),
                 complexity: cm.clone(),
+                norm_churn: (norm_churn * 10000.0).round() / 10000.0,
+                norm_compl: (norm_compl * 10000.0).round() / 10000.0,
                 hotspot_score: (score * 10000.0).round() / 10000.0,
                 risk_level: risk_level.to_string(),
                 top_symbols: None,
@@ -283,19 +317,31 @@ fn collect_git_churn(project_root: &Path, since: &str) -> (HashMap<String, Churn
         for path in &commit.files {
             let entry = churn_map.entry(path.clone()).or_insert_with(|| ChurnInfo {
                 commit_count: 0,
+                bot_commit_count: 0,
                 authors: HashSet::new(),
                 // H-1 fix: last_changed is Option<String>, None instead of ""
                 last_changed: commit.date.clone(),
             });
             entry.commit_count += 1;
             if let Some(ref author) = commit.author {
                 entry.authors.insert(author.clone());
+                if is_bot_author(author) {
+                    entry.bot_commit_count += 1;
+                }
             }
         }
     }
     (churn_map, true)
 }
 
+/// GitHub's bot accounts (dependabot[bot], renovate[bot], github-actions[bot],
+/// etc.) all carry `[bot]` in their noreply author email (`%ae`), which is
+/// what `commits_with_files` captures — this substring check covers them
+/// without hardcoding a specific bot allowlist.
+fn is_bot_author(author: &str) -> bool {
+    author.contains("[bot]")
+}
+
 fn collect_complexity(conn: &Connection) -> HashMap<String, ComplexityInfo> {
     // is_test = 0: test functions structurally call production code, so they
     // almost always end up with coreness > 0 despite never being called
@@ -512,6 +558,100 @@ mod tests {
         assert_eq!(output.hotspots[0].churn.commit_count, 2);
     }
 
+    #[test]
+    fn test_churn_source_classifies_human_bot_mixed_and_unknown() {
+        let unknown = ChurnInfo {
+            commit_count: 0,
+            bot_commit_count: 0,
+            authors: HashSet::new(),
+            last_changed: None,
+        };
+        assert_eq!(unknown.churn_source(), "unknown");
+
+        let human = ChurnInfo {
+            commit_count: 3,
+            bot_commit_count: 0,
+            authors: HashSet::new(),
+            last_changed: None,
+        };
+        assert_eq!(human.churn_source(), "human");
+
+        let bot_dominated = ChurnInfo {
+            commit_count: 3,
+            bot_commit_count: 3,
+            authors: HashSet::new(),
+            last_changed: None,
+        };
+        assert_eq!(bot_dominated.churn_source(), "bot_dominated");
+
+        let mixed = ChurnInfo {
+            commit_count: 4,
+            bot_commit_count: 1,
+            authors: HashSet::new(),
+            last_changed: None,
+        };
+        assert_eq!(mixed.churn_source(), "mixed");
+    }
+
+    #[test]
+    fn test_is_bot_author_matches_bot_suffix() {
+        assert!(is_bot_author(
+            "49699333+dependabot[bot]@users.noreply.github.com"
+        ));
+        assert!(is_bot_author(
+            "29139614+renovate[bot]@users.noreply.github.com"
+        ));
+        assert!(!is_bot_author("jane@example.com"));
+    }
+
+    #[test]
+    fn test_collect_git_churn_tallies_bot_commits_separately() {
+        let dir = tempfile::tempdir().unwrap();
+        run_git(dir.path(), &["init", "-q"]);
+        run_git(dir.path(), &["config", "user.email", "human@example.com"]);
+        run_git(dir.path(), &["config", "user.name", "Human"]);
+        std::fs::write(dir.path().join("f.py"), "x = 1\n").unwrap();
+        run_git(dir.path(), &["add", "f.py"]);
+        run_git(dir.path(), &["commit", "-q", "-m", "init"]);
+
+        run_git(
+            dir.path(),
+            &[
+                "config",
+                "user.email",
+                "49699333+dependabot[bot]@users.noreply.github.com",
+            ],
+        );
+        run_git(dir.path(), &["config", "user.name", "dependabot[bot]"]);
+        std::fs::write(dir.path().join("f.py"), "x = 2\n").unwrap();
+        run_git(dir.path(), &["commit", "-q", "-am", "bump"]);
+
+        let (churn_map, git_available) = collect_git_churn(dir.path(), "1 year");
+        assert!(git_available);
+        let entry = churn_map.get("f.py").unwrap();
+        assert_eq!(entry.commit_count, 2);
+        assert_eq!(entry.bot_commit_count, 1);
+        assert_eq!(entry.churn_source(), "mixed");
+    }
+
+    #[test]
+    fn test_norm_compl_and_norm_churn_are_exposed_without_git() {
+        let conn = setup_db();
+        let dir = tempfile::tempdir().unwrap();
+        insert_symbol(&conn, "a.func1", "/a.py", 0, false, 0);
+        insert_symbol(&conn, "b.func1", "/b.py", 10, true, 5);
+        insert_symbol(&conn, "b.func2", "/b.py", 2, false, 1);
+
+        let config = HotspotsConfig::default();
+        let output = compute_hotspots(dir.path(), &conn, &config, 10, "6 months ago", 2, false);
+        assert!(!output.git_available);
+        // No churn signal at all when git is unavailable.
+        assert!(output.hotspots.iter().all(|h| h.norm_churn == 0.0));
+        // b.py is the most complex candidate, so its complexity share is 1.0.
+        let b = output.hotspots.iter().find(|h| h.path == "/b.py").unwrap();
+        assert_eq!(b.norm_compl, 1.0);
+    }
+
     /// Regression: `compute_absolute_hotspot_risk` must NOT saturate to
     /// ~1.0 just because one file is *relatively* the busiest in a small,
     /// healthy repo — unlike `compute_hotspots`' `hotspot_score` (min-max