diff --git a/internal/analysis/connectivity.go b/internal/analysis/connectivity.go index 4958aa8e..d67aa3b5 100644 --- a/internal/analysis/connectivity.go +++ b/internal/analysis/connectivity.go @@ -13,10 +13,13 @@ import ( // This is deliberately DISTINCT from dead-code analysis (FindDeadCode): // // - Dead-code analysis reports symbols with zero *incoming usage* -// edges — genuinely unreachable code. Such a symbol is still a -// normally extracted node: its file `defines` it, a method is -// `member_of` its type. The finding is actionable — the code is -// unused and can be removed. +// edges. Such a symbol is still a normally extracted node: its file +// `defines` it, a method is `member_of` its type. That is a narrower +// signal than "unreachable", and it is not a removal verdict: a call +// site the resolver left unresolved — an ambiguous member call, a +// name-only match — leaves exactly the same zero-incoming shape as +// genuinely unused code. Confirm a dead-code row against the source +// before deleting anything. // // - This analyzer reports *isolated* nodes — nodes with zero edges of // *any* kind, structural edges included. A normally extracted @@ -98,8 +101,11 @@ const connectivityNote = "Connectivity health is a graph-EXTRACTION diagnostic, "symbol always has at least a structural edge, so an isolated node " + "signals the indexer mis-extracted the symbol, NOT that the code is " + "unused. This is distinct from dead code (analyze kind=dead_code), " + - "which reports symbols with zero INCOMING usage edges — genuinely " + - "unreachable code that is safe to remove." + "which reports symbols with zero INCOMING usage edges. That is a " + + "narrower signal, not a removal verdict: a call site the resolver " + + "left unresolved — an ambiguous member call, a name-only match — " + + "leaves the same zero-incoming shape as genuinely unused code, so " + + "confirm a dead_code row against the source before removing anything." // GraphConnectivity computes the connectivity-health report over the // supplied nodes. The caller passes the node slice (e.g. a diff --git a/internal/analysis/connectivity_test.go b/internal/analysis/connectivity_test.go index 0bfdeb70..f8aae067 100644 --- a/internal/analysis/connectivity_test.go +++ b/internal/analysis/connectivity_test.go @@ -2,6 +2,7 @@ package analysis import ( "math" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -247,6 +248,47 @@ func TestGraphConnectivity_IsolatedIsNotDeadCode(t *testing.T) { "the structurally-linked dead-code node is NOT an extraction gap") } +// TestConnectivityNote_MatchesSharedZeroEdgeStance pins the standing note +// against the per-symbol caveat vocabulary this analyzer says it stays in +// lockstep with. graph.ClassifyZeroEdge — reused here for the +// isolated/leaf split — deliberately refuses to call a zero-incoming +// symbol proof of anything, because an unresolved or name-only call site +// leaves the same shape. The note describing kind=dead_code must not +// contradict that by promising the row is safe to delete. +func TestConnectivityNote_MatchesSharedZeroEdgeStance(t *testing.T) { + nodes := []connNode{ + {"a.go", graph.KindFile, "a.go"}, + // Zero incoming *usage* edges, one structural edge — exactly the + // shape kind=dead_code reports on. + {"a.go::Unused", graph.KindFunction, "a.go"}, + } + g, allNodes := buildConnGraph(nodes, []connEdge{{"a.go", "a.go::Unused"}}) + + // Control: the shared classifier sees this very shape and still hedges. + caveat := graph.CaveatForZeroEdge(g, "a.go::Unused") + require.NotNil(t, caveat, "the dead-code shape carries a zero-edge caveat") + require.Equal(t, graph.ZeroEdgeLikelyUnused, caveat.Class) + require.Contains(t, caveat.Message, "not proof", + "the shared caveat refuses to call zero incoming usage edges proof") + + report := GraphConnectivity(g, allNodes, 0) + require.Equal(t, connectivityNote, report.Note, "the report carries the standing note") + + // The distinction the note exists to draw must survive any rewording. + assert.Contains(t, report.Note, "dead_code", "the note still names the analyzer it contrasts with") + assert.Contains(t, report.Note, "INCOMING", "the note still states the dead_code signal") + + // What must never come back: a removal verdict the classifier withholds. + lowered := strings.ToLower(report.Note) + for _, verdict := range []string{ + "safe to remove", "safe to delete", + "can be removed", "can be deleted", + } { + assert.NotContains(t, lowered, verdict, + "the note must not promise removal safety the shared classifier withholds") + } +} + // TestGraphConnectivity_NilGraph asserts a nil graph yields a zero // report rather than panicking. func TestGraphConnectivity_NilGraph(t *testing.T) { diff --git a/internal/mcp/tools_analyze_connectivity.go b/internal/mcp/tools_analyze_connectivity.go index 87992a3e..97376935 100644 --- a/internal/mcp/tools_analyze_connectivity.go +++ b/internal/mcp/tools_analyze_connectivity.go @@ -7,10 +7,11 @@ // leaf nodes. // // It is deliberately DISTINCT from kind=dead_code. dead_code reports -// symbols with zero *incoming usage* edges — genuinely unreachable -// code, a real finding to act on (delete it). This analyzer reports -// isolated nodes — zero edges of *any* kind, structural edges -// included — which a normally extracted symbol never has. An isolated +// symbols with zero *incoming usage* edges — a narrower signal than +// "unreachable", and not a removal verdict, since an unresolved or +// name-only call site leaves the same zero-incoming shape. This +// analyzer reports isolated nodes — zero edges of *any* kind, structural +// edges included — which a normally extracted symbol never has. An isolated // node signals the indexer mis-extracted the symbol or its file, not // that the code is unused; the response carries a `note` spelling out // the distinction so a reader does not delete live code.