ci: fail comment blocks over two lines that a PR adds or edits

.github/copilot-instructions.md

@@ -5,3 +5,5 @@ SPDX-License-Identifier: Apache-2.0
 
 When reviewing a pull request, follow `REVIEW.md` at the repository root.
 When writing code, follow `AGENTS.md`.
+Keep code comments to at most two lines that state the constraint or the reason;
+link an issue or PR for the backstory. CI fails longer comment blocks.

.github/workflows/ci.yml

@@ -198,6 +198,12 @@ jobs:
           skip=()
           if [ "$SKIP" = true ]; then skip=(--skip-label); fi
           python scripts/check_changelog_fragment.py --base "origin/$BASE_REF" --head HEAD "${skip[@]}"
+      # Fails a comment block over two lines that the PR adds or edits; AGENTS.md has the rule.
+      - name: Comment blocks
+        if: ${{ !cancelled() && github.event_name == 'pull_request' }}
+        env:
+          BASE_REF: ${{ github.event.pull_request.base.ref }}
+        run: python scripts/check_comment_blocks.py --base "origin/$BASE_REF" --head HEAD
       # Lists the docs pages whose `depends_on` front matter matches a file this PR
       # changes while the PR leaves the page alone, in the job summary and as a
       # warning on each page (scripts/check_docs_review_triggers.py). A prompt to

AGENTS.md

@@ -41,6 +41,10 @@ also holds a PR to:
   (`tests/test_docs_code_refs.py`): rename in code, rename in docs.
 - Tests run offline: a connection off loopback fails the test, so mock LLM and
   network calls.
+- Comments of at most two lines (`scripts/check_comment_blocks.py`): say the
+  constraint or the reason, and end with an issue or PR URL when a reader needs
+  the backstory. A block of three or more `#` lines fails once the PR adds or
+  edits a line in it, so shorten an old block when you touch it.
 - User outcomes (`tests/outcomes/*.json`, checked by `pytest --outcomes`): a
   test marked `@pytest.mark.outcome(...)` is evidence for a promise in the docs;
   weaken or drop it only together with the doc and the contract. The same

REVIEW.md

@@ -18,8 +18,8 @@ checks.
 - A command gives a wrong number, verdict or exit code, or crashes where it
   should print a message.
 - A test passes when the behaviour it names is broken.
-- A doc, docstring, PR title or changelog fragment says something the code
-  does not do.
+- A doc, docstring, comment, PR title or changelog fragment says something the
+  code does not do.
 - A stored record is written or read with keys or a shape other than the
   schema's.
 - Behaviour differs on Windows or with a `None`, empty or missing input.
@@ -39,6 +39,9 @@ checks.
 - A nit is one the author can take: no style point the configured ruff rules
   leave out (line length, quotes, import order), and no suggestion that fails
   CI, such as one that adds a line to a file at its growth baseline.
+- CI fails a comment block over two lines. A shorter comment that tells the
+  backstory (run IDs, measurements, alternatives tried) instead of the reason
+  is a nit: suggest the short form with an issue or PR link.
 - A bug in code the PR did not change: mention it once as pre-existing. It
   does not block the PR unless the change makes it worse.
 - Do not repeat a finding already raised on the PR, unless its fix did not

dokimi/CONTRIBUTING.md

@@ -134,6 +134,11 @@ CI additionally holds a PR to:
   `changelog.d/` (`scriv create`) instead of editing `CHANGELOG.md`; see
   [changelog.d/README.md](changelog.d/README.md). A change with no user-visible
   effect gets the `skip-changelog` label instead; adding or removing the label re-runs the check.
+- **Comment blocks of at most two lines** — `python scripts/check_comment_blocks.py
+  --base <remote>/main --head HEAD`. A run of three or more `#` comment lines in a
+  Python, shell, YAML or TOML file, a Makefile or a Dockerfile fails once the PR adds
+  or edits a line in it, so shorten an old block when you edit it. Keep the reason,
+  and link an issue or PR for the backstory.
 
 **pre-commit.** `.pre-commit-config.yaml` at the repo root runs the fast
 offline checks on the files a commit stages: `ruff check` (the version and

dokimi/scripts/check_comment_blocks.py

@@ -0,0 +1,167 @@
+#!/usr/bin/env python
+# Copyright (C) 2026 Intel Corporation
+# SPDX-License-Identifier: Apache-2.0
+
+"""Fail a PR that adds or edits a line of a comment block longer than two lines.
+
+A comment states the constraint or the reason in at most two lines; the
+backstory goes in the PR or an issue, linked from the comment (``AGENTS.md``).
+A block is a run of consecutive full-line ``#`` comments in a Python, shell,
+YAML or TOML file, a Makefile or a Dockerfile. Python comments come from the
+tokenizer, so a ``#`` line inside a string is not one. A block of three or more
+lines fails when the PR adds or changes any of its lines, so an existing long
+block passes until a PR edits it. Not counted, and ending a block: a shebang on
+line 1, the SPDX header lines, and tool pragmas (``# noqa``, ``# type:``,
+``# pragma:``, ``# fmt:`` and the like). A ruler line such as ``# -----`` stays in
+its block but does not count toward its length, so a banner is one line. Files
+under ``EXCLUDE`` are third-party copies.
+
+    python scripts/check_comment_blocks.py --base origin/main --head HEAD
+
+Exits 1 after an ``::error file=...`` line per block, 2 when git cannot diff
+``--base``/``--head``.
+"""
+
+from __future__ import annotations
+
+import argparse
+import io
+import re
+import subprocess
+import sys
+import tokenize
+from pathlib import Path
+
+REPO = Path(__file__).resolve().parents[2]
+MAX_LINES = 2
+SUFFIXES = {".py", ".sh", ".yml", ".yaml", ".toml"}
+NAMES = {"Makefile", "Dockerfile"}
+# Copied unchanged from other projects, as ruff's extend-exclude says.
+EXCLUDE = ("dokimi/tests/fixtures/task_formats/",)
+_LICENSE = re.compile(r"#\s*(Copyright \(C\)|SPDX-License-Identifier:)")
+_PRAGMA = re.compile(
+    r"#\s*(noqa\b|type:|pragma:|fmt:|ruff:|mypy:|pylint:|nosec\b|isort:|shellcheck\s|yamllint\s|-\*-)")
+_RULER = re.compile(r"#+\s*([-=*#~_+])\1{2,}\s*$")
+_HUNK = re.compile(r"^@@ -\d+(?:,\d+)? \+(\d+)(?:,(\d+))? @@", re.MULTILINE)
+
+
+def _git(*args: str) -> subprocess.CompletedProcess[str]:
+    return subprocess.run(["git", *args], cwd=REPO, capture_output=True, text=True,
+                          encoding="utf-8", errors="replace", check=False)
+
+
+def checked(path: str) -> bool:
+    """Whether the check reads ``path`` (repository-relative, ``/``-separated)."""
+    name = path.rsplit("/", 1)[-1]
+    return (Path(name).suffix in SUFFIXES or name in NAMES) and not path.startswith(EXCLUDE)
+
+
+def _python_comments(text: str) -> set[int] | None:
+    """Lines whose first token is a comment; None when ``text`` does not tokenize."""
+    found = set()
+    try:
+        for tok in tokenize.generate_tokens(io.StringIO(text).readline):
+            if tok.type == tokenize.COMMENT and not tok.line[:tok.start[1]].strip():
+                found.add(tok.start[0])
+    except (tokenize.TokenError, SyntaxError):
+        return None
+    return found
+
+
+def _exempt(number: int, stripped: str) -> bool:
+    return ((number == 1 and stripped.startswith("#!"))
+            or bool(_LICENSE.match(stripped) or _PRAGMA.match(stripped)))
+
+
+def comment_lines(text: str, python: bool) -> set[int]:
+    """1-based numbers of the full-line comments in ``text`` that count toward a block."""
+    # Not splitlines(): it also splits on form feeds, so line numbers would drift from git's.
+    lines = text.split("\n")
+    found = _python_comments(text) if python else None
+    if found is None:
+        found = {i for i, line in enumerate(lines, 1) if line.lstrip().startswith("#")}
+    return {i for i in found if not _exempt(i, lines[i - 1].strip())}
+
+
+def blocks(numbers: set[int]) -> list[tuple[int, int]]:
+    """(first, last) line of each run of consecutive numbers."""
+    runs: list[tuple[int, int]] = []
+    for n in sorted(numbers):
+        if runs and n == runs[-1][1] + 1:
+            runs[-1] = (runs[-1][0], n)
+        else:
+            runs.append((n, n))
+    return runs
+
+
+def long_blocks(text: str, path: str, changed: set[int]) -> list[tuple[int, int, int]]:
+    """(first, last, length) of the blocks over ``MAX_LINES`` that hold a line of ``changed``."""
+    lines, found = text.split("\n"), []
+    for first, last in blocks(comment_lines(text, path.endswith(".py"))):
+        size = sum(not _RULER.match(lines[n - 1].strip()) for n in range(first, last + 1))
+        if size > MAX_LINES and any(first <= n <= last for n in changed):
+            found.append((first, last, size))
+    return found
+
+
+def changed_paths(base: str, head: str) -> list[tuple[str, ...]]:
+    """(path at head, path at base if renamed) of each file ``base...head`` adds or changes."""
+    result = _git("diff", "-z", "-M", "--name-status", "--diff-filter=AMR", f"{base}...{head}")
+    result.check_returncode()
+    fields, paths, i = result.stdout.split("\0")[:-1], [], 0
+    while i < len(fields):
+        if fields[i].startswith("R"):
+            paths.append((fields[i + 2], fields[i + 1]))
+            i += 3
+        else:
+            paths.append((fields[i + 1],))
+            i += 2
+    return paths
+
+
+def added_lines(base: str, head: str, paths: tuple[str, ...]) -> set[int]:
+    """Line numbers at ``head`` that ``base...head`` adds or changes in ``paths[0]``."""
+    result = _git("diff", "-U0", "-M", f"{base}...{head}", "--", *paths)
+    result.check_returncode()
+    lines: set[int] = set()
+    for m in _HUNK.finditer(result.stdout):
+        start = int(m.group(1))
+        lines.update(range(start, start + int(m.group(2) or 1)))
+    return lines
+
+
+def findings(base: str, head: str) -> list[tuple[str, int, int, int]]:
+    """(path, first, last, length) of every long block the PR adds to or edits."""
+    found = []
+    for paths in changed_paths(base, head):
+        if not checked(paths[0]):
+            continue
+        shown = _git("show", f"{head}:{paths[0]}")
+        shown.check_returncode()
+        changed = added_lines(base, head, paths)
+        found += [(paths[0], *block) for block in long_blocks(shown.stdout, paths[0], changed)]
+    return found
+
+
+def main(argv: list[str] | None = None) -> int:
+    parser = argparse.ArgumentParser(description=__doc__.splitlines()[0])
+    parser.add_argument("--base", default="origin/main")
+    parser.add_argument("--head", default="HEAD")
+    args = parser.parse_args(argv)
+    try:
+        found = findings(args.base, args.head)
+    except subprocess.CalledProcessError as exc:
+        print(f"cannot diff {args.base}...{args.head}: {exc.stderr.strip()}", file=sys.stderr)
+        return 2
+    for path, first, last, size in found:
+        print(f"::error file={path},line={first},endLine={last},title=Comment block::"
+              f"{size} comment lines; keep it to {MAX_LINES} that say the reason, "
+              "and link an issue or PR for the rest (AGENTS.md)")
+    if found:
+        return 1
+    print(f"No comment block over {MAX_LINES} lines among the lines this PR adds or changes.")
+    return 0
+
+
+if __name__ == "__main__":
+    sys.exit(main())

dokimi/tests/test_check_comment_blocks.py

@@ -0,0 +1,131 @@
+# Copyright (C) 2026 Intel Corporation
+# SPDX-License-Identifier: Apache-2.0
+
+"""Tests for scripts/check_comment_blocks.py — the comment block length gate."""
+
+from __future__ import annotations
+
+import importlib.util
+import subprocess
+from pathlib import Path
+
+import pytest
+
+_SCRIPT = Path(__file__).resolve().parents[1] / "scripts" / "check_comment_blocks.py"
+_spec = importlib.util.spec_from_file_location("check_comment_blocks", _SCRIPT)
+assert _spec and _spec.loader
+ccb = importlib.util.module_from_spec(_spec)
+_spec.loader.exec_module(ccb)
+
+LONG = "# one\n# two\n# three\nx = 1\n"
+
+
+@pytest.mark.parametrize(("path", "read"), [
+    ("dokimi/dokimi/cli.py", True),
+    ("dokimi/Makefile", True),
+    (".agent-eval/Dockerfile", True),
+    (".github/workflows/ci.yml", True),
+    (".pre-commit-config.yaml", True),
+    ("dokimi/pyproject.toml", True),
+    ("dokimi/docs/README.md", False),        # `#` is a heading there
+    ("dokimi/requirements.lock.txt", False),  # generated
+    ("dokimi/tests/fixtures/task_formats/x/solve.sh", False),  # third-party copy
+])
+def test_which_files_are_read(path, read):
+    assert ccb.checked(path) is read
+
+
+def test_a_hash_inside_a_python_string_is_not_a_comment():
+    text = 'x = """\n# a\n# b\n# c\n"""\n'
+    assert ccb.comment_lines(text, python=True) == set()
+
+
+def test_a_trailing_comment_does_not_join_a_block():
+    text = "# a\n# b\nx = 1  # c\n"
+    assert ccb.blocks(ccb.comment_lines(text, python=True)) == [(1, 2)]
+
+
+def test_shebang_license_and_pragmas_do_not_count():
+    text = ("#!/usr/bin/env python\n# Copyright (C) 2026 Intel Corporation\n"
+            "# SPDX-License-Identifier: Apache-2.0\n# a\n# b\n# fmt: off\n# c\n")
+    assert ccb.blocks(ccb.comment_lines(text, python=True)) == [(4, 5), (7, 7)]
+
+
+def test_python_that_does_not_tokenize_falls_back_to_lines():
+    text = "# a\n# b\n# c\nx = '\n"
+    assert ccb.comment_lines(text, python=True) == {1, 2, 3}
+
+
+def test_yaml_comments_are_read_by_line_including_in_run_scripts():
+    text = "steps:\n  - run: |\n      # a\n      # b\n      # c\n      make\n"
+    assert ccb.blocks(ccb.comment_lines(text, python=False)) == [(3, 5)]
+
+
+def test_a_form_feed_keeps_line_numbers_in_step_with_git():
+    text = "a: 1\f\n# x\n# y\n# z\n"
+    assert ccb.long_blocks(text, "f.yml", {2}) == [(2, 4, 3)]
+
+
+def test_a_banner_is_one_line_but_rulers_do_not_hide_a_long_block():
+    banner = "# ------\n# Section\n# ------\nx = 1\n"
+    assert ccb.long_blocks(banner, "f.py", {2}) == []
+    ruled = "# ======\n# a\n# b\n# c\n# ======\n"
+    assert ccb.long_blocks(ruled, "f.py", {3}) == [(1, 5, 3)]
+
+
+@pytest.mark.parametrize(("changed", "flagged"), [
+    ({2}, [(1, 3, 3)]),   # an edited line inside the block
+    ({4}, []),            # only the code below it changed
+    (set(), []),          # an existing block nobody touched
+])
+def test_a_long_block_fails_only_when_the_pr_touches_it(changed, flagged):
+    assert ccb.long_blocks(LONG, "f.py", changed) == flagged
+
+
+def test_two_lines_pass_even_when_both_are_new():
+    assert ccb.long_blocks("# one\n# two\nx = 1\n", "f.py", {1, 2}) == []
+
+
+def _git_repo(path: Path) -> None:
+    for cmd in (["init", "-q"], ["config", "user.email", "t@example.com"],
+                ["config", "user.name", "t"], ["config", "commit.gpgsign", "false"]):
+        subprocess.run(["git", *cmd], cwd=path, check=True)
+
+
+def _commit(path: Path, msg: str, tag: str | None = None) -> None:
+    subprocess.run(["git", "add", "-A"], cwd=path, check=True)
+    subprocess.run(["git", "commit", "-q", "-m", msg], cwd=path, check=True)
+    if tag:
+        subprocess.run(["git", "tag", tag], cwd=path, check=True)
+
+
+def test_main_flags_a_new_long_block_with_its_lines(tmp_path, monkeypatch, capsys):
+    _git_repo(tmp_path)
+    (tmp_path / "a.py").write_text("x = 1\n", encoding="utf-8")
+    _commit(tmp_path, "base", tag="base")
+    (tmp_path / "a.py").write_text("x = 1\n" + LONG, encoding="utf-8")
+    _commit(tmp_path, "pr")
+    monkeypatch.setattr(ccb, "REPO", tmp_path)
+    assert ccb.main(["--base", "base", "--head", "HEAD"]) == 1
+    assert "::error file=a.py,line=2,endLine=4," in capsys.readouterr().out
+
+
+def test_main_passes_an_untouched_long_block_and_a_renamed_file(tmp_path, monkeypatch):
+    _git_repo(tmp_path)
+    (tmp_path / "old.py").write_text(LONG + "y = 2\n" * 10, encoding="utf-8")
+    (tmp_path / "b.yml").write_text(LONG, encoding="utf-8")
+    _commit(tmp_path, "base", tag="base")
+    (tmp_path / "old.py").rename(tmp_path / "new.py")
+    (tmp_path / "b.yml").write_text(LONG + "z: 3\n", encoding="utf-8")
+    _commit(tmp_path, "move and append")
+    monkeypatch.setattr(ccb, "REPO", tmp_path)
+    assert ccb.findings("base", "HEAD") == []
+
+
+def test_main_exits_2_for_a_base_git_cannot_diff(tmp_path, monkeypatch, capsys):
+    _git_repo(tmp_path)
+    (tmp_path / "f.py").write_text("x = 1\n", encoding="utf-8")
+    _commit(tmp_path, "base")
+    monkeypatch.setattr(ccb, "REPO", tmp_path)
+    assert ccb.main(["--base", "no-such-ref", "--head", "HEAD"]) == 2
+    assert "no-such-ref" in capsys.readouterr().err