feat(codebase): detect low module cohesion

Author: LyleMi

crates/reforge-engine/src/api.rs

@@ -255,6 +255,7 @@ fn validate_public_values(value: &toml::Value) -> Result<()> {
         "codebase.max-public-items",
         "codebase.max-functions-per-file",
         "codebase.max-functions-per-100-lines",
+        "codebase.min-module-functions",
         "codebase.min-repeated-literal-occurrences",
         "codebase.min-data-clump-occurrences",
         "codebase.churn-window-days",
@@ -277,6 +278,7 @@ fn validate_public_values(value: &toml::Value) -> Result<()> {
         }
     }
     validate_percentage(value, "codebase.max-small-function-ratio")?;
+    validate_percentage(value, "codebase.min-clustered-function-percent")?;
     validate_percentage(value, "dataflow.relay.min-relay-percent")?;
     if let Some(candidate) = value_at(value, "codebase.function-similarity")
         && candidate

crates/reforge-engine/src/api/config.rs

@@ -1,5 +1,32 @@
 use super::*;
 
+const CODEBASE_CONFIG_KEYS: &[&str] = &[
+    "preset",
+    "max-file-lines",
+    "max-dir-files",
+    "min-similar-functions",
+    "min-function-tokens",
+    "function-similarity",
+    "max-function-lines",
+    "max-function-complexity",
+    "max-nesting-depth",
+    "max-function-parameters",
+    "max-type-lines",
+    "max-type-members",
+    "max-imports",
+    "max-public-items",
+    "max-functions-per-file",
+    "max-functions-per-100-lines",
+    "max-small-function-ratio",
+    "min-module-functions",
+    "min-clustered-function-percent",
+    "min-repeated-literal-occurrences",
+    "min-data-clump-occurrences",
+    "churn",
+    "churn-window-days",
+    "churn-max-commit-lines",
+];
+
 pub(super) fn validate_public_keys(value: &toml::Value) -> Result<()> {
     validate_table_keys(
         value,
@@ -27,34 +54,7 @@ pub(super) fn validate_public_keys(value: &toml::Value) -> Result<()> {
             "ignore-paths",
         ],
     )?;
-    validate_table_keys(
-        value,
-        ANALYSIS_CODEBASE,
-        &[
-            "preset",
-            "max-file-lines",
-            "max-dir-files",
-            "min-similar-functions",
-            "min-function-tokens",
-            "function-similarity",
-            "max-function-lines",
-            "max-function-complexity",
-            "max-nesting-depth",
-            "max-function-parameters",
-            "max-type-lines",
-            "max-type-members",
-            "max-imports",
-            "max-public-items",
-            "max-functions-per-file",
-            "max-functions-per-100-lines",
-            "max-small-function-ratio",
-            "min-repeated-literal-occurrences",
-            "min-data-clump-occurrences",
-            "churn",
-            "churn-window-days",
-            "churn-max-commit-lines",
-        ],
-    )?;
+    validate_table_keys(value, ANALYSIS_CODEBASE, CODEBASE_CONFIG_KEYS)?;
     validate_table_keys(
         value,
         ANALYSIS_DATAFLOW,

crates/reforge-engine/src/api/tests.rs

@@ -73,7 +73,7 @@ fn repository_dogfood_enables_every_preview_rule_without_changing_defaults() {
     let dogfood = Config::parse_toml(&std::fs::read_to_string(path).unwrap()).unwrap();
     let registry = crate::detectors::manifest::rule_registry();
 
-    assert_eq!(registry.len(), 33);
+    assert_eq!(registry.len(), 34);
     assert_eq!(dogfood.rules.enabled.len(), registry.len());
     assert!(
         registry.iter().all(|rule| {
@@ -86,6 +86,108 @@ fn repository_dogfood_enables_every_preview_rule_without_changing_defaults() {
     assert!(Config::defaults().rules.enabled.is_empty());
 }
 
+#[test]
+fn low_module_cohesion_thresholds_follow_presets_and_overrides() {
+    for (preset, functions, percent) in [
+        ("strict", 16, 40),
+        ("balanced", 20, 50),
+        ("relaxed", 30, 60),
+    ] {
+        let config =
+            Config::parse_toml(&format!("version = 2\n[codebase]\npreset = \"{preset}\"\n"))
+                .unwrap();
+        let resolved = crate::scan::config::effective_scan_config_with(
+            &crate::execution::EffectiveConfig::default(),
+            Some(&config.engine),
+        )
+        .unwrap();
+        assert_eq!(resolved.args.min_module_functions, functions);
+        assert_eq!(resolved.args.min_clustered_function_percent, percent);
+    }
+
+    let config = Config::parse_toml(
+        "version = 2\n[codebase]\npreset = \"strict\"\nmin-module-functions = 18\nmin-clustered-function-percent = 75\n",
+    )
+    .unwrap();
+    let resolved = crate::scan::config::effective_scan_config_with(
+        &crate::execution::EffectiveConfig::default(),
+        Some(&config.engine),
+    )
+    .unwrap();
+    assert_eq!(resolved.args.min_module_functions, 18);
+    assert_eq!(resolved.args.min_clustered_function_percent, 75);
+}
+
+#[test]
+fn low_module_cohesion_config_rejects_invalid_boundaries() {
+    for (setting, expected) in [
+        ("min-module-functions = 0", "positive integer"),
+        ("min-clustered-function-percent = -1", "between 0 and 100"),
+        ("min-clustered-function-percent = 101", "between 0 and 100"),
+    ] {
+        let error = Config::parse_toml(&format!("version = 2\n[codebase]\n{setting}\n"))
+            .unwrap_err()
+            .to_string();
+        assert!(error.contains(expected), "{error}");
+    }
+    Config::parse_toml(
+        "version = 2\n[codebase]\nmin-module-functions = 1\nmin-clustered-function-percent = 0\n",
+    )
+    .unwrap();
+}
+
+#[test]
+fn low_module_cohesion_report_is_schema_stable_and_python_is_not_applicable() {
+    let root = std::env::temp_dir().join(format!(
+        "reforge-low-module-cohesion-{}",
+        std::process::id()
+    ));
+    let _ = std::fs::remove_dir_all(&root);
+    std::fs::create_dir_all(&root).unwrap();
+    std::fs::write(
+        root.join("monolith.js"),
+        "function renderPage(){renderHeader();renderBody();}\nfunction renderHeader(){}\nfunction renderBody(){}\nfunction diffPage(){diffHeader();diffBody();}\nfunction diffHeader(){}\nfunction diffBody(){}\n",
+    )
+    .unwrap();
+    let config = Config::parse_toml(
+        "version = 2\n[rules]\nenable = [\"reforge.codebase.low_module_cohesion\"]\n[codebase]\nmin-module-functions = 6\nmin-clustered-function-percent = 100\nchurn = \"off\"\n",
+    )
+    .unwrap();
+    let analyze_once = || {
+        analyze(&AnalyzeOptions {
+            root: root.clone(),
+            config: config.clone(),
+            reproducible: true,
+            metrics_output: None,
+            flow_ir_output: None,
+        })
+        .unwrap()
+    };
+    let first = analyze_once();
+    let second = analyze_once();
+    first.validate().unwrap();
+    let serialized = serde_json::to_vec(&first).unwrap();
+    let round_trip: Report = serde_json::from_slice(&serialized).unwrap();
+    round_trip.validate().unwrap();
+    assert_eq!(serialized, serde_json::to_vec(&second).unwrap());
+    assert_eq!(first.issues.len(), 1);
+    assert_eq!(first.issues[0].id, round_trip.issues[0].id);
+    assert_eq!(
+        first.issues[0].evidence[0].id,
+        round_trip.issues[0].evidence[0].id
+    );
+
+    std::fs::remove_file(root.join("monolith.js")).unwrap();
+    std::fs::write(root.join("monolith.py"), "def render_page():\n    pass\n").unwrap();
+    let python = analyze_once();
+    assert!(python.issues.is_empty());
+    assert_eq!(
+        python.coverage["codebase"].rules["reforge.codebase.low_module_cohesion"].status,
+        CoverageStatus::NotApplicable
+    );
+    std::fs::remove_dir_all(root).unwrap();
+}
+
 fn options(root: &Path, config: &Path, enabled: BTreeSet<Analysis>) -> AnalyzeOptions {
     let mut config = Config::parse_toml(&std::fs::read_to_string(config).unwrap()).unwrap();
     config.set_enabled(enabled).unwrap();

crates/reforge-engine/src/detectors/concepts.rs

@@ -0,0 +1,84 @@
+const STOP_WORDS: &[&str] = &[
+    "api", "app", "cmd", "for", "from", "get", "has", "impl", "index", "main", "mod", "new", "old",
+    "src", "test", "tests", "the", "this", "type", "use", "with",
+];
+
+pub(crate) fn split_identifier_words(identifier: &str) -> Vec<String> {
+    let mut words = Vec::new();
+    let mut current = String::new();
+    let mut previous_lowercase = false;
+
+    for character in identifier.chars() {
+        if character == '_' || character == '-' || character == '/' || character == '\\' {
+            push_word(&mut words, &mut current);
+            previous_lowercase = false;
+            continue;
+        }
+        if character.is_ascii_uppercase() && previous_lowercase {
+            push_word(&mut words, &mut current);
+        }
+        if character.is_ascii_alphanumeric() {
+            previous_lowercase = character.is_ascii_lowercase() || character.is_ascii_digit();
+            current.push(character.to_ascii_lowercase());
+        } else {
+            push_word(&mut words, &mut current);
+            previous_lowercase = false;
+        }
+    }
+    push_word(&mut words, &mut current);
+    words
+}
+
+fn push_word(words: &mut Vec<String>, current: &mut String) {
+    if current.is_empty() {
+        return;
+    }
+    let word = normalize_word(current);
+    if !word.is_empty() {
+        words.push(word);
+    }
+    current.clear();
+}
+
+pub(crate) fn normalize_word(word: &str) -> String {
+    let mut normalized = word
+        .trim_matches(|character: char| !character.is_ascii_alphanumeric())
+        .to_ascii_lowercase();
+    if normalized.len() > 4 && normalized.ends_with("ies") {
+        normalized.truncate(normalized.len() - 3);
+        normalized.push('y');
+    } else if normalized.len() > 4 && normalized.ends_with('s') {
+        normalized.truncate(normalized.len() - 1);
+    }
+    normalized
+}
+
+pub(crate) fn is_useful_concept_word(word: &str) -> bool {
+    word.len() > 2
+        && !STOP_WORDS.contains(&word)
+        && !word.chars().all(|character| character.is_ascii_digit())
+}
+
+pub(crate) fn identifier_concepts(identifier: &str) -> std::collections::BTreeSet<String> {
+    split_identifier_words(identifier)
+        .into_iter()
+        .filter(|word| is_useful_concept_word(word))
+        .collect()
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn splits_and_normalizes_identifier_concepts() {
+        assert_eq!(
+            identifier_concepts("renderUserEntries"),
+            std::collections::BTreeSet::from([
+                "entry".to_string(),
+                "render".to_string(),
+                "user".to_string(),
+            ])
+        );
+    }
+}

crates/reforge-engine/src/detectors/drift/analysis.rs

@@ -2,3 +2,5 @@ include!("analysis/messages_and_boundaries.rs");
 include!("analysis/source_syntax.rs");
 include!("analysis/naming.rs");
 include!("analysis/constants.rs");
+
+use crate::detectors::concepts::{is_useful_concept_word, normalize_word, split_identifier_words};

crates/reforge-engine/src/detectors/drift/analysis/constants.rs

@@ -129,11 +129,6 @@ pub(super) const GENERIC_BUCKET_WORDS: &[&str] = &[
     "common", "helper", "helpers", "lib", "misc", "shared", "util", "utils",
 ];
 
-pub(super) const STOP_WORDS: &[&str] = &[
-    "api", "app", "cmd", "for", "from", "get", "has", "impl", "index", "main", "mod", "new", "old",
-    "src", "test", "tests", "the", "this", "type", "use", "with",
-];
-
 pub(super) const CONFIG_KEY_WORDS: &[&str] = &[
     "api",
     "auth",

crates/reforge-engine/src/detectors/drift/analysis/naming.rs

@@ -38,65 +38,6 @@ pub(super) fn concept_key(words: &[String], stop_words: &[&str], max_words: usiz
     concepts.join(" ")
 }
 
-pub(super) fn split_identifier_words(identifier: &str) -> Vec<String> {
-    let mut words = Vec::new();
-    let mut current = String::new();
-    let mut previous_lowercase = false;
-
-    for character in identifier.chars() {
-        if character == '_' || character == '-' || character == '/' || character == '\\' {
-            push_word(&mut words, &mut current);
-            previous_lowercase = false;
-            continue;
-        }
-
-        if character.is_ascii_uppercase() && previous_lowercase {
-            push_word(&mut words, &mut current);
-        }
-
-        if character.is_ascii_alphanumeric() {
-            previous_lowercase = character.is_ascii_lowercase() || character.is_ascii_digit();
-            current.push(character.to_ascii_lowercase());
-        } else {
-            push_word(&mut words, &mut current);
-            previous_lowercase = false;
-        }
-    }
-    push_word(&mut words, &mut current);
-
-    words
-}
-
-pub(super) fn push_word(words: &mut Vec<String>, current: &mut String) {
-    if current.is_empty() {
-        return;
-    }
-    let word = normalize_word(current);
-    if !word.is_empty() {
-        words.push(word);
-    }
-    current.clear();
-}
-
-pub(super) fn normalize_word(word: &str) -> String {
-    let mut normalized = word
-        .trim_matches(|character: char| !character.is_ascii_alphanumeric())
-        .to_ascii_lowercase();
-    if normalized.len() > 4 && normalized.ends_with("ies") {
-        normalized.truncate(normalized.len() - 3);
-        normalized.push('y');
-    } else if normalized.len() > 4 && normalized.ends_with('s') {
-        normalized.truncate(normalized.len() - 1);
-    }
-    normalized
-}
-
-pub(super) fn is_useful_concept_word(word: &str) -> bool {
-    word.len() > 2
-        && !STOP_WORDS.contains(&word)
-        && !word.chars().all(|character| character.is_ascii_digit())
-}
-
 pub(super) fn path_words(path: &Path) -> Vec<String> {
     path.components()
         .filter_map(|component| component.as_os_str().to_str())

crates/reforge-engine/src/detectors/drift/mod.rs

@@ -1,6 +1,7 @@
 use std::collections::{BTreeMap, BTreeSet};
 use std::path::{Path, PathBuf};
 
+use crate::detectors::concepts::{is_useful_concept_word, split_identifier_words};
 use crate::detectors::similarity::SourceFile;
 use crate::evidence_analysis::DetectedEvidenceInput;
 use crate::model::{DetectedEvidence, DetectedMeasurement, RelatedLocation, Rule};