From 961e24537c0fc639900e06a021ebea87cd011990 Mon Sep 17 00:00:00 2001 From: "F.N. Claessen" Date: Tue, 4 Aug 2026 00:15:15 +0200 Subject: [PATCH] Add an advisory agent hook for mid-phrase line breaks The docstring convention -- break lines only after punctuation -- has been missed in three PRs in a row (#2364, #2218, #2385), each time caught by a reviewer rather than by the author. #2384 documented the rule harder, and the very next PR broke it, so documentation is evidently not the missing piece. Adds .claude/hooks/reflow_check.py and wires it as a PreToolUse hook on `git commit`, next to the existing pre-commit and worktree-guard hooks. Two deliberate choices: It is an agent hook, not a pre-commit hook. The misses have been an agent's, and contributors should not pay a false-positive tax for that. It is also the only form that works: the check is a heuristic, and a heuristic gate that a human hits on embedded OpenAPI YAML gets disabled, whereas an agent can read "line 2028 may break mid-phrase", look, and judge. It is advisory and never blocks. It exits 0 always, and only inspects lines being added, so a legacy file is not a wall -- the repo has ~4350 candidate hits, and reflowing unrelated prose is not a thing a commit should do. Suppressing non-prose (bullets, RST directives, doctests, embedded YAML and JSON, shell continuations) cuts the noise substantially where docstrings carry API specs: sensors.py 494 hits to 59, assets.py 385 to 15. Files that are genuinely prose-heavy stay high (storage.py 199 to 173), which is the honest answer rather than a tuned one. Self-tested both directions: a docstring broken mid-phrase is reported, a correctly reflowed one is silent. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01X2jLjsQzwztDc7Nxz1ozmQ Signed-off-by: F.N. Claessen --- .claude/hooks/reflow-check.sh | 9 +++ .claude/hooks/reflow_check.py | 135 ++++++++++++++++++++++++++++++++++ .claude/settings.json | 8 +- 3 files changed, 151 insertions(+), 1 deletion(-) create mode 100755 .claude/hooks/reflow-check.sh create mode 100644 .claude/hooks/reflow_check.py diff --git a/.claude/hooks/reflow-check.sh b/.claude/hooks/reflow-check.sh new file mode 100755 index 0000000000..3530ec0fe1 --- /dev/null +++ b/.claude/hooks/reflow-check.sh @@ -0,0 +1,9 @@ +#!/usr/bin/env bash +# Advisory check, run before an agent commits: flag docstring and comment lines +# that break mid-phrase, looking only at the lines being added. +# Never blocks -- the check is a heuristic, and the agent judges each hit. +set -uo pipefail +cd "${CLAUDE_PROJECT_DIR:-.}" || exit 0 +command -v python3 >/dev/null 2>&1 || exit 0 +python3 .claude/hooks/reflow_check.py --staged 2>/dev/null || true +exit 0 diff --git a/.claude/hooks/reflow_check.py b/.claude/hooks/reflow_check.py new file mode 100644 index 0000000000..072f9d0188 --- /dev/null +++ b/.claude/hooks/reflow_check.py @@ -0,0 +1,135 @@ +"""Flag docstring and comment lines that break in the middle of a phrase. + +The repo convention (see .github/instructions/docstrings.instructions.md) is that +each physical line of a docstring or comment ends at punctuation, +so that review comments and text search stay stable. +The mechanical form of that rule: +inside a multi-line docstring or comment block, every line but the last ends in punctuation. + +This is a heuristic, and deliberately an advisory one -- +docstrings here also hold OpenAPI YAML, shell commands and bullet lists, +none of which are prose and none of which end in punctuation. +The agent reading the output is expected to judge each hit rather than obey it. + +Usage: + python reflow_check.py ... # whole files + python reflow_check.py --staged # only lines added in the git index +""" + +from __future__ import annotations + +import ast +import re +import subprocess +import sys +from pathlib import Path + +PUNCTUATION = (".", ",", ";", ":", "!", "?", ")", "]", "}", "-", "—", "–") + +#: Lines that are not prose, and so are not expected to end in punctuation. +NOT_PROSE = re.compile( + r"""^( + \s*[-*+]\s # bullet list item + | \s*\.\.\s # RST directive + | \s*>>> # doctest + | \s*\$ # shell prompt + | \s*\w[\w\s-]*:\s*\S # "key: value", i.e. embedded YAML + | \s*[\[{] # start of an embedded JSON/dict literal + )""", + re.VERBOSE, +) + + +def _offending_lines(block: list[str], first_line: int) -> list[tuple[int, str]]: + """Lines of one block that end mid-phrase, ignoring the last line of the block.""" + found = [] + body = [(i, ln) for i, ln in enumerate(block) if ln.strip()] + for i, line in body[:-1]: + text = re.sub(r"^\s*#:?\s?", "", line).strip().strip('"').strip("'").strip() + if not text or text.endswith("\\") or NOT_PROSE.match(line): + continue + if not text.endswith(PUNCTUATION): + found.append((first_line + i, line.strip()[:110])) + return found + + +def check(path: Path) -> list[tuple[int, str]]: + """Every mid-phrase line break in one Python file.""" + try: + source = path.read_text() + tree = ast.parse(source) + except (SyntaxError, UnicodeDecodeError, OSError): + return [] + + found: list[tuple[int, str]] = [] + for node in ast.walk(tree): + if isinstance( + node, (ast.Module, ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef) + ): + docstring = ast.get_docstring(node, clean=False) + if docstring and "\n" in docstring: + first = node.body[0].lineno if node.body else 1 + found += _offending_lines(docstring.split("\n"), first) + + block: list[str] = [] + start = 0 + for number, line in enumerate(source.split("\n"), start=1): + if line.strip().startswith("#"): + start = start or number + block.append(line) + continue + if len(block) > 1: + found += _offending_lines(block, start) + block, start = [], 0 + if len(block) > 1: + found += _offending_lines(block, start) + return found + + +def staged_lines() -> dict[str, set[int]]: + """Line numbers added to each staged Python file.""" + diff = subprocess.run( + ["git", "diff", "--cached", "-U0", "--", "*.py"], + capture_output=True, + text=True, + ).stdout + added: dict[str, set[int]] = {} + current = None + for line in diff.split("\n"): + if line.startswith("+++ b/"): + current = line[6:] + added.setdefault(current, set()) + header = re.match(r"^@@ -\S+ \+(\d+)(?:,(\d+))?", line) + if header and current: + first = int(header.group(1)) + added[current].update(range(first, first + int(header.group(2) or 1))) + return added + + +def main() -> int: + if "--staged" in sys.argv: + targets = staged_lines() + else: + targets = {argument: None for argument in sys.argv[1:]} + + hits = [] + for name, wanted in targets.items(): + path = Path(name) + if not path.exists(): + continue + for number, text in sorted(set(check(path))): + if wanted is None or number in wanted: + hits.append(f"{name}:{number}: {text}") + + if hits: + print("Possible mid-phrase line breaks in docstrings or comments:") + print("\n".join(f" {hit}" for hit in hits)) + print( + "\nEach line above should end at punctuation, or be reflowed so it does." + "\nIgnore any that are not prose (embedded YAML, shell commands, list items)." + ) + return 0 # advisory: never blocks the commit + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/.claude/settings.json b/.claude/settings.json index 5e8fc5e6ae..1a42941c25 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -34,6 +34,12 @@ { "matcher": "Bash", "hooks": [ + { + "type": "command", + "command": "bash \"$CLAUDE_PROJECT_DIR/.claude/hooks/reflow-check.sh\"", + "if": "Bash(git commit:*)", + "timeout": 30 + }, { "type": "command", "command": "bash \"$CLAUDE_PROJECT_DIR/.claude/hooks/pre-commit-check.sh\"", @@ -56,4 +62,4 @@ } ] } -} \ No newline at end of file +}