Tune Ruby/Rails architecture findings to cut false positives

Rails snapshots produced noisy, often-wrong findings: Ruby modules are
directories and coupling is synthesized from autoloaded constant
references, so the module graph is nearly complete and bidirectional and
the generic OOP/hexagonal explainers over-fire.

- layers: re-level the Rails pattern to two tiers (delivery=controllers/
  views, domain=everything else) so idiomatic model->service/job/helper
  references are no longer "violations"; only domain->delivery smells
  fire. Also classify interactors/presenters/serializers/notifiers/types.
- god-class, hotspots: dedupe candidates by symbol name (a constant
  reopened across N files no longer yields N identical findings, e.g.
  RailsAdmin::Config::Actions x50) and cap god-class output at 25.
- cycles: report SCCs larger than 8 modules as one low-confidence
  "highly coupled module cluster" note instead of a confidence-1.0
  cyclic-dependency alarm; in autoloaded Rails a giant SCC is expected.
- ruby extractor: tag synthetic coupling edges with coupling_kind and
  exclude ActiveRecord associations from cycle detection (has_many/
  belongs_to pairs are bidirectional by nature); add framework constants
  (I18n, Rails, Logger, GlobalID, Mime) to the fan-in ignore list.
- bump cacheVersion v94->v95 (+ cachecov coverage entry).

Author: dejo1307

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

@@ -78,8 +78,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 +118,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 {

internal/explainers/cycles/cycles.go

@@ -9,6 +9,20 @@ 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.
+const maxCycleModules = 8
+
+// 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 +37,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 +53,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 +68,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 +84,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

internal/explainers/godclass/godclass.go

@@ -22,6 +22,11 @@ const (
 	stdDevK = 2.0
 	// maxEvidence caps how many dependents are listed as evidence per insight.
 	maxEvidence = 8
+	// maxInsights caps how many high-fan-in symbols are reported, matching the
+	// sibling explainers (surface=20, complexity=15, depth=10). Without a cap a
+	// large repo can emit hundreds of findings dominated by central-by-design
+	// framework types.
+	maxInsights = 25
 )
 
 // GodClassExplainer detects high-fan-in symbols.
@@ -50,10 +55,27 @@ func (e *GodClassExplainer) Explain(ctx context.Context, store *facts.Store) ([]
 		return nil, nil
 	}
 
-	// Fan-in per symbol and the distribution used for outlier detection.
-	fanIn := make(map[string]int, len(symbols))
-	values := make([]float64, 0, len(symbols))
+	// Report each distinct symbol name once. A constant reopened across many files
+	// (Ruby STI/concerns, monkey-patched framework namespaces like
+	// RailsAdmin::Config::Actions) yields one symbol fact per file, all sharing a
+	// Name. Fan-in is keyed by name, so without de-duping they produced identical
+	// findings AND skewed the outlier distribution below (the repeated high value
+	// dragged the threshold up). De-dupe here so both the distribution and the
+	// candidate set are over distinct symbols.
+	distinct := make([]facts.Fact, 0, len(symbols))
+	seen := make(map[string]bool, len(symbols))
 	for _, s := range symbols {
+		if seen[s.Name] {
+			continue
+		}
+		seen[s.Name] = true
+		distinct = append(distinct, s)
+	}
+
+	// Fan-in per symbol and the distribution used for outlier detection.
+	fanIn := make(map[string]int, len(distinct))
+	values := make([]float64, 0, len(distinct))
+	for _, s := range distinct {
 		n := len(reverse[s.Name])
 		fanIn[s.Name] = n
 		values = append(values, float64(n))
@@ -67,7 +89,7 @@ func (e *GodClassExplainer) Explain(ctx context.Context, store *facts.Store) ([]
 		labels []string
 	}
 	var candidates []candidate
-	for _, s := range symbols {
+	for _, s := range distinct {
 		n := fanIn[s.Name]
 		if n < minFanIn || float64(n) <= threshold {
 			continue
@@ -88,6 +110,10 @@ func (e *GodClassExplainer) Explain(ctx context.Context, store *facts.Store) ([]
 		return candidates[i].fact.Name < candidates[j].fact.Name
 	})
 
+	if len(candidates) > maxInsights {
+		candidates = candidates[:maxInsights]
+	}
+
 	var insights []facts.Insight
 	for _, c := range candidates {
 		evidence := make([]facts.Evidence, 0, maxEvidence+1)

internal/explainers/godclass/godclass_test.go

@@ -77,6 +77,54 @@ func TestExplain_DetectsGodClass(t *testing.T) {
 	}
 }
 
+// TestExplain_DedupReopenedSymbol: a constant reopened across many files (Ruby
+// STI/concerns, monkey-patched framework namespaces) yields one symbol fact per
+// file, all sharing a Name. Fan-in is keyed by name, so each produced an identical
+// insight — the RailsAdmin::Config::Actions ×50 flood. Report the name once.
+func TestExplain_DedupReopenedSymbol(t *testing.T) {
+	store := makeStore("core.Hub", manyCallers(12), nil)
+	// Simulate the same constant reopened in 4 more files.
+	for i := 0; i < 4; i++ {
+		store.Add(facts.Fact{Kind: facts.KindSymbol, Name: "core.Hub", File: fmt.Sprintf("reopen/%d.go", i)})
+	}
+	store.BuildGraph()
+
+	insights, err := New().Explain(context.Background(), store)
+	if err != nil {
+		t.Fatalf("Explain: %v", err)
+	}
+	if len(insights) != 1 {
+		t.Fatalf("reopened symbol should yield 1 insight, got %d: %+v", len(insights), insights)
+	}
+}
+
+// TestExplain_CapsInsightCount: no more than maxInsights findings are emitted even
+// when many distinct symbols exceed the outlier threshold.
+func TestExplain_CapsInsightCount(t *testing.T) {
+	s := facts.NewStore()
+	// 40 distinct hubs, each with 12 dependents -> all well above the floor.
+	for h := 0; h < 40; h++ {
+		hub := fmt.Sprintf("core.Hub%d", h)
+		s.Add(facts.Fact{Kind: facts.KindSymbol, Name: hub, File: fmt.Sprintf("core/hub%d.go", h)})
+		for i := 0; i < 12; i++ {
+			s.Add(facts.Fact{
+				Kind: facts.KindSymbol, Name: fmt.Sprintf("caller%d_%d.Fn", h, i),
+				File:      fmt.Sprintf("callers/%d_%d.go", h, i),
+				Relations: []facts.Relation{{Kind: facts.RelCalls, Target: hub}},
+			})
+		}
+	}
+	s.BuildGraph()
+
+	insights, err := New().Explain(context.Background(), s)
+	if err != nil {
+		t.Fatalf("Explain: %v", err)
+	}
+	if len(insights) > maxInsights {
+		t.Errorf("insight count not capped: got %d, want <= %d", len(insights), maxInsights)
+	}
+}
+
 func TestExplain_BelowFloor(t *testing.T) {
 	// Hub has only 5 dependents — below minFanIn even if it's the max.
 	store := makeStore("core.Hub", manyCallers(5), nil)

internal/explainers/hotspots/hotspots.go

@@ -54,9 +54,24 @@ func (e *HotspotExplainer) Explain(ctx context.Context, store *facts.Store) ([]f
 		return nil, nil
 	}
 
-	scores := make(map[string]int, len(symbols))
-	values := make([]float64, 0, len(symbols))
+	// Report each distinct symbol name once. A constant reopened across many files
+	// (Ruby STI/concerns, monkey-patched framework namespaces) yields one symbol
+	// fact per file, all sharing a Name and therefore identical in/out degree.
+	// De-dupe here so both the outlier distribution and the candidate set are over
+	// distinct symbols (repeated values would otherwise skew the threshold).
+	distinct := make([]facts.Fact, 0, len(symbols))
+	seen := make(map[string]bool, len(symbols))
 	for _, s := range symbols {
+		if seen[s.Name] {
+			continue
+		}
+		seen[s.Name] = true
+		distinct = append(distinct, s)
+	}
+
+	scores := make(map[string]int, len(distinct))
+	values := make([]float64, 0, len(distinct))
+	for _, s := range distinct {
 		in := len(reverse[s.Name])
 		out := len(forward[s.Name])
 		score := in * out
@@ -73,7 +88,7 @@ func (e *HotspotExplainer) Explain(ctx context.Context, store *facts.Store) ([]f
 		score int
 	}
 	var candidates []candidate
-	for _, s := range symbols {
+	for _, s := range distinct {
 		in := len(reverse[s.Name])
 		out := len(forward[s.Name])
 		if in < minDegree || out < minDegree {