Skip to content

Commit 25a32d3

Browse files
committed
feat(audit): dead-code findings auto-annotated with impact deletion_safety (closes #238)
`audit --check dead-code` reported `status: dead` from the registry with no signal about whether a finding is actually safe to delete — an agent had to manually chain a separate `context --check trace --direction up` call per finding to rule out entry points (exactly the caveat already documented in CONTEXT.md: "status: dead != aman dihapus"). `impact_engine.analyze_impact(name, action="delete")` already computes this exact signal (risk level from real dependents), it just wasn't wired into the dead-code command output. Each finding (capped at --verify-impact-limit, default 20, across all categories combined) now gets a `deletion_safety` field: safe / caution / entry_point_likely / unknown (on a per-item analyze_impact failure — never crashes the whole report). Opt out entirely with --no-verify-impact. Verified on a real workspace: AdGate.tsx's default export (flagged unused_exports — genuinely never imported directly, confirmed earlier this session) is correctly tagged entry_point_likely because analyze_impact finds 6 real direct dependents through other paths, exactly the false-confidence trap this issue set out to close.
1 parent 03f470c commit 25a32d3

3 files changed

Lines changed: 185 additions & 0 deletions

File tree

scripts/commands/audit.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,13 @@ def add_args(parser):
110110
help="perf-hint: single category filter")
111111
parser.add_argument("--no-confirm-hash", action="store_true", default=False,
112112
help="staleness: skip content-hash confirmation")
113+
parser.add_argument("--no-verify-impact", dest="verify_impact",
114+
action="store_false", default=True,
115+
help="dead-code: skip per-finding deletion_safety cross-check "
116+
"against impact analysis (issue #238)")
117+
parser.add_argument("--verify-impact-limit", type=int, default=None,
118+
help="dead-code: max findings to cross-check with impact "
119+
"analysis (default: 20)")
113120

114121

115122
def _parse_checks(check_arg: str) -> List[str]:
@@ -138,6 +145,8 @@ def _build_namespace(base_args, check_name: str) -> argparse.Namespace:
138145
ns.categories = getattr(base_args, "categories", None)
139146
ns.max_files = getattr(base_args, "max_files", None) or 3000
140147
ns.max_results = getattr(base_args, "max_results", None) or 100
148+
ns.verify_impact = getattr(base_args, "verify_impact", True)
149+
ns.verify_impact_limit = getattr(base_args, "verify_impact_limit", None) or 20
141150
elif check_name == "complexity":
142151
ns.name = getattr(base_args, "name", None)
143152
ns.file = getattr(base_args, "file", None)

scripts/commands/dead_code.py

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,15 @@ def add_args(parser):
1313
help="Max files to scan (default: 3000)")
1414
parser.add_argument("--max-results", type=int, default=100,
1515
help="Max results per category (default: 100)")
16+
parser.add_argument("--no-verify-impact", dest="verify_impact",
17+
action="store_false", default=True,
18+
help="Skip per-finding deletion_safety cross-check against "
19+
"impact analysis (issue #238). On by default; disable "
20+
"on very large result sets if it's too slow.")
21+
parser.add_argument("--verify-impact-limit", type=int, default=20,
22+
help="Max number of findings to cross-check with impact "
23+
"analysis per run (default: 20, highest-confidence "
24+
"findings first)")
1625

1726

1827
def execute(args, workspace):
@@ -82,6 +91,47 @@ def execute(args, workspace):
8291
if "stats" not in result:
8392
result["stats"] = {}
8493
result["stats"]["confidence_distribution"] = dist
94+
95+
# Issue #238: per-finding deletion_safety cross-check.
96+
#
97+
# "status: dead" in the registry only means "no CALLS edge found" — it
98+
# does NOT mean safe to delete (entry points like HTTP handlers, CLI
99+
# subcommands, and exported APIs routinely have zero inbound edges but
100+
# are still critical). Previously an agent had to manually chain
101+
# `audit --check dead-code` -> `context --check trace --direction up`
102+
# to verify this per finding; analyze_impact() already computes exactly
103+
# this signal for the "delete" action, it just wasn't wired in here.
104+
if getattr(args, "verify_impact", True):
105+
try:
106+
from impact_engine import analyze_impact
107+
all_items = []
108+
for cat_items in result.get("results", {}).values():
109+
if isinstance(cat_items, list):
110+
all_items.extend(cat_items)
111+
limit = max(0, getattr(args, "verify_impact_limit", 20) or 0)
112+
_risk_to_safety = {
113+
"low": "safe",
114+
"medium": "caution",
115+
"high": "entry_point_likely",
116+
"critical": "entry_point_likely",
117+
}
118+
for item in all_items[:limit]:
119+
if not isinstance(item, dict):
120+
continue
121+
name = item.get("name")
122+
if not name:
123+
continue
124+
try:
125+
impact_result = analyze_impact(
126+
name, workspace, action="delete", depth=3
127+
)
128+
risk = impact_result.get("risk", "low")
129+
item["deletion_safety"] = _risk_to_safety.get(risk, "caution")
130+
item["deletion_impact_stats"] = impact_result.get("stats")
131+
except Exception:
132+
item["deletion_safety"] = "unknown"
133+
except ImportError:
134+
pass
85135
return result
86136

87137
# Issue #199: deprecated "dead-code" alias registration removed; this module is now an implementation module imported by the "audit" umbrella command.

tests/test_dead_code_command.py

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
"""Tests for the dead-code command's deletion_safety cross-check (issue #238).
2+
3+
`audit --check dead-code` previously reported `status: dead` from the
4+
registry with no signal about whether a finding is actually safe to delete
5+
— an agent had to manually chain a separate `context --check trace
6+
--direction up` call per finding to check for entry points. This wires
7+
`impact_engine.analyze_impact(action="delete")` (which already computes
8+
exactly this signal) directly into the dead-code command output.
9+
"""
10+
11+
import argparse
12+
import os
13+
import sys
14+
from unittest import mock
15+
16+
import pytest
17+
18+
SCRIPT_DIR = os.path.join(
19+
os.path.dirname(os.path.dirname(os.path.abspath(__file__))), "scripts"
20+
)
21+
if SCRIPT_DIR not in sys.path:
22+
sys.path.insert(0, SCRIPT_DIR)
23+
24+
from commands import dead_code # noqa: E402
25+
26+
27+
def _base_args(**overrides):
28+
ns = argparse.Namespace(
29+
workspace=".",
30+
categories=None,
31+
max_files=3000,
32+
max_results=100,
33+
verify_impact=True,
34+
verify_impact_limit=20,
35+
)
36+
for k, v in overrides.items():
37+
setattr(ns, k, v)
38+
return ns
39+
40+
41+
def _fake_dead_code_result():
42+
return {
43+
"status": "ok",
44+
"stats": {"total_dead_code": 2},
45+
"results": {
46+
"unused_exports": [
47+
{"file": "AdGate.tsx", "line": 39, "name": "AdGate", "type": "default_export"},
48+
],
49+
"registry_dead": [
50+
{"file": "utils.ts", "line": 10, "name": "reallyUnusedHelper", "type": "function"},
51+
],
52+
},
53+
}
54+
55+
56+
class TestDeletionSafetyCrossCheck:
57+
def test_high_risk_symbol_flagged_entry_point_likely(self):
58+
"""A dead-code finding that analyze_impact reports as high-risk
59+
(real dependents exist) must not be silently labeled safe."""
60+
with mock.patch("commands.dead_code.detect_dead_code", return_value=_fake_dead_code_result()), \
61+
mock.patch("hybrid_engine.create_hybrid_engine", side_effect=ImportError), \
62+
mock.patch(
63+
"impact_engine.analyze_impact",
64+
side_effect=lambda name, ws, **kw: {
65+
"risk": "high" if name == "AdGate" else "low",
66+
"stats": {"direct_dependents": 6 if name == "AdGate" else 0},
67+
},
68+
):
69+
result = dead_code.execute(_base_args(), ".")
70+
71+
items_by_name = {
72+
item["name"]: item
73+
for cat in result["results"].values()
74+
for item in cat
75+
}
76+
assert items_by_name["AdGate"]["deletion_safety"] == "entry_point_likely"
77+
assert items_by_name["reallyUnusedHelper"]["deletion_safety"] == "safe"
78+
79+
def test_no_verify_impact_flag_skips_cross_check(self):
80+
"""--no-verify-impact must not call analyze_impact at all."""
81+
with mock.patch("commands.dead_code.detect_dead_code", return_value=_fake_dead_code_result()), \
82+
mock.patch("hybrid_engine.create_hybrid_engine", side_effect=ImportError), \
83+
mock.patch("impact_engine.analyze_impact") as mock_analyze:
84+
result = dead_code.execute(_base_args(verify_impact=False), ".")
85+
86+
mock_analyze.assert_not_called()
87+
for cat in result["results"].values():
88+
for item in cat:
89+
assert "deletion_safety" not in item
90+
91+
def test_verify_impact_limit_caps_calls(self):
92+
"""Only the first N findings (across all categories combined) are
93+
cross-checked, to bound cost on large dead-code result sets."""
94+
many_findings = {
95+
"status": "ok",
96+
"stats": {"total_dead_code": 10},
97+
"results": {
98+
"unused_exports": [
99+
{"file": f"f{i}.ts", "line": i, "name": f"fn{i}", "type": "function"}
100+
for i in range(10)
101+
],
102+
},
103+
}
104+
with mock.patch("commands.dead_code.detect_dead_code", return_value=many_findings), \
105+
mock.patch("hybrid_engine.create_hybrid_engine", side_effect=ImportError), \
106+
mock.patch(
107+
"impact_engine.analyze_impact",
108+
return_value={"risk": "low", "stats": {}},
109+
) as mock_analyze:
110+
dead_code.execute(_base_args(verify_impact_limit=3), ".")
111+
112+
assert mock_analyze.call_count == 3
113+
114+
def test_analyze_impact_failure_does_not_crash_command(self):
115+
"""If analyze_impact raises for a given symbol, the command must
116+
still return successfully with an 'unknown' safety label — not
117+
propagate the exception and break the whole dead-code report."""
118+
with mock.patch("commands.dead_code.detect_dead_code", return_value=_fake_dead_code_result()), \
119+
mock.patch("hybrid_engine.create_hybrid_engine", side_effect=ImportError), \
120+
mock.patch("impact_engine.analyze_impact", side_effect=RuntimeError("boom")):
121+
result = dead_code.execute(_base_args(), ".")
122+
123+
assert result["status"] == "ok"
124+
for cat in result["results"].values():
125+
for item in cat:
126+
assert item["deletion_safety"] == "unknown"

0 commit comments

Comments
 (0)