diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5130eb34..87d3a828 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -90,6 +90,15 @@ jobs: # Rejects raw error_code="LITERAL" string literals (must use ErrorCode). run: uv run python scripts/check_error_codes.py + - name: File-size budget check + # Enforces the CONTRIBUTING.md per-layer budgets in CODE LINES + # (docstrings/comments excluded, so documenting a module is free). + # Files already over their hard ceiling are grandfathered in + # scripts/file_size_baseline.json and may only shrink -- this catches + # NEW oversized modules and growth of the existing ones, which review + # reliably misses because a diff never shows the resulting file size. + run: uv run python scripts/check_file_size.py + # ──────────────────────────────────────────────────────────────────────── # Test suite across every supported interpreter. pyproject declares # `requires-python = ">=3.12"`, so the matrix is 3.12 + 3.13 (3.10/3.11 are diff --git a/CLAUDE.md b/CLAUDE.md index ba15445b..3d25d7b1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -197,7 +197,7 @@ Full author checklist: see `CONTRIBUTING.md` > "Releasing a beta (pre-release) v ## Coding Conventions -> **0. (BINDING) Follow [CONTRIBUTING.md](CONTRIBUTING.md) in full.** Every code change -- human or AI agent -- must satisfy the rules in `CONTRIBUTING.md`. Specifically, the "Code Quality Patterns" section is non-negotiable: dataclasses (not bare tuples) for multi-value returns; categorical arguments before variable ones; `ErrorCode` enum (never raw strings); file-size budgets; context managers over lambdas; named functions over assigned anonymous functions; `ty` clean for new code. The `.claude/settings.json` post-edit hooks run `ruff check --fix`, `ruff format`, and `ty check` after every edit -- when an AI agent edits a file in this repo, those checks fire automatically and any failure must be addressed before continuing. If a rule conflicts with an existing pattern in legacy code, **fix it in the PR you are touching** or open a follow-up issue; do not propagate the pattern. +> **0. (BINDING) Follow [CONTRIBUTING.md](CONTRIBUTING.md) in full.** Every code change -- human or AI agent -- must satisfy the rules in `CONTRIBUTING.md`. Specifically, the "Code Quality Patterns" section is non-negotiable: dataclasses (not bare tuples) for multi-value returns; categorical arguments before variable ones; `ErrorCode` enum (never raw strings); file-size budgets (measured in CODE LINES -- docstrings and comments are free; `make loc-check`); context managers over lambdas; named functions over assigned anonymous functions; `ty` clean for new code. The `.claude/settings.json` post-edit hooks run `ruff check --fix`, `ruff format`, and `ty check` after every edit -- when an AI agent edits a file in this repo, those checks fire automatically and any failure must be addressed before continuing. If a rule conflicts with an existing pattern in legacy code, **fix it in the PR you are touching** or open a follow-up issue; do not propagate the pattern. 1. **Typer commands** are thin - they parse arguments, call a service, and format output. No business logic in commands. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ed5563b3..e97a891e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -182,15 +182,32 @@ If a new category appears, **add it to `ErrorCode`** and `_ERROR_CODE_TO_TYPE` i ### File-size budgets -- split when concerns drift -Hard ceiling per file: +Budgets are measured in **code lines**, not raw line count. Docstrings, comments and blank lines do **not** count. + +```bash +make loc-check # the gate; part of `make check` +make loc-report # every module by code lines, largest first +make loc-baseline # re-record grandfathered files AFTER a split +``` | Layer | Soft ceiling | Hard ceiling | |-------|--------------|--------------| -| `commands/*.py` | 800 LOC | 1200 LOC | -| `services/*.py` | 1000 LOC | 1500 LOC | -| `client/*.py` (per module) / `manage_client.py` | 1500 LOC | 2000 LOC | +| `commands/*.py` | 800 | 1200 | +| `services/*.py` | 1000 | 1500 | +| `client/*.py` (per module) / `manage_client.py` | 1500 | 2000 | +| `server/*.py` | 800 | 1200 | +| `sync/*.py` | 1000 | 1500 | +| everything else in the package | 1000 | 1500 | + +**Why code lines and not LOC.** This codebase deliberately writes long rationale-carrying docstrings -- they are the reason it stays navigable, for humans and for the AI agents that work in it. A raw-LOC budget taxes exactly that and pushes toward *less* explanation, which is backwards. The gap is not marginal: `services/version_service.py` is 1252 lines but 705 lines of code (36% prose), and `constants.py` is 574 lines but 190 lines of code (56% prose). Run `make loc-report` for the current numbers rather than trusting these. + +The line the metric draws: a **docstring** (the bare leading string of a module, class or function) is prose and is exempt. A string **assigned to a name** -- a SQL block, a template, the `CHANGELOG` tables -- is data, is counted, and cannot be used to hide content from the budget. + +**Soft vs hard.** Crossing the **soft** ceiling means the next PR that adds material to the file should split it first; `loc-check` prints a warning but stays green. Crossing the **hard** ceiling fails the check: split before merging more functionality. + +**The grandfather ratchet.** Files that were already over their hard ceiling when the gate landed are recorded in `scripts/file_size_baseline.json` at their then-current size. They are allowed to stay that big but **may only shrink** -- growing one fails `loc-check`. That is what lets the gate block on day one without demanding a repo-wide refactor first: it stops new debt and stops existing debt getting worse. After you split a baselined file, run `make loc-baseline` to re-record it. Never run it to silence a file you just grew -- the diff makes that obvious in review. -When a file crosses the **soft** ceiling, the next PR that adds material to it should split first. When a file crosses the **hard** ceiling, splitting is required before merging more functionality into it. +Two files are exempt outright (`scripts/check_file_size.py` `_EXEMPT`): `changelog.py` and `commands/context.py` are documentation payloads that happen to live in `.py` files, and a ceiling on them would only push prose out of the repo. Keep that list short -- an exemption is an admission the budget does not model the file. How to split: - A client mixing multiple Keboola subsystems (Storage, Queue, Sandboxes, ...) → split by **endpoint family** into a package, e.g. `client/storage_tables.py`, `client/queue.py`, `client/configs.py`, composed into one class via mixins. Keep `BaseHttpClient` shared. (This is exactly what `client.py` -> the `client/` package was in #520.) diff --git a/Makefile b/Makefile index fa3fdd81..e5242229 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,6 @@ .DEFAULT_GOAL := help -.PHONY: help install install-mcp install-server sync test test-unit test-integration test-e2e test-e2e-local test-e2e-invite test-e2e-feature test-e2e-stream test-file test-cov lint lint-fix format format-check typecheck typecheck-warn skill-check skill-gen version-sync version-check changelog changelog-check check-error-codes parity-check command-sync-check gen-command-reference check clean hooks web-install web-dev-backend web-dev-frontend web-build web-clean +.PHONY: help install install-mcp install-server sync test test-unit test-integration test-e2e test-e2e-local test-e2e-invite test-e2e-feature test-e2e-stream test-file test-cov lint lint-fix format format-check typecheck typecheck-warn skill-check skill-gen version-sync version-check changelog changelog-check check-error-codes loc-check loc-report loc-baseline parity-check command-sync-check gen-command-reference check clean hooks web-install web-dev-backend web-dev-frontend web-build web-clean help: ## Show this help message @grep -E '^[a-zA-Z0-9_-]+:.*?## .*$$' $(MAKEFILE_LIST) | awk 'BEGIN {FS = ":.*?## "}; {printf " \033[36m%-18s\033[0m %s\n", $$1, $$2}' @@ -92,6 +92,15 @@ version-check: ## Check version-bearing files match pyproject.toml (fails if mis exit 1; \ fi +loc-check: ## Check per-layer file-size budgets in CODE LINES (docstrings/comments excluded) + uv run python scripts/check_file_size.py + +loc-report: ## List every module by code lines, largest first + uv run python scripts/check_file_size.py --report + +loc-baseline: ## Re-record grandfathered over-budget files (run AFTER a split, never to silence growth) + uv run python scripts/check_file_size.py --update-baseline + changelog: ## Generate changelog skeleton from GitHub releases uv run python scripts/generate_changelog.py @@ -115,7 +124,7 @@ hooks: ## Install git pre-commit hook (lint + format on staged files) chmod +x .git/hooks/pre-commit @echo "Pre-commit hook installed." -check: lint format-check typecheck skill-check version-check command-sync-check changelog-check check-error-codes test ## Run all checks (lint + format + typecheck + skill + version + command-sync + changelog + error-codes + test) +check: lint format-check typecheck skill-check version-check command-sync-check changelog-check check-error-codes loc-check test ## Run all checks (lint + format + typecheck + skill + version + command-sync + changelog + error-codes + file-size + test) clean: ## Remove build artifacts and caches find . -type d -name __pycache__ -exec rm -rf {} + 2>/dev/null || true diff --git a/scripts/check_file_size.py b/scripts/check_file_size.py new file mode 100644 index 00000000..065fa518 --- /dev/null +++ b/scripts/check_file_size.py @@ -0,0 +1,335 @@ +"""CI guard: enforce the per-layer file-size budgets from CONTRIBUTING.md. + +Budgets are measured in **code lines**, not raw line count: docstrings, +comments and blank lines are excluded. Raw LOC would tax the long +rationale-carrying docstrings this codebase deliberately writes -- they are +what makes it navigable, so a metric that punishes them pushes in exactly the +wrong direction. The gap is not marginal: `services/version_service.py` is 1252 +lines but 705 lines of code (36% prose), and `constants.py` is 574 lines but +190 lines of code (56% prose). + +What counts as a code line: any physical line carrying at least one token that +is not a comment, a docstring, or pure layout (NEWLINE/NL/INDENT/DEDENT). A +module-level string *assigned to a name* (the CHANGELOG tables, SQL blocks) is +data, not prose, and is counted -- only true docstrings, i.e. the bare leading +string of a module, class, or function, are exempt. + +Usage (run from repo root): + python scripts/check_file_size.py # exits 1 if any HARD ceiling is exceeded + python scripts/check_file_size.py --report # print every file, largest first + python scripts/check_file_size.py --top 20 # report mode, limited + +Exit codes: 0 = all within hard ceilings (soft-ceiling overruns are warnings +only, printed but non-fatal); 1 = at least one hard ceiling exceeded. +""" + +import argparse +import ast +import io +import json +import sys +import tokenize +from dataclasses import dataclass +from pathlib import Path + +REPO_ROOT = Path(__file__).parent.parent +PKG_ROOT = REPO_ROOT / "src" / "keboola_agent_cli" + +# Tokens that never make a line "code" on their own. +_LAYOUT_TOKENS = frozenset( + { + tokenize.COMMENT, + tokenize.NL, + tokenize.NEWLINE, + tokenize.INDENT, + tokenize.DEDENT, + tokenize.ENDMARKER, + tokenize.ENCODING, + } +) + + +@dataclass(frozen=True) +class Budget: + """Per-layer code-line ceilings. + + ``soft`` is advisory -- crossing it means the next PR adding material to the + file should split it first. ``hard`` blocks: splitting is required before + more functionality lands. + """ + + label: str + soft: int + hard: int + + +# Ordered: the FIRST matching prefix wins, so specific layers precede the +# catch-all. Numbers carried over from the CONTRIBUTING.md table -- switching +# the metric from raw LOC to code lines already relaxes them for well-commented +# files, which is the intent; they were not additionally loosened. +_BUDGETS: tuple[tuple[str, Budget], ...] = ( + ("commands/", Budget("commands", soft=800, hard=1200)), + ("services/", Budget("services", soft=1000, hard=1500)), + ("client/", Budget("client", soft=1500, hard=2000)), + ("manage_client.py", Budget("client", soft=1500, hard=2000)), + ("server/", Budget("server", soft=800, hard=1200)), + ("sync/", Budget("sync", soft=1000, hard=1500)), +) +# Everything else under the package: top-level modules, helpers, generated data. +_DEFAULT_BUDGET = Budget("module", soft=1000, hard=1500) + +# Files exempt from the ceiling, with the reason. Keep this list SHORT and +# justified -- an exemption is an admission the budget does not model the file. +# Both entries below are documentation payloads that happen to live in .py +# files: a ceiling on them would only push prose out of the repo. +_EXEMPT: dict[str, str] = { + "changelog.py": ( + "append-only release-note data, one block per version; splitting it " + "would just move the append point without reducing anything" + ), + "commands/context.py": ( + "a single AGENT_CONTEXT string literal (~1600 lines of CLI documentation " + "served by `kbagent context`); it grows with every new command by design" + ), +} + +# Ratchet baseline: files that already exceeded their hard ceiling when the +# check was introduced (0.78.0). They are grandfathered at their recorded size +# and may only shrink -- growth fails the check. This is what lets the gate be +# blocking on day one without a repo-wide refactor first: it stops NEW debt and +# stops existing debt getting worse, rather than demanding it all be paid now. +BASELINE_PATH = REPO_ROOT / "scripts" / "file_size_baseline.json" + + +@dataclass(frozen=True) +class FileMetrics: + """Line accounting for one Python file.""" + + path: Path + total: int + code: int + docstring: int + comment: int + blank: int + + @property + def prose_ratio(self) -> float: + """Share of the file that is docstring or comment.""" + return (self.docstring + self.comment) / self.total if self.total else 0.0 + + +def _docstring_lines(tree: ast.Module) -> set[int]: + """Physical line numbers occupied by true docstrings. + + Only the bare leading string of a module / class / function qualifies -- + :func:`ast.get_docstring` semantics. A string assigned to a name is data + and stays counted as code. + """ + lines: set[int] = set() + scopes: tuple[type[ast.AST], ...] = ( + ast.Module, + ast.ClassDef, + ast.FunctionDef, + ast.AsyncFunctionDef, + ) + for node in ast.walk(tree): + if not isinstance(node, scopes): + continue + body = getattr(node, "body", None) + if not body: + continue + first = body[0] + if not isinstance(first, ast.Expr) or not isinstance(first.value, ast.Constant): + continue + if not isinstance(first.value.value, str): + continue + end = first.end_lineno or first.lineno + lines.update(range(first.lineno, end + 1)) + return lines + + +def measure(path: Path) -> FileMetrics: + """Count code / docstring / comment / blank lines in one file.""" + source = path.read_text(encoding="utf-8") + total = len(source.splitlines()) + doc_lines = _docstring_lines(ast.parse(source)) + + code_lines: set[int] = set() + comment_lines: set[int] = set() + for token in tokenize.generate_tokens(io.StringIO(source).readline): + if token.type == tokenize.COMMENT: + comment_lines.add(token.start[0]) + continue + if token.type in _LAYOUT_TOKENS: + continue + code_lines.update(range(token.start[0], token.end[0] + 1)) + + code_lines -= doc_lines + comment_lines -= code_lines # a trailing comment rides on a code line + blank = sum(1 for line in source.splitlines() if not line.strip()) + return FileMetrics( + path=path, + total=total, + code=len(code_lines), + docstring=len(doc_lines), + comment=len(comment_lines), + blank=blank, + ) + + +def budget_for(relative_path: str) -> Budget: + """Resolve the budget for a package-relative path.""" + for prefix, budget in _BUDGETS: + if relative_path.startswith(prefix) or relative_path == prefix: + return budget + return _DEFAULT_BUDGET + + +def _iter_package_files() -> list[Path]: + """Every checked-in Python module in the package, excluding caches.""" + return sorted(p for p in PKG_ROOT.rglob("*.py") if "__pycache__" not in p.parts) + + +def _report(metrics: list[FileMetrics], limit: int | None) -> None: + """Print every file largest-first with its budget headroom.""" + ranked = sorted(metrics, key=lambda m: m.code, reverse=True) + if limit is not None: + ranked = ranked[:limit] + print(f"{'file':52} {'code':>6} {'total':>6} {'prose':>6} budget") + for metric in ranked: + rel = metric.path.relative_to(PKG_ROOT).as_posix() + budget = budget_for(rel) + # Full-path match only, exactly like the gate below. A basename fallback + # would print `commands/changelog.py` as exempt (it shares a name with + # the exempt top-level `changelog.py`) while main() still measured it -- + # the report would promise a budget the build does not honour. + if _is_exempt(rel): + state = "exempt" + elif metric.code > budget.hard: + state = f"HARD >{budget.hard}" + elif metric.code > budget.soft: + state = f"soft >{budget.soft}" + else: + state = f"ok ({budget.soft}/{budget.hard})" + print( + f"{rel:52} {metric.code:6} {metric.total:6} " + f"{metric.prose_ratio:5.0%} {budget.label}: {state}" + ) + + +def _is_exempt(relative_path: str) -> bool: + return relative_path in _EXEMPT + + +def _load_baseline() -> dict[str, int]: + """Read the grandfathered sizes, or an empty ratchet if none is recorded.""" + if not BASELINE_PATH.is_file(): + return {} + data = json.loads(BASELINE_PATH.read_text(encoding="utf-8")) + return {str(k): int(v) for k, v in data.get("files", {}).items()} + + +def _write_baseline(metrics: list[FileMetrics]) -> dict[str, int]: + """Record every currently-over-ceiling file at its present size.""" + recorded: dict[str, int] = {} + for metric in sorted(metrics, key=lambda m: m.path.as_posix()): + rel = metric.path.relative_to(PKG_ROOT).as_posix() + if _is_exempt(rel): + continue + if metric.code > budget_for(rel).hard: + recorded[rel] = metric.code + payload = { + "_comment": ( + "Grandfathered files over their CONTRIBUTING.md hard ceiling, in CODE LINES " + "(see scripts/check_file_size.py). They may only shrink. Regenerate with " + "`make loc-baseline` after a split -- never to silence a file you just grew." + ), + "files": recorded, + } + BASELINE_PATH.write_text(json.dumps(payload, indent=2) + "\n", encoding="utf-8") + return recorded + + +def main(argv: list[str] | None = None) -> int: + """Run the gate. ``argv`` defaults to the process arguments.""" + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--report", action="store_true", help="list every file, largest first") + parser.add_argument("--top", type=int, default=None, help="limit --report to N files") + parser.add_argument( + "--update-baseline", + action="store_true", + help="re-record the grandfathered sizes (run after a split)", + ) + args = parser.parse_args(argv) + + metrics = [measure(path) for path in _iter_package_files()] + + if args.report or args.top is not None: + _report(metrics, args.top) + return 0 + + if args.update_baseline: + recorded = _write_baseline(metrics) + print(f"Recorded {len(recorded)} grandfathered files in {BASELINE_PATH.name}.") + return 0 + + baseline = _load_baseline() + new_debt: list[tuple[FileMetrics, Budget]] = [] + regressions: list[tuple[FileMetrics, int]] = [] + healed: list[str] = [] + over_soft: list[tuple[FileMetrics, Budget]] = [] + + for metric in metrics: + rel = metric.path.relative_to(PKG_ROOT).as_posix() + if _is_exempt(rel): + continue + budget = budget_for(rel) + allowance = baseline.get(rel) + if allowance is not None: + # Grandfathered: the ceiling is its recorded size, and it may only shrink. + if metric.code > allowance: + regressions.append((metric, allowance)) + elif metric.code <= budget.hard: + healed.append(rel) + continue + if metric.code > budget.hard: + new_debt.append((metric, budget)) + elif metric.code > budget.soft: + over_soft.append((metric, budget)) + + for metric, budget in sorted(over_soft, key=lambda pair: pair[0].code, reverse=True): + rel = metric.path.relative_to(PKG_ROOT).as_posix() + print( + f"WARN: {rel} is {metric.code} code lines, over the {budget.label} soft " + f"ceiling of {budget.soft}. The next PR adding material here should split it first." + ) + for rel in sorted(healed): + print(f"NOTE: {rel} is back within its ceiling -- drop it via `make loc-baseline`.") + + if new_debt or regressions: + print() + for metric, budget in sorted(new_debt, key=lambda pair: pair[0].code, reverse=True): + rel = metric.path.relative_to(PKG_ROOT).as_posix() + print( + f"FAIL: {rel} is {metric.code} code lines, over the {budget.label} HARD " + f"ceiling of {budget.hard}. Split it before merging more functionality." + ) + for metric, allowance in sorted(regressions, key=lambda pair: pair[0].code, reverse=True): + rel = metric.path.relative_to(PKG_ROOT).as_posix() + print( + f"FAIL: {rel} grew to {metric.code} code lines, past its grandfathered " + f"{allowance}. This file is already over budget -- shrink it, do not extend it." + ) + print("\nSee CONTRIBUTING.md > 'File-size budgets'. `--report` shows the whole tree.") + return 1 + + checked = len(metrics) - len(_EXEMPT) + print( + f"OK: {checked} modules within budget " + f"({len(over_soft)} over soft, {len(baseline)} grandfathered)." + ) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/file_size_baseline.json b/scripts/file_size_baseline.json new file mode 100644 index 00000000..c1851b14 --- /dev/null +++ b/scripts/file_size_baseline.json @@ -0,0 +1,11 @@ +{ + "_comment": "Grandfathered files over their CONTRIBUTING.md hard ceiling, in CODE LINES (see scripts/check_file_size.py). They may only shrink. Regenerate with `make loc-baseline` after a split -- never to silence a file you just grew.", + "files": { + "commands/config.py": 2007, + "commands/lineage.py": 1271, + "commands/storage.py": 2246, + "services/data_app_service.py": 1726, + "services/storage_service.py": 1733, + "services/sync_service.py": 1655 + } +} diff --git a/tests/test_check_file_size.py b/tests/test_check_file_size.py new file mode 100644 index 00000000..fae111df --- /dev/null +++ b/tests/test_check_file_size.py @@ -0,0 +1,224 @@ +"""Tests for scripts/check_file_size.py -- the code-line budget gate. + +Two things must hold for the gate to be worth having: + +1. The **measurement** is honest. Raw LOC was rejected precisely because it + taxes docstrings, so the code-line count has to exclude prose exactly and + not quietly exclude real code (a module-level data string is code). +2. The **ratchet** actually ratchets. A grandfathered file may shrink but never + grow, and a brand-new oversized file is rejected outright. + +The real repo tree is never mutated: measurement runs against tmp files and the +ratchet logic against synthetic metrics. +""" + +from __future__ import annotations + +import importlib.util +import json +import sys +from pathlib import Path + +import pytest + +# Load scripts/check_file_size.py as a module without having to install it. +SCRIPTS_DIR = Path(__file__).parent.parent / "scripts" +SPEC = importlib.util.spec_from_file_location( + "_check_file_size_under_test", + SCRIPTS_DIR / "check_file_size.py", +) +assert SPEC is not None and SPEC.loader is not None +_mod = importlib.util.module_from_spec(SPEC) +sys.modules["_check_file_size_under_test"] = _mod +SPEC.loader.exec_module(_mod) + + +def _measure_source(tmp_path: Path, source: str): + target = tmp_path / "sample.py" + target.write_text(source, encoding="utf-8") + return _mod.measure(target) + + +class TestMeasurement: + """What counts as a code line.""" + + def test_docstrings_and_comments_are_not_code(self, tmp_path): + metrics = _measure_source( + tmp_path, + '''"""Module docstring. + +Spanning several lines. +""" + +# A standalone comment. +def f() -> int: + """Function docstring.""" + return 1 +''', + ) + # Code lines: `def f() -> int:` and `return 1`. + assert metrics.code == 2 + assert metrics.docstring == 5 # 4-line module docstring + 1-line function one + assert metrics.comment == 1 + assert metrics.blank == 2 + + def test_module_level_data_string_counts_as_code(self, tmp_path): + """A string ASSIGNED to a name is data, not prose. + + This is the line between `changelog.py` (data, counted) and a docstring + (prose, exempt); getting it wrong would let real content hide from the + budget behind a triple quote. + """ + metrics = _measure_source( + tmp_path, + 'TEMPLATE = """\nline one\nline two\nline three\n"""\n', + ) + assert metrics.code == 5 + assert metrics.docstring == 0 + + def test_class_and_nested_function_docstrings_are_found(self, tmp_path): + metrics = _measure_source( + tmp_path, + '''class A: + """Class docstring.""" + + def m(self) -> None: + """Method docstring.""" + pass +''', + ) + assert metrics.docstring == 2 + assert metrics.code == 3 # class A:, def m, pass + + def test_multiline_expression_counts_each_line_once(self, tmp_path): + metrics = _measure_source(tmp_path, "x = [\n 1,\n 2,\n]\n") + assert metrics.code == 4 + + def test_trailing_comment_does_not_shadow_its_code_line(self, tmp_path): + """A comment riding on a code line must not be double-counted.""" + metrics = _measure_source(tmp_path, "x = 1 # explain\n") + assert metrics.code == 1 + assert metrics.comment == 0 + + def test_prose_ratio_reflects_documentation_weight(self, tmp_path): + metrics = _measure_source( + tmp_path, + '"""Doc."""\n# comment\nx = 1\ny = 2\n', + ) + assert metrics.total == 4 + assert metrics.prose_ratio == pytest.approx(0.5) + + +class TestBudgetResolution: + """First matching prefix wins; everything else gets the default.""" + + @pytest.mark.parametrize( + ("relative_path", "expected_label"), + [ + ("commands/storage.py", "commands"), + ("services/version_service.py", "services"), + ("client/queue.py", "client"), + ("manage_client.py", "client"), + ("server/app.py", "server"), + ("sync/engine.py", "sync"), + ("http_base.py", "module"), + ("auto_update.py", "module"), + ], + ) + def test_layer_is_resolved_from_the_path(self, relative_path, expected_label): + assert _mod.budget_for(relative_path).label == expected_label + + def test_soft_is_always_below_hard(self): + budgets = [budget for _, budget in _mod._BUDGETS] + [_mod._DEFAULT_BUDGET] + assert all(b.soft < b.hard for b in budgets) + + +class TestRatchet: + """Grandfathered files may shrink, never grow.""" + + @pytest.fixture + def baseline_file(self, tmp_path, monkeypatch): + path = tmp_path / "baseline.json" + monkeypatch.setattr(_mod, "BASELINE_PATH", path) + return path + + def test_missing_baseline_is_an_empty_ratchet(self, baseline_file): + assert _mod._load_baseline() == {} + + def test_roundtrip_records_only_over_ceiling_files(self, baseline_file, tmp_path): + pkg = tmp_path / "pkg" + (pkg / "services").mkdir(parents=True) + monkeypatch_root = pkg + with pytest.MonkeyPatch.context() as mp: + mp.setattr(_mod, "PKG_ROOT", monkeypatch_root) + big = _mod.FileMetrics( + path=pkg / "services" / "huge.py", + total=3000, + code=1600, # services hard ceiling is 1500 + docstring=0, + comment=0, + blank=0, + ) + small = _mod.FileMetrics( + path=pkg / "services" / "fine.py", + total=100, + code=50, + docstring=0, + comment=0, + blank=0, + ) + recorded = _mod._write_baseline([big, small]) + assert recorded == {"services/huge.py": 1600} + assert _mod._load_baseline() == {"services/huge.py": 1600} + + def test_baseline_payload_explains_itself(self, baseline_file, tmp_path): + """The file is read by humans mid-review; it must say what it is.""" + with pytest.MonkeyPatch.context() as mp: + mp.setattr(_mod, "PKG_ROOT", tmp_path) + _mod._write_baseline([]) + payload = json.loads(baseline_file.read_text(encoding="utf-8")) + assert "may only shrink" in payload["_comment"] + assert payload["files"] == {} + + +class TestRepoState: + """The gate must be green on the tree it ships with. + + A blocking check that is red on arrival gets disabled, not fixed -- which is + exactly why the ratchet exists. If this fails, either a file grew past its + grandfathered size or new oversized code landed. + """ + + def test_repo_passes_its_own_gate(self, capsys): + assert _mod.main([]) == 0 + + def test_every_baselined_file_still_exists(self): + """A stale baseline entry silently grants a budget to nothing.""" + for relative_path in _mod._load_baseline(): + assert (_mod.PKG_ROOT / relative_path).is_file(), ( + f"{relative_path} is baselined but gone -- run `make loc-baseline`" + ) + + def test_report_and_gate_agree_on_what_is_exempt(self, capsys): + """The report must not promise a budget the gate does not honour. + + `commands/changelog.py` shares a basename with the exempt top-level + `changelog.py`. A basename-based exemption in the report would print it + as unlimited while main() still measured it -- a wasted debugging cycle + the first time it crossed the ceiling. + """ + _mod.main(["--report"]) + lines = capsys.readouterr().out.splitlines() + exempt_in_report = { + line.split()[0] for line in lines[1:] if line.strip().endswith("exempt") + } + assert exempt_in_report == set(_mod._EXEMPT), ( + "report exemptions must match _EXEMPT exactly (full paths, no basename match)" + ) + + def test_exemptions_are_justified_and_real(self): + for relative_path, reason in _mod._EXEMPT.items(): + assert (_mod.PKG_ROOT / relative_path).is_file() or ( + relative_path in {p.name for p in _mod.PKG_ROOT.rglob("*.py")} + ), f"{relative_path} is exempt but does not exist" + assert len(reason) > 40, f"{relative_path} needs a real justification"