From c7538f691bd53f5ad9a084b44917a35594450652 Mon Sep 17 00:00:00 2001 From: Krzysztof kuhy Rudnicki Date: Sat, 15 Aug 2026 16:35:01 +0200 Subject: [PATCH] Enforce the shared 250-line file cap Replace this repo's stale file-length hook with the shared gate every other repo will use. The old hook capped at 500 lines, called a repo-local scripts/check_file_length.py, and filtered to types_or: [python, shell] -- so it never looked at the Flutter app in app/ or at any prose. Two caps that disagree is worse than one, so the old one is deleted rather than kept. The new hook checks only the files in the commit and exits 1; CI runs --all as the backstop, since a fresh clone that never ran `pre-commit install` has no hooks at all. The exemption list lives with the checker in ~/utils/file_length/config.py so the gate and the cross-repo survey cannot disagree about what "too long" means. Two things this repo has that ~/todo (the reference implementation) did not: - A pre-commit CI job running --all-files, where $HOME/utils does not exist. The shared gate is checked out to .utils and reached via UTILS_ROOT, or the hook fails on a missing checker rather than on anything in this repo. - Tests that isolate live user state with dotted-string patches (`patch("diet_guard._state.FOOD_LOG_FILE", ...)`). unittest.mock.patch fails OPEN: once a refactor moves a name, the old path still imports, the patch redirects nothing, the test still passes -- and the suite writes to the real ~/.local/share/diet_guard, which diet-guard-sync.timer publishes within 15 minutes. check_patch_targets.py resolves all 30 patch targets statically so that failure is loud instead of silent. It is landed before any split for exactly that reason. The gate is red at 52 files; the splits that follow drive it to zero. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01MK3HCZrM75sRHL1gL8usve --- .github/workflows/file-length.yml | 33 ++++++++++ .github/workflows/pre-commit.yml | 14 +++++ .pre-commit-config.yaml | 30 +++++++-- app/pubspec.yaml | 2 +- scripts/check_file_length.py | 26 -------- scripts/check_file_length.sh | 31 +++++++++ scripts/check_patch_targets.py | 101 ++++++++++++++++++++++++++++++ 7 files changed, 205 insertions(+), 32 deletions(-) create mode 100644 .github/workflows/file-length.yml delete mode 100755 scripts/check_file_length.py create mode 100755 scripts/check_file_length.sh create mode 100755 scripts/check_patch_targets.py diff --git a/.github/workflows/file-length.yml b/.github/workflows/file-length.yml new file mode 100644 index 0000000..afc1578 --- /dev/null +++ b/.github/workflows/file-length.yml @@ -0,0 +1,33 @@ +name: file-length + +# The 250-line cap, enforced on push as well as at commit time. The local +# pre-commit hook can be skipped (a fresh clone that never ran `pre-commit +# install` has no hooks at all), so the cap needs a second gate that no one +# can forget to install. +# +# Deliberately a SEPARATE workflow from release-apk.yml: a file that grew too +# long should fail lint, not block publishing an APK. +on: + push: + branches: [main] + pull_request: + workflow_dispatch: + +jobs: + file-length: + name: file length <= 250 lines + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + # The gate and its exemption list live in the shared utils repo so every + # repo enforces the same cap. Checking it out beats vendoring a copy + # here, which is exactly how a gate and a survey start disagreeing. + - name: Check out the shared gate + uses: actions/checkout@v4 + with: + repository: kuhyx/utils + path: .utils + + - name: Enforce the cap + run: bash .utils/scripts/check_file_length.sh --all diff --git a/.github/workflows/pre-commit.yml b/.github/workflows/pre-commit.yml index 98911e8..01b8b08 100644 --- a/.github/workflows/pre-commit.yml +++ b/.github/workflows/pre-commit.yml @@ -16,4 +16,18 @@ jobs: python-version: "3.12" - name: Install dependencies run: pip install -r requirements.txt + + # The file-length hook delegates to the shared gate in kuhyx/utils, which + # a runner does not have at $HOME/utils. Check it out and point the shim + # at it via UTILS_ROOT, or `pre-commit run --all-files` fails on a + # missing checker rather than on anything in this repo. `.utils` is in + # the gate's own EXCLUDED_DIRS, so it does not lint itself. + - name: Check out the shared gate + uses: actions/checkout@v4 + with: + repository: kuhyx/utils + path: .utils + - uses: pre-commit/action@v3.0.1 + env: + UTILS_ROOT: ${{ github.workspace }}/.utils diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index a350ccf..892b7c8 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -27,7 +27,7 @@ repos: - id: name-tests-test args: [--pytest-test-first] # Shared fake widgets, not a test module. Split out of conftest.py only - # because that file passed the 500-line cap. + # because that file passed the 250-line cap. exclude: ^diet_guard/tests/_(tk_fakes|gate_layout_probe|gate_keyboard_probe)\.py$ - id: check-ast - id: mixed-line-ending @@ -170,8 +170,28 @@ repos: # shebang (SC2148) and its variables are read by makepkg rather than by # the file itself (SC2034). namcap is what actually lints these. exclude: '^packaging/aur/PKGBUILD$' - - id: max-file-length - name: Max file length (500 lines) - entry: python3 scripts/check_file_length.py + # The shared 250-line cap. Checks only the files in the commit, so an + # unrelated commit never breaks; exits 1 rather than warning. No + # baseline and no allowlist -- those are suppressions. The exemption + # list (generated/vendored/data files) lives with the checker in + # ~/utils/file_length/config.py so the gate and the survey cannot + # disagree. + # + # Deliberately unfiltered: no types_or and no files. The cap applies to + # Dart and prose too, and a type filter here is what would silently + # exempt the whole Flutter app and every .md. + - id: file-length + name: file length <= 250 lines + entry: scripts/check_file_length.sh language: system - types_or: [python, shell] + + # unittest.mock.patch fails OPEN: once a refactor moves a name, a patch + # target that still imports redirects nothing, the test passes, and the + # suite writes to the real ~/.local/share/diet_guard. This resolves every + # patch target statically so a stale path fails the commit instead. + - id: patch-targets + name: test patch targets resolve + entry: python3 scripts/check_patch_targets.py + language: system + pass_filenames: false + files: ^diet_guard/ diff --git a/app/pubspec.yaml b/app/pubspec.yaml index fd41847..ccc9f71 100644 --- a/app/pubspec.yaml +++ b/app/pubspec.yaml @@ -2,7 +2,7 @@ name: diet_guard_app description: "Companion phone app for diet_guard: log meals on the go." publish_to: 'none' -version: 1.0.11+1 +version: 1.0.12+1 environment: sdk: ^3.12.2 diff --git a/scripts/check_file_length.py b/scripts/check_file_length.py deleted file mode 100755 index d30d747..0000000 --- a/scripts/check_file_length.py +++ /dev/null @@ -1,26 +0,0 @@ -#!/usr/bin/env python3 -"""Pre-commit hook: fail if any file exceeds MAX_LINES lines.""" - -from pathlib import Path -import sys - -MAX_LINES = 500 - - -def main() -> int: - """Return 1 if any file exceeds the line limit, else 0.""" - failed = False - for filepath in sys.argv[1:]: - try: - with Path(filepath).open(encoding="utf-8", errors="replace") as fh: - count = sum(1 for _ in fh) - except OSError: - failed = True - continue - if count > MAX_LINES: - failed = True - return 1 if failed else 0 - - -if __name__ == "__main__": - sys.exit(main()) diff --git a/scripts/check_file_length.sh b/scripts/check_file_length.sh new file mode 100755 index 0000000..45d8370 --- /dev/null +++ b/scripts/check_file_length.sh @@ -0,0 +1,31 @@ +#!/bin/bash + +# ============================================================================ +# Fail if any file in the commit exceeds the shared 250-line cap. +# +# Thin delegate to the shared gate in ~/utils, which owns the cap and the +# exemption list (generated / vendored / data files). Copying that logic here +# is what lets one repo's idea of "too long" drift from every other repo's -- +# so this script only locates the shared checker and forwards its arguments. +# +# Usage: +# scripts/check_file_length.sh [ ...] # pre-commit passes these +# scripts/check_file_length.sh --all # whole tree, from cwd +# ============================================================================ + +set -euo pipefail + +readonly SHARED_GATE="${UTILS_ROOT:-$HOME/utils}/scripts/check_file_length.sh" + +main() { + if [[ ! -x "$SHARED_GATE" ]]; then + echo "Error: shared file-length gate not found at $SHARED_GATE" >&2 + echo " Clone github.com/kuhyx/utils to ~/utils, or set" >&2 + echo " UTILS_ROOT to where it lives." >&2 + exit 1 + fi + + exec bash "$SHARED_GATE" "$@" +} + +main "$@" diff --git a/scripts/check_patch_targets.py b/scripts/check_patch_targets.py new file mode 100755 index 0000000..fb152b8 --- /dev/null +++ b/scripts/check_patch_targets.py @@ -0,0 +1,101 @@ +#!/usr/bin/env python3 +"""Fail if a test patches a dotted path that no longer resolves. + +``unittest.mock.patch("diet_guard._state.FOOD_LOG_FILE", ...)`` fails *open*. +Once a refactor moves ``FOOD_LOG_FILE`` to another module, the old path still +imports, the patch redirects nothing, and the test keeps passing -- while the +suite writes to the real ``~/.local/share/diet_guard``, which +``diet-guard-sync.timer`` then publishes to every other device. + +That is why this gate exists rather than a note in CLAUDE.md: the failure it +guards against is silent and green, so only a static check catches it. + +Usage: + python3 scripts/check_patch_targets.py +""" + +from __future__ import annotations + +import ast +import importlib +import pathlib +import sys +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from collections.abc import Iterator + +#: Only paths into this package are checked; a patch of `requests` or `tk` is +#: not ours to validate. +PACKAGE = "diet_guard" + +TESTS_ROOT = pathlib.Path(PACKAGE) / "tests" + + +def patch_targets(root: pathlib.Path) -> Iterator[tuple[pathlib.Path, int, str]]: + """Yield ``(file, lineno, dotted_target)`` for every literal patch target.""" + for path in sorted(root.rglob("*.py")): + tree = ast.parse(path.read_text(), str(path)) + for node in ast.walk(tree): + if not isinstance(node, ast.Call): + continue + name = getattr(node.func, "attr", None) or getattr(node.func, "id", None) + if name != "patch" or not node.args: + continue + first = node.args[0] + if ( + isinstance(first, ast.Constant) + and isinstance(first.value, str) + and first.value.startswith(PACKAGE) + ): + yield path, node.lineno, first.value + + +def resolves(dotted: str) -> bool: + """Whether ``dotted`` names a real attribute reachable from a real module. + + Tries the longest importable module prefix first, then walks the remaining + segments as attributes -- the same resolution order ``mock.patch`` uses. + """ + parts = dotted.split(".") + for split in range(len(parts), 0, -1): + try: + module = importlib.import_module(".".join(parts[:split])) + except ImportError: + continue + obj: object = module + for attr in parts[split:]: + if not hasattr(obj, attr): + return False + obj = getattr(obj, attr) + return True + return False + + +def main() -> int: + """Report every unresolvable patch target; return 1 if any were found.""" + if not TESTS_ROOT.is_dir(): + sys.stderr.write(f"Error: {TESTS_ROOT} not found; run from the repo root.\n") + return 1 + + stale = [ + (path, lineno, target) + for path, lineno, target in patch_targets(TESTS_ROOT) + if not resolves(target) + ] + for path, lineno, target in stale: + sys.stderr.write(f"{path}:{lineno}: patch target does not resolve: {target}\n") + + if stale: + plural = "" if len(stale) == 1 else "s" + sys.stderr.write( + f"\n{len(stale)} stale patch target{plural}. A patch that does not " + f"resolve silently redirects nothing -- the test passes while writing " + f"to real user state. Update the path to where the name now lives.\n" + ) + return 1 + return 0 + + +if __name__ == "__main__": + raise SystemExit(main())