From 1cdbf48c577703862040d41cc83f37511cae6116 Mon Sep 17 00:00:00 2001 From: Manoj21k Date: Fri, 31 Jul 2026 19:37:39 +0530 Subject: [PATCH] fix(detect): gate coverage/ pruning on report artefacts (#2339) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "coverage" was an unconditional _SKIP_DIRS entry and _is_noise_dir matches directory names at any depth, so a repo where coverage is a legitimate package name lost the whole package from the graph — no warning, no skipped_sensitive entry, nothing in the report. The failure is quiet in the worst way: the package's dependents survive, so queries keep returning plausible neighbours while the package itself has no nodes. The entry's comment cites Vitest/Istanbul/nyc HTML reports (#870), but the pruning is language-agnostic, so it also removes Python/Go/Rust packages that happen to be called coverage. detect.py has been fixed twice for this exact shape — #1666 gated a bare snapshots/ on real .snap evidence, #2058 gated env/.env/*_env on real virtualenv markers — so this applies the established pattern rather than a new mechanism. _has_coverage_artifacts() mirrors _has_venv_markers(): same OSError guard, same "cannot verify, keep a possibly-real code dir" contract when no parent is available. Evidence is a file a coverage tool actually writes (lcov.info, coverage-final.json, clover.xml, coverage.xml, cobertura-coverage.xml, jacoco.xml, .coverage, index.html) or an lcov-report/ / html-report/ subtree, covering lcov, nyc/Istanbul, coverage.py, JaCoCo and Cobertura. lcov-report stays unconditional: it has no false-positive class, so gating it would add filesystem probes for no benefit — the same split #1666 made between the unambiguous __snapshots__ and the bare snapshots. --- graphify/detect.py | 43 +++++++++++++++++++++++- tests/test_detect.py | 79 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 121 insertions(+), 1 deletion(-) diff --git a/graphify/detect.py b/graphify/detect.py index 0b569e4b..fb94fffb 100644 --- a/graphify/detect.py +++ b/graphify/detect.py @@ -795,7 +795,9 @@ _SKIP_DIRS = { ".tox", ".nox", ".eggs", "*.egg-info", # nox is tox's successor, same .nox/ venv shape (#1804) "graphify-out", GRAPHIFY_OUT_NAME, # never treat own output as source input (#524); honour GRAPHIFY_OUT (#1423) # Coverage/test-artefact dirs — generated, never architecturally meaningful - "coverage", "lcov-report", # Vitest/Istanbul/nyc HTML reports (#870) + "lcov-report", # Vitest/Istanbul/nyc HTML reports (#870); + # bare "coverage" is gated on report + # artefacts below (#2339) "visual-tests", "visual-test", # Playwright/visual-regression bundles (#869) "__snapshots__", # Jest/Vitest snapshot dir (unambiguous) "storybook-static", # Storybook production build output @@ -824,6 +826,39 @@ _SKIP_FILES = { # unconditionally pruned above; only the ambiguous bare name is gated here. _JS_SNAPSHOT_TEST_ROOTS = frozenset({"__tests__", "__test__"}) +# Files a coverage tool writes into its own output dir. Any one of them is proof +# the directory is generated: lcov (lcov.info), nyc/Istanbul (coverage-final.json, +# clover.xml, the lcov-report/ subtree), coverage.py (coverage.xml, .coverage), +# JaCoCo/Cobertura (jacoco.xml, cobertura-coverage.xml). +_COVERAGE_ARTIFACT_FILES = frozenset({ + "lcov.info", "coverage-final.json", "coverage-summary.json", + "clover.xml", "coverage.xml", "cobertura-coverage.xml", "jacoco.xml", + ".coverage", "index.html", +}) +_COVERAGE_ARTIFACT_DIRS = frozenset({"lcov-report", "html-report"}) + + +def _has_coverage_artifacts(d: "Path") -> bool: + """True only when *d* holds files a coverage tool actually generated. + + ``coverage`` is a legitimate package name (a Python package, a Go/Rust module, + a domain namespace), so pruning it by name alone silently drops real source — + an entire 5-module package in #2339, with its dependents left in the graph so + queries still returned plausible neighbours. Prune it only on real evidence, + mirroring the ``snapshots``/``env`` gating (#1666/#2058): a coverage report + file, or an Istanbul/lcov HTML report subtree. + """ + try: + for name in _COVERAGE_ARTIFACT_FILES: + if (d / name).is_file(): + return True + for name in _COVERAGE_ARTIFACT_DIRS: + if (d / name).is_dir(): + return True + except OSError: + pass + return False + def _has_venv_markers(d: "Path") -> bool: """True only when *d* has actual virtualenv/conda structure on disk. @@ -858,6 +893,12 @@ def _is_noise_dir(part: str, parent: "Path | None" = None) -> bool: if parent is None: return False # cannot verify; keep a possibly-real code dir return _has_venv_markers(parent / part) + if part == "coverage": + # Ambiguous: a generated report dir OR a real package named coverage. + # Prune only on actual coverage-artefact evidence (#2339). + if parent is None: + return False # cannot verify; keep a possibly-real code dir + return _has_coverage_artifacts(parent / part) if part == "snapshots": # Prune only when it looks like an actual JS/Vitest snapshot dir. if parent is None: diff --git a/tests/test_detect.py b/tests/test_detect.py index 63b13a00..4a90367e 100644 --- a/tests/test_detect.py +++ b/tests/test_detect.py @@ -686,6 +686,85 @@ def test_detect_skips_coverage_dir(tmp_path): assert any("main.py" in f for f in all_files) +def test_detect_skips_coverage_dir_by_lcov_info(tmp_path): + """A coverage/ dir is still pruned on any single artefact file — an lcov.info + with no lcov-report/ subtree is enough evidence (#870, #2339).""" + cov = tmp_path / "coverage" + cov.mkdir() + (cov / "lcov.info").write_text("TN:\nSF:src/app.ts\nend_of_record\n") + (cov / "prettify.js").write_text("var PR_SHOULD_USE_CONTINUATION=true;") + (tmp_path / "main.py").write_text("def hello(): pass") + result = detect(tmp_path) + all_files = [f for files in result["files"].values() for f in files] + assert not any(f.startswith(str(cov)) for f in all_files) + assert any("main.py" in f for f in all_files) + + +def test_detect_keeps_coverage_code_namespace(tmp_path): + """#2339: a coverage/ dir holding real modules and no coverage artefacts is a + legitimate package name, not a generated report, and must NOT be pruned. + + Pruning it by name dropped an entire production package while leaving its + dependents in the graph, so queries kept returning plausible neighbours and + nothing in the report or skipped lists showed the loss.""" + pkg = tmp_path / "auditor_toolkit" / "assurance" / "coverage" + pkg.mkdir(parents=True) + (pkg / "__init__.py").write_text("from .impact import Impact\n") + (pkg / "impact.py").write_text("class Impact:\n def score(self): return 1\n") + (pkg / "inventory.py").write_text("def inventory():\n return []\n") + (tmp_path / "app.py").write_text("from auditor_toolkit.assurance import coverage\n") + result = detect(tmp_path) + all_files = [f for files in result["files"].values() for f in files] + assert any("impact.py" in f for f in all_files) + assert any("inventory.py" in f for f in all_files) + assert any(f.endswith("coverage" + os.sep + "__init__.py") for f in all_files) + + +def test_collect_files_keeps_coverage_code_namespace(tmp_path): + """#2339 as reported: collect_files returned [] for a real coverage package, + both when it is the walk target and when it is reached through the repo root. + A genuine report dir alongside it must still be skipped.""" + from graphify.extract import collect_files + + pkg = tmp_path / "auditor_toolkit" / "assurance" / "coverage" + pkg.mkdir(parents=True) + for name in ("__init__.py", "impact.py", "mapping.py"): + (pkg / name).write_text("def f(): pass\n") + + report = tmp_path / "webapp" / "coverage" + report.mkdir(parents=True) + (report / "index.html").write_text("coverage") + (report / "base.css").write_text("body{}") + (report / "prettify.js").write_text("var PR=1;") + + assert sorted(p.name for p in collect_files(pkg)) == [ + "__init__.py", "impact.py", "mapping.py", + ] + walked = {str(p.relative_to(tmp_path)) for p in collect_files(tmp_path)} + assert any(p.endswith("impact.py") for p in walked) + assert not any("webapp" in p for p in walked), ( + "a generated Istanbul report dir must still be pruned (#870)" + ) + + +def test_is_noise_dir_coverage_is_evidence_gated(tmp_path): + """The gate itself: name alone is not enough, and an unverifiable call + (no parent) keeps a possibly-real code dir — same contract as env/snapshots.""" + src = tmp_path / "coverage" + src.mkdir() + (src / "__init__.py").write_text("") + assert detect_mod._is_noise_dir("coverage", tmp_path) is False + + generated = tmp_path / "report" / "coverage" + generated.mkdir(parents=True) + (generated / "coverage-final.json").write_text("{}") + assert detect_mod._is_noise_dir("coverage", tmp_path / "report") is True + + assert detect_mod._is_noise_dir("coverage") is False + # lcov-report stays unconditional — no package is ever named that. + assert detect_mod._is_noise_dir("lcov-report") is True + + def test_detect_skips_visual_tests_dir(tmp_path): """visual-tests/ bundles and snapshots are noise — must be excluded (#869).""" vt = tmp_path / "visual-tests"