diff --git a/.file_length_baseline.json b/.file_length_baseline.json new file mode 100644 index 0000000..813dcd1 --- /dev/null +++ b/.file_length_baseline.json @@ -0,0 +1,16 @@ +{ + "examples/flight_booking_server.py": 582, + "nitrostack/auth/oauth.py": 516, + "nitrostack/cli/main.py": 2019, + "nitrostack/cli/pack.py": 507, + "nitrostack/core/app.py": 2273, + "nitrostack/transports/http.py": 797, + "nitrostack/widgets/route_templates.py": 870, + "nitrostack/widgets/views.py": 569, + "tests/test_cli.py": 1589, + "tests/test_oauth.py": 1011, + "tests/test_secret_value.py": 515, + "tests/test_tasks.py": 574, + "tests/test_transports.py": 732, + "tests/test_widgets.py": 629 +} diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index a18cf80..b33a230 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -30,5 +30,8 @@ jobs: run: | python -m pip install -e ".[dev]" + - name: Check Python file length policy + run: python scripts/check_file_lengths.py + - name: Run pytest with coverage run: pytest tests --cov=nitrostack --cov-report=term-missing --timeout=60 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000..72b8092 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,56 @@ +# Contributing to Nitrostack Python SDK + +Welcome! This guide explains our development workflow and coding policies to help you contribute smoothly. + +--- + +## Quick Start for Developers + +```bash +# 1. Set up virtual environment and install dev dependencies +python -m venv .venv +source .venv/bin/activate +pip install -e ".[dev]" + +# 2. Run tests +pytest tests + +# 3. Check Python file-length policy +python scripts/check_file_lengths.py +``` + +--- + +## Python File-Length Policy + +To keep modules focused and reviewable, we enforce a **500 physical-line limit** on Python files. + +### The Rule +* **New files & standard files:** Must be **500 lines or fewer**. +* **Lines counted:** Total physical lines (including blanks, comments, docstrings, and multiline strings). +* **Scope:** Tracked `*.py` files in `nitrostack/`, `tests/`, `examples/`, and `scripts/`. Embedded templates in Python files count toward this limit. +* **Exempt:** Documentation (`.md`), static assets (HTML/CSS/JS), lockfiles, vendored packages, and generated build output. + +--- + +## Working with Existing Oversized Files (The Ratchet) + +A set of pre-existing legacy files exceed 500 lines. Instead of forcing immediate file splits, we use a **ratchet** tracked in [`.file_length_baseline.json`](.file_length_baseline.json): + +1. **They cannot grow:** CI fails if any legacy file exceeds its current baseline number. +2. **They can shrink:** When you refactor or shrink an oversized file, lower its baseline automatically with: + ```bash + python scripts/check_file_lengths.py --update-baseline + ``` + Then commit the updated `.file_length_baseline.json` with your PR. +3. **Graduation:** Once a legacy file reaches 500 lines or fewer, `--update-baseline` automatically removes it from the baseline. From that point on, it is capped at 500 lines forever. + +> **Need an exception?** Any baseline increase is exceptional and must be explicitly justified and approved during code review. + +--- + +## Continuous Integration (CI) + +Our GitHub Actions workflow automatically checks every PR across Python 3.10, 3.11, and 3.12: +1. **File-length check:** Runs `python scripts/check_file_lengths.py`. +2. **Test suite:** Runs `pytest tests --cov=nitrostack --cov-report=term-missing --timeout=60`. diff --git a/scripts/check_file_lengths.py b/scripts/check_file_lengths.py new file mode 100755 index 0000000..8f13f6f --- /dev/null +++ b/scripts/check_file_lengths.py @@ -0,0 +1,229 @@ +from __future__ import annotations + +import argparse +import json +import os +import subprocess +import sys +from pathlib import Path +from typing import Dict, List, NamedTuple, Optional, Tuple + +DEFAULT_MAX_LINES = 500 +SCOPED_DIRECTORIES = ("nitrostack", "tests", "examples", "scripts") +REPO_ROOT = Path(__file__).resolve().parents[1] +DEFAULT_BASELINE_FILE = REPO_ROOT / ".file_length_baseline.json" + + +class Violation(NamedTuple): + path: str + count: int + allowed: int + violation_type: str # 'new_file', 'baseline_growth', or 'stale_baseline' + message: str + + +def count_physical_lines(file_path: Path | str) -> int: + """Count physical lines in a file, matching (Get-Content file).Count semantics. + + Includes blank lines, comments, docstrings, and lines inside multiline strings. + A trailing newline does not add a phantom line. + """ + path = Path(file_path) + if not path.is_file(): + return 0 + with open(path, "rb") as f: + content = f.read() + return len(content.splitlines()) + + +def get_tracked_python_files( + repo_root: Path, + scoped_dirs: Tuple[str, ...] = SCOPED_DIRECTORIES, +) -> List[str]: + """Return relative POSIX paths of tracked Python files under scoped directories.""" + try: + result = subprocess.run( + ["git", "ls-files"], + cwd=repo_root, + capture_output=True, + check=True, + text=True, + ) + files = result.stdout.splitlines() + except (subprocess.SubprocessError, FileNotFoundError): + files = [] + for d in scoped_dirs: + dir_path = repo_root / d + if dir_path.is_dir(): + for root, _, filenames in os.walk(dir_path): + for filename in filenames: + rel = Path(root, filename).relative_to(repo_root).as_posix() + files.append(rel) + + matched = [] + for f in files: + if not f.endswith(".py"): + continue + # Check if the file is within one of the scoped directories + parts = Path(f).parts + if parts and parts[0] in scoped_dirs: + matched.append(Path(f).as_posix()) + + return sorted(matched) + + +def load_baseline(baseline_path: Path) -> Dict[str, int]: + """Load baseline mapping of file path -> allowed line count.""" + if not baseline_path.exists(): + return {} + with open(baseline_path, "r", encoding="utf-8") as f: + data = json.load(f) + # Ensure all paths are normalized POSIX strings + return {Path(k).as_posix(): int(v) for k, v in data.items()} + + +def save_baseline(baseline_path: Path, baseline: Dict[str, int]) -> None: + """Save baseline mapping to JSON sorted alphabetically.""" + sorted_baseline = {k: baseline[k] for k in sorted(baseline.keys())} + with open(baseline_path, "w", encoding="utf-8") as f: + json.dump(sorted_baseline, f, indent=2) + f.write("\n") + + +def check_files( + repo_root: Path, + tracked_files: List[str], + baseline: Dict[str, int], + max_lines: int = DEFAULT_MAX_LINES, +) -> Tuple[List[Violation], Dict[str, int]]: + """Check tracked files against the maximum line limit and ratchet baseline. + + Returns: + (violations, new_ratcheted_baseline) + """ + violations: List[Violation] = [] + ratcheted_baseline: Dict[str, int] = dict(baseline) + + # Track files currently present + checked_paths = set(tracked_files) + + for rel_path in tracked_files: + full_path = repo_root / rel_path + if not full_path.is_file(): + continue + + count = count_physical_lines(full_path) + allowed = baseline.get(rel_path, max_lines) + + if count > allowed: + if rel_path in baseline: + vtype = "baseline_growth" + msg = ( + f"File grew above baseline: {rel_path} has {count} lines " + f"(allowed baseline: {allowed})" + ) + else: + vtype = "new_file" + msg = ( + f"File exceeds line limit: {rel_path} has {count} lines " + f"(allowed maximum: {allowed})" + ) + violations.append( + Violation( + path=rel_path, + count=count, + allowed=allowed, + violation_type=vtype, + message=msg, + ) + ) + elif rel_path in baseline: + # Ratchet mechanism: file shrank below baseline! + if count <= max_lines: + # File reached or dropped below standard threshold -> remove from baseline + del ratcheted_baseline[rel_path] + elif count < allowed: + # File shrank, lower the baseline + ratcheted_baseline[rel_path] = count + + # Also clean up any baseline entries for files that no longer exist + for baseline_file in list(ratcheted_baseline.keys()): + if baseline_file not in checked_paths: + del ratcheted_baseline[baseline_file] + + return violations, ratcheted_baseline + + +def run_check( + repo_root: Path, + baseline_path: Path, + max_lines: int = DEFAULT_MAX_LINES, + update_baseline: bool = False, +) -> int: + """Run checker and report results. + + Returns exit code (0 for success, 1 for violations). + """ + tracked = get_tracked_python_files(repo_root) + baseline = load_baseline(baseline_path) + violations, ratcheted_baseline = check_files( + repo_root=repo_root, + tracked_files=tracked, + baseline=baseline, + max_lines=max_lines, + ) + + ratchet_needed = ratcheted_baseline != baseline + + if violations: + print(f"FAILED: Found {len(violations)} Python file-length violation(s):") + for v in violations: + print(f" [{v.violation_type}] {v.path}: count={v.count}, allowed={v.allowed}") + print(f" -> {v.message}") + + if ratchet_needed: + if update_baseline: + save_baseline(baseline_path, ratcheted_baseline) + print(f"Updated baseline file at {baseline_path} with ratcheted counts.") + else: + print("\nNOTE: Baseline ratchet improvement detected:") + for k, old_count in baseline.items(): + new_count = ratcheted_baseline.get(k) + if new_count is None: + print(f" {k}: was {old_count}, now <= {max_lines} (entry removed from baseline)") + elif new_count < old_count: + print(f" {k}: was {old_count}, lowered to {new_count}") + print("Run with --update-baseline to update the baseline file.") + + if violations: + return 1 + + print( + f"SUCCESS: All {len(tracked)} tracked Python files in scope satisfy file-length limits " + f"(max {max_lines} lines or within baseline ratchet)." + ) + return 0 + + +def main(argv: Optional[List[str]] = None) -> int: + parser = argparse.ArgumentParser( + description="Check Python file lengths against limit (500 lines) and baseline ratchet." + ) + parser.add_argument( + "--update-baseline", + action="store_true", + help="Update the baseline file if files have shrunk or reached <= 500 lines", + ) + + args = parser.parse_args(argv) + + return run_check( + repo_root=REPO_ROOT, + baseline_path=DEFAULT_BASELINE_FILE, + max_lines=DEFAULT_MAX_LINES, + update_baseline=args.update_baseline, + ) + + +if __name__ == "__main__": + sys.exit(main()) \ No newline at end of file diff --git a/tests/test_file_length_check.py b/tests/test_file_length_check.py new file mode 100644 index 0000000..17ff5de --- /dev/null +++ b/tests/test_file_length_check.py @@ -0,0 +1,189 @@ +"""Tests for the dependency-free file-length checker and baseline ratchet.""" + +import sys +from pathlib import Path + +# Ensure repo root is in sys.path so scripts can be imported cleanly +ROOT = Path(__file__).resolve().parents[1] +if str(ROOT) not in sys.path: + sys.path.insert(0, str(ROOT)) + + +from scripts.check_file_lengths import ( + check_files, + count_physical_lines, + load_baseline, + run_check, + save_baseline, +) + + +def test_count_physical_lines(tmp_path: Path): + file_path = tmp_path / "sample.py" + # Content with blank lines, comments, docstrings, multiline strings + content = '"""Module docstring.\nLine 2.\n"""\n\n# A comment\nx = 1\n' + file_path.write_text(content, encoding="utf-8") + assert count_physical_lines(file_path) == 6 + + # Trailing newline does not add a phantom line + file_path_no_newline = tmp_path / "no_newline.py" + file_path_no_newline.write_text("a = 1\nb = 2", encoding="utf-8") + assert count_physical_lines(file_path_no_newline) == 2 + + # Non-existent file + assert count_physical_lines(tmp_path / "non_existent.py") == 0 + + +def test_file_exactly_at_500_lines(tmp_path: Path): + repo_root = tmp_path + py_file = repo_root / "nitrostack" / "boundary.py" + py_file.parent.mkdir(parents=True) + # Write exactly 500 lines + py_file.write_text("\n".join(f"x_{i} = {i}" for i in range(500)), encoding="utf-8") + + tracked = ["nitrostack/boundary.py"] + baseline = {} + + violations, ratcheted = check_files( + repo_root=repo_root, + tracked_files=tracked, + baseline=baseline, + max_lines=500, + ) + assert len(violations) == 0 + assert ratcheted == {} + + +def test_new_file_over_500_lines(tmp_path: Path): + repo_root = tmp_path + py_file = repo_root / "nitrostack" / "large.py" + py_file.parent.mkdir(parents=True) + # Write 501 lines + py_file.write_text("\n".join(f"x_{i} = {i}" for i in range(501)), encoding="utf-8") + + tracked = ["nitrostack/large.py"] + baseline = {} + + violations, _ = check_files( + repo_root=repo_root, + tracked_files=tracked, + baseline=baseline, + max_lines=500, + ) + assert len(violations) == 1 + v = violations[0] + assert v.path == "nitrostack/large.py" + assert v.count == 501 + assert v.allowed == 500 + assert v.violation_type == "new_file" + assert "File exceeds line limit" in v.message + + +def test_baseline_growth(tmp_path: Path): + repo_root = tmp_path + py_file = repo_root / "nitrostack" / "legacy.py" + py_file.parent.mkdir(parents=True) + # File has 600 lines, baseline is 550 + py_file.write_text("\n".join(f"x_{i} = {i}" for i in range(600)), encoding="utf-8") + + tracked = ["nitrostack/legacy.py"] + baseline = {"nitrostack/legacy.py": 550} + + violations, _ = check_files( + repo_root=repo_root, + tracked_files=tracked, + baseline=baseline, + max_lines=500, + ) + assert len(violations) == 1 + v = violations[0] + assert v.path == "nitrostack/legacy.py" + assert v.count == 600 + assert v.allowed == 550 + assert v.violation_type == "baseline_growth" + assert "File grew above baseline" in v.message + + +def test_file_below_its_baseline_ratchet(tmp_path: Path): + repo_root = tmp_path + py_file = repo_root / "nitrostack" / "legacy.py" + py_file.parent.mkdir(parents=True) + # File previously was 700 lines (in baseline), now shrunk to 620 lines (> 500) + py_file.write_text("\n".join(f"x_{i} = {i}" for i in range(620)), encoding="utf-8") + + tracked = ["nitrostack/legacy.py"] + baseline = {"nitrostack/legacy.py": 700} + + violations, ratcheted = check_files( + repo_root=repo_root, + tracked_files=tracked, + baseline=baseline, + max_lines=500, + ) + assert len(violations) == 0 + # Ratchet lowered the baseline + assert ratcheted["nitrostack/legacy.py"] == 620 + + +def test_file_drops_to_or_below_500_removed_from_baseline(tmp_path: Path): + repo_root = tmp_path + py_file = repo_root / "nitrostack" / "legacy.py" + py_file.parent.mkdir(parents=True) + # File was 600 in baseline, now shrunk to 500 lines + py_file.write_text("\n".join(f"x_{i} = {i}" for i in range(500)), encoding="utf-8") + + tracked = ["nitrostack/legacy.py"] + baseline = {"nitrostack/legacy.py": 600} + + violations, ratcheted = check_files( + repo_root=repo_root, + tracked_files=tracked, + baseline=baseline, + max_lines=500, + ) + assert len(violations) == 0 + # Ratchet removed it from baseline because it now satisfies the standard 500 threshold + assert "nitrostack/legacy.py" not in ratcheted + + +def test_baseline_save_and_load(tmp_path: Path): + baseline_path = tmp_path / "baseline.json" + data = {"b.py": 600, "a.py": 700} + save_baseline(baseline_path, data) + + # Should be saved sorted by key + loaded = load_baseline(baseline_path) + assert list(loaded.keys()) == ["a.py", "b.py"] + assert loaded == {"a.py": 700, "b.py": 600} + + +def test_run_check_flow_and_update_baseline(tmp_path: Path, capsys): + repo_root = tmp_path + baseline_path = repo_root / ".file_length_baseline.json" + + # Create tracked files under nitrostack and tests + f1 = repo_root / "nitrostack" / "f1.py" + f1.parent.mkdir(parents=True) + f1.write_text("\n".join(f"x_{i} = {i}" for i in range(550)), encoding="utf-8") + + save_baseline(baseline_path, {"nitrostack/f1.py": 600}) + + # Run check without update: should succeed (550 <= 600) and notify ratchet + code = run_check(repo_root=repo_root, baseline_path=baseline_path, update_baseline=False) + assert code == 0 + captured = capsys.readouterr().out + assert "Baseline ratchet improvement detected" in captured + + # Run check with update: should update baseline + code = run_check(repo_root=repo_root, baseline_path=baseline_path, update_baseline=True) + assert code == 0 + updated_baseline = load_baseline(baseline_path) + assert updated_baseline["nitrostack/f1.py"] == 550 + + # Grow file to 580: should fail + f1.write_text("\n".join(f"x_{i} = {i}" for i in range(580)), encoding="utf-8") + code = run_check(repo_root=repo_root, baseline_path=baseline_path, update_baseline=False) + assert code == 1 + captured = capsys.readouterr().out + assert "FAILED: Found 1 Python file-length violation(s)" in captured + assert "baseline_growth" in captured