Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion internal/explainers/godclass/godclass.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,10 @@ func (e *GodClassExplainer) Explain(ctx context.Context, store *facts.Store) ([]
if graph == nil {
return nil, nil
}
reverse := graph.Reverse()
// Architectural fan-in only: reference-only facts (test_ref/file_ref) carry
// RelCalls edges into production code but are not symbols. Counting them
// inflates fan-in and drifts the outlier threshold below (GAP-XL-15).
reverse := graph.ArchitecturalReverse()

symbols := store.ByKind(facts.KindSymbol)
if len(symbols) == 0 {
Expand Down
68 changes: 68 additions & 0 deletions internal/explainers/godclass/godclass_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,74 @@ func TestExplain_MultipleHubsOrderedByFanIn(t *testing.T) {
}
}

// TestExplain_ExcludesTestRefFanIn: a hub referenced by production symbols AND by
// test_ref/file_ref facts (spec files, initializers) must report a fan-in that
// counts only the architectural (symbol) dependents. Reference-only facts carry
// RelCalls edges into production code but are not symbols; counting them inflates
// the fan-in and drifts the outlier threshold (GAP-XL-15).
func TestExplain_ExcludesTestRefFanIn(t *testing.T) {
s := facts.NewStore()
const hub = "core.Hub"
s.Add(facts.Fact{Kind: facts.KindSymbol, Name: hub, File: "core/hub.go"})
// 8 real symbol dependents — clears the minFanIn floor on symbols alone.
for i := 0; i < 8; i++ {
s.Add(facts.Fact{
Kind: facts.KindSymbol, Name: fmt.Sprintf("pkg/c%d.Call", i),
File: fmt.Sprintf("pkg/c%d.go", i),
Relations: []facts.Relation{{Kind: facts.RelCalls, Target: hub}},
})
}
// 5 reference-only dependents that also call the hub — must NOT count.
for i := 0; i < 3; i++ {
s.Add(facts.Fact{
Kind: facts.KindTestRef, Name: fmt.Sprintf("spec/c%d_spec.rb", i),
File: fmt.Sprintf("spec/c%d_spec.rb", i),
Relations: []facts.Relation{{Kind: facts.RelCalls, Target: hub}},
})
}
for i := 0; i < 2; i++ {
s.Add(facts.Fact{
Kind: facts.KindFileRef, Name: fmt.Sprintf("config/init%d.rb", i),
File: fmt.Sprintf("config/init%d.rb", i),
Relations: []facts.Relation{{Kind: facts.RelCalls, Target: hub}},
})
}
// Low-fan-in noise so the outlier threshold is meaningful.
s.Add(facts.Fact{Kind: facts.KindSymbol, Name: "leaf.A", File: "leaf/a.go"})
s.Add(facts.Fact{Kind: facts.KindSymbol, Name: "leaf.B", File: "leaf/b.go"})
s.Add(facts.Fact{Kind: facts.KindSymbol, Name: "leaf.C", File: "leaf/c.go"})
s.BuildGraph()

insights, err := New().Explain(context.Background(), s)
if err != nil {
t.Fatalf("Explain: %v", err)
}

var hubInsight *facts.Insight
for i := range insights {
if strings.Contains(insights[i].Title, hub) {
hubInsight = &insights[i]
break
}
}
if hubInsight == nil {
t.Fatalf("hub %q not reported as a god-class; got %v", hub, insights)
}
// Fan-in must be 8 (symbols), not 13 (symbols + 3 test_ref + 2 file_ref).
if !strings.Contains(hubInsight.Title, "(8 dependents)") {
t.Errorf("fan-in should exclude reference-only facts; title = %q, want it to report 8 dependents", hubInsight.Title)
}
if strings.Contains(hubInsight.Title, "(13 dependents)") {
t.Errorf("fan-in wrongly includes test_ref/file_ref facts; title = %q", hubInsight.Title)
}
// No spec file or initializer should appear as a dependent in the evidence.
for _, ev := range hubInsight.Evidence[1:] {
if strings.HasPrefix(ev.Symbol, "spec/") || strings.HasPrefix(ev.Symbol, "config/") {
t.Errorf("reference-only fact %q leaked into god-class evidence", ev.Symbol)
}
}
}

// TestConfidenceMath locks the 0.5→1.0 scaling and its clamps.
func TestConfidenceMath(t *testing.T) {
tests := []struct {
Expand Down
6 changes: 5 additions & 1 deletion internal/explainers/hotspots/hotspots.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,11 @@ func (e *HotspotExplainer) Explain(ctx context.Context, store *facts.Store) ([]f
return nil, nil
}
forward := graph.Forward()
reverse := graph.Reverse()
// Architectural fan-in only: reference-only facts (test_ref/file_ref) are not
// symbols, so counting their RelCalls edges inflates the centrality score and
// the outlier distribution (GAP-XL-15). Fan-out is unaffected — a symbol never
// calls a reference node.
reverse := graph.ArchitecturalReverse()

symbols := store.ByKind(facts.KindSymbol)
if len(symbols) == 0 {
Expand Down
67 changes: 67 additions & 0 deletions internal/explainers/hotspots/hotspots_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,73 @@ func TestExplain_NeighborsCappedAndSorted(t *testing.T) {
}
}

// TestExplain_ExcludesTestRefFanIn: a pinch point's fan-in counts only
// architectural (symbol) callers. test_ref/file_ref facts carry RelCalls edges
// into the hub but are not symbols; counting them inflates the centrality score
// and the outlier distribution (GAP-XL-15).
func TestExplain_ExcludesTestRefFanIn(t *testing.T) {
s := facts.NewStore()
const hub = "core.Hub"
// Fan-out: hub calls 5 targets.
calls := make([]facts.Relation, 0, 5)
for i := 0; i < 5; i++ {
tgt := fmt.Sprintf("dep/t%d.Fn", i)
calls = append(calls, facts.Relation{Kind: facts.RelCalls, Target: tgt})
s.Add(facts.Fact{Kind: facts.KindSymbol, Name: tgt, File: "dep/t.go"})
}
s.Add(facts.Fact{Kind: facts.KindSymbol, Name: hub, File: "core/hub.go", Relations: calls})
// 4 real symbol callers (fan-in) — clears minDegree on symbols alone.
for i := 0; i < 4; i++ {
s.Add(facts.Fact{
Kind: facts.KindSymbol, Name: fmt.Sprintf("caller/c%d.Fn", i),
File: "caller/c.go",
Relations: []facts.Relation{{Kind: facts.RelCalls, Target: hub}},
})
}
// 3 reference-only callers that must not count toward fan-in.
for i := 0; i < 2; i++ {
s.Add(facts.Fact{
Kind: facts.KindTestRef, Name: fmt.Sprintf("spec/c%d_spec.rb", i),
File: fmt.Sprintf("spec/c%d_spec.rb", i),
Relations: []facts.Relation{{Kind: facts.RelCalls, Target: hub}},
})
}
s.Add(facts.Fact{
Kind: facts.KindFileRef, Name: "config/init.rb", File: "config/init.rb",
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)
}

var hubInsight *facts.Insight
for i := range insights {
if strings.Contains(insights[i].Title, hub) {
hubInsight = &insights[i]
break
}
}
if hubInsight == nil {
t.Fatalf("hub %q not reported as a hotspot; got %v", hub, insights)
}
// Fan-in must be 4 (symbols), not 7 (symbols + 2 test_ref + 1 file_ref).
if !strings.Contains(hubInsight.Title, "fan-in 4") {
t.Errorf("fan-in should exclude reference-only facts; title = %q, want fan-in 4", hubInsight.Title)
}
if strings.Contains(hubInsight.Title, "fan-in 7") {
t.Errorf("fan-in wrongly includes test_ref/file_ref facts; title = %q", hubInsight.Title)
}
// No spec file or initializer should appear as an in-caller in the evidence.
for _, ev := range hubInsight.Evidence[1:] {
if strings.HasPrefix(ev.Symbol, "spec/") || strings.HasPrefix(ev.Symbol, "config/") {
t.Errorf("reference-only fact %q leaked into hotspot evidence", ev.Symbol)
}
}
}

// TestExplain_OrderedByScore: the higher fanIn×fanOut hotspot ranks first.
func TestExplain_OrderedByScore(t *testing.T) {
s := facts.NewStore()
Expand Down
39 changes: 39 additions & 0 deletions internal/facts/graph.go
Original file line number Diff line number Diff line change
Expand Up @@ -710,6 +710,45 @@ func (g *Graph) Reverse() map[string][]Edge {
return g.reverse
}

// isReferenceOnlyKind reports whether a fact kind carries only reference
// (RelCalls) edges into production code — test_ref and file_ref. They exist so
// the dead-code detector can see a production symbol is used from a test/spec or
// a file-scope block; by contract "no other explainer is affected"
// (pkg/plugin/plugin.go). They are not part of the architectural coupling graph,
// so counting them as dependents inflates god-class fan-in and hotspots
// centrality and drifts the outlier threshold (GAP-XL-15).
func isReferenceOnlyKind(kind string) bool {
return kind == KindTestRef || kind == KindFileRef
}

// ArchitecturalReverse returns a reverse adjacency map restricted to edges whose
// SOURCE fact is part of the architectural coupling graph — i.e. excluding
// reference-only kinds (test_ref/file_ref). The outlier explainers (god-class,
// hotspots) use this instead of Reverse() so their fan-in/centrality and the
// distribution they threshold over count only real symbol coupling. orphans,
// impact_analysis, traverse and find_path keep using the unfiltered Reverse()
// index — they intentionally surface those references. (GAP-XL-15)
func (g *Graph) ArchitecturalReverse() map[string][]Edge {
g.mu.RLock()
defer g.mu.RUnlock()
out := make(map[string][]Edge, len(g.reverse))
for target, edges := range g.reverse {
kept := make([]Edge, 0, len(edges))
for _, e := range edges {
// In a reverse edge, e.Target holds the SOURCE fact name.
if idx, ok := g.factIdx[e.Target]; ok && idx < len(g.facts) &&
isReferenceOnlyKind(g.facts[idx].Kind) {
continue
}
kept = append(kept, e)
}
if len(kept) > 0 {
out[target] = kept
}
}
return out
}

// NodeCount returns the number of unique nodes in the graph.
func (g *Graph) NodeCount() int {
g.mu.RLock()
Expand Down
39 changes: 39 additions & 0 deletions internal/facts/graph_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -938,3 +938,42 @@ func impactNodes(res ImpactResult) []TraversalNode {
}
return out
}

// TestArchitecturalReverse_ExcludesReferenceKinds verifies that reference-only
// facts (test_ref/file_ref) are dropped from the architectural reverse index.
// Their RelCalls edges must not count as dependents — otherwise they inflate
// god-class fan-in and hotspots centrality and drift the outlier threshold
// (GAP-XL-15). The unfiltered Reverse() index must still surface them, since
// orphans/impact_analysis rely on seeing test/file references.
func TestArchitecturalReverse_ExcludesReferenceKinds(t *testing.T) {
s := NewStore()
s.Add(
Fact{Kind: KindSymbol, Name: "Prod", File: "app/prod.rb"},
Fact{Kind: KindSymbol, Name: "Caller", File: "app/caller.rb", Relations: []Relation{
{Kind: RelCalls, Target: "Prod"},
}},
Fact{Kind: KindTestRef, Name: "spec/prod_spec.rb", File: "spec/prod_spec.rb", Relations: []Relation{
{Kind: RelCalls, Target: "Prod"},
}},
Fact{Kind: KindFileRef, Name: "config/initializers/boot.rb", File: "config/initializers/boot.rb", Relations: []Relation{
{Kind: RelCalls, Target: "Prod"},
}},
)
s.BuildGraph()
g := s.Graph()

// Unfiltered: all three sources (symbol + test_ref + file_ref) are dependents.
if got := len(g.Reverse()["Prod"]); got != 3 {
t.Fatalf("Reverse()[Prod] = %d edges, want 3 (Caller + test_ref + file_ref)", got)
}

// Architectural: only the symbol dependent survives.
arch := g.ArchitecturalReverse()["Prod"]
if len(arch) != 1 {
t.Fatalf("ArchitecturalReverse()[Prod] = %d edges, want 1 (symbol only): %+v", len(arch), arch)
}
// In a reverse edge, Edge.Target holds the SOURCE fact name.
if arch[0].Target != "Caller" {
t.Errorf("surviving dependent = %q, want Caller (the symbol source)", arch[0].Target)
}
}
Loading