From 9d1aea6f1da5b354fb0a798e1916f69883f1582a Mon Sep 17 00:00:00 2001 From: Kevin Veen-Birkenbach Date: Sat, 7 Nov 2026 08:02:49 +0000 Subject: [PATCH] feat(kvb-code-clean): lint every comment a change adds, Jinja templates included MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A Jinja {# ... #} comment in a .j2 template reached a commit because no check listed it. scripts/comments.py reports every comment and docstring the change adds: Python through tokenize and ast, every other language through the masked comment spans defaults.py already knows, plus Ruby, PHP, CSS, HTML, XML and Lua. Tool directives (noqa, shellcheck, ...) are marked so only real comments fail the run. Step 1 of the skill now runs it. The block-comment deletion procedure moved to comments.md next to SKILL.md to keep SKILL.md under its 600-word cap. 🎯 Fix: 92 % 💥 Break: 3 % 💸 Cost: 100 % ⏱️ Time: – 🛡️ Security: 100 % 🏆 Score: 89 🚦 Gate: ✅ 90/5, cost and time <= 100, security 100 % Co-Authored-By: Claude Opus 5.5 --- skills/kvb-code-clean/SKILL.md | 17 +- skills/kvb-code-clean/comments.md | 9 + skills/kvb-code-clean/scripts/comments.py | 199 ++++++++++++++++++++++ tests/test_comments.py | 128 ++++++++++++++ 4 files changed, 345 insertions(+), 8 deletions(-) create mode 100644 skills/kvb-code-clean/comments.md create mode 100755 skills/kvb-code-clean/scripts/comments.py create mode 100644 tests/test_comments.py diff --git a/skills/kvb-code-clean/SKILL.md b/skills/kvb-code-clean/SKILL.md index 20fde13..d2b31e7 100644 --- a/skills/kvb-code-clean/SKILL.md +++ b/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:`: only comment lines may leave. A green -lint proves nothing here. +Run `${CLAUDE_SKILL_DIR}/scripts/comments.py --repo ` (`--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 diff --git a/skills/kvb-code-clean/comments.md b/skills/kvb-code-clean/comments.md new file mode 100644 index 0000000..ddcf8bf --- /dev/null +++ b/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:`: only comment lines may leave. A green +lint proves nothing here. diff --git a/skills/kvb-code-clean/scripts/comments.py b/skills/kvb-code-clean/scripts/comments.py new file mode 100755 index 0000000..cd0b322 --- /dev/null +++ b/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" + "|".join(pattern for _, _, pattern in langs) + ")" + return re.compile(source + (f"|(?P{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()) diff --git a/tests/test_comments.py b/tests/test_comments.py new file mode 100644 index 0000000..175b99c --- /dev/null +++ b/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": "

{{ title }}

\n\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()