fix(detect): gate coverage/ pruning on real report markers (#2339) - #2353
fix(detect): gate coverage/ pruning on real report markers (#2339)#2353Rishet11 wants to merge 1 commit into
Conversation
…Labs#2339) "coverage" and "lcov-report" were in the unconditional _SKIP_DIRS set, and _is_noise_dir matches a bare directory name at any depth with no content check, so a real source package named coverage/ was pruned before anything read it. Move both names out of the unconditional set and prune them only on actual report evidence via a new _has_coverage_markers helper, mirroring the existing gating for env/.env/*_env (_has_venv_markers) and snapshots. As with those, a None parent cannot be verified and keeps the directory.
There was a problem hiding this comment.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Graphify reviewed this change.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Graphify review — findings
This PR changes how the directory-pruning logic in graphify/detect.py handles directories named coverage and lcov-report. Instead of always skipping them by name, it removes them from the unconditional _SKIP_DIRS set and adds a new _has_coverage_markers helper plus a branch in _is_noise_dir so these dirs are only pruned when actual coverage/report evidence is present on disk (e.g. lcov.info, coverage-final.json, an index.html with Istanbul assets, *.gcov, or a nested lcov-report/). The test suite adds several new cases in tests/test_detect.py covering both the "real source package named coverage/ is kept" and "genuine report dir is still pruned" scenarios, including nested paths and the no-parent case, and imports _is_noise_dir to test it directly. The surface area is limited to the noise-directory detection path and its associated tests.
No blocking issues surfaced. 2 lower-confidence candidates did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 1180 functions depend on the 433 functions this change touches.
Health — this change adds coupling hotspots:
- worse:
detect()— 85 callers, 11 callees - worse:
_is_noise_dir()— 8 callers, 2 callees
Verification — 1180 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 626 function(s) in the blast radius were not formally verified this run
· 1 grounded finding(s) anchored inline below; 1 more finding(s) on lines outside this diff (see the check run).
| return False | ||
|
|
||
|
|
||
| def _is_noise_dir(part: str, parent: "Path | None" = None) -> bool: |
There was a problem hiding this comment.
_is_noise_dir()
8 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
|
Correction to my own PR body, before you hit it while reviewing. I wrote that the only
That claim came from a review pass I ran, and I repeated it without grepping it myself. My mistake. What it means in practice, having looked at each:
Worth noting this is not new behavior I am introducing so much as the existing shape of the gate: The |
Closes #2339
The bug
"coverage"and"lcov-report"were in the unconditional_SKIP_DIRSset (graphify/detect.py:798), and_is_noise_dir(part, parent)matches a bare directory name at any depth with no content check. A real source package namedcoverage/was therefore pruned before anything read it, with no warning.The fix
No new mechanism — this follows the precedent already in the same function.
env/.env/*_envandsnapshotswere previously moved out of the unconditional set and are pruned only on real evidence (_has_venv_markers, and the*.snap/_JS_SNAPSHOT_TEST_ROOTSgate) for exactly this bug class (#1666 / #2058).coverage/lcov-reportmove out of_SKIP_DIRS._has_coverage_markers(d), styled after_has_venv_markers: prunes onlcov.info,coverage-final.json,clover.xml,coverage.xml,cobertura.xml, anindex.htmlalongside Istanbul/nyc static assets (base.css/prettify.js/block-navigation.js/sorter.js), any*.gcov, or a nestedlcov-report/.Noneparent cannot be verified, so the directory is kept.__snapshots__andstorybook-staticstay unconditional — those names are unambiguous.Scope decision, stated rather than hidden
visual-tests/visual-test(detect.py:799) anddist-protected(:802) are pruned the same unconditional way and are plausible real directory names, so they are the same bug class. I looked for evidence as crisp as the coverage markers and did not find any — Playwright/visual-regression output has no equivalent oflcov.infoorindex.html+base.css. Rather than guess at a marker set, I left both alone. Happy to follow up if you know the right signal.Verification
tests/test_detect.py: realcoverage/package kept, genuine lcov/Istanbul report still pruned, both again for acoverage/nested deeper in the tree, and_is_noise_dir("coverage", None) is False.detect.py, 3 of the 6 fail (the 3 "still pruned" ones pass either way by design).tests/test_detect.py: 232 passed.test_detect_skips_coverage_dirandtest_detect_skips_visual_tests_dirstay green for the right reason — they plant report-like files, so the evidence gate still prunes them.v8tree in this environment (1 intest_llm_backends.pyfrom a missingopenaimodule, 7 env-dependent ollama backend-detection tests). Failure set unchanged — I verified that by stashing and re-running, not by assuming.ruffclean.python -m tools.skillgen --check→check OK: 134 artifact(s) match committed output and expected/.Reviewed by a fresh-eyes pass that traced
_is_noise_dirend to end, confirmed the new gate is actually reached (no earlier_SKIP_DIRSshort-circuit), confirmed the only_is_noise_dircaller (detect.py:1334, theos.walkprune loop) always passes a real parent, and confirmed_has_coverage_markersruns once per directory rather than once per file.