fix(checks): floor ImportFanOutOutlier thresholds and stop double-reporting (CD-338) - #218
Merged
Conversation
…orting (CD-338) CD-333 left the fan-out threshold purely relative. Only in-project resolved edges count, so a codebase importing mainly from node_modules has a sparse graph where mean 0.1 and stddev 0.3 put mean + 3*stddev below 1 — a file importing one local module cleared the gate and was reported as a god module. Its regression test passed only because the synthetic leaf had fan-out 0; real leaves import something. Both metrics now also require an absolute floor of 8. The amended fan-in condition was additionally a strict superset of the fan-out condition, and the engine has no issue-dedup pass, so a qualifying file received two issues with the same check id and location. The branches are now exclusive: a combined hub finding when both metrics qualify, a fan-out finding otherwise, never both. Across five local repos this takes 43 findings to 20. Every removed report is fan-in-only on a shared module (an error adapter imported by 25 files, a test helper by 58); every survivor imports 15 to 54 others. Also corrects the CHANGELOG's [Unreleased] section, where a prior conflict resolution ran two bullets together and duplicated a third. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019afugU2bTYCtXn6Vy9dn1v
Criterion benchmark comparison (PR head vs base branch) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes two defects introduced by CD-333 (#217), found by adversarial review.
The fan-out threshold had no absolute floor
threshold_out = mean_out + 3*stddev_outis purely relative. Onlyresolvedin-project edges count, so a project importing mainly fromnode_moduleshas a sparse local graph — mean 0.1, stddev 0.3, threshold ~1.0. A file importing a single local module cleared the gate and was reported as a god module.CD-333's regression test passed only because its synthetic leaf had
fan_out == 0. Real leaves import something. Both metrics now require>= MIN_ABSOLUTE_FAN(8) as well as the relative bar.Fan-in findings were emitted twice
The amended fan-in condition was a strict superset of the fan-out condition, so fan-in could never fire alone — and the engine has no issue-dedup pass (
finalize_and_filterdoes override-retain, suppression-retain, severity stamp and sort). A qualifying file received two issues with the same check id, file and 1:1 location.The branches are now exclusive: a combined hub finding when both metrics qualify, a fan-out finding otherwise, never both.
Measured effect
Across five local repos,
Design.ImportFanOutOutliergoes from 43 findings to 20.Every removed finding is fan-in-only on a shared module — projektor's
services/errors.ts(47 importers),test/helpers.ts(58),utils/api-client.ts(33). Every survivor is a file importing 15 to 54 others.Tests
Two new tests in both
import_fan_out_outlier.rsanddesign/mod.rs, each confirmed failing before the fix:sparse_graph_does_not_flag_a_leaf_that_imports_one_module— the case the existing suite misses, because its leaf has fan-out 0. Failed with 17 spurious findings.a_file_is_never_reported_twice— asserts exactly one issue, not merely that a message matches. Failed with 2.Also
Corrects the CHANGELOG
[Unreleased]section, where a prior conflict resolution ran two bullets together and duplicated a third.🤖 Generated with Claude Code
https://claude.ai/code/session_019afugU2bTYCtXn6Vy9dn1v