From 156ee380b6170cc1d5916926a57dab279450fe5b Mon Sep 17 00:00:00 2001 From: Bharat Kaurav <117659673+TheNetherWatcher@users.noreply.github.com> Date: Fri, 16 Oct 2026 19:17:49 +0530 Subject: [PATCH] fix(analyzer): exclude isolated modules from coupling averages Modules with no in-project imports and no importers (Ca + Ce = 0) have undefined instability, which defaulted to 0 and produced Distance 1. They were averaged into MainSequenceDeviation and AverageInstability, marked high risk, and ranked as refactoring candidates, so a fully decoupled package scored worse than an import chain. Leave isolated modules out of the instability/distance averages and the instability variance used by SystemComplexity, skip the distance term in refactoring priority for them, and report them as low risk. Fixes #850 --- docs/algorithms/dependency.md | 2 +- internal/analyzer/coupling_metrics.go | 42 +++++++++++++------ internal/analyzer/coupling_metrics_test.go | 47 ++++++++++++++++++++++ service/system_analysis_service.go | 7 +++- service/system_analysis_service_test.go | 3 +- 5 files changed, 85 insertions(+), 16 deletions(-) create mode 100644 internal/analyzer/coupling_metrics_test.go diff --git a/docs/algorithms/dependency.md b/docs/algorithms/dependency.md index 79db236b..de71df5d 100644 --- a/docs/algorithms/dependency.md +++ b/docs/algorithms/dependency.md @@ -288,7 +288,7 @@ I = Ce / (Ca + Ce) | I = 0 | Maximally stable -- only depended upon, depends on nothing | | I = 1 | Maximally unstable -- depends on others, nobody depends on it | -If `Ca + Ce = 0` (isolated module), instability defaults to 0. +If `Ca + Ce = 0` (isolated module), instability is undefined. It is reported as 0, but isolated modules are left out of the system-wide averages (average instability and main sequence deviation), are not refactoring candidates by distance, and are always low risk. ### Abstractness (A) diff --git a/internal/analyzer/coupling_metrics.go b/internal/analyzer/coupling_metrics.go index d61ac96a..8e0e4a87 100644 --- a/internal/analyzer/coupling_metrics.go +++ b/internal/analyzer/coupling_metrics.go @@ -175,6 +175,7 @@ func (calc *CouplingMetricsCalculator) calculateSystemMetrics( // Aggregate metrics var totalFanIn, totalFanOut float64 var totalInstability, totalAbstractness, totalDistance float64 + coupledModules := 0 for _, metrics := range moduleMetrics { if err := ctx.Err(); err != nil { @@ -182,8 +183,13 @@ func (calc *CouplingMetricsCalculator) calculateSystemMetrics( } totalFanIn += float64(metrics.AfferentCoupling) totalFanOut += float64(metrics.EfferentCoupling) - totalInstability += metrics.Instability totalAbstractness += metrics.Abstractness + // Instability and distance are undefined for isolated modules. + if IsIsolatedModule(metrics) { + continue + } + coupledModules++ + totalInstability += metrics.Instability totalDistance += metrics.Distance } @@ -193,9 +199,11 @@ func (calc *CouplingMetricsCalculator) calculateSystemMetrics( systemMetrics.AverageFanIn = totalFanIn / moduleCount systemMetrics.AverageFanOut = totalFanOut / moduleCount systemMetrics.DependencyRatio = float64(systemMetrics.TotalDependencies) / moduleCount - systemMetrics.AverageInstability = totalInstability / moduleCount systemMetrics.AverageAbstractness = totalAbstractness / moduleCount - systemMetrics.MainSequenceDeviation = totalDistance / moduleCount + if coupledModules > 0 { + systemMetrics.AverageInstability = totalInstability / float64(coupledModules) + systemMetrics.MainSequenceDeviation = totalDistance / float64(coupledModules) + } // Calculate modularity index systemMetrics.ModularityIndex = calc.calculateModularityIndex() @@ -222,6 +230,13 @@ func (calc *CouplingMetricsCalculator) calculateSystemMetrics( return systemMetrics, nil } +// IsIsolatedModule reports whether a module has no in-project imports and no +// importers (Ca + Ce = 0). Martin's instability Ce/(Ca+Ce) is undefined for +// such modules, so their instability and distance carry no signal. +func IsIsolatedModule(metrics *ModuleMetrics) bool { + return metrics.AfferentCoupling+metrics.EfferentCoupling == 0 +} + func isStableModule(metrics *ModuleMetrics) bool { return metrics.Instability <= lowInstability } @@ -332,15 +347,18 @@ func (calc *CouplingMetricsCalculator) calculateSystemComplexity( // Coupling complexity (variance in instability) var instabilityVariance float64 - if len(moduleMetrics) > 1 { - var sumSquaredDiffs float64 - - for _, metrics := range moduleMetrics { - diff := metrics.Instability - averageInstability - sumSquaredDiffs += diff * diff + var sumSquaredDiffs float64 + coupledModules := 0 + for _, metrics := range moduleMetrics { + if IsIsolatedModule(metrics) { + continue } - - instabilityVariance = sumSquaredDiffs / float64(len(moduleMetrics)) + diff := metrics.Instability - averageInstability + sumSquaredDiffs += diff * diff + coupledModules++ + } + if coupledModules > 1 { + instabilityVariance = sumSquaredDiffs / float64(coupledModules) } couplingComplexity := math.Sqrt(instabilityVariance) * 10 // Scale to reasonable range @@ -364,7 +382,7 @@ func (calc *CouplingMetricsCalculator) identifyRefactoringPriorities( priority := 0.0 // High priority for poor architectural position - if metrics.Distance > 0.5 { + if !IsIsolatedModule(metrics) && metrics.Distance > 0.5 { priority += metrics.Distance * 50 } diff --git a/internal/analyzer/coupling_metrics_test.go b/internal/analyzer/coupling_metrics_test.go new file mode 100644 index 00000000..1464f446 --- /dev/null +++ b/internal/analyzer/coupling_metrics_test.go @@ -0,0 +1,47 @@ +package analyzer + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func calculateTestCouplingMetrics(t *testing.T, graph *DependencyGraph) { + t.Helper() + topology, err := AnalyzeDependencyTopology(context.Background(), graph, 0) + require.NoError(t, err) + calculator := NewCouplingMetricsCalculator(graph, DefaultCouplingMetricsOptions()) + require.NoError(t, calculator.CalculateMetrics(context.Background(), topology)) +} + +func TestCouplingMetricsAllIsolatedModulesHaveNoDeviation(t *testing.T) { + graph := dependencyGraphWithModules("pkg", "pkg.a", "pkg.b", "pkg.c", "pkg.d") + + calculateTestCouplingMetrics(t, graph) + + for name, metrics := range graph.ModuleMetrics { + assert.True(t, IsIsolatedModule(metrics), name) + } + assert.Zero(t, graph.SystemMetrics.MainSequenceDeviation) + assert.Zero(t, graph.SystemMetrics.AverageInstability) + assert.Empty(t, graph.SystemMetrics.RefactoringPriority) +} + +func TestCouplingMetricsExcludeIsolatedModulesFromAverages(t *testing.T) { + graph := dependencyGraphWithModules("entry", "leaf", "isolated") + graph.AddDependency("entry", "leaf", DependencyEdgeImport, nil) + + calculateTestCouplingMetrics(t, graph) + + entry := graph.ModuleMetrics["entry"] + leaf := graph.ModuleMetrics["leaf"] + require.True(t, IsIsolatedModule(graph.ModuleMetrics["isolated"])) + require.False(t, IsIsolatedModule(entry)) + require.False(t, IsIsolatedModule(leaf)) + + assert.InDelta(t, (entry.Distance+leaf.Distance)/2, graph.SystemMetrics.MainSequenceDeviation, 1e-9) + assert.InDelta(t, (entry.Instability+leaf.Instability)/2, graph.SystemMetrics.AverageInstability, 1e-9) + assert.NotContains(t, graph.SystemMetrics.RefactoringPriority, "isolated") +} diff --git a/service/system_analysis_service.go b/service/system_analysis_service.go index a1a31242..e1f2c8e1 100644 --- a/service/system_analysis_service.go +++ b/service/system_analysis_service.go @@ -1577,8 +1577,11 @@ func (s *SystemAnalysisServiceImpl) extractModuleMetrics(graph *analyzer.Depende metrics.Distance = analyzerMetrics.Distance metrics.AbstractClassCount = analyzerMetrics.AbstractClassCount - // Determine risk level based on distance - if analyzerMetrics.Distance > 0.7 { + // Determine risk level based on distance; it is undefined for + // isolated modules, so they stay low risk. + if analyzer.IsIsolatedModule(analyzerMetrics) { + metrics.RiskLevel = domain.RiskLevelLow + } else if analyzerMetrics.Distance > 0.7 { metrics.RiskLevel = domain.RiskLevelHigh } else if analyzerMetrics.Distance > 0.4 { metrics.RiskLevel = domain.RiskLevelMedium diff --git a/service/system_analysis_service_test.go b/service/system_analysis_service_test.go index 0c313bb4..c5fd61e1 100644 --- a/service/system_analysis_service_test.go +++ b/service/system_analysis_service_test.go @@ -518,7 +518,8 @@ func TestExtractModuleMetrics(t *testing.T) { // Add high-risk analyzer metrics graph.ModuleMetrics = make(map[string]*analyzer.ModuleMetrics) graph.ModuleMetrics["risky.module"] = &analyzer.ModuleMetrics{ - Distance: 0.8, // High distance from main sequence + AfferentCoupling: 3, + Distance: 0.8, // High distance from main sequence } return service, graph