From 10c33690b9eeb550edeb9c70a07dae751f1a9e9b Mon Sep 17 00:00:00 2001 From: Krzysztof kuhy Rudnicki Date: Sun, 23 Aug 2026 19:27:09 +0200 Subject: [PATCH] Add the 250-line file-length gate as a pre-commit hook Vendors ~/utils/file_length into scripts/file_length so the gate is reachable in CI: pre-commit's `entry:` is not shell-expanded and GitHub runners have no ~/utils, so an external reference would pass locally and silently skip on push. The existing pre-commit/action all-files job picks the hook up with no workflow change. The vendored copy is excluded from ruff/ruff-format/mypy/pylint and must stay byte-identical to upstream -- ruff --unsafe-fixes otherwise rewrites its print() calls to `pass` (T201/F841), which leaves the gate exiting 1 while reporting nothing. Also: - name-tests-test now exempts tests/_*.py, so the 35 test-file splits have a landing zone for shared helpers (it rejects any non-test_*.py module under tests/). - pylint max-module-lines 1000 -> 250, so both limits agree. Note: .git/hooks/pre-commit was never installed in this repo (only pre-push), so no commit-stage hook had ever run. Installed via `pre-commit install`. 55 files are currently over the cap; they are split in following commits. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01PPpmMpTSkp8UziRc5u8B7P --- .pre-commit-config.yaml | 10 ++ pyproject.toml | 7 +- scripts/check_file_length.sh | 40 ++++++ scripts/file_length/__init__.py | 5 + scripts/file_length/_tables.py | 201 ++++++++++++++++++++++++++++++ scripts/file_length/check.py | 183 +++++++++++++++++++++++++++ scripts/file_length/config.py | 45 +++++++ scripts/file_length/exemptions.py | 82 ++++++++++++ 8 files changed, 571 insertions(+), 2 deletions(-) create mode 100755 scripts/check_file_length.sh create mode 100644 scripts/file_length/__init__.py create mode 100644 scripts/file_length/_tables.py create mode 100644 scripts/file_length/check.py create mode 100644 scripts/file_length/config.py create mode 100644 scripts/file_length/exemptions.py diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 3d3e56d..81da094 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -29,6 +29,7 @@ repos: - id: debug-statements - id: name-tests-test args: [--pytest-test-first] + exclude: ^steam_backlog_enforcer/tests/_[a-z_]+\.py$ - id: check-ast - id: check-builtin-literals - id: check-docstring-first @@ -51,6 +52,11 @@ repos: entry: '(?i)#\s*ruff:\s*noqa' language: pygrep types: [python] + - id: file-length + name: Files must be at most 250 lines + entry: bash scripts/check_file_length.sh + language: system + types: [text] - id: requirements-sync name: requirements.txt covers all package imports entry: python3 scripts/check_requirements_sync.py @@ -65,9 +71,11 @@ repos: rev: v0.15.2 hooks: - id: ruff + exclude: ^scripts/file_length/ args: [--fix, --unsafe-fixes, --exit-non-zero-on-fix, --show-fixes] types_or: [python, pyi] - id: ruff-format + exclude: ^scripts/file_length/ types_or: [python, pyi] # =========================================================================== @@ -77,6 +85,7 @@ repos: rev: v1.13.0 hooks: - id: mypy + exclude: ^scripts/file_length/ args: - --ignore-missing-imports - --no-error-summary @@ -109,6 +118,7 @@ repos: rev: v3.3.2 hooks: - id: pylint + exclude: ^scripts/file_length/ args: - --rcfile=pyproject.toml - --fail-under=8.0 diff --git a/pyproject.toml b/pyproject.toml index 6558805..e047273 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -36,7 +36,10 @@ include = ["steam_backlog_enforcer*"] [tool.ruff] target-version = "py310" include = ["*.py", "**/*.py"] -exclude = [".git", ".venv", "__pycache__", "build", "dist", ".eggs"] +# scripts/file_length is a VENDORED copy of ~/utils/file_length and must stay +# byte-identical to upstream; ruff --unsafe-fixes otherwise strips its print() +# calls (T201) and silently guts the gate's reporting. +exclude = [".git", ".venv", "__pycache__", "build", "dist", ".eggs", "scripts/file_length"] [tool.ruff.lint] select = ["ALL"] @@ -121,7 +124,7 @@ disable = [] [tool.pylint.design] min-public-methods = 0 -max-module-lines = 1000 +max-module-lines = 250 max-attributes = 10 [tool.pylint.typecheck] diff --git a/scripts/check_file_length.sh b/scripts/check_file_length.sh new file mode 100755 index 0000000..20a359a --- /dev/null +++ b/scripts/check_file_length.sh @@ -0,0 +1,40 @@ +#!/bin/bash + +# ============================================================================ +# Fail if any file exceeds the shared 250-line cap. +# +# A file that cannot be read in one piece forces re-reads and partial edits, +# which is the largest avoidable cost in an LLM-assisted workflow. The cap +# applies to code AND prose; generated files, markup and data are exempt. +# +# This is a VENDORED copy of ~/utils/file_length/. It is vendored rather than +# referenced because pre-commit's `entry:` is not shell-expanded and CI +# runners have no ~/utils -- an external reference would pass locally and +# silently skip on push. +# +# Usage: +# scripts/check_file_length.sh [ ...] # pre-commit passes these +# scripts/check_file_length.sh --all # whole tree, from cwd +# ============================================================================ + +set -euo pipefail + +SCRIPTS_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +readonly SCRIPTS_ROOT +readonly CHECKER="$SCRIPTS_ROOT/file_length/check.py" + +main() { + if [[ $# -eq 0 ]]; then + echo "Usage: $(basename "$0") ... | --all" >&2 + exit 1 + fi + if [[ ! -f "$CHECKER" ]]; then + echo "Error: checker not found at $CHECKER" >&2 + exit 1 + fi + + PYTHONPATH="$SCRIPTS_ROOT${PYTHONPATH:+:$PYTHONPATH}" \ + python3 "$CHECKER" "$@" +} + +main "$@" diff --git a/scripts/file_length/__init__.py b/scripts/file_length/__init__.py new file mode 100644 index 0000000..8945ab8 --- /dev/null +++ b/scripts/file_length/__init__.py @@ -0,0 +1,5 @@ +"""Shared 250-line file-length cap: config, gate, and survey.""" + +from file_length.config import MAX_LINES + +__all__ = ["MAX_LINES"] diff --git a/scripts/file_length/_tables.py b/scripts/file_length/_tables.py new file mode 100644 index 0000000..5c1b1b7 --- /dev/null +++ b/scripts/file_length/_tables.py @@ -0,0 +1,201 @@ +"""Exemption tables for the 250-line file-length cap. + +The data half: which extensions are capped, and which paths are generated, +vendored, third-party or session artifacts. :mod:`file_length.exemptions` +holds the rules that read these, and :mod:`file_length.config` re-exports +both as the public surface. + +Duplicating these lists in shell is what lets a gate and a survey silently +disagree, so they live here once. +""" + +from __future__ import annotations + +import re + +#: Hard cap. Files above this fail the gate. Code and prose alike. +MAX_LINES = 250 + +#: Extensions the cap applies to: real programming languages plus prose. +#: Markup (.html/.css/.scss) is deliberately absent -- templates split into +#: partials, not functions, so a line cap does not express anything useful. +CAPPED_EXTENSIONS = { + # code + ".py", + ".dart", + ".ts", + ".tsx", + ".js", + ".jsx", + ".sh", + ".bash", + ".zsh", + ".c", + ".h", + ".cpp", + ".cc", + ".hpp", + ".go", + ".rs", + ".java", + ".kt", + ".kts", + ".swift", + ".rb", + ".lua", + ".sql", + ".vue", + ".cs", + ".php", + # prose + ".md", + ".markdown", + ".txt", + ".rst", + ".tex", + ".org", +} + +#: Generated files: regenerated by a tool, so splitting them is meaningless +#: (the tool would just rewrite the original next run). +GENERATED_PATTERN = re.compile( + r"\.g\.dart$" + r"|\.freezed\.dart$" + r"|\.gr\.dart$" + r"|\.config\.dart$" + r"|\.mocks\.dart$" + r"|\.min\.(js|css)$" + r"|\.pb\.(go|py)$" + r"|_pb2(_grpc)?\.py$" + r"|\.generated\.ts$" + r"|\.d\.ts$" + r"|/l10n/generated/" + r"|/generated/" +) + +#: A marker in the first few lines also marks a file generated. Covers tools +#: that do not use a distinguishing extension. +GENERATED_MARKERS = ( + "GENERATED CODE - DO NOT MODIFY", + "@generated", + "DO NOT EDIT", + "Autogenerated", + "AUTOGENERATED", + "generated by", +) + +#: Directory names never descended into: dependencies, build output, caches. +EXCLUDED_DIRS = frozenset( + { + ".git", + "node_modules", + ".dart_tool", + "build", + ".venv", + "venv", + "env", + "site-packages", + "target", + "ephemeral", + "__pycache__", + ".mypy_cache", + ".pytest_cache", + ".ruff_cache", + "dist", + ".next", + "vendor", + "third_party", + "Pods", + ".gradle", + ".idea", + ".cxx", + "coverage", + "htmlcov", + ".tox", + "Library", + "Temp", + "obj", + ".terraform", + ".fvm", + ".angular", + ".cache", + ".svelte-kit", + "migrations", + ".ci-mirror-venv", + ".hippo", + ".build-deps", + # Where CI checks out the shared gate itself (kuhyx/utils). Without + # this, a repo's lint job fails on utils' own long files -- a gate + # that fails for reasons the repo being gated cannot fix. + ".utils", + } +) + +#: Third-party content living *inside* a repo you own, keyed by repo dir name. +#: Verified by git remote / vendor manifest, not guessed. +VENDORED_SUBPATHS = { + "mcp-servers": ("servers",), + "dufs-cloud": ("app/assets/pdfjs", "web/assets/pdfjs"), + ".claude": ( + "projects", + "shell-snapshots", + "cache", + "backups", + "todos", + "statsig", + "history", + "chrome", + "downloads", + "plugins", + "ide", + ), + # Agent skill bundles vendored from Anthropic (todo: commit f91311f). + # A SKILL.md is a fixed-name file with frontmatter; detail belongs in a + # sibling references/ dir, so splitting one into SKILL.part2.md would + # break the format rather than improve readability. + "todo": (".agents/skills",), +} + +#: Directory names that mark a vendored agent-skill tree wherever they appear. +#: Keyed by name rather than repo because these bundles get dropped into any +#: repo that wants them, not just the one they were first seen in. +VENDORED_ANYWHERE = ("/.agents/skills/", "/.claude/skills/") + +#: Frozen records of past agent sessions: an implementation plan or an +#: append-only session log describes what was done at one moment. Splitting one +#: rewrites a historical record for no reading benefit -- nobody navigates them, +#: they are read whole or not at all. Same category as generated files. +#: +#: Deliberately NOT the whole of docs/superpowers/: `specs/` and `contracts/` +#: stay capped, because those are living documents that get re-read and edited. +SESSION_ARTIFACTS_ANYWHERE = ( + "/docs/superpowers/plans/", + "/docs/superpowers/sessions/", +) + +#: Whole repos under ~ that are third-party clones, not kuhy's work. +#: Verified 2026-08-14 by `git remote get-url origin` + commit authors. +THIRD_PARTY_REPOS = frozenset( + { + "StringTheory", # github.com/AnthonySf/TheStringTheory + "reaper-mcp", # github.com/TwelveTake-Studios/reaper-mcp + "freesound-mcp", # github.com/timjrobinson/FreesoundMCPServer + "llama_cpp_dart_local", # generated FFI bindings + "aseprite-mcp", + "Mapperatorinator", # github.com/OliBomby/Mapperatorinator + "aur", + "build-your-own-x", + "build_your_x", + "byox-ladder", + "warriorjs", + "untools", + "awesome-mcp-explorer", + "runnerup", + "depotdownloader", + "actions-runner", + } +) + +#: A `.txt` whose mean line is shorter than this is data (a wordlist, a +#: coordinate dump), not prose, so the cap does not apply. +DATA_TXT_MAX_MEAN_LINE = 25 diff --git a/scripts/file_length/check.py b/scripts/file_length/check.py new file mode 100644 index 0000000..ea11177 --- /dev/null +++ b/scripts/file_length/check.py @@ -0,0 +1,183 @@ +"""Gate: fail if any given file exceeds the 250-line cap. + +Invoked by `scripts/check_file_length.sh`, which pre-commit calls with the +staged files. Exits 1 and lists every violation; exits 0 silently otherwise. + +The check is deliberately deterministic code rather than a judgement call -- +adjudication belongs in an exit code, not in a model. +""" + +from __future__ import annotations + +import argparse +import os +from pathlib import Path +import subprocess +import sys + +if __package__ in (None, ""): # invoked as a plain script + sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +from file_length.config import ( + CAPPED_EXTENSIONS, + EXCLUDED_DIRS, + MAX_LINES, + is_data_text, + is_generated, + is_session_artifact, + is_vendored, +) + + +def count_lines(path: Path) -> int | None: + """Number of lines, or None if the file is binary or unreadable.""" + try: + with path.open("rb") as handle: + first = handle.read(8192) + if b"\x00" in first: + return None + count = first.count(b"\n") + while chunk := handle.read(1 << 20): + count += chunk.count(b"\n") + if first and not first.endswith(b"\n") and count == 0: + count = 1 + return count + except OSError: + return None + + +def absolutize(path: Path) -> Path: + """`path` anchored at the cwd, without resolving symlinks. + + The exemption rules are written against full paths ('/.claude/skills/'), + but pre-commit passes paths relative to the repo root ('skills/x.md') while + --all walks absolute ones. Matching a relative path against those rules + silently drops the repo-name context, so the same file passed the gate one + way and failed it the other. + + Deliberately NOT `Path.resolve()`: build and asset dirs here are symlinks + out to ../testsAndMisc_builds/, and resolving would move those paths + outside the repo and change which exemptions match. + """ + return path if path.is_absolute() else Path.cwd() / path + + +def exempt_reason(path: Path, lines: int) -> str | None: + """Why this file is not subject to the cap, or None if it is subject.""" + if path.suffix.lower() not in CAPPED_EXTENSIONS: + return "not a capped extension" + if is_vendored(absolutize(path)): + return "vendored / third-party" + if is_session_artifact(absolutize(path)): + return "frozen session artifact" + if is_generated(path): + return "generated" + try: + size = path.stat().st_size + except OSError: + return "unreadable" + if is_data_text(path, lines, size): + return "data-ish text (wordlist)" + return None + + +def git_ignored(paths: list[Path], root: Path) -> set[Path]: + """The subset of `paths` that git ignores, empty if root is not a repo. + + Git-ignored files are build output or generated data the repo does not + own -- an app's own export (todo's BACKLOG.md), a log, a scratch dump. + They are rewritten wholesale by whatever produces them, so splitting one + is meaningless: the next run restores the original. Pre-commit never sees + them either (they cannot be staged), so skipping them makes --all agree + with the hook instead of failing on files no commit could ever fix. + """ + if not paths: + return set() + try: + result = subprocess.run( + ["git", "-C", str(root), "check-ignore", "--stdin"], + input="\n".join(str(p) for p in paths), + capture_output=True, + text=True, + check=False, + ) + except OSError: # git not installed + return set() + # 0 = some ignored, 1 = none ignored, 128 = not a repo. + if result.returncode not in (0, 1): + return set() + return {Path(line) for line in result.stdout.splitlines() if line} + + +def iter_all(root: Path): + """Every file under root, skipping excluded and git-ignored files.""" + found: list[Path] = [] + for dirpath, dirnames, filenames in os.walk(root, followlinks=False): + dirnames[:] = [d for d in dirnames if d not in EXCLUDED_DIRS] + for name in filenames: + candidate = Path(dirpath) / name + if not candidate.is_symlink(): + found.append(candidate) + ignored = git_ignored(found, root) + return [path for path in found if path not in ignored] + + +def main() -> int: + parser = argparse.ArgumentParser( + description=f"Fail if any file exceeds {MAX_LINES} lines." + ) + parser.add_argument("paths", nargs="*", type=Path) + parser.add_argument( + "--all", + action="store_true", + help="check the whole tree under the current directory", + ) + parser.add_argument( + "--explain", action="store_true", help="also report why files were skipped" + ) + args = parser.parse_args() + + if args.all: + targets = list(iter_all(Path.cwd())) + elif args.paths: + # Explicitly-named paths get the same git-ignore skip as --all, so + # naming a file directly cannot report a violation the hook never + # would. Without this the two modes disagree on exactly the files + # no commit could ever fix (todo's exported BACKLOG.md, logs). + existing = [p for p in args.paths if p.is_file()] + ignored = git_ignored(existing, Path.cwd()) + targets = [p for p in existing if p not in ignored] + else: + parser.error("give file paths or --all") + + violations: list[tuple[Path, int]] = [] + for path in targets: + if not path.is_file(): + continue + lines = count_lines(path) + if lines is None: + continue + reason = exempt_reason(path, lines) + if reason is not None: + if args.explain: + print(f" skip {path}: {reason}") + continue + if lines > MAX_LINES: + violations.append((path, lines)) + + if not violations: + return 0 + + print( + f"File-length gate FAILED: {len(violations)} file(s) over {MAX_LINES} lines\n", + file=sys.stderr, + ) + for path, lines in sorted(violations, key=lambda item: -item[1]): + over = lines - MAX_LINES + print(f" {path}: {lines} lines (over by {over})", file=sys.stderr) + print("\nSplit them, or see refactor_claude_todo.md in this repo.", file=sys.stderr) + return 1 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/scripts/file_length/config.py b/scripts/file_length/config.py new file mode 100644 index 0000000..700ace3 --- /dev/null +++ b/scripts/file_length/config.py @@ -0,0 +1,45 @@ +"""Shared configuration for the 250-line file-length cap. + +Single source of truth for the cap and its exemptions, imported by both the +pre-commit gate (`check.py`) and the survey. This module is the public +surface: :mod:`file_length._tables` holds the data, and +:mod:`file_length.exemptions` the rules that read it. +""" + +from __future__ import annotations + +from file_length._tables import ( + CAPPED_EXTENSIONS, + DATA_TXT_MAX_MEAN_LINE, + EXCLUDED_DIRS, + GENERATED_MARKERS, + GENERATED_PATTERN, + MAX_LINES, + SESSION_ARTIFACTS_ANYWHERE, + THIRD_PARTY_REPOS, + VENDORED_ANYWHERE, + VENDORED_SUBPATHS, +) +from file_length.exemptions import ( + is_data_text, + is_generated, + is_session_artifact, + is_vendored, +) + +__all__ = [ + "CAPPED_EXTENSIONS", + "DATA_TXT_MAX_MEAN_LINE", + "EXCLUDED_DIRS", + "GENERATED_MARKERS", + "GENERATED_PATTERN", + "MAX_LINES", + "SESSION_ARTIFACTS_ANYWHERE", + "THIRD_PARTY_REPOS", + "VENDORED_ANYWHERE", + "VENDORED_SUBPATHS", + "is_data_text", + "is_generated", + "is_session_artifact", + "is_vendored", +] diff --git a/scripts/file_length/exemptions.py b/scripts/file_length/exemptions.py new file mode 100644 index 0000000..be7ffa4 --- /dev/null +++ b/scripts/file_length/exemptions.py @@ -0,0 +1,82 @@ +"""Predicates deciding whether a file is exempt from the 250-line cap. + +Split from :mod:`file_length.config`, which holds the tables these read. +Keeping the rules separate from the data they consult means a new exemption +is a table entry, not a new branch, and the gate and the survey keep sharing +one definition of "exempt". + +Re-exported from :mod:`file_length.config`, so existing +``from file_length.config import is_generated`` imports keep working. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +from file_length._tables import ( + DATA_TXT_MAX_MEAN_LINE, + EXCLUDED_DIRS, + GENERATED_MARKERS, + GENERATED_PATTERN, + SESSION_ARTIFACTS_ANYWHERE, + THIRD_PARTY_REPOS, + VENDORED_ANYWHERE, + VENDORED_SUBPATHS, +) + +if TYPE_CHECKING: + from pathlib import Path + + +def is_generated(path: Path) -> bool: + """True if the file is tool-generated and must not be split by hand.""" + if GENERATED_PATTERN.search(path.as_posix()): + return True + try: + with path.open("r", encoding="utf-8", errors="replace") as handle: + head = "".join(next(handle, "") for _ in range(5)) + except OSError: + return False + return any(marker in head for marker in GENERATED_MARKERS) + + +def is_data_text(path: Path, lines: int, size: int) -> bool: + """True for wordlist-style .txt: many lines, very short ones.""" + if path.suffix.lower() != ".txt" or lines <= 0: + return False + return (size / lines) < DATA_TXT_MAX_MEAN_LINE + + +def is_vendored(path: Path) -> bool: + """True if the path sits under a known vendored subtree or excluded dir. + + Callers must pass an absolute path (see `check.absolutize`). Every rule + below needs the repo-name context that a repo-relative path has already + thrown away: '/.claude/skills/' cannot match 'skills/x.md', so a relative + path would quietly skip the exemption instead of applying it. + """ + parts = path.parts + if any(part in EXCLUDED_DIRS for part in parts): + return True + posix = path.as_posix() + if any(marker in posix for marker in VENDORED_ANYWHERE): + return True + for repo, subs in VENDORED_SUBPATHS.items(): + for sub in subs: + if f"/{repo}/{sub}/" in posix: + return True + return any(f"/{repo}/" in posix for repo in THIRD_PARTY_REPOS) + + +def is_session_artifact(path: Path) -> bool: + """True for a frozen agent-session record (a plan or a session log). + + Kept separate from `is_vendored` so the gate reports why a file is exempt: + these are kuhy's own files, not third-party code. + + Callers must pass an absolute path (see `check.absolutize`), for the same + reason `is_vendored` does -- a repo-relative path has already dropped the + leading directory these markers match on. + """ + posix = path.as_posix() + return any(marker in posix for marker in SESSION_ARTIFACTS_ANYWHERE)