Tune ruby rails explainers

internal/cachecov/coverage_test.go

@@ -126,6 +126,7 @@ var versionCoverage = map[int][]string{
 	92: {"TestRoutes_SymbolPathArg", "TestRoutes_ScopeBareSymbolPrefix", "TestRoutes_ResourcePathOverride"},                                                                                                                                                                                        // Ruby symbol path args + scope :symbol + resource path: override
 	93: {"TestSwitchReturns_MultiLineCaseLabels", "TestExtractEndpointFacts_MultiLineMethodCase"},                                                                                                                                                                                                 // Swift multi-line case-label method parsing
 	94: {"TestExtractEndpointFacts_ConstantMethod"},                                                                                                                                                                                                                                              // Swift single-value (constant) method property
+	95: {"TestResolveImports_CouplingKindTagged", "TestResolveImports_ReferenceBeatsAssociation"},                                                                                                                                                                                                // Ruby synthetic-edge coupling_kind prop + framework-const ignore list
 }
 
 func TestCacheVersionCoverage(t *testing.T) {

internal/engine/cache.go

@@ -249,7 +249,11 @@ import (
 // v94: Swift endpoint extractor reads a single-value method property (`var method:
 // HTTPMethod { return .post }`, no switch) and applies its lone verb to every case, instead
 // of defaulting to GET. Cached Swift snapshots must re-extract.
-const cacheVersion = "v94"
+// v95: Ruby extractor tags synthetic coupling edges with a coupling_kind prop (so the cycles
+// explainer can exclude ActiveRecord associations) and adds common Rails framework constants
+// (I18n, Rails, Logger, ...) to the builtin-const ignore list. Cached Ruby snapshots must
+// re-extract to pick up the new edge props and suppressed references.
+const cacheVersion = "v95"
 
 // extractorCache holds per-extractor facts keyed by a content hash of the files
 // the extractor depends on. It is loaded from disk at the start of a snapshot and

internal/engine/testdata/golden/ruby_sample.facts.jsonl

@@ -1,5 +1,5 @@
 {"kind":"dependency","name":"Reporting::ReportWorker -\u003e Trackable","file":"app/services/report_worker.rb","line":8,"repo":"ruby_sample","props":{"language":"ruby","mixin_kind":"include","source":"internal"},"relations":[{"kind":"implements","target":"Trackable"}]}
-{"kind":"dependency","name":"app/services -\u003e app/models/concerns","file":"app/services/_coupling.rb","repo":"ruby_sample","props":{"framework":"rails","language":"ruby","source":"internal","synthetic_coupling":true},"relations":[{"kind":"imports","target":"app/models/concerns"}]}
+{"kind":"dependency","name":"app/services -\u003e app/models/concerns","file":"app/services/_coupling.rb","repo":"ruby_sample","props":{"coupling_kind":"mixin","framework":"rails","language":"ruby","source":"internal","synthetic_coupling":true},"relations":[{"kind":"imports","target":"app/models/concerns"}]}
 {"kind":"dependency","name":"config -\u003e rails/all","file":"config/application.rb","line":5,"repo":"ruby_sample","props":{"language":"ruby","source":"external"},"relations":[{"kind":"imports","target":"rails/all"}]}
 {"kind":"file_ref","name":"app/services/report_worker.rb","file":"app/services/report_worker.rb","repo":"ruby_sample","props":{"dynamic_send_prefixes":["report_"],"language":"ruby"}}
 {"kind":"file_ref","name":"app/views/reports/show.html.erb","file":"app/views/reports/show.html.erb","repo":"ruby_sample","props":{"language":"ruby"},"relations":[{"kind":"calls","target":"ReportPresenter"},{"kind":"calls","target":"ReportPresenter.render_summary"},{"kind":"calls","target":"can_view_reports?"},{"kind":"calls","target":"current_user"},{"kind":"calls","target":"render_summary"}]}

internal/explainers/common/common.go

@@ -12,6 +12,15 @@ import (
 	"github.com/enola-labs/enola/internal/facts"
 )
 
+// OversizedClusterModules is the module-count above which a strongly-connected
+// component is treated as a coupling *cluster* rather than a discrete, actionable
+// tangle. In an autoloaded language (Ruby/Rails) mutual constant references across
+// many directories are the expected topology, so a large SCC is a coupling-density
+// signal, not a fixable cycle or a genuine deep layering. Shared by the cycles
+// explainer (softens such SCCs into a cluster note) and the depth explainer (counts
+// such an SCC as one logical layer instead of its full size).
+const OversizedClusterModules = 8
+
 // FileDir returns the directory portion of a file path, which enola uses as the
 // canonical module name. A path with no separator maps to ".".
 func FileDir(file string) string {
@@ -78,8 +87,26 @@ func ResolveRelativeImport(sourceModule, target string) string {
 // roles. Modules with an absent or non-test role are kept (consumers treat an
 // absent role as included).
 func BuildModuleGraph(store *facts.Store) map[string][]string {
+	return BuildModuleGraphExcluding(store)
+}
+
+// BuildModuleGraphExcluding is BuildModuleGraph with the ability to drop synthetic
+// coupling edges by their Props["coupling_kind"] (see facts.Coupling* constants).
+// A dependency fact whose coupling_kind is in excludeKinds contributes no edge.
+// The cycles explainer uses this to exclude ActiveRecord associations, whose
+// inherent bidirectionality would otherwise manufacture false cycles. With no
+// excludeKinds it is identical to BuildModuleGraph.
+func BuildModuleGraphExcluding(store *facts.Store, excludeKinds ...string) map[string][]string {
 	graph := make(map[string][]string)
 
+	var excluded map[string]bool
+	if len(excludeKinds) > 0 {
+		excluded = make(map[string]bool, len(excludeKinds))
+		for _, k := range excludeKinds {
+			excluded[k] = true
+		}
+	}
+
 	modules := store.ByKind(facts.KindModule)
 	moduleNames := make(map[string]bool)
 	testModules := make(map[string]bool)
@@ -100,6 +127,11 @@ func BuildModuleGraph(store *facts.Store) map[string][]string {
 		if testModules[sourceModule] {
 			continue // edge out of a test bundle — not production architecture
 		}
+		if excluded != nil {
+			if ck, _ := dep.Props[facts.PropCouplingKind].(string); excluded[ck] {
+				continue
+			}
+		}
 
 		for _, rel := range dep.Relations {
 			if rel.Kind != facts.RelImports {
@@ -124,6 +156,48 @@ func BuildModuleGraph(store *facts.Store) map[string][]string {
 	return graph
 }
 
+// rubyFrameworkBaseClasses are exact Rails/framework base-class names whose high
+// fan-in comes from being inherited, not from being a god class.
+var rubyFrameworkBaseClasses = map[string]bool{
+	"ApplicationRecord": true, "ApplicationController": true, "ApplicationJob": true,
+	"ApplicationMailer": true, "ApplicationService": true, "ApplicationSerializer": true,
+	"ApplicationCable": true, "ApplicationPolicy": true, "ApplicationInteractor": true,
+	"ApplicationPresenter": true,
+}
+
+// rubyBaseClassSuffixes are naming conventions for user-defined base classes
+// (NotifierBase, ApiBaseController, Pusher::Base, ...). A class named like this is
+// a base others inherit from, so its inbound degree is inheritance, not coupling.
+var rubyBaseClassSuffixes = []string{
+	"BaseController", "BaseJob", "BaseService", "BaseMailer", "BaseSerializer",
+	"BasePolicy", "BasePresenter", "BaseInteractor", "Base",
+}
+
+// IsRubyFrameworkBaseSymbol reports whether a symbol is a Rails/framework base
+// class — one whose high fan-in is a product of inheritance (every subclass
+// "depends on" it) rather than a design smell. Gated on a .rb file so non-Ruby
+// symbols are never affected. Used by the god-class and hotspots explainers to keep
+// framework scaffolding (ApplicationRecord/Controller/Job, *BaseController, *::Base)
+// out of their findings while still surfacing genuine central domain types.
+func IsRubyFrameworkBaseSymbol(name, file string) bool {
+	if !strings.HasSuffix(file, ".rb") {
+		return false
+	}
+	seg := name
+	if i := strings.LastIndex(seg, "::"); i >= 0 {
+		seg = seg[i+2:]
+	}
+	if rubyFrameworkBaseClasses[seg] {
+		return true
+	}
+	for _, suffix := range rubyBaseClassSuffixes {
+		if strings.HasSuffix(seg, suffix) {
+			return true
+		}
+	}
+	return false
+}
+
 // SymbolModule returns the module a symbol belongs to. Symbol names encode the
 // module as the prefix before the first ".", e.g. "internal/auth.Login.Verify"
 // -> "internal/auth". Names without a "." are returned unchanged.

internal/explainers/cycles/cycles.go

@@ -9,6 +9,21 @@ import (
 	"github.com/enola-labs/enola/internal/facts"
 )
 
+// maxCycleModules is the largest SCC still reported as a discrete, actionable
+// "Cyclic dependency". Larger components are not a fixable cycle — in an
+// autoloaded language (Ruby/Rails) mutual constant references across many
+// directories are the expected topology, so a 99-module SCC is a coupling-density
+// signal, not a defect. Such components are reported once as a softer,
+// lower-confidence "Highly coupled module cluster" note instead of an alarming
+// confidence-1.0 cycle with advice ("introduce an interface") that cannot
+// meaningfully be applied to a 99-node cluster. Shared with the depth explainer
+// via common.OversizedClusterModules.
+const maxCycleModules = common.OversizedClusterModules
+
+// maxClusterMembers caps how many representative members are listed as evidence
+// for an oversized coupling cluster.
+const maxClusterMembers = 12
+
 // CycleExplainer detects cyclic dependencies between modules using Tarjan's SCC algorithm.
 type CycleExplainer struct{}
 
@@ -23,8 +38,11 @@ func (e *CycleExplainer) Name() string {
 
 // Explain builds a dependency graph from import relations and detects cycles.
 func (e *CycleExplainer) Explain(ctx context.Context, store *facts.Store) ([]facts.Insight, error) {
-	// Build adjacency list from dependency facts
-	graph := common.BuildModuleGraph(store)
+	// Build adjacency list from dependency facts. ActiveRecord associations are
+	// excluded: has_many/belongs_to pairs are inherently bidirectional domain
+	// relationships, not load-order dependencies, and would otherwise manufacture
+	// spurious two-module cycles between associated models.
+	graph := common.BuildModuleGraphExcluding(store, facts.CouplingAssociation)
 
 	// Run Tarjan's SCC
 	sccs := tarjanSCC(graph)
@@ -36,6 +54,11 @@ func (e *CycleExplainer) Explain(ctx context.Context, store *facts.Store) ([]fac
 			continue
 		}
 
+		if len(scc) > maxCycleModules {
+			insights = append(insights, coupledClusterInsight(scc))
+			continue
+		}
+
 		cyclePath := strings.Join(scc, " -> ") + " -> " + scc[0]
 		evidence := make([]facts.Evidence, 0, len(scc))
 		for _, mod := range scc {
@@ -46,6 +69,7 @@ func (e *CycleExplainer) Explain(ctx context.Context, store *facts.Store) ([]fac
 		}
 
 		insights = append(insights, facts.Insight{
+			// Title prefix is parsed by pkg/explain (Code health section); keep stable.
 			Title:       fmt.Sprintf("Cyclic dependency detected (%d modules)", len(scc)),
 			Description: fmt.Sprintf("The following modules form a dependency cycle: %s. This can cause initialization issues, make refactoring harder, and indicates tight coupling.", cyclePath),
 			Confidence:  1.0, // Deterministic
@@ -61,6 +85,39 @@ func (e *CycleExplainer) Explain(ctx context.Context, store *facts.Store) ([]fac
 	return insights, nil
 }
 
+// coupledClusterInsight reports an oversized SCC as a soft coupling signal rather
+// than a discrete cycle. The title deliberately does NOT start with "Cyclic
+// dependency" so pkg/explain does not fold it into the cycle count.
+func coupledClusterInsight(scc []string) facts.Insight {
+	members := scc
+	if len(members) > maxClusterMembers {
+		members = members[:maxClusterMembers]
+	}
+	evidence := make([]facts.Evidence, 0, len(members))
+	for _, mod := range members {
+		evidence = append(evidence, facts.Evidence{
+			Fact:   mod,
+			Detail: fmt.Sprintf("module %q is part of the cluster", mod),
+		})
+	}
+	return facts.Insight{
+		Title: fmt.Sprintf("Highly coupled module cluster (%d modules)", len(scc)),
+		Description: fmt.Sprintf(
+			"%d modules reference each other mutually. In an autoloaded codebase "+
+				"(e.g. Rails) this is expected — constant references between directories "+
+				"resolve lazily, so this is not a load-order cycle. Treat it as an overall "+
+				"coupling-density signal, not a defect to break.",
+			len(scc),
+		),
+		Confidence: 0.4,
+		Evidence:   evidence,
+		Actions: []string{
+			"Look for a few high-traffic modules whose extraction would thin the cluster",
+			"Prefer narrowing individual module responsibilities over a single big refactor",
+		},
+	}
+}
+
 // tarjanSCC computes strongly connected components of the module graph. It
 // delegates to common.StronglyConnectedComponents, whose output is deterministic
 // (sorted components with sorted members) — so the emitted cycle path, evidence

internal/explainers/cycles/cycles_test.go

@@ -2,6 +2,7 @@ package cycles
 
 import (
 	"context"
+	"fmt"
 	"reflect"
 	"sort"
 	"strings"
@@ -202,6 +203,74 @@ func TestExplain_WithCycle(t *testing.T) {
 	}
 }
 
+// TestExplain_OversizedClusterSoftened: an SCC larger than maxCycleModules is not
+// a fixable cycle (in autoloaded Ruby/Rails it is the expected topology). It must
+// be reported once as a soft, low-confidence "Highly coupled module cluster" note
+// whose title does NOT start with "Cyclic dependency" (so pkg/explain won't count
+// it as a cycle), not as a confidence-1.0 alarm.
+func TestExplain_OversizedClusterSoftened(t *testing.T) {
+	n := maxCycleModules + 3
+	modules := make([]string, n)
+	deps := map[string][]string{}
+	for i := 0; i < n; i++ {
+		modules[i] = fmt.Sprintf("app/m%02d", i)
+	}
+	// One big ring: m0 -> m1 -> ... -> m(n-1) -> m0, so all n form a single SCC.
+	for i := 0; i < n; i++ {
+		deps[modules[i]] = []string{modules[(i+1)%n]}
+	}
+	store := makeStore(modules, deps)
+
+	insights, err := New().Explain(context.Background(), store)
+	if err != nil {
+		t.Fatalf("Explain: %v", err)
+	}
+	if len(insights) != 1 {
+		t.Fatalf("expected 1 cluster insight, got %d: %+v", len(insights), insights)
+	}
+	in := insights[0]
+	if strings.HasPrefix(in.Title, "Cyclic dependency") {
+		t.Errorf("oversized SCC should not be titled as a cyclic dependency: %q", in.Title)
+	}
+	if !strings.HasPrefix(in.Title, "Highly coupled module cluster") {
+		t.Errorf("expected a coupling-cluster title, got %q", in.Title)
+	}
+	if in.Confidence >= 1.0 {
+		t.Errorf("cluster confidence should be soft (<1.0), got %v", in.Confidence)
+	}
+	if len(in.Evidence) > maxClusterMembers {
+		t.Errorf("cluster evidence not capped: got %d, want <= %d", len(in.Evidence), maxClusterMembers)
+	}
+}
+
+// TestExplain_AssociationEdgesExcluded: a two-model "cycle" formed solely by
+// ActiveRecord associations (Order has_many LineItems, LineItem belongs_to Order)
+// is bidirectional by nature, not a load-order cycle, and must not be reported.
+func TestExplain_AssociationEdgesExcluded(t *testing.T) {
+	s := facts.NewStore()
+	s.Add(facts.Fact{Kind: facts.KindModule, Name: "app/models/order"})
+	s.Add(facts.Fact{Kind: facts.KindModule, Name: "app/models/line_item"})
+	// Synthetic association edges both ways (as emitEdges would produce).
+	s.Add(facts.Fact{
+		Kind: facts.KindDependency, File: "app/models/order/_coupling.rb",
+		Props:     map[string]any{facts.PropCouplingKind: facts.CouplingAssociation},
+		Relations: []facts.Relation{{Kind: facts.RelImports, Target: "app/models/line_item"}},
+	})
+	s.Add(facts.Fact{
+		Kind: facts.KindDependency, File: "app/models/line_item/_coupling.rb",
+		Props:     map[string]any{facts.PropCouplingKind: facts.CouplingAssociation},
+		Relations: []facts.Relation{{Kind: facts.RelImports, Target: "app/models/order"}},
+	})
+
+	insights, err := New().Explain(context.Background(), s)
+	if err != nil {
+		t.Fatalf("Explain: %v", err)
+	}
+	if len(insights) != 0 {
+		t.Errorf("association-only 2-cycle should not be reported, got %d: %+v", len(insights), insights)
+	}
+}
+
 // TestExplain_Deterministic guards BUG-2: the cycle path, evidence order, and
 // multi-cycle insight order used to depend on Go's randomized map iteration
 // (tarjanSCC ranged the graph map directly and never sorted). Each Explain call