Add concise-comments skill; wire into /spec & /process-reviews

skills/concise-comments/SKILL.md

@@ -0,0 +1,41 @@
+---
+name: concise-comments
+description: Rules for concise code comments/docstrings, plus a pass to trim verbose ones. Use when writing or reviewing code, or when asked to reduce comment verbosity / trim over-commented code / cut docstring bloat. Referenced by /spec and /process-reviews.
+---
+
+# Concise comments & docstrings
+
+Prevention: follow these when writing. Cleanup: run the pass below when invoked as `/concise-comments`.
+
+## The test
+Keep a comment only if a competent reader of THIS codebase couldn't infer it from the code and names. Otherwise delete it, or cut it to the one missing fact in one line.
+
+## For / not for
+For: the non-obvious *why* (constraint, invariant, footgun, edge-case gotcha) and a one-line *what* on public APIs.
+Not for: design narrative, alternatives-considered, defending the approach, repro steps, byte-count walkthroughs, ticket history, or restating the code. Those go in the PR description, DECISIONS.md, the issue, or a test.
+
+## Limits (defaults; exceed only with a stated reason)
+- Inline comment: 1 line. Needing 3+ lines to justify a line of code → rename things or move the rationale out.
+- Function/method docstring: 1-line summary; +≤3 lines only for genuinely subtle behaviour.
+- Module docstring: ≤8 lines. Test docstring: 1 line, or none if the name says it.
+- Section banner: ≤1 label line, no `---` paragraphs.
+
+Say it once at the definition; don't re-explain at every call site.
+
+## Delete on sight
+- **Restates the code** (`# strip prefix` above `x.split(...)`).
+- **Ticket-ID noise** (`DEV-1234:` on every line/attribute; the value is the fact).
+- **Design essay / justification debate** (paragraphs on why this is right and alternatives wrong).
+- **Worked examples & repros** (byte counts, sample output) → move to the test.
+- **Docstring echoing the name** (`def test_x: """Test x."""`).
+- **Emphasis theatre** (ALL-CAPS sentences, "astronomically", "provably", "THE … requirement").
+
+Keep, as ≤1 line: a real cross-dialect/edge gotcha, a non-obvious invariant, a "looks wrong but isn't", an ordering/side-effect warning. When unsure a *why* is inferable, keep it but shorten it.
+
+## The pass (`/concise-comments`)
+1. Scope: default to the branch diff (`git diff $(git merge-base <base> HEAD) -- '*.py'`); or the path/file the user names. Only added/modified comments unless told to sweep.
+2. Edit comments/docstrings ONLY — never code, string literals, test data, asserts, imports, markers.
+3. Verify: run affected tests + linter (green); confirm `git diff` changed no code line; measure before/after with `scripts/count_comments.py --range <mergebase> <paths>`.
+4. Report per-file and total lines before → after + ratio, and that tests/lint pass.
+
+Scale to the ask: "trim a bit" = kill the essays; "3x" / "massively" = apply every limit hard.

skills/concise-comments/scripts/count_comments.py

@@ -0,0 +1,87 @@
+#!/usr/bin/env python3
+"""Count comment + docstring lines in Python files.
+
+Comments via tokenize, docstrings via AST (module/class/function first stmt).
+
+  count_comments.py <file>...              # per-file + total counts
+  count_comments.py --range <ref> <path>...  # net added vs a git ref (working tree)
+"""
+from __future__ import annotations
+
+import ast
+import io
+import subprocess
+import sys
+import tokenize
+
+
+def counts(src: str | None) -> tuple[int, int]:
+    if src is None:
+        return (0, 0)
+    try:
+        comment = sum(
+            1 for t in tokenize.generate_tokens(io.StringIO(src).readline)
+            if t.type == tokenize.COMMENT
+        )
+    except Exception:
+        comment = 0
+    doc = 0
+    try:
+        tree = ast.parse(src)
+    except Exception:
+        return (comment, 0)
+    kinds = (ast.Module, ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef)
+    for node in ast.walk(tree):
+        if isinstance(node, kinds):
+            body = getattr(node, "body", [])
+            first = body[0] if body else None
+            if (
+                isinstance(first, ast.Expr)
+                and isinstance(getattr(first, "value", None), ast.Constant)
+                and isinstance(first.value.value, str)
+            ):
+                doc += first.end_lineno - first.lineno + 1
+    return (comment, doc)
+
+
+def git_show(ref: str, path: str) -> str | None:
+    r = subprocess.run(["git", "show", f"{ref}:{path}"], capture_output=True, text=True)
+    return r.stdout if r.returncode == 0 else None
+
+
+def read(path: str) -> str | None:
+    try:
+        return open(path, encoding="utf-8").read()
+    except FileNotFoundError:
+        return None
+
+
+def main(argv: list[str]) -> int:
+    if argv and argv[0] == "--range":
+        ref = argv[1]
+        paths = argv[2:]
+        tc = td = 0
+        for p in paths:
+            b = counts(git_show(ref, p))
+            h = counts(read(p))
+            dc, dd = h[0] - b[0], h[1] - b[1]
+            print(f"{dc + dd:+5d}  comment={dc:+4d} doc={dd:+4d}  {p}")
+            tc += dc
+            td += dd
+        print(f"{'=' * 50}\nNET ADDED total={tc + td} (comment {tc}, docstring {td})")
+        return 0
+    if not argv:
+        print(__doc__)
+        return 1
+    tc = td = 0
+    for p in argv:
+        c, d = counts(read(p))
+        print(f"{c + d:5d}  comment={c:4d} doc={d:4d}  {p}")
+        tc += c
+        td += d
+    print(f"{'=' * 50}\nTOTAL={tc + td} (comment {tc}, docstring {td})")
+    return 0
+
+
+if __name__ == "__main__":
+    raise SystemExit(main(sys.argv[1:]))

skills/process-reviews/SKILL.md

@@ -119,7 +119,7 @@ Don't dedupe across sources blindly — if multiple sources flag the same area,
 
 For each entry, classify as VALID or INVALID using a source-appropriate signal:
 
-- **CodeRabbit / Sonar / Codex** — READ the cited file at the cited line(s). Apply the user's global rules from `~/.claude/CLAUDE.md` (trust internal code, validate only at boundaries; imports at the top; etc.). Lean toward VALID when uncertain — false positives are easier to defend later than missed bugs now.
+- **CodeRabbit / Sonar / Codex** — READ the cited file at the cited line(s). Apply the user's global rules from `~/.claude/CLAUDE.md` (trust internal code, validate only at boundaries; imports at the top; etc.). Lean toward VALID when uncertain — false positives are easier to defend later than missed bugs now. Over-verbose comments/docstrings in the diff (design essays, ticket-ID-on-every-line, code-restating comments, name-echo docstrings) are a valid maintainability finding per the `concise-comments` skill — flag them even if no bot did.
 - **CI failure** — READ the failed-step log excerpt in the JSON. Classify as INVALID only when the failure is clearly unrelated to this PR's changes (network blip / runner died / well-known flaky test that the user has previously confirmed is flaky / out-of-date Action that times out before doing anything). Otherwise VALID. **A test failure that points at code this PR touched is always VALID** — don't argue your way out of it.
 
 Codex-specific validation hazards (apply on top of the shared CodeRabbit/Sonar rules):
@@ -237,6 +237,7 @@ Do **not** start writing code until the user picks.
 - NEVER call any "resolve" mutation on CodeRabbit threads (no `resolveReviewThread`, no UI-equivalent). Only `/replies`. This is a hard global rule from the user.
 - NOSONAR suppressions MUST include the rule key and a reason. Bare `// NOSONAR` is forbidden. The rule key inside the parentheses must be alphanumeric only — never include the `python:` / `javascript:` / etc. language prefix (it makes Sonar treat the suppression as malformed).
 - This skill stops at the plan. Code fixes happen only after the user picks a group.
+- When you later write fixes, keep any comments/docstrings concise per the `concise-comments` skill — don't add design essays or code-restating comments while resolving findings.
 - If validation makes you LESS than 80% confident an issue is invalid, classify it as VALID and put it in the plan. Better to over-fix than to silently dismiss a real bug.
 
 ## When NOT to use this skill

skills/spec/SKILL.md

@@ -41,6 +41,14 @@ Interview me in detail.
 Cover implementation approach, edge cases, gotchas, design choices, tradeoffs, and constraints.
 Skip obvious questions.
 Ask one at a time and build on my answers.
+
+**IMPORTANT — WHENEVER you ask me to make a choice, you MUST give, for the
+decision, explicit PROS and CONS of each option AND a clear RECOMMENDATION of
+which one you'd pick and why. This is non-negotiable. Never present options as
+a bare menu. If you use a structured/multiple-choice prompt, spell out the
+pros, the cons, and the recommendation inside it — do not rely on a one-line
+description to carry them.**
+
 When you think you have enough information, return a detailed, complete spec.
 In the spec you write, NEVER take shortcuts or make simplifications or
 extensions to the original requirements without asking me about each one first.
@@ -80,6 +88,11 @@ Implement the plan. Run the full non-integration test suite (per
 `feedback_unit_tests_only.md`) after changes and fix any failures. Do not
 declare done until every test from Step 4/5 passes.
 
+When writing code AND tests (Steps 4 and 6), follow the `concise-comments`
+skill: no design essays, ticket-ID-on-every-line, or code-restating comments;
+docstrings to a line. Rationale belongs in the spec / PR description / this
+issue — not in code — so verbose comments never get written in the first place.
+
 ## Step 7 — Ask me to commit, push, and PR
 
 Once all tests pass, stop and ask me whether to commit, push, and open a PR.