From 26178436cfbba761cf3e671a513e40e80db8e74a Mon Sep 17 00:00:00 2001 From: Utkarsh Date: Mon, 28 Sep 2026 18:02:10 +0530 Subject: [PATCH] ci: add PR gate workflow, structure/rules checker, algorithm tests Adds a GitHub Actions workflow that runs on every pull request against main, so a review starts from a green or red signal instead of a manual read-through. .github/check_pr.py is the gate. It diffs the PR against the base branch and enforces the parts of CONTRIBUTING.md that can be checked mechanically: - structure: no new top-level entries, no moves, renames or deletions, no subdirectories or stray extensions inside scripts/, notebooks/, docs/ and tests/ - third-party imports must be declared in requirements.txt - scripts that move or delete files must offer --dry-run - notebooks must not carry saved output containing local paths Missing tests and oversized notebook output are warnings, not errors, so the existing files that break those two rules do not block contributors today. `--selftest` covers the helpers. tests/test_algorithms.py adds 12 unittest cases for the pure functions in the search and sort scripts. They are loaded by path because 01.TwoSum.py is not an importable module name. The remaining scripts call input() or sleep at import time and need a refactor before they can be tested. Also strips the NUL bytes from the qrcode[pil] line in requirements.txt, which was appended as UTF-16 into a UTF-8 file and made the whole file unreadable to pip. Co-Authored-By: Claude Opus 5 --- .github/check_pr.py | 204 +++++++++++++++++++++++++++++++++ .github/workflows/pr-check.yml | 26 +++++ requirements.txt | Bin 276 -> 223 bytes tests/test_algorithms.py | 101 ++++++++++++++++ 4 files changed, 331 insertions(+) create mode 100755 .github/check_pr.py create mode 100644 .github/workflows/pr-check.yml create mode 100644 tests/test_algorithms.py diff --git a/.github/check_pr.py b/.github/check_pr.py new file mode 100755 index 0000000..52546e0 --- /dev/null +++ b/.github/check_pr.py @@ -0,0 +1,204 @@ +#!/usr/bin/env python3 +"""PR gate: repo structure + the rules in CONTRIBUTING.md. Run from the repo root.""" + +import argparse +import ast +import json +import re +import subprocess +import sys +from pathlib import Path + +# Every entry allowed at the repo root. Adding one is a structural change. +TOP_LEVEL = { + ".github", ".gitignore", "CONTRIBUTING.md", "LICENSE", "README.md", + "requirements.txt", "data", "Decorators & Namespaces", "docs", + "notebooks", "resources", "scripts", "tests", +} +# Which extension belongs in which directory, and how deep a new file may sit. +DIR_RULES = {"scripts": ".py", "notebooks": ".ipynb", "docs": ".md", "tests": ".py"} +FREEFORM = {"data", "resources", "Decorators & Namespaces", ".github"} + +IMPORT_ALIASES = { + "cv2": "opencv-python", "PIL": "pillow", "sklearn": "scikit-learn", + "bs4": "beautifulsoup4", "yaml": "pyyaml", "dotenv": "python-dotenv", +} +LOCAL_PATH = re.compile(r"/Users/|/home/[a-z]|[A-Z]:\\Users") +DESTRUCTIVE = re.compile(r"shutil\.(rmtree|move)|os\.(remove|unlink|rmdir|rename)|\.unlink\(|\.replace\(") + +errors, warnings = [], [] + + +def fail(path, msg): + errors.append(f"{path}: {msg}") + + +def warn(path, msg): + warnings.append(f"{path}: {msg}") + + +def changed_files(base): + out = subprocess.run( + ["git", "diff", "--name-status", "--find-renames", f"{base}...HEAD"], + capture_output=True, text=True, check=True, + ).stdout + changes = [] + for line in out.splitlines(): + parts = line.split("\t") + changes.append((parts[0][0], parts[1:])) + return changes + + +def requirement_names(): + names = set() + for line in Path("requirements.txt").read_text().splitlines(): + line = line.split("#")[0].strip() + if line: + names.add(re.split(r"[<>=!\[;\s]", line)[0].lower().replace("_", "-")) + return names + + +def imported_roots(tree): + roots = set() + for node in ast.walk(tree): + if isinstance(node, ast.Import): + roots.update(a.name.split(".")[0] for a in node.names) + elif isinstance(node, ast.ImportFrom) and not node.level and node.module: + roots.add(node.module.split(".")[0]) + return roots + + +def check_structure(changes): + for status, paths in changes: + target = paths[-1] + top = target.split("/")[0] + if status in "AR" and top not in TOP_LEVEL: + fail(target, f"new top-level entry '{top}'. Put it in one of: {', '.join(sorted(DIR_RULES))}") + continue + if status == "R": + fail(target, f"renamed/moved from '{paths[0]}'. Open an issue first — moves break links in the docs and notebooks") + if status == "D": + fail(target, "deleted. Deletions need a maintainer's sign-off; open an issue instead") + if status == "A" and top in DIR_RULES: + bits = target.split("/") + if len(bits) != 2: + fail(target, f"nested under {top}/. One flat file per contribution, no subdirectories") + elif not target.endswith(DIR_RULES[top]): + fail(target, f"{top}/ holds only {DIR_RULES[top]} files") + + +def test_sources(_cache={}): + # Why: a script counts as checked if a test file names it + if not _cache: + files = list(Path("tests").glob("test_*.py")) + list(Path("scripts").glob("test_*.py")) + _cache["text"] = "\n".join(p.read_text(errors="replace") for p in files) + return _cache["text"] + + +def check_script(path, reqs, repo_modules): + source = Path(path).read_text(encoding="utf-8", errors="replace") + try: + tree = ast.parse(source, filename=path) + except SyntaxError as exc: + fail(path, f"does not parse: line {exc.lineno}: {exc.msg}") + return + + for root in sorted(imported_roots(tree)): + if root in sys.stdlib_module_names or root in repo_modules: + continue + pkg = IMPORT_ALIASES.get(root, root).lower().replace("_", "-") + if pkg not in reqs: + fail(path, f"imports third-party '{root}' — add '{pkg}' to requirements.txt and say why in a comment") + + if DESTRUCTIVE.search(source) and "--dry-run" not in source: + fail(path, "moves or deletes files but has no --dry-run flag") + + has_logic = any(isinstance(n, (ast.For, ast.While, ast.FunctionDef)) for n in ast.walk(tree)) + stem = Path(path).stem + tested = Path(f"tests/test_{stem}.py").exists() or stem in test_sources() + if has_logic and "assert" not in source and not tested: + warn(path, "real logic but no check — add a few asserts under `if __name__ == \"__main__\":`") + + +def check_notebook(path): + try: + nb = json.loads(Path(path).read_text(encoding="utf-8")) + except json.JSONDecodeError as exc: + fail(path, f"is not valid notebook JSON: {exc}") + return + size = 0 + for cell in nb.get("cells", []): + blob = json.dumps(cell.get("outputs", [])) + size += len(blob) + if LOCAL_PATH.search(blob): + fail(path, "output contains a local filesystem path — clear the outputs before committing") + return + if size > 1_000_000: + warn(path, f"{size // 1000} KB of saved output — clear the outputs to keep clones small") + + +def main(): + ap = argparse.ArgumentParser(description=__doc__) + ap.add_argument("--base", default="origin/main", help="branch or ref to diff against") + ap.add_argument("--selftest", action="store_true") + args = ap.parse_args() + + if args.selftest: + selftest() + return 0 + + if not Path("CONTRIBUTING.md").exists(): + print("Run this from the repo root.", file=sys.stderr) + return 2 + + try: + changes = changed_files(args.base) + except subprocess.CalledProcessError: + print(f"Cannot diff against '{args.base}'. Fetch it first: git fetch origin main", file=sys.stderr) + return 2 + + if not changes: + print(f"No changes against {args.base}.") + return 0 + + check_structure(changes) + reqs = requirement_names() + # Why: sibling scripts and the scripts/ and tests/ packages are local, not PyPI + repo_modules = {p.stem for p in Path("scripts").glob("*.py")} | set(DIR_RULES) + for status, paths in changes: + target = paths[-1] + if status == "D" or not Path(target).exists(): + continue + if target.endswith(".py"): + check_script(target, reqs, repo_modules) + elif target.endswith(".ipynb"): + check_notebook(target) + + for line in warnings: + print(f"warning: {line}") + for line in errors: + print(f"error: {line}") + print(f"\n{len(changes)} changed file(s), {len(errors)} error(s), {len(warnings)} warning(s).") + if errors: + print("See CONTRIBUTING.md for the rule behind each error.") + return 1 if errors else 0 + + +def selftest(): + tree = ast.parse("import os, numpy as np\nfrom pathlib import Path\nfrom . import x") + assert imported_roots(tree) == {"os", "numpy", "pathlib"} + assert re.split(r"[<>=!\[;\s]", "pdf2docx>=0.5")[0] == "pdf2docx" + assert LOCAL_PATH.search("/Users/someone/data.csv") + assert not LOCAL_PATH.search("data/sample.txt") + assert DESTRUCTIVE.search("shutil.rmtree(p)") and not DESTRUCTIVE.search("os.listdir(p)") + errors.clear() + check_structure([("A", ["tools/x.py"]), ("A", ["scripts/deep/x.py"]), ("A", ["scripts/a.txt"]), ("D", ["README.md"])]) + assert len(errors) == 4, errors + errors.clear() + check_structure([("A", ["scripts/ok.py"]), ("M", ["README.md"])]) + assert not errors, errors + print("selftest ok") + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/.github/workflows/pr-check.yml b/.github/workflows/pr-check.yml new file mode 100644 index 0000000..f6c6119 --- /dev/null +++ b/.github/workflows/pr-check.yml @@ -0,0 +1,26 @@ +name: PR check + +on: + pull_request: + branches: [main] + +jobs: + check: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - uses: actions/setup-python@v5 + with: + python-version: "3.11" + + - name: Structure and CONTRIBUTING rules + run: python .github/check_pr.py --base origin/${{ github.base_ref }} + + - name: Every script compiles + run: python -m compileall -q scripts + + - name: Tests + run: python -m unittest discover tests -v diff --git a/requirements.txt b/requirements.txt index ec909cf01256dacbc53e7d5e1962412db9ba91db..fc9e5ff83a6ed43531283f3f1150b4c6140d20d4 100644 GIT binary patch delta 59 zcmbQjbf0m;YLlYm{FKz_g3O#)1qEe=(&E$hurvLx| delta 112 zcmcc5IE87#>P&_rhGd3(h7^WWhG>QYhD?SWhFAs#AXEn8QifuncnXk}#83$&i-Bs3 rfNBaDN`P#AhC(1O9