Skip to content

Commit 7cb2ee3

Browse files
committed
fix(coverage): count coverage gaps by one shared rule
The snapshot receipt classified a service as a coverage gap whenever it had any unresolved outbound call site, while coverage_report and the coverage explainer both required outbound == 0 — no resolved edges at all. Since unresolved is derived as detected - resolved - external, every healthy client service has some, so the receipt's metric saturated: it equalled services_total on healthy input and could not signal ill health. llm_context.md prints that number behind a warning sign, so the file an agent reads without a tool call carried the inflated count. Hoist the rule into facts.ClassifyService, with facts.DependsOnCount replacing the two duplicate outbound counters. coverageSummary, buildCoverageReport and the coverage explainer now all classify through it, so they cannot drift apart again. Also decouple UnresolvedEdges from the gap condition — it accumulates for every service, gapped or not. On a four-service multi-repo snapshot the receipt now agrees with coverage_report; unresolved_edges is unchanged. facts.jsonl and insights.json are byte-identical across the change. No cacheVersion bump: coverageSummary feeds receipt.json only (via both engine.go and global_receipt.go), no extractor output changes, and TestGolden passes without -update. The existing svcCoverage test helper attaches no relations, so every fixture had outbound == 0 and TestCoverageSummary_ExternalBucket returned the same answer under either rule. Add the case that distinguishes them.
1 parent d60547e commit 7cb2ee3

7 files changed

Lines changed: 158 additions & 40 deletions

File tree

internal/engine/coverage_summary_test.go

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package engine
55
// and excluded from the internal blind-spot count (unresolved) and gap tally.
66

77
import (
8+
"fmt"
89
"testing"
910

1011
"github.com/enola-labs/enola/internal/facts"
@@ -28,12 +29,49 @@ func svcCoverage(name string, resolved, unresolved, external int) facts.Fact {
2829
}
2930
}
3031

32+
// withDeps attaches n resolved outbound cross-repo dependencies to a service node.
33+
// Without this, every fixture has outbound == 0, which is the one input that tells
34+
// a coverage gap apart from a merely partially-covered service.
35+
func withDeps(f facts.Fact, n int) facts.Fact {
36+
for i := 0; i < n; i++ {
37+
f.Relations = append(f.Relations, facts.Relation{
38+
Kind: facts.RelDependsOn,
39+
Target: fmt.Sprintf("dep-%d", i),
40+
})
41+
}
42+
return f
43+
}
44+
45+
// A service with a resolved outbound edge is "partial coverage", which both
46+
// coverage_report and the coverage explainer classify as connected — not a gap.
47+
// The receipt must agree with them.
48+
func TestCoverageSummary_PartiallyCoveredServiceIsNotAGap(t *testing.T) {
49+
st := facts.NewStore()
50+
st.Add(
51+
withDeps(svcCoverage("partial", 5, 2, 0), 1), // resolved edge + unresolved -> connected
52+
svcCoverage("gap", 0, 3, 0), // no resolved edge + unresolved -> gap
53+
withDeps(svcCoverage("clean", 4, 0, 0), 1), // nothing unresolved -> connected
54+
)
55+
56+
sum := coverageSummary(st)
57+
if sum == nil {
58+
t.Fatal("expected a CoverageSummary")
59+
}
60+
if sum.CoverageGaps != 1 {
61+
t.Errorf("CoverageGaps = %d, want 1 (only the service with no resolved outbound edge)", sum.CoverageGaps)
62+
}
63+
// UnresolvedEdges must keep accumulating across every service, gapped or not.
64+
if sum.UnresolvedEdges != 5 {
65+
t.Errorf("UnresolvedEdges = %d, want 5 (2+3, counted regardless of classification)", sum.UnresolvedEdges)
66+
}
67+
}
68+
3169
func TestCoverageSummary_ExternalBucket(t *testing.T) {
3270
st := facts.NewStore()
3371
st.Add(
34-
svcCoverage("a", 5, 2, 0), // 2 internal unresolved -> a coverage gap
35-
svcCoverage("b", 4, 0, 3), // only external -> NOT a gap
36-
svcCoverage("c", 1, 1, 4), // both -> gap, external counted separately
72+
svcCoverage("a", 5, 2, 0), // 2 internal unresolved -> a coverage gap
73+
svcCoverage("b", 4, 0, 3), // only external -> NOT a gap
74+
svcCoverage("c", 1, 1, 4), // both -> gap, external counted separately
3775
)
3876

3977
sum := coverageSummary(st)

internal/engine/receipt.go

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -105,19 +105,25 @@ func runGit(repoPath string, args ...string) (string, error) {
105105
// coverageSummary rolls up the per-service edge_coverage counts the cross-repo
106106
// linker records into a single snapshot-level summary. It returns nil for a
107107
// single-repo snapshot (no service nodes), matching the coverage explainer.
108+
//
109+
// A gap is counted through facts.ClassifyService, so the receipt, coverage_report
110+
// and the coverage explainer cannot drift apart. UnresolvedEdges accumulates for
111+
// every service regardless of class — a partially-covered service still has blind
112+
// spots worth reporting, it just is not a gap.
108113
func coverageSummary(store *facts.Store) *facts.CoverageSummary {
109114
services := store.ByKind(facts.KindService)
110115
if len(services) == 0 {
111116
return nil
112117
}
113118
sum := &facts.CoverageSummary{ServicesTotal: len(services)}
114119
for _, svc := range services {
120+
detected := readCoverageField(svc, "detected")
115121
unresolved := readCoverageField(svc, "unresolved")
116-
if unresolved > 0 {
122+
sum.UnresolvedEdges += unresolved
123+
sum.ExternalEdges += readCoverageField(svc, "external")
124+
if facts.ClassifyService(facts.DependsOnCount(svc), detected, unresolved) == facts.ServiceCoverageGap {
117125
sum.CoverageGaps++
118-
sum.UnresolvedEdges += unresolved
119126
}
120-
sum.ExternalEdges += readCoverageField(svc, "external")
121127
}
122128
return sum
123129
}

internal/explainers/coverage/coverage.go

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -44,11 +44,11 @@ func (e *CoverageExplainer) Explain(ctx context.Context, store *facts.Store) ([]
4444
continue
4545
}
4646

47-
outbound := dependsOnCount(svc)
47+
outbound := facts.DependsOnCount(svc)
4848
evidence := []facts.Evidence{{Fact: svc.Name, Detail: coverageDetail(cov)}}
4949

5050
var insight facts.Insight
51-
if outbound == 0 {
51+
if facts.ClassifyService(outbound, detected, unresolved) == facts.ServiceCoverageGap {
5252
insight = facts.Insight{
5353
Title: fmt.Sprintf("Coverage gap: service %s appears isolated but has %d unresolved outbound call site(s)",
5454
svc.Name, unresolved),
@@ -106,18 +106,6 @@ func coverageDetail(cov []coverageEntry) string {
106106
return out
107107
}
108108

109-
// dependsOnCount returns how many resolved outbound (cross-repo) dependencies a
110-
// service node carries.
111-
func dependsOnCount(svc facts.Fact) int {
112-
n := 0
113-
for _, rel := range svc.Relations {
114-
if rel.Kind == facts.RelDependsOn {
115-
n++
116-
}
117-
}
118-
return n
119-
}
120-
121109
// readCoverage extracts the edge_coverage entries from a service node's props,
122110
// tolerating both the in-memory shape ([]map[string]any with int values) and the
123111
// shape that survives a facts.jsonl JSON round-trip ([]any of map[string]any with

internal/facts/coverage.go

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
package facts
2+
3+
// Service coverage classifications. A service node's class answers "did enola
4+
// resolve this repo's outbound calls, and if not, is that a blind spot or is the
5+
// repo genuinely a leaf?"
6+
const (
7+
// ServiceConnected: at least one outbound call site resolved to a loaded repo.
8+
// Some call sites may still be unresolved (an unloaded repo, a third-party API);
9+
// that is partial coverage, not a blind spot.
10+
ServiceConnected = "connected"
11+
// ServiceCoverageGap: nothing resolved, yet outbound call sites were detected.
12+
// The repo looks isolated but almost certainly is not — verify against source.
13+
ServiceCoverageGap = "coverage_gap"
14+
// ServiceIsolated: no resolved outbound edges and nothing unresolved to explain
15+
// them away — genuinely a leaf.
16+
ServiceIsolated = "isolated"
17+
)
18+
19+
// ClassifyService is the single definition of a service's cross-repo coverage
20+
// class. The snapshot receipt (internal/engine), coverage_report (internal/server)
21+
// and the coverage explainer all classify through here; they previously each
22+
// carried their own copy and the receipt's had drifted, counting every service
23+
// with any unresolved call site as a gap. Because unresolved is derived as
24+
// detected-resolved-external, that made the metric saturate — on a healthy
25+
// multi-repo snapshot every client has some unresolved call site, so the count
26+
// equalled the number of services and could not signal ill health.
27+
//
28+
// outbound is the count of resolved outbound cross-repo dependencies (see
29+
// DependsOnCount); detected and unresolved are summed across the service's
30+
// edge_coverage entries.
31+
func ClassifyService(outbound, detected, unresolved int) string {
32+
if outbound > 0 {
33+
return ServiceConnected
34+
}
35+
// external-only call sites are expected, not a blind spot, so a service with no
36+
// resolved edges but only external calls stays isolated, not a gap.
37+
if detected > 0 && unresolved > 0 {
38+
return ServiceCoverageGap
39+
}
40+
return ServiceIsolated
41+
}
42+
43+
// DependsOnCount returns how many resolved outbound (cross-repo) dependencies a
44+
// service node carries.
45+
func DependsOnCount(svc Fact) int {
46+
n := 0
47+
for _, rel := range svc.Relations {
48+
if rel.Kind == RelDependsOn {
49+
n++
50+
}
51+
}
52+
return n
53+
}

internal/facts/coverage_test.go

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
package facts
2+
3+
import "testing"
4+
5+
func TestClassifyService(t *testing.T) {
6+
tests := []struct {
7+
name string
8+
outbound, detected, unresolved int
9+
want string
10+
}{
11+
{"resolved edge, everything covered", 1, 4, 0, ServiceConnected},
12+
{"resolved edge plus unresolved is partial coverage, not a gap", 1, 4, 1, ServiceConnected},
13+
{"many unresolved still connected while one edge resolved", 1, 1219, 342, ServiceConnected},
14+
{"nothing resolved but call sites detected", 0, 3, 3, ServiceCoverageGap},
15+
{"genuine leaf", 0, 0, 0, ServiceIsolated},
16+
{"external-only call sites are expected, not a blind spot", 0, 3, 0, ServiceIsolated},
17+
// unresolved is derived as detected-resolved-external, so unresolved>0 with
18+
// detected==0 cannot arise from the linker. A hand-authored or truncated fact
19+
// can still produce it; classify it isolated rather than inventing a gap.
20+
{"unresolved without detected", 0, 0, 2, ServiceIsolated},
21+
}
22+
for _, tt := range tests {
23+
t.Run(tt.name, func(t *testing.T) {
24+
if got := ClassifyService(tt.outbound, tt.detected, tt.unresolved); got != tt.want {
25+
t.Errorf("ClassifyService(%d, %d, %d) = %q, want %q",
26+
tt.outbound, tt.detected, tt.unresolved, got, tt.want)
27+
}
28+
})
29+
}
30+
}
31+
32+
func TestDependsOnCount(t *testing.T) {
33+
svc := Fact{
34+
Kind: KindService,
35+
Name: "svc",
36+
Relations: []Relation{
37+
{Kind: RelDependsOn, Target: "a"},
38+
{Kind: RelCalls, Target: "b"},
39+
{Kind: RelDependsOn, Target: "c"},
40+
{Kind: RelImports, Target: "d"},
41+
},
42+
}
43+
if got := DependsOnCount(svc); got != 2 {
44+
t.Errorf("DependsOnCount = %d, want 2 (only depends_on relations)", got)
45+
}
46+
if got := DependsOnCount(Fact{Kind: KindService, Name: "leaf"}); got != 0 {
47+
t.Errorf("DependsOnCount(no relations) = %d, want 0", got)
48+
}
49+
}

internal/facts/model.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,7 @@ type ParseError struct {
231231
// without the consumer running coverage_report.
232232
type CoverageSummary struct {
233233
ServicesTotal int `json:"services_total"`
234-
CoverageGaps int `json:"coverage_gaps"` // services with unresolved (internal) outbound call sites
234+
CoverageGaps int `json:"coverage_gaps"` // services classified ServiceCoverageGap: no resolved outbound edge, yet unresolved call sites were detected. A service that resolved some edges is partially covered, not a gap
235235
UnresolvedEdges int `json:"unresolved_edges"` // detected outbound edges that did not resolve to a loaded service (internal blind spots; excludes external)
236236
ExternalEdges int `json:"external_edges,omitempty"` // detected outbound edges to hardcoded external hosts (third-party APIs) — expected, not a blind spot
237237
}

internal/server/server.go

Lines changed: 3 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1437,12 +1437,7 @@ func buildCoverageReport(store *facts.Store, repo string) []serviceCoverage {
14371437
continue
14381438
}
14391439

1440-
outbound := 0
1441-
for _, rel := range svc.Relations {
1442-
if rel.Kind == facts.RelDependsOn {
1443-
outbound++
1444-
}
1445-
}
1440+
outbound := facts.DependsOnCount(svc)
14461441

14471442
cov := readEdgeCoverage(svc)
14481443
detected, unresolved, external := 0, 0, 0
@@ -1452,20 +1447,9 @@ func buildCoverageReport(store *facts.Store, repo string) []serviceCoverage {
14521447
external += c.External
14531448
}
14541449

1455-
class := "connected"
1456-
if outbound == 0 {
1457-
// external-only call sites are expected, not a blind spot, so a service
1458-
// with no resolved edges but only external calls stays isolated, not a gap.
1459-
if detected > 0 && unresolved > 0 {
1460-
class = "coverage_gap"
1461-
} else {
1462-
class = "isolated"
1463-
}
1464-
}
1465-
14661450
out = append(out, serviceCoverage{
14671451
Service: svc.Name,
1468-
Classification: class,
1452+
Classification: facts.ClassifyService(outbound, detected, unresolved),
14691453
OutboundEdges: outbound,
14701454
EdgeCoverage: cov,
14711455
UnresolvedTotal: unresolved,
@@ -1531,7 +1515,7 @@ func renderCoverageReport(report []serviceCoverage) string {
15311515

15321516
gaps := 0
15331517
for _, sc := range report {
1534-
if sc.Classification == "coverage_gap" {
1518+
if sc.Classification == facts.ServiceCoverageGap {
15351519
gaps++
15361520
}
15371521
}

0 commit comments

Comments
 (0)