Merge feature/kvb-code-clean/comments into unify/skills-20261112

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

skills/kvb-code-clean/SKILL.md

@@ -28,15 +28,16 @@ Delete every comment the change adds except three kinds:
 Restated code, step narration, banners, "Note that", rationale prose and
 whatever the code, a name or the commit message already carries go. When in
 doubt, delete. Docstrings and script headers count: prose outside the
-parameter doc goes too, and a `#` or `//` grep misses it.
+parameter doc goes too.
 
-Block-comment languages are not line-oriented: in Jinja (`{# … #}`), C-like
-(`/* … */`), HTML (`<!-- … -->`) and kin, a closer alone on its line looks
-like a comment line; deleted with the rest, the opener swallows every live
-statement down to the next closer. Delete such a block by hand, count the
-delimiters (`grep -c '{#'` against `grep -c '#}'`), re-parse the file and
-diff it against `git show HEAD:<path>`: only comment lines may leave. A green
-lint proves nothing here.
+Run `${CLAUDE_SKILL_DIR}/scripts/comments.py --repo <dir>` (`--staged`,
+`--file` as in Step 2): it lists every comment and docstring the change adds,
+Jinja `{# … #}` in `.j2` templates included; exit 1 means hits. Judge each
+hit not marked `directive` by the three kinds above, and run it again until
+only kept hits remain. A repository's own comment lint runs too, because a
+`--no-verify` commit skips the hook that would run it. Before deleting a
+block comment (`{# … #}`, `/* … */`, `<!-- … -->`), read `comments.md` next
+to this file.
 
 ## Step 2: defaults
 

skills/kvb-code-clean/comments.md

@@ -0,0 +1,9 @@
+# Deleting block comments
+
+Block-comment languages are not line-oriented: in Jinja (`{# … #}`), C-like
+(`/* … */`), HTML (`<!-- … -->`) and kin, a closer alone on its line looks
+like a comment line; deleted with the rest, the opener swallows every live
+statement down to the next closer. Delete such a block by hand, count the
+delimiters (`grep -c '{#'` against `grep -c '#}'`), re-parse the file and
+diff it against `git show HEAD:<path>`: only comment lines may leave. A green
+lint proves nothing here.

skills/kvb-code-clean/scripts/comments.py

@@ -0,0 +1,199 @@
+#!/usr/bin/env python3
+"""Report every comment and docstring a change adds, for the agent to delete or keep.
+
+Args:
+    --repo: a directory inside the git work tree to scan.
+    --staged: scan only the staged changes (git diff --cached) against the
+        index version of each file. Without it: every change against HEAD,
+        staged and unstaged, against the working tree file.
+    --file: scan this whole file, every line counted as added; repeatable,
+        relative to --repo or absolute. Untracked files are in no diff.
+    --json: print the hits as a JSON list instead of tab-separated lines.
+
+Output:
+    One line per hit: file:line, language, kind (comment or docstring), the
+    text, and `directive` for a tool directive (noqa, nocheck, shellcheck, ...).
+
+Exit codes:
+    0 no hits but directives, 1 hits, 2 usage error, git failure, or a file
+    that cannot be read or, for Python, parsed.
+"""
+
+from __future__ import annotations
+
+import argparse
+import ast
+import bisect
+import io
+import json
+import re
+import sys
+import tokenize
+from dataclasses import asdict, dataclass
+from pathlib import Path, PurePosixPath
+
+import defaults
+
+EXTRA = {
+    ".py": ("python", defaults.DQ + "|" + defaults.SQ, defaults.HASH),
+    ".rb": ("ruby", defaults.DQ + "|" + defaults.SQ, defaults.HASH),
+    ".php": ("php", defaults.DQ + "|" + defaults.SQ, defaults.HASH + "|" + defaults.SLASH),
+    ".css": ("css", defaults.DQ + "|" + defaults.SQ, r"/\*.*?\*/"),
+    ".scss": ("css", defaults.DQ + "|" + defaults.SQ, defaults.SLASH),
+    ".html": ("html", "", r"<!--.*?-->"),
+    ".xml": ("html", "", r"<!--.*?-->"),
+    ".lua": ("lua", defaults.DQ + "|" + defaults.SQ, r"--\[\[.*?\]\]|--[^\n]*"),
+}
+DIRECTIVE = re.compile(
+    r"\b(?:noqa|nocheck|nosec|type:\s*ignore|pragma|pylint|mypy|ruff|pyright|isort|fmt:\s*(?:on|off)"
+    r"|coding[:=]|shellcheck|yamllint|hadolint|ansible-lint|markdownlint|eslint|stylelint|prettier"
+    r"|rubocop|frozen_string_literal|phpcs|phpstan|psalm|jscpd|checkov|tflint)\b",
+    re.IGNORECASE,
+)
+
+
+@dataclass(frozen=True)
+class Hit:
+    file: str
+    line: int
+    language: str
+    kind: str
+    text: str
+    directive: bool
+
+
+def patterns(path: str) -> list[tuple[str, str, str]]:
+    found = [(lang, *defaults.LANGS[lang][:2]) for lang in defaults.languages(path) if lang != "python"]
+    stem = defaults.JINJA_SUFFIX.sub("", PurePosixPath(path).name)
+    extra = EXTRA.get(PurePosixPath(stem).suffix)
+    return found + ([extra] if extra else [])
+
+
+def python_lang(path: str) -> bool:
+    return defaults.languages(path) == ["python"]
+
+
+def known(path: str) -> bool:
+    return python_lang(path) or bool(patterns(path))
+
+
+def touched(lines: set[int], wanted: set[int] | None) -> bool:
+    return wanted is None or bool(lines & wanted)
+
+
+def python_comments(path: str, tokens: list[tokenize.TokenInfo], wanted: set[int] | None) -> list[Hit]:
+    return [
+        Hit(path, token.start[0], "python", "comment", defaults.short(token.string), bool(DIRECTIVE.search(token.string)))
+        for token in tokens
+        if token.type == tokenize.COMMENT
+        and not (token.start[0] == 1 and token.string.startswith("#!"))
+        and touched({token.start[0]}, wanted)
+    ]
+
+
+def python_docstrings(path: str, tree: ast.AST, wanted: set[int] | None) -> list[Hit]:
+    nodes = [
+        node
+        for node in ast.walk(tree)
+        if isinstance(node, ast.Expr) and isinstance(node.value, ast.Constant) and isinstance(node.value.value, str)
+    ]
+    return [
+        Hit(path, node.lineno, "python", "docstring", defaults.short(node.value.value), False)
+        for node in nodes
+        if touched(set(range(node.lineno, (node.end_lineno or node.lineno) + 1)), wanted)
+    ]
+
+
+def scan_python(path: str, text: str, wanted: set[int] | None) -> list[Hit]:
+    try:
+        tree = ast.parse(text, filename=path)
+        tokens = list(tokenize.generate_tokens(io.StringIO(text).readline))
+    except (SyntaxError, ValueError, tokenize.TokenError) as error:
+        raise defaults.ScanError(f"{path}: unparseable Python: {error}") from error
+    return python_comments(path, tokens, wanted) + python_docstrings(path, tree, wanted)
+
+
+def masked_pattern(langs: list[tuple[str, str, str]]) -> re.Pattern[str]:
+    strings = "|".join(pattern for _, pattern, _ in langs if pattern)
+    source = "(?P<comment>" + "|".join(pattern for _, _, pattern in langs) + ")"
+    return re.compile(source + (f"|(?P<string>{strings})" if strings else ""), re.MULTILINE | re.DOTALL)
+
+
+def scan_masked(path: str, text: str, langs: list[tuple[str, str, str]], wanted: set[int] | None) -> list[Hit]:
+    _, spans = defaults.mask(text, masked_pattern(langs))
+    starts = [0] + [newline.end() for newline in re.finditer("\n", text)]
+    language = "+".join(lang for lang, _, _ in langs)
+    hits = []
+    for begin, end in spans:
+        body = text[begin:end]
+        if begin == 0 and body.startswith("#!"):
+            continue
+        first = bisect.bisect_right(starts, begin)
+        lines = set(range(first, bisect.bisect_right(starts, max(begin, end - 1)) + 1))
+        if touched(lines, wanted):
+            hits.append(Hit(path, first, language, "comment", defaults.short(body), bool(DIRECTIVE.search(body))))
+    return hits
+
+
+def scan(path: str, text: str, wanted: set[int] | None) -> list[Hit]:
+    if python_lang(path):
+        return scan_python(path, text, wanted)
+    langs = patterns(path)
+    return scan_masked(path, text, langs, wanted) if langs else []
+
+
+def changed_targets(root: Path, staged: bool) -> dict[str, tuple[str, set[int] | None]]:
+    targets: dict[str, tuple[str, set[int] | None]] = {}
+    for path, lines in defaults.added_lines(root, staged).items():
+        if known(path):
+            text = defaults.git(root, "cat-file", "blob", f":{path}") if staged else defaults.read(root / path)
+            targets[path] = (text, lines)
+    return targets
+
+
+def file_targets(repo: Path, root: Path, files: list[str]) -> dict[str, tuple[str, set[int] | None]]:
+    targets: dict[str, tuple[str, set[int] | None]] = {}
+    for name in files:
+        target = (repo / name).resolve()
+        if not target.is_file():
+            raise defaults.ScanError(f"{name}: no such file")
+        shown = target.relative_to(root).as_posix() if target.is_relative_to(root) else str(target)
+        if known(shown):
+            targets[shown] = (defaults.read(target), None)
+        else:
+            print(f"comments: {shown}: no comment syntax known for this file type", file=sys.stderr)
+    return targets
+
+
+def collect(repo: Path, staged: bool, files: list[str]) -> list[Hit]:
+    if not repo.is_dir():
+        raise defaults.ScanError(f"{repo} is not a directory")
+    root = Path(defaults.git(repo, "rev-parse", "--show-toplevel").strip()).resolve()
+    targets = changed_targets(root, staged) | file_targets(repo, root, files)
+    hits = [hit for path, (text, wanted) in targets.items() for hit in scan(path, text, wanted)]
+    return sorted(hits, key=lambda hit: (hit.file, hit.line, hit.kind))
+
+
+def main() -> int:
+    parser = argparse.ArgumentParser(description=__doc__.splitlines()[0])
+    parser.add_argument("--repo", required=True)
+    parser.add_argument("--staged", action="store_true")
+    parser.add_argument("--file", action="append", default=[])
+    parser.add_argument("--json", action="store_true")
+    args = parser.parse_args()
+    try:
+        hits = collect(Path(args.repo), args.staged, args.file)
+    except defaults.ScanError as error:
+        print(f"comments: {error}", file=sys.stderr)
+        return 2
+    if args.json:
+        print(json.dumps([asdict(hit) for hit in hits], indent=1))
+    else:
+        for hit in hits:
+            row = [f"{hit.file}:{hit.line}", hit.language, hit.kind, hit.text] + (["directive"] if hit.directive else [])
+            print("\t".join(row))
+    return 1 if any(not hit.directive for hit in hits) else 0
+
+
+if __name__ == "__main__":
+    sys.exit(main())

tests/test_comments.py

@@ -0,0 +1,128 @@
+from __future__ import annotations
+
+import json
+import subprocess
+import sys
+import tempfile
+import unittest
+from pathlib import Path
+
+REPO_ROOT = Path(__file__).resolve().parents[1]
+SCRIPT = REPO_ROOT / "skills" / "kvb-code-clean" / "scripts" / "comments.py"
+BASE = {
+    "casc.yaml.j2": "# Header kept from the base.\njenkins:\n  url: x\n",
+    "app.py": "def old():\n    return 1\n",
+    "run.sh": "#!/bin/bash\necho old\n",
+}
+UNSTAGED = {
+    "casc.yaml.j2": BASE["casc.yaml.j2"]
+    + "{# Without an override the plugin requests every scope. #}\n"
+    + "  scopes: \"openid\"\n"
+    + "  # a yaml narration line\n"
+    + '  note: "{{ value }}"\n',
+    "app.py": BASE["app.py"]
+    + "\n\ndef new(a):\n"
+    + '    """Return a."""\n'
+    + "    # explain the next line\n"
+    + "    return a  # noqa: E501\n",
+    "run.sh": BASE["run.sh"]
+    + "# shellcheck disable=SC2086\n"
+    + 'echo "#not a comment" $X\n',
+}
+STAGED = {
+    "style.css": "a { color: red; }\n/* palette narration */\n",
+    "page.html.j2": "<p>{{ title }}</p>\n<!-- layout note -->\n{# jinja note #}\n",
+    "Dockerfile": "FROM debian\n# install the tools\nRUN true\n",
+}
+UNSTAGED_HITS = [
+    ("app.py:6", "python", "docstring", False),
+    ("app.py:7", "python", "comment", False),
+    ("app.py:8", "python", "comment", True),
+    ("casc.yaml.j2:4", "jinja+yaml", "comment", False),
+    ("casc.yaml.j2:6", "jinja+yaml", "comment", False),
+    ("run.sh:3", "shell", "comment", True),
+]
+STAGED_HITS = [
+    ("Dockerfile:2", "dockerfile", "comment", False),
+    ("page.html.j2:2", "jinja+html", "comment", False),
+    ("page.html.j2:3", "jinja+html", "comment", False),
+    ("style.css:2", "css", "comment", False),
+]
+
+
+class TestComments(unittest.TestCase):
+    def setUp(self):
+        tmp = tempfile.TemporaryDirectory()
+        self.addCleanup(tmp.cleanup)
+        self.repo = Path(tmp.name) / "repo"
+        self.repo.mkdir()
+        self.env = {
+            "PATH": "/usr/bin:/bin",
+            "HOME": tmp.name,
+            "GIT_CONFIG_NOSYSTEM": "1",
+            "GIT_CEILING_DIRECTORIES": tmp.name,
+            "GIT_AUTHOR_NAME": "t",
+            "GIT_AUTHOR_EMAIL": "t@example.org",
+            "GIT_COMMITTER_NAME": "t",
+            "GIT_COMMITTER_EMAIL": "t@example.org",
+        }
+        self._git("init", "-q", "-b", "main")
+        self._write(BASE)
+        self._git("add", ".")
+        self._git("commit", "-q", "-m", "base")
+
+    def _git(self, *args: str) -> str:
+        return subprocess.run(
+            ["git", *args], cwd=self.repo, env=self.env, capture_output=True, text=True, check=True
+        ).stdout
+
+    def _write(self, files: dict[str, str]) -> None:
+        for name, text in files.items():
+            (self.repo / name).write_text(text, encoding="utf-8")
+
+    def _change(self) -> None:
+        self._write(STAGED)
+        self._git("add", *STAGED)
+        self._write(UNSTAGED)
+
+    def _hits(self, *args: str) -> tuple[int, list[tuple]]:
+        result = subprocess.run(
+            [sys.executable, str(SCRIPT), "--repo", str(self.repo), "--json", *args],
+            env=self.env,
+            capture_output=True,
+            text=True,
+            check=False,
+        )
+        self.assertIn(result.returncode, (0, 1), result.stderr)
+        rows = json.loads(result.stdout)
+        return result.returncode, sorted(
+            (f"{r['file']}:{r['line']}", r["language"], r["kind"], r["directive"]) for r in rows
+        )
+
+    def test_worktree_reports_every_added_comment_and_nothing_from_the_base(self):
+        self._change()
+        code, hits = self._hits()
+        self.assertEqual(code, 1)
+        self.assertEqual(hits, sorted(UNSTAGED_HITS + STAGED_HITS))
+
+    def test_staged_reads_the_index_version(self):
+        self._change()
+        code, hits = self._hits("--staged")
+        self.assertEqual(code, 1)
+        self.assertEqual(hits, sorted(STAGED_HITS))
+
+    def test_directives_alone_exit_zero(self):
+        self._write({"run.sh": BASE["run.sh"] + "# shellcheck disable=SC2086\necho $X\n"})
+        code, hits = self._hits()
+        self.assertEqual(code, 0)
+        self.assertEqual(hits, [("run.sh:3", "shell", "comment", True)])
+
+    def test_file_scans_an_untracked_template_whole(self):
+        (self.repo / "new.yml.j2").write_text("a: 1\n{# narration #}\n", encoding="utf-8")
+        code, hits = self._hits("--file", "new.yml.j2")
+        self.assertEqual(code, 1)
+        self.assertEqual(hits, [("new.yml.j2:2", "jinja+yaml", "comment", False)])
+
+
+if __name__ == "__main__":
+    unittest.main()