Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions scripts/commands/audit.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,13 @@
help="perf-hint: single category filter")
parser.add_argument("--no-confirm-hash", action="store_true", default=False,
help="staleness: skip content-hash confirmation")
parser.add_argument("--no-verify-impact", dest="verify_impact",
action="store_false", default=True,
help="dead-code: skip per-finding deletion_safety cross-check "
"against impact analysis (issue #238)")
parser.add_argument("--verify-impact-limit", type=int, default=None,
help="dead-code: max findings to cross-check with impact "
"analysis (default: 20)")


def _parse_checks(check_arg: str) -> List[str]:
Expand All @@ -127,7 +134,7 @@
return parts or list(ALL_CHECKS)


def _build_namespace(base_args, check_name: str) -> argparse.Namespace:

Check failure on line 137 in scripts/commands/audit.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this function to reduce its Cognitive Complexity from 16 to the 15 allowed.

See more on https://sonarcloud.io/project/issues?id=Wolfvin_CodeLens&issues=AZ9WWWCJOsIJlDz02mKE&open=AZ9WWWCJOsIJlDz02mKE&pullRequest=243
ns = argparse.Namespace()
for attr in ("format", "top", "max_tokens", "lite", "deep", "db_path",
"diff_base", "diff_scope", "disable_suppression",
Expand All @@ -138,6 +145,8 @@
ns.categories = getattr(base_args, "categories", None)
ns.max_files = getattr(base_args, "max_files", None) or 3000
ns.max_results = getattr(base_args, "max_results", None) or 100
ns.verify_impact = getattr(base_args, "verify_impact", True)
ns.verify_impact_limit = getattr(base_args, "verify_impact_limit", None) or 20
elif check_name == "complexity":
ns.name = getattr(base_args, "name", None)
ns.file = getattr(base_args, "file", None)
Expand Down
50 changes: 50 additions & 0 deletions scripts/commands/dead_code.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,15 @@ def add_args(parser):
help="Max files to scan (default: 3000)")
parser.add_argument("--max-results", type=int, default=100,
help="Max results per category (default: 100)")
parser.add_argument("--no-verify-impact", dest="verify_impact",
action="store_false", default=True,
help="Skip per-finding deletion_safety cross-check against "
"impact analysis (issue #238). On by default; disable "
"on very large result sets if it's too slow.")
parser.add_argument("--verify-impact-limit", type=int, default=20,
help="Max number of findings to cross-check with impact "
"analysis per run (default: 20, highest-confidence "
"findings first)")


def execute(args, workspace):
Expand Down Expand Up @@ -82,6 +91,47 @@ def execute(args, workspace):
if "stats" not in result:
result["stats"] = {}
result["stats"]["confidence_distribution"] = dist

# Issue #238: per-finding deletion_safety cross-check.
#
# "status: dead" in the registry only means "no CALLS edge found" — it
# does NOT mean safe to delete (entry points like HTTP handlers, CLI
# subcommands, and exported APIs routinely have zero inbound edges but
# are still critical). Previously an agent had to manually chain
# `audit --check dead-code` -> `context --check trace --direction up`
# to verify this per finding; analyze_impact() already computes exactly
# this signal for the "delete" action, it just wasn't wired in here.
if getattr(args, "verify_impact", True):
try:
from impact_engine import analyze_impact
all_items = []
for cat_items in result.get("results", {}).values():
if isinstance(cat_items, list):
all_items.extend(cat_items)
limit = max(0, getattr(args, "verify_impact_limit", 20) or 0)
_risk_to_safety = {
"low": "safe",
"medium": "caution",
"high": "entry_point_likely",
"critical": "entry_point_likely",
}
for item in all_items[:limit]:
if not isinstance(item, dict):
continue
name = item.get("name")
if not name:
continue
try:
impact_result = analyze_impact(
name, workspace, action="delete", depth=3
)
risk = impact_result.get("risk", "low")
item["deletion_safety"] = _risk_to_safety.get(risk, "caution")
item["deletion_impact_stats"] = impact_result.get("stats")
except Exception:
item["deletion_safety"] = "unknown"
except ImportError:
pass
return result

# Issue #199: deprecated "dead-code" alias registration removed; this module is now an implementation module imported by the "audit" umbrella command.
126 changes: 126 additions & 0 deletions tests/test_dead_code_command.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
"""Tests for the dead-code command's deletion_safety cross-check (issue #238).

`audit --check dead-code` previously 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 check for entry points. This wires
`impact_engine.analyze_impact(action="delete")` (which already computes
exactly this signal) directly into the dead-code command output.
"""

import argparse
import os
import sys
from unittest import mock

import pytest

SCRIPT_DIR = os.path.join(
os.path.dirname(os.path.dirname(os.path.abspath(__file__))), "scripts"
)
if SCRIPT_DIR not in sys.path:
sys.path.insert(0, SCRIPT_DIR)

from commands import dead_code # noqa: E402


def _base_args(**overrides):
ns = argparse.Namespace(
workspace=".",
categories=None,
max_files=3000,
max_results=100,
verify_impact=True,
verify_impact_limit=20,
)
for k, v in overrides.items():
setattr(ns, k, v)
return ns


def _fake_dead_code_result():
return {
"status": "ok",
"stats": {"total_dead_code": 2},
"results": {
"unused_exports": [
{"file": "AdGate.tsx", "line": 39, "name": "AdGate", "type": "default_export"},
],
"registry_dead": [
{"file": "utils.ts", "line": 10, "name": "reallyUnusedHelper", "type": "function"},
],
},
}


class TestDeletionSafetyCrossCheck:
def test_high_risk_symbol_flagged_entry_point_likely(self):
"""A dead-code finding that analyze_impact reports as high-risk
(real dependents exist) must not be silently labeled safe."""
with mock.patch("commands.dead_code.detect_dead_code", return_value=_fake_dead_code_result()), \
mock.patch("hybrid_engine.create_hybrid_engine", side_effect=ImportError), \
mock.patch(
"impact_engine.analyze_impact",
side_effect=lambda name, ws, **kw: {
"risk": "high" if name == "AdGate" else "low",
"stats": {"direct_dependents": 6 if name == "AdGate" else 0},
},
):
result = dead_code.execute(_base_args(), ".")

items_by_name = {
item["name"]: item
for cat in result["results"].values()
for item in cat
}
assert items_by_name["AdGate"]["deletion_safety"] == "entry_point_likely"
assert items_by_name["reallyUnusedHelper"]["deletion_safety"] == "safe"

def test_no_verify_impact_flag_skips_cross_check(self):
"""--no-verify-impact must not call analyze_impact at all."""
with mock.patch("commands.dead_code.detect_dead_code", return_value=_fake_dead_code_result()), \
mock.patch("hybrid_engine.create_hybrid_engine", side_effect=ImportError), \
mock.patch("impact_engine.analyze_impact") as mock_analyze:
result = dead_code.execute(_base_args(verify_impact=False), ".")

mock_analyze.assert_not_called()
for cat in result["results"].values():
for item in cat:
assert "deletion_safety" not in item

def test_verify_impact_limit_caps_calls(self):
"""Only the first N findings (across all categories combined) are
cross-checked, to bound cost on large dead-code result sets."""
many_findings = {
"status": "ok",
"stats": {"total_dead_code": 10},
"results": {
"unused_exports": [
{"file": f"f{i}.ts", "line": i, "name": f"fn{i}", "type": "function"}
for i in range(10)
],
},
}
with mock.patch("commands.dead_code.detect_dead_code", return_value=many_findings), \
mock.patch("hybrid_engine.create_hybrid_engine", side_effect=ImportError), \
mock.patch(
"impact_engine.analyze_impact",
return_value={"risk": "low", "stats": {}},
) as mock_analyze:
dead_code.execute(_base_args(verify_impact_limit=3), ".")

assert mock_analyze.call_count == 3

def test_analyze_impact_failure_does_not_crash_command(self):
"""If analyze_impact raises for a given symbol, the command must
still return successfully with an 'unknown' safety label — not
propagate the exception and break the whole dead-code report."""
with mock.patch("commands.dead_code.detect_dead_code", return_value=_fake_dead_code_result()), \
mock.patch("hybrid_engine.create_hybrid_engine", side_effect=ImportError), \
mock.patch("impact_engine.analyze_impact", side_effect=RuntimeError("boom")):
result = dead_code.execute(_base_args(), ".")

assert result["status"] == "ok"
for cat in result["results"].values():
for item in cat:
assert item["deletion_safety"] == "unknown"
Loading