Skip to content

Commit 6efb114

Browse files
authored
fix(explainers): fix 7 correctness/determinism bugs and harden test coverage (#73)
Bulletproof the explainer suite with unit tests and fix the bugs found while doing so. Explainer output feeds query_insights and pkg/explain, so correctness and byte-for-byte determinism matter. Confirmed bugs fixed (each with a regression test proven to fail pre-fix): - pkg/explain: metric parsing corrupted by digits in symbol/module names — allInts scanned the whole title, so a hotspot on "Sha256Hash" rendered "fan-in 256 / out 30" and a surface on "oauth2" rendered "2/40 (44%)". Parse only the metric section via metricInts(title, marker). - cycles: nondeterministic cycle path / evidence order / insight order (map iteration + unsorted neighbors). Factored a deterministic common.StronglyConnectedComponents shared with depth. - depth: under-counted chains hidden behind a cycle (a memoized chain truncated by the back-edge cut was reused globally). Rewrote over the SCC condensation (longest path on the DAG, cycle = its full member count as a cycle-safe upper bound). - crossrepo: "via" detail dropped after a facts.jsonl round-trip (asserted []string only; JSON decodes arrays as []any). Handle both shapes. layers fixes: - deterministic pattern-insight evidence (was map iteration order) - dedup layer violations by (source, target) module so the count isn't inflated - resolve relative import targets (./x, ../y) before matching a layer Tests: added threshold-boundary, determinism, float64/JSONL prop, evidence-cap, and sort-order coverage across every explainer and common. Full suite green under -race; engine golden/determinism gate unaffected (fixes touch insights, not facts.jsonl). Docs: refreshed the Apache Airflow --explain example in README.md (fact counts, hotspots, and the now-corrected depth numbers).
1 parent 972a724 commit 6efb114

19 files changed

Lines changed: 1397 additions & 191 deletions

File tree

README.md

Lines changed: 57 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -315,95 +315,97 @@ enola --explain /path/to/repo
315315

316316
Every finding carries a confidence score, and it means something exact: `1.0` is a structural fact (a cycle exists; an export ratio measured), while anything below is a flagged heuristic for you to review (a god class is a statistical fan-in outlier, not a rule). The analyses are computed by graph algorithms — Tarjan's SCC for cycles, longest-path for dependency depth, mean+2σ outlier tests for the rest — so the same commit yields the same report.
317317

318-
Here's the actual report for [Apache Airflow](https://github.com/apache/airflow) — a large polyglot codebase (Python, Java, TypeScript, and OpenAPI specs) analyzed in a single pass, 112,792 facts in ~3.5s (extraction parses files in parallel across cores; timing measured on a 16-core machine):
318+
Here's the actual report for [Apache Airflow](https://github.com/apache/airflow) — a large polyglot codebase (Python, Java, TypeScript, gRPC, and OpenAPI specs) analyzed in a single pass, 122,772 facts in ~2s (extraction parses files in parallel across cores):
319319

320320
```
321321
════════════════════════════════════════════════════════════
322322
Repository explanation: apache/airflow
323323
════════════════════════════════════════════════════════════
324324
325325
Overview
326-
Generated: 2026-06-24T11:28:08Z
327-
Analysis time: 3.522345875s
328-
Languages: java, openapi, python, typescript
329-
Total facts: 112792
326+
Generated: 2026-07-07T20:51:51Z
327+
Analysis time: 2.100326959s
328+
Languages: python, typescript, java, grpc
329+
Total facts: 122772
330330
331331
Architectural kinds
332-
module 2492
333-
symbol 64192
334-
route 6359
332+
module 2499
333+
symbol 64148
334+
route 6426
335335
storage 64
336-
dependency 39685
336+
dependency 44371
337337
338338
Symbol breakdown
339-
method 41127
340-
function 12371
341-
class 8621
339+
method 40976
340+
function 12409
341+
class 8583
342342
type 1393
343-
variable 636
344-
interface 36
343+
variable 732
344+
interface 37
345+
struct 10
345346
enum 6
346347
constant 2
347348
348349
API & data surface
349-
routes 6359
350-
(unspecified) 6182
351-
GET 105
352-
POST 28
353-
PATCH 22
354-
DELETE 16
355-
PUT 6
350+
routes 6426
351+
PATCH 6038
352+
GET 247
353+
POST 74
354+
DELETE 46
355+
PUT 20
356+
HEAD 1
356357
storage 64
357358
358359
Dependencies
359-
internal 17810
360-
stdlib 12430
361-
external 9445
360+
internal 20905
361+
stdlib 12858
362+
external 10607
363+
unclassified 1
362364
363365
Architecture
364366
Pattern: (none detected)
365-
cyclic dependencies 26
367+
cyclic dependencies 27
366368
layer violations 0
367369
368370
Impact analysis (hotspots)
369-
coupled modules 840
370-
high criticality 486
371-
medium criticality 354
371+
coupled modules 915
372+
high criticality 547
373+
medium criticality 368
372374
Top hotspots (by coupling):
373375
module fan-in fan-out crit blast radius
374-
airflow-core/src/airflow/models 1396 315 high 56732
375-
devel-common/src/tests_common/t… 1450 85 high 35780
376-
providers/common/compat/src/air… 1214 1 high 51068
377-
airflow-core/src/airflow/utils 1029 69 high 62870
378-
airflow-core/src/airflow 871 39 high 67265
379-
providers/common/compat/tests/u… 690 0 high 61945
380-
providers/google/src/airflow/pr… 323 284 high 14305
381-
providers/amazon/tests/system/a… 1 510 high 177
376+
airflow-core/src/airflow/models 1661 380 high 66716
377+
devel-common/src/tests_common/t… 1496 161 high 39422
378+
providers/common/compat/src/air… 1295 2 high 59376
379+
airflow-core/src/airflow/utils 1124 132 high 68653
380+
airflow-core/src/airflow 1172 71 high 71697
381+
providers/common/compat/tests/u… 776 0 high 65589
382+
providers/google/src/airflow/pr… 327 329 high 14701
383+
task-sdk/src/airflow/sdk 591 15 high 64026
382384
383385
Code health
384-
god classes (high fan-in) 298
385-
airflow-core/src/airflow/ui/openapi-gen/req… 153 dependents
386-
airflow-ctl/tests/airflow_ctl/api/test_oper… 79 dependents
387-
providers/cncf/kubernetes/tests/unit/cncf/k… 71 dependents
388-
airflow-core/tests/unit/ti_deps/deps/test_t… 49 dependents
389-
providers/hashicorp/tests/unit/hashicorp/ho… 45 dependents
390-
call-graph hotspots 133
391-
airflow-core/src/airflow/ui/openapi-gen/req… fan-in 153 / out 12
392-
providers/cncf/kubernetes/tests/unit/cncf/k… fan-in 71 / out 6
393-
providers/openlineage/tests/unit/openlineag… fan-in 3 / out 21
394-
providers/edge3/src/airflow/providers/edge3… fan-in 3 / out 3
395-
providers/google/src/airflow/providers/goog… fan-in 10 / out 18
386+
god classes (high fan-in) 254
387+
chart/tests/chart_utils/helm_template_gener… 1193 dependents
388+
devel-common/src/tests_common/test_utils/co… 557 dependents
389+
devel-common/src/tests_common/test_utils/sy… 476 dependents
390+
dev/breeze/src/airflow_breeze/utils/console… 376 dependents
391+
airflow-core/src/airflow/utils/session.crea… 288 dependents
392+
call-graph hotspots 152
393+
chart/tests/chart_utils/helm_template_gener… fan-in 1193 / out 7
394+
devel-common/src/tests_common/test_utils/co… fan-in 557 / out 10
395+
task-sdk/src/airflow/sdk/execution_time/tas… fan-in 101 / out 37
396+
airflow-core/src/airflow/utils/session.crea… fan-in 288 / out 6
397+
dev/breeze/src/airflow_breeze/utils/run_uti… fan-in 179 / out 9
396398
deep dependency chains 10
397-
airflow-core/tests/unit/api_fastapi/core_ap… depth 57
398-
airflow-core/tests/unit/api_fastapi/core_ap… depth 56
399-
airflow-core/tests/unit/assets depth 56
400-
airflow-core/tests/unit/jobs depth 56
401-
providers/fab/tests/unit/fab/plugins depth 56
399+
chart/docs depth 196
400+
dev/breeze/tests depth 196
401+
dev/breeze/tests/integration_tests depth 196
402+
scripts/tools depth 196
403+
airflow-core/tests/unit/api_fastapi/core_ap… depth 195
402404
large public surfaces 20
403-
airflow-core/src/airflow/ui/openapi-gen/req… 911/911 (100%)
404405
airflow-core/src/airflow/ui/openapi-gen/que… 864/864 (100%)
406+
airflow-core/src/airflow/ui/openapi-gen/req… 720/720 (100%)
405407
task-sdk/src/airflow/sdk/execution_time/com… 119/129 (92%)
406-
providers/edge3/src/airflow/providers/edge3… 3/3 (100%)
408+
providers/edge3/src/airflow/providers/edge3… 100/100 (100%)
407409
task-sdk/src/airflow/sdk/definitions/mapped… 100/111 (90%)
408410
complexity outliers 15
409411
airflow-core/src/airflow/jobs/scheduler_job… complexity 69

internal/explainers/common/common.go

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package common
66

77
import (
88
"math"
9+
"sort"
910
"strings"
1011

1112
"github.com/enola-labs/enola/internal/facts"
@@ -161,3 +162,78 @@ func OutlierThreshold(values []float64, k float64) float64 {
161162
mean, std := MeanStdDev(values)
162163
return mean + k*std
163164
}
165+
166+
// StronglyConnectedComponents returns the strongly-connected components of a
167+
// directed graph (adjacency list: node -> successors) using Tarjan's algorithm.
168+
//
169+
// The result is deterministic regardless of Go's randomized map iteration: nodes
170+
// are visited in sorted order with sorted neighbor lists, each component's members
171+
// are sorted, and the component list is ordered by each component's smallest
172+
// member. Every key of graph appears in exactly one component (a node that appears
173+
// only as a neighbor is included as its own singleton). Shared by the cycles and
174+
// dependency-depth explainers, which both need a cycle-safe view of the module
175+
// graph — factoring it here keeps a single, tested implementation.
176+
func StronglyConnectedComponents(graph map[string][]string) [][]string {
177+
nodes := make([]string, 0, len(graph))
178+
for v := range graph {
179+
nodes = append(nodes, v)
180+
}
181+
sort.Strings(nodes)
182+
183+
var (
184+
index int
185+
stack []string
186+
onStack = make(map[string]bool)
187+
indices = make(map[string]int)
188+
lowlinks = make(map[string]int)
189+
sccs [][]string
190+
)
191+
192+
var strongConnect func(v string)
193+
strongConnect = func(v string) {
194+
indices[v] = index
195+
lowlinks[v] = index
196+
index++
197+
stack = append(stack, v)
198+
onStack[v] = true
199+
200+
neighbors := append([]string(nil), graph[v]...)
201+
sort.Strings(neighbors)
202+
for _, w := range neighbors {
203+
if _, visited := indices[w]; !visited {
204+
strongConnect(w)
205+
if lowlinks[w] < lowlinks[v] {
206+
lowlinks[v] = lowlinks[w]
207+
}
208+
} else if onStack[w] {
209+
if indices[w] < lowlinks[v] {
210+
lowlinks[v] = indices[w]
211+
}
212+
}
213+
}
214+
215+
if lowlinks[v] == indices[v] {
216+
var scc []string
217+
for {
218+
w := stack[len(stack)-1]
219+
stack = stack[:len(stack)-1]
220+
onStack[w] = false
221+
scc = append(scc, w)
222+
if w == v {
223+
break
224+
}
225+
}
226+
sort.Strings(scc)
227+
sccs = append(sccs, scc)
228+
}
229+
}
230+
231+
for _, v := range nodes {
232+
if _, visited := indices[v]; !visited {
233+
strongConnect(v)
234+
}
235+
}
236+
237+
sort.Slice(sccs, func(i, j int) bool { return sccs[i][0] < sccs[j][0] })
238+
return sccs
239+
}

internal/explainers/common/common_test.go

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package common
22

33
import (
44
"math"
5+
"strings"
56
"testing"
67

78
"github.com/enola-labs/enola/internal/facts"
@@ -148,3 +149,87 @@ func TestMeanStdDev(t *testing.T) {
148149
t.Errorf("OutlierThreshold(vals, 2) = %v, want 9", got)
149150
}
150151
}
152+
153+
// TestOutlierThreshold_ZeroStdDev: with identical values the std dev is 0, so the
154+
// threshold equals the mean — every value is <= it, so nothing is a strict
155+
// outlier (why an all-equal fan-in/complexity distribution flags nothing).
156+
func TestOutlierThreshold_ZeroStdDev(t *testing.T) {
157+
vals := []float64{7, 7, 7, 7}
158+
mean, std := MeanStdDev(vals)
159+
if std != 0 {
160+
t.Errorf("std of identical values = %v, want 0", std)
161+
}
162+
if got := OutlierThreshold(vals, 2); got != mean {
163+
t.Errorf("OutlierThreshold(identical, 2) = %v, want mean %v", got, mean)
164+
}
165+
}
166+
167+
// sccKey renders a component partition as a stable string for comparison.
168+
func sccKey(sccs [][]string) string {
169+
parts := make([]string, len(sccs))
170+
for i, scc := range sccs {
171+
parts[i] = strings.Join(scc, ",")
172+
}
173+
return strings.Join(parts, " | ")
174+
}
175+
176+
func TestStronglyConnectedComponents(t *testing.T) {
177+
tests := []struct {
178+
name string
179+
graph map[string][]string
180+
want string // sccKey of the expected (sorted) partition
181+
}{
182+
{"empty", map[string][]string{}, ""},
183+
{"single isolated", map[string][]string{"a": nil}, "a"},
184+
{"simple cycle", map[string][]string{"b": {"a"}, "a": {"b"}}, "a,b"},
185+
{"triangle + tail", map[string][]string{"a": {"b"}, "b": {"c"}, "c": {"a", "d"}, "d": nil}, "a,b,c | d"},
186+
{"two disjoint cycles", map[string][]string{"a": {"b"}, "b": {"a"}, "c": {"d"}, "d": {"c"}}, "a,b | c,d"},
187+
{"self loop is singleton", map[string][]string{"a": {"a"}}, "a"},
188+
{"neighbor-only node included", map[string][]string{"a": {"b"}}, "a | b"},
189+
}
190+
for _, tt := range tests {
191+
t.Run(tt.name, func(t *testing.T) {
192+
if got := sccKey(StronglyConnectedComponents(tt.graph)); got != tt.want {
193+
t.Errorf("SCC(%v) = %q, want %q", tt.graph, got, tt.want)
194+
}
195+
})
196+
}
197+
}
198+
199+
// TestStronglyConnectedComponents_Deterministic: repeated calls re-range the
200+
// graph map (Go randomizes iteration), but the sorted output must be identical.
201+
func TestStronglyConnectedComponents_Deterministic(t *testing.T) {
202+
graph := map[string][]string{
203+
"a": {"b"}, "b": {"c"}, "c": {"a", "d"},
204+
"d": {"e"}, "e": {"d"},
205+
"f": {"a"}, "g": nil,
206+
}
207+
want := sccKey(StronglyConnectedComponents(graph))
208+
for i := 0; i < 50; i++ {
209+
if got := sccKey(StronglyConnectedComponents(graph)); got != want {
210+
t.Fatalf("non-deterministic SCC output on iteration %d:\nwant %q\ngot %q", i, want, got)
211+
}
212+
}
213+
}
214+
215+
// TestBuildModuleGraph_ExcludesTestRole: modules tagged module_role=test are
216+
// dropped as both nodes and edge endpoints.
217+
func TestBuildModuleGraph_ExcludesTestRole(t *testing.T) {
218+
s := facts.NewStore()
219+
s.Add(facts.Fact{Kind: facts.KindModule, Name: "src/app"})
220+
s.Add(facts.Fact{Kind: facts.KindModule, Name: "src/apptest",
221+
Props: map[string]any{facts.PropModuleRole: facts.ModuleRoleTest}})
222+
// app imports the test module and vice versa; the test edges must not appear.
223+
s.Add(facts.Fact{Kind: facts.KindDependency, File: "src/app/f.go",
224+
Relations: []facts.Relation{{Kind: facts.RelImports, Target: "src/apptest"}}})
225+
s.Add(facts.Fact{Kind: facts.KindDependency, File: "src/apptest/f.go",
226+
Relations: []facts.Relation{{Kind: facts.RelImports, Target: "src/app"}}})
227+
228+
graph := BuildModuleGraph(s)
229+
if _, ok := graph["src/apptest"]; ok {
230+
t.Error("test-role module should not be a graph node")
231+
}
232+
if len(graph["src/app"]) != 0 {
233+
t.Errorf("edge to a test-role module should be dropped, got %v", graph["src/app"])
234+
}
235+
}

0 commit comments

Comments
 (0)