Skip to content

Commit 33b2c30

Browse files
committed
Exclude reference-only facts from god-class & hotspots fan-in (GAP-XL-15)
test_ref/file_ref nodes are indexed into the coupling graph, so their RelCalls edges landed in the reverse adjacency map and were counted as dependents by the two reverse-index explainers — inflating fan-in and centrality and, worse, drifting the outlier threshold so genuine findings dropped below the cut. The plugin contract promised "no other explainer is affected"; it was. Add Graph.ArchitecturalReverse(), which filters reverse edges by source fact kind, and point god-class and hotspots at it. orphans, impact_analysis, traverse and find_path keep the unfiltered Reverse() — they intentionally surface those references. Explainer read-path + a derived index only; facts.jsonl is byte-identical, so no cacheVersion bump and no golden regeneration. Tests: facts.TestArchitecturalReverse_ExcludesReferenceKinds, {godclass,hotspots}.TestExplain_ExcludesTestRefFanIn.
1 parent 7aeec82 commit 33b2c30

6 files changed

Lines changed: 222 additions & 2 deletions

File tree

internal/explainers/godclass/godclass.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,10 @@ func (e *GodClassExplainer) Explain(ctx context.Context, store *facts.Store) ([]
4848
if graph == nil {
4949
return nil, nil
5050
}
51-
reverse := graph.Reverse()
51+
// Architectural fan-in only: reference-only facts (test_ref/file_ref) carry
52+
// RelCalls edges into production code but are not symbols. Counting them
53+
// inflates fan-in and drifts the outlier threshold below (GAP-XL-15).
54+
reverse := graph.ArchitecturalReverse()
5255

5356
symbols := store.ByKind(facts.KindSymbol)
5457
if len(symbols) == 0 {

internal/explainers/godclass/godclass_test.go

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,74 @@ func TestExplain_MultipleHubsOrderedByFanIn(t *testing.T) {
254254
}
255255
}
256256

257+
// TestExplain_ExcludesTestRefFanIn: a hub referenced by production symbols AND by
258+
// test_ref/file_ref facts (spec files, initializers) must report a fan-in that
259+
// counts only the architectural (symbol) dependents. Reference-only facts carry
260+
// RelCalls edges into production code but are not symbols; counting them inflates
261+
// the fan-in and drifts the outlier threshold (GAP-XL-15).
262+
func TestExplain_ExcludesTestRefFanIn(t *testing.T) {
263+
s := facts.NewStore()
264+
const hub = "core.Hub"
265+
s.Add(facts.Fact{Kind: facts.KindSymbol, Name: hub, File: "core/hub.go"})
266+
// 8 real symbol dependents — clears the minFanIn floor on symbols alone.
267+
for i := 0; i < 8; i++ {
268+
s.Add(facts.Fact{
269+
Kind: facts.KindSymbol, Name: fmt.Sprintf("pkg/c%d.Call", i),
270+
File: fmt.Sprintf("pkg/c%d.go", i),
271+
Relations: []facts.Relation{{Kind: facts.RelCalls, Target: hub}},
272+
})
273+
}
274+
// 5 reference-only dependents that also call the hub — must NOT count.
275+
for i := 0; i < 3; i++ {
276+
s.Add(facts.Fact{
277+
Kind: facts.KindTestRef, Name: fmt.Sprintf("spec/c%d_spec.rb", i),
278+
File: fmt.Sprintf("spec/c%d_spec.rb", i),
279+
Relations: []facts.Relation{{Kind: facts.RelCalls, Target: hub}},
280+
})
281+
}
282+
for i := 0; i < 2; i++ {
283+
s.Add(facts.Fact{
284+
Kind: facts.KindFileRef, Name: fmt.Sprintf("config/init%d.rb", i),
285+
File: fmt.Sprintf("config/init%d.rb", i),
286+
Relations: []facts.Relation{{Kind: facts.RelCalls, Target: hub}},
287+
})
288+
}
289+
// Low-fan-in noise so the outlier threshold is meaningful.
290+
s.Add(facts.Fact{Kind: facts.KindSymbol, Name: "leaf.A", File: "leaf/a.go"})
291+
s.Add(facts.Fact{Kind: facts.KindSymbol, Name: "leaf.B", File: "leaf/b.go"})
292+
s.Add(facts.Fact{Kind: facts.KindSymbol, Name: "leaf.C", File: "leaf/c.go"})
293+
s.BuildGraph()
294+
295+
insights, err := New().Explain(context.Background(), s)
296+
if err != nil {
297+
t.Fatalf("Explain: %v", err)
298+
}
299+
300+
var hubInsight *facts.Insight
301+
for i := range insights {
302+
if strings.Contains(insights[i].Title, hub) {
303+
hubInsight = &insights[i]
304+
break
305+
}
306+
}
307+
if hubInsight == nil {
308+
t.Fatalf("hub %q not reported as a god-class; got %v", hub, insights)
309+
}
310+
// Fan-in must be 8 (symbols), not 13 (symbols + 3 test_ref + 2 file_ref).
311+
if !strings.Contains(hubInsight.Title, "(8 dependents)") {
312+
t.Errorf("fan-in should exclude reference-only facts; title = %q, want it to report 8 dependents", hubInsight.Title)
313+
}
314+
if strings.Contains(hubInsight.Title, "(13 dependents)") {
315+
t.Errorf("fan-in wrongly includes test_ref/file_ref facts; title = %q", hubInsight.Title)
316+
}
317+
// No spec file or initializer should appear as a dependent in the evidence.
318+
for _, ev := range hubInsight.Evidence[1:] {
319+
if strings.HasPrefix(ev.Symbol, "spec/") || strings.HasPrefix(ev.Symbol, "config/") {
320+
t.Errorf("reference-only fact %q leaked into god-class evidence", ev.Symbol)
321+
}
322+
}
323+
}
324+
257325
// TestConfidenceMath locks the 0.5→1.0 scaling and its clamps.
258326
func TestConfidenceMath(t *testing.T) {
259327
tests := []struct {

internal/explainers/hotspots/hotspots.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,11 @@ func (e *HotspotExplainer) Explain(ctx context.Context, store *facts.Store) ([]f
4747
return nil, nil
4848
}
4949
forward := graph.Forward()
50-
reverse := graph.Reverse()
50+
// Architectural fan-in only: reference-only facts (test_ref/file_ref) are not
51+
// symbols, so counting their RelCalls edges inflates the centrality score and
52+
// the outlier distribution (GAP-XL-15). Fan-out is unaffected — a symbol never
53+
// calls a reference node.
54+
reverse := graph.ArchitecturalReverse()
5155

5256
symbols := store.ByKind(facts.KindSymbol)
5357
if len(symbols) == 0 {

internal/explainers/hotspots/hotspots_test.go

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,73 @@ func TestExplain_NeighborsCappedAndSorted(t *testing.T) {
195195
}
196196
}
197197

198+
// TestExplain_ExcludesTestRefFanIn: a pinch point's fan-in counts only
199+
// architectural (symbol) callers. test_ref/file_ref facts carry RelCalls edges
200+
// into the hub but are not symbols; counting them inflates the centrality score
201+
// and the outlier distribution (GAP-XL-15).
202+
func TestExplain_ExcludesTestRefFanIn(t *testing.T) {
203+
s := facts.NewStore()
204+
const hub = "core.Hub"
205+
// Fan-out: hub calls 5 targets.
206+
calls := make([]facts.Relation, 0, 5)
207+
for i := 0; i < 5; i++ {
208+
tgt := fmt.Sprintf("dep/t%d.Fn", i)
209+
calls = append(calls, facts.Relation{Kind: facts.RelCalls, Target: tgt})
210+
s.Add(facts.Fact{Kind: facts.KindSymbol, Name: tgt, File: "dep/t.go"})
211+
}
212+
s.Add(facts.Fact{Kind: facts.KindSymbol, Name: hub, File: "core/hub.go", Relations: calls})
213+
// 4 real symbol callers (fan-in) — clears minDegree on symbols alone.
214+
for i := 0; i < 4; i++ {
215+
s.Add(facts.Fact{
216+
Kind: facts.KindSymbol, Name: fmt.Sprintf("caller/c%d.Fn", i),
217+
File: "caller/c.go",
218+
Relations: []facts.Relation{{Kind: facts.RelCalls, Target: hub}},
219+
})
220+
}
221+
// 3 reference-only callers that must not count toward fan-in.
222+
for i := 0; i < 2; i++ {
223+
s.Add(facts.Fact{
224+
Kind: facts.KindTestRef, Name: fmt.Sprintf("spec/c%d_spec.rb", i),
225+
File: fmt.Sprintf("spec/c%d_spec.rb", i),
226+
Relations: []facts.Relation{{Kind: facts.RelCalls, Target: hub}},
227+
})
228+
}
229+
s.Add(facts.Fact{
230+
Kind: facts.KindFileRef, Name: "config/init.rb", File: "config/init.rb",
231+
Relations: []facts.Relation{{Kind: facts.RelCalls, Target: hub}},
232+
})
233+
s.BuildGraph()
234+
235+
insights, err := New().Explain(context.Background(), s)
236+
if err != nil {
237+
t.Fatalf("Explain: %v", err)
238+
}
239+
240+
var hubInsight *facts.Insight
241+
for i := range insights {
242+
if strings.Contains(insights[i].Title, hub) {
243+
hubInsight = &insights[i]
244+
break
245+
}
246+
}
247+
if hubInsight == nil {
248+
t.Fatalf("hub %q not reported as a hotspot; got %v", hub, insights)
249+
}
250+
// Fan-in must be 4 (symbols), not 7 (symbols + 2 test_ref + 1 file_ref).
251+
if !strings.Contains(hubInsight.Title, "fan-in 4") {
252+
t.Errorf("fan-in should exclude reference-only facts; title = %q, want fan-in 4", hubInsight.Title)
253+
}
254+
if strings.Contains(hubInsight.Title, "fan-in 7") {
255+
t.Errorf("fan-in wrongly includes test_ref/file_ref facts; title = %q", hubInsight.Title)
256+
}
257+
// No spec file or initializer should appear as an in-caller in the evidence.
258+
for _, ev := range hubInsight.Evidence[1:] {
259+
if strings.HasPrefix(ev.Symbol, "spec/") || strings.HasPrefix(ev.Symbol, "config/") {
260+
t.Errorf("reference-only fact %q leaked into hotspot evidence", ev.Symbol)
261+
}
262+
}
263+
}
264+
198265
// TestExplain_OrderedByScore: the higher fanIn×fanOut hotspot ranks first.
199266
func TestExplain_OrderedByScore(t *testing.T) {
200267
s := facts.NewStore()

internal/facts/graph.go

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -710,6 +710,45 @@ func (g *Graph) Reverse() map[string][]Edge {
710710
return g.reverse
711711
}
712712

713+
// isReferenceOnlyKind reports whether a fact kind carries only reference
714+
// (RelCalls) edges into production code — test_ref and file_ref. They exist so
715+
// the dead-code detector can see a production symbol is used from a test/spec or
716+
// a file-scope block; by contract "no other explainer is affected"
717+
// (pkg/plugin/plugin.go). They are not part of the architectural coupling graph,
718+
// so counting them as dependents inflates god-class fan-in and hotspots
719+
// centrality and drifts the outlier threshold (GAP-XL-15).
720+
func isReferenceOnlyKind(kind string) bool {
721+
return kind == KindTestRef || kind == KindFileRef
722+
}
723+
724+
// ArchitecturalReverse returns a reverse adjacency map restricted to edges whose
725+
// SOURCE fact is part of the architectural coupling graph — i.e. excluding
726+
// reference-only kinds (test_ref/file_ref). The outlier explainers (god-class,
727+
// hotspots) use this instead of Reverse() so their fan-in/centrality and the
728+
// distribution they threshold over count only real symbol coupling. orphans,
729+
// impact_analysis, traverse and find_path keep using the unfiltered Reverse()
730+
// index — they intentionally surface those references. (GAP-XL-15)
731+
func (g *Graph) ArchitecturalReverse() map[string][]Edge {
732+
g.mu.RLock()
733+
defer g.mu.RUnlock()
734+
out := make(map[string][]Edge, len(g.reverse))
735+
for target, edges := range g.reverse {
736+
kept := make([]Edge, 0, len(edges))
737+
for _, e := range edges {
738+
// In a reverse edge, e.Target holds the SOURCE fact name.
739+
if idx, ok := g.factIdx[e.Target]; ok && idx < len(g.facts) &&
740+
isReferenceOnlyKind(g.facts[idx].Kind) {
741+
continue
742+
}
743+
kept = append(kept, e)
744+
}
745+
if len(kept) > 0 {
746+
out[target] = kept
747+
}
748+
}
749+
return out
750+
}
751+
713752
// NodeCount returns the number of unique nodes in the graph.
714753
func (g *Graph) NodeCount() int {
715754
g.mu.RLock()

internal/facts/graph_test.go

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -938,3 +938,42 @@ func impactNodes(res ImpactResult) []TraversalNode {
938938
}
939939
return out
940940
}
941+
942+
// TestArchitecturalReverse_ExcludesReferenceKinds verifies that reference-only
943+
// facts (test_ref/file_ref) are dropped from the architectural reverse index.
944+
// Their RelCalls edges must not count as dependents — otherwise they inflate
945+
// god-class fan-in and hotspots centrality and drift the outlier threshold
946+
// (GAP-XL-15). The unfiltered Reverse() index must still surface them, since
947+
// orphans/impact_analysis rely on seeing test/file references.
948+
func TestArchitecturalReverse_ExcludesReferenceKinds(t *testing.T) {
949+
s := NewStore()
950+
s.Add(
951+
Fact{Kind: KindSymbol, Name: "Prod", File: "app/prod.rb"},
952+
Fact{Kind: KindSymbol, Name: "Caller", File: "app/caller.rb", Relations: []Relation{
953+
{Kind: RelCalls, Target: "Prod"},
954+
}},
955+
Fact{Kind: KindTestRef, Name: "spec/prod_spec.rb", File: "spec/prod_spec.rb", Relations: []Relation{
956+
{Kind: RelCalls, Target: "Prod"},
957+
}},
958+
Fact{Kind: KindFileRef, Name: "config/initializers/boot.rb", File: "config/initializers/boot.rb", Relations: []Relation{
959+
{Kind: RelCalls, Target: "Prod"},
960+
}},
961+
)
962+
s.BuildGraph()
963+
g := s.Graph()
964+
965+
// Unfiltered: all three sources (symbol + test_ref + file_ref) are dependents.
966+
if got := len(g.Reverse()["Prod"]); got != 3 {
967+
t.Fatalf("Reverse()[Prod] = %d edges, want 3 (Caller + test_ref + file_ref)", got)
968+
}
969+
970+
// Architectural: only the symbol dependent survives.
971+
arch := g.ArchitecturalReverse()["Prod"]
972+
if len(arch) != 1 {
973+
t.Fatalf("ArchitecturalReverse()[Prod] = %d edges, want 1 (symbol only): %+v", len(arch), arch)
974+
}
975+
// In a reverse edge, Edge.Target holds the SOURCE fact name.
976+
if arch[0].Target != "Caller" {
977+
t.Errorf("surviving dependent = %q, want Caller (the symbol source)", arch[0].Target)
978+
}
979+
}

0 commit comments

Comments
 (0)