fix(checks): floor ImportFanOutOutlier thresholds and stop double-reporting (CD-338)

CHANGELOG.md

@@ -16,9 +16,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Fixed
 - `Refactor.DuplicateBlock` missed clones that differ only in their literal values (CD-331). The report blamed unmodified identifiers, which was wrong: identifiers were already canonicalised to window-relative positional indices, confirmed with a fixture that renamed every one and still fired. String and number literals were the gap — they were hashed by value, so three worker files differing in three strings produced nothing. Literals are now normalised to positional placeholders too, behind a `normalize_literals` option that is on by default.
 - Import blocks are no longer eligible to be duplicates. Normalising module specifiers made every run of `import` statements match every other one — one repo went from 8 findings to 66, almost all on line 1 — and an import list cannot be extracted into a shared helper, so the finding was never actionable. `import`, `export * from` and `export { x } from` are excluded from statement windows outright; a plain `export { x }` or `export function` still participates. **On upgrade this can move a finding's primary span further down a file.** Suppression targets a comment's next non-blank line rather than a range, so a `cofferdam-ignore` anchored to an import may no longer cover the block it was written for, surfacing a duplicate that had been suppressed. Move the comment to the first statement of the block itself.
-- `Refactor.NearDuplicateBlock` is new, and reports the near-clones that literal normalisation newly finds. They are worth seeing — a copy that has drifted is where bugs hide — but on real projects they are mostly test setup blocks differing in their fixture values, and `--fail-on` defaults to `medium`, so reporting them under the existing check would turn CI red for duplication nobody chose to gate on. The engine stamps severity per check id rather than per issue, so a second id is the only way to separate them. `Refactor.DuplicateBlock` keeps `medium` and now reports verbatim clones only; `NearDuplicateBlock` defaults to `low`, printing without failing a build. Raise it with `[checks."Refactor.NearDuplicateBlock"] severity = "medium"` to gate on it deliberately. Both read one corpus slot and share a single overlap-claim pass, so they never report the same region twice.- `Design.LayerViolation`'s catalogue page no longer recommends `.cofferdamignore` for excluding tests. That is the exact configuration that makes `Design.OrphanExport` claim a symbol is "never imported in the project" when its only importers are the ignored tests — `.cofferdamignore` prunes files before discovery, so they contribute no import edges either (CD-325). The page now points at an `[[overrides]]` block with `disabled = true`, which turns off the one check without removing the files from the graph. The underlying behaviour is unchanged and tracked separately.
-- `Design.ImportFanOutOutlier`'s fan-in branch no longer fires on fan-in alone (CD-333). Any successfully shared module is a fan-in outlier by construction, so the check was flagging leaf utilities for being reused: a manual audit across five local repos found 23 fan-in findings, every one a false positive (buttons, config, utils, type modules, an API client), and not one genuine over-centralised god object. The fan-in branch now also requires the file's fan-out to be a statistical outlier, matching the check's stated target of a hub with high fan-in *and* high fan-out. Fan-in findings are rare by design as a result; the fan-out branch is unchanged.
+- `Refactor.NearDuplicateBlock` is new, and reports the near-clones that literal normalisation newly finds. They are worth seeing — a copy that has drifted is where bugs hide — but on real projects they are mostly test setup blocks differing in their fixture values, and `--fail-on` defaults to `medium`, so reporting them under the existing check would turn CI red for duplication nobody chose to gate on. The engine stamps severity per check id rather than per issue, so a second id is the only way to separate them. `Refactor.DuplicateBlock` keeps `medium` and now reports verbatim clones only; `NearDuplicateBlock` defaults to `low`, printing without failing a build. Raise it with `[checks."Refactor.NearDuplicateBlock"] severity = "medium"` to gate on it deliberately. Both read one corpus slot and share a single overlap-claim pass, so they never report the same region twice.
 - `Design.LayerViolation`'s catalogue page no longer recommends `.cofferdamignore` for excluding tests. That is the exact configuration that makes `Design.OrphanExport` claim a symbol is "never imported in the project" when its only importers are the ignored tests — `.cofferdamignore` prunes files before discovery, so they contribute no import edges either (CD-325). The page now points at an `[[overrides]]` block with `disabled = true`, which turns off the one check without removing the files from the graph. The underlying behaviour is unchanged and tracked separately.
+- `Design.ImportFanOutOutlier` no longer flags shared utility modules (CD-333, CD-338). Any successfully shared module is a fan-in outlier by construction, so the check was reporting leaf utilities for being reused: an audit across five local repos found 23 fan-in findings, every one a false positive, and not one genuine over-centralised god object. A fan-in finding now also requires the file's fan-out to be an outlier, matching the check's stated target of a hub with high fan-in *and* high fan-out, and such a file is reported once as a combined finding rather than twice. Both metrics must additionally clear an absolute floor of eight. The relative bar alone was not enough: only in-project edges count, so a codebase that imports mainly from `node_modules` has a sparse graph where the mean is 0.1 and three standard deviations reach 1.0, and a file importing one local module cleared it. Across the same five repos this drops 43 findings to 20, all 23 removed being fan-in-only reports of shared modules — an error adapter used by 25 files, a test helper used by 58 — while every surviving report is a file importing between 15 and 54 others.
 - `Refactor.PurityHeuristic` can be switched on (CD-324). Its one option is called `enabled`, and the config loader stripped that key out of every check's option bag before validation as a forward-compatible placeholder — so the check's own catalogue page told the user to write `enabled = true`, and the check never saw it. A registered check no user could reach is worse than an unrecognised key, so `enabled` now flows through to the option bag like any other option, and is dropped again only for the checks that do not declare it. Configs carrying a stray `enabled` therefore keep loading rather than failing on upgrade, which is why the fix is silent rather than a warning: making the inert case loud needs a warning sink `options_for_raw` does not have, and is not worth a breaking change to reach.
 - `scripts/version.mjs set X.Y.Z --regen` could stamp the previous version into `docs/public/checks.json` and `docs/public/llms.txt` (CD-317). A stale cargo fingerprint after a `Cargo.toml` revert-then-rebump left `cargo build --workspace` reporting `Finished` without recompiling `cofferdam-cli`, so the subsequent `gen-docs` ran the old binary and wrote the old version, and `node scripts/version.mjs check` then failed on locations that regen was meant to fix. `--regen` now runs `cargo clean -p cofferdam-cli` before the build to force that crate's recompile, checks the built binary's own `--version` against the target before trusting it to run `gen-docs`, and re-reads `checks.json`/`llms.txt` afterwards to confirm they landed on the target version — failing loudly and naming both versions if not.
 - `--hide-baselined` now applies to every output format, not text alone (CD-315). The flag filtered findings inside the text formatter, so `--format=json`, `compact` and `sarif` received the untouched list and printed baselined findings anyway, marked `"baselined": true`. Nothing in the help text said so, and a CI pipeline reading JSON was quietly counting pre-existing findings as new work. The filter now runs once in the CLI, ahead of the per-format branch, so all four formats render the same list. Summary counts are deliberately left whole — `total`, `new` and `baselined` still describe the full run, matching what the text formatter has always done — so the gate count stays visible when the entries are hidden.

crates/cofferdam-checks/docs/Design.ImportFanOutOutlier.md

@@ -27,7 +27,9 @@ export * from "./b";
 export * from "./c";
 ```
 
-Statistics: computed only over in-project (resolved) import edges — an external package import (`react`, `lodash`) doesn't count toward either metric, including a bare specifier the resolver traced into `node_modules` (a vendor package's own internal import graph never counts toward fan-in/fan-out, and vendor files never enter the population). `index.*`/`types.*` basenames are excluded from the statistical population entirely (not just from being flagged), since including their legitimately extreme counts would inflate the mean/stddev for every other file. Below 8 non-excluded files in the project, or when a metric's standard deviation is 0 (every file has the same count), nothing is flagged for that metric — there isn't enough variation to call anything an outlier. A file's fan-out must exceed the project mean plus three standard deviations to be flagged on its own. Fan-in requires both: the file's fan-in must clear its own three-standard-deviation bar and its fan-out must clear the fan-out bar too. Fan-in alone can never distinguish a healthy leaf module — reused everywhere, importing little — from a real god object, so the check no longer treats it as sufficient by itself. This makes fan-in findings rare by design: most true hubs show up on the fan-out branch anyway, and the fan-in branch now exists only to catch a hub that also gets pulled in from everywhere.
+Statistics: computed only over in-project (resolved) import edges — an external package import (`react`, `lodash`) doesn't count toward either metric, including a bare specifier the resolver traced into `node_modules` (a vendor package's own internal import graph never counts toward fan-in/fan-out, and vendor files never enter the population). `index.*`/`types.*` basenames are excluded from the statistical population entirely (not just from being flagged), since including their legitimately extreme counts would inflate the mean/stddev for every other file. Below 8 non-excluded files in the project, or when a metric's standard deviation is 0 (every file has the same count), nothing is flagged for that metric — there isn't enough variation to call anything an outlier. A file's fan-out must exceed the project mean plus three standard deviations to be flagged on its own. Fan-in requires both: the file's fan-in must clear its own three-standard-deviation bar and its fan-out must clear the fan-out bar too.
+
+Each metric must also clear an absolute floor of eight, not merely the relative bar. In a project whose files import mainly from `node_modules` the in-project graph is sparse — a mean of 0.1 and a standard deviation of 0.3 put the relative threshold below one — so without the floor a file importing a single local module would read as an outlier. A file is reported at most once: a combined hub finding when both metrics qualify, a fan-out finding otherwise. Fan-in alone can never distinguish a healthy leaf module — reused everywhere, importing little — from a real god object, so the check no longer treats it as sufficient by itself. This makes fan-in findings rare by design: most true hubs show up on the fan-out branch anyway, and the fan-in branch now exists only to catch a hub that also gets pulled in from everywhere.
 
 Suppressing a legitimate hub: findings are pinned to line 1, column 1 of the file (it's a whole-file metric, not tied to a specific import), so the usual next-line `// cofferdam-ignore: Design.ImportFanOutOutlier` comment has no real "line 0" to sit on. Use the file-wide directive instead — `// cofferdam-ignore-file: Design.ImportFanOutOutlier: <reason>` anywhere in the file — which matches regardless of the finding's line number.
 

crates/cofferdam-checks/src/design/import_fan_out_outlier.rs

@@ -20,6 +20,13 @@ const MIN_FILES: usize = 8;
 /// to count as an outlier.
 const STDDEV_MULTIPLIER: f64 = 3.0;
 
+/// A metric must also clear this absolute count, not just the relative
+/// stddev threshold. In a project whose files import mainly from
+/// node_modules the in-project graph is sparse enough that
+/// `mean + 3*stddev` falls below 1, and a file importing a single local
+/// module would otherwise read as an outlier.
+const MIN_ABSOLUTE_FAN: u32 = 8;
+
 /// Basenames treated as intentional aggregator hubs — always high
 /// fan-in/fan-out by design (a barrel `index.ts`, a shared `types.ts`).
 /// Excluded from BOTH the flaggable set AND the statistical population,
@@ -75,11 +82,14 @@ const META: CheckMeta = CheckMeta {
 };
 
 /// `Design.ImportFanOutOutlier` — finalize-stage check (CD-130) that
-/// flags a file whose import fan-in (files that import it) or fan-out
-/// (files it imports) is more than `STDDEV_MULTIPLIER` standard
-/// deviations above the project mean, computed over the project's
-/// in-project (resolved) import edges only — external package imports
-/// don't count toward either metric.
+/// flags a file whose import fan-out (files it imports) is more than
+/// `STDDEV_MULTIPLIER` standard deviations above the project mean AND
+/// at least `MIN_ABSOLUTE_FAN`, computed over the project's in-project
+/// (resolved) import edges only — external package imports don't count
+/// toward either metric. Fan-in (files that import it) is never
+/// sufficient on its own; when it clears the same two bars alongside
+/// fan-out, the file is reported once as a combined hub finding rather
+/// than twice.
 ///
 /// Scope: files matching `HUB_BASENAMES` (barrel `index.*`, `types.*`)
 /// or resolving as the nearest `package.json`'s declared entry point
@@ -224,11 +234,17 @@ fn compute_outliers(
     let mut sorted_population = population;
     sorted_population.sort_by(|a, b| a.display.cmp(&b.display));
     for stats in sorted_population {
-        if stddev_in > 0.0
+        let fan_in_is_outlier = stddev_in > 0.0
             && stats.fan_in as f64 > threshold_in
-            && stddev_out > 0.0
+            && stats.fan_in >= MIN_ABSOLUTE_FAN;
+        let fan_out_is_outlier = stddev_out > 0.0
             && stats.fan_out as f64 > threshold_out
-        {
+            && stats.fan_out >= MIN_ABSOLUTE_FAN;
+        // Exclusive branches: fan-in never qualifies without fan-out also
+        // qualifying (see the module doc), so at most one issue per file
+        // — a combined finding when both are outliers, else a fan-out-only
+        // finding, never both for the same file.
+        if fan_in_is_outlier && fan_out_is_outlier {
             issues.push(Issue {
                 check_id: META.id.to_string(),
                 message: format!(
@@ -241,8 +257,7 @@ fn compute_outliers(
                 severity: Severity::Medium,
                 related: Vec::new(),
             });
-        }
-        if stddev_out > 0.0 && stats.fan_out as f64 > threshold_out {
+        } else if fan_out_is_outlier {
             issues.push(Issue {
                 check_id: META.id.to_string(),
                 message: format!(
@@ -366,4 +381,59 @@ mod tests {
         );
         assert!(hub_issues[0].message.contains("fan-out"));
     }
+
+    /// A leaf imported by many files but itself importing only one local
+    /// module must not be flagged: `high_fan_in_alone_is_not_flagged`
+    /// above only passes because its leaf has fan_out == 0, which is
+    /// unrealistic — a leaf with a single import has a fan_out that can
+    /// still clear a sparse graph's relative threshold without an
+    /// absolute floor.
+    #[test]
+    fn sparse_graph_does_not_flag_a_leaf_that_imports_one_module() {
+        let leaf = PathBuf::from("/proj/leaf.ts");
+        let leaf_dep = PathBuf::from("/proj/leaf_dep.ts");
+        let mut all_files = HashSet::new();
+        all_files.insert(leaf.clone());
+        all_files.insert(leaf_dep.clone());
+        let mut imports = Vec::new();
+        for i in 0..15 {
+            let importer = PathBuf::from(format!("/proj/importer_{i}.ts"));
+            all_files.insert(importer.clone());
+            imports.push(edge(&importer, &leaf));
+        }
+        imports.push(edge(&leaf, &leaf_dep));
+        for i in 0..150 {
+            all_files.insert(PathBuf::from(format!("/proj/filler_{i}.ts")));
+        }
+
+        let issues = compute_outliers(&imports, &[], &all_files);
+        assert!(
+            issues.is_empty(),
+            "expected no findings for a leaf with fan_out 1 in a sparse graph, got {issues:?}"
+        );
+    }
+
+    /// The same file must never receive both a fan-out finding and a
+    /// combined fan-in/fan-out finding.
+    #[test]
+    fn a_file_is_never_reported_twice() {
+        let hub = PathBuf::from("/proj/hub.ts");
+        let mut all_files = HashSet::new();
+        all_files.insert(hub.clone());
+        let mut imports = Vec::new();
+        for i in 0..15 {
+            let sibling = PathBuf::from(format!("/proj/sibling_{i}.ts"));
+            all_files.insert(sibling.clone());
+            imports.push(edge(&sibling, &hub));
+            imports.push(edge(&hub, &sibling));
+        }
+
+        let issues = compute_outliers(&imports, &[], &all_files);
+        let hub_issues: Vec<&Issue> = issues.iter().filter(|i| i.file == hub).collect();
+        assert_eq!(
+            hub_issues.len(),
+            1,
+            "expected exactly one issue for hub, got {issues:?}"
+        );
+    }
 }

crates/cofferdam-checks/src/design/mod.rs

@@ -1770,6 +1770,49 @@ export function computeTotal(items: number[]): number {
         assert_eq!(fan_in_issues[0].file, hub);
     }
 
+    #[test]
+    fn sparse_graph_does_not_flag_a_leaf_that_imports_one_module() {
+        // A leaf imported by many files but itself importing only one
+        // local module must not be flagged: `high_fan_in_alone_is_not_flagged`
+        // above only passes because its leaf has fan_out == 0, which is
+        // unrealistic — a leaf with a single import can still clear a
+        // sparse graph's relative threshold without an absolute floor.
+        let leaf = PathBuf::from("/p/leaf.ts");
+        let leaf_dep = PathBuf::from("/p/leaf_dep.ts");
+        let mut imports: Vec<ImportRecord> = (0..14)
+            .map(|i| {
+                let importer = PathBuf::from(format!("/p/importer{i}.ts"));
+                internal_import(&importer, &leaf)
+            })
+            .collect();
+        imports.push(internal_import(&leaf, &leaf_dep));
+        let issues = run_fan_out_outlier(imports);
+        assert!(
+            issues.is_empty(),
+            "expected no findings for a leaf with fan_out 1 in a sparse graph; got {issues:?}"
+        );
+    }
+
+    #[test]
+    fn a_file_is_never_reported_twice() {
+        // The same file must never receive both a fan-out finding and a
+        // combined fan-in/fan-out finding.
+        let hub = PathBuf::from("/p/hub.ts");
+        let mut imports = Vec::new();
+        for i in 0..14 {
+            let sibling = PathBuf::from(format!("/p/sibling{i}.ts"));
+            imports.push(internal_import(&sibling, &hub));
+            imports.push(internal_import(&hub, &sibling));
+        }
+        let issues = run_fan_out_outlier(imports);
+        let hub_issues: Vec<&CoreIssue> = issues.iter().filter(|i| i.file == hub).collect();
+        assert_eq!(
+            hub_issues.len(),
+            1,
+            "expected exactly one issue for hub; got {issues:?}"
+        );
+    }
+
     #[test]
     fn below_min_files_emits_nothing() {
         // Only 3 files total, well under MIN_FILES (8).