From e774c4117fb9319e59351014f51bd1535fe06bce Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Wed, 6 May 2026 21:08:44 +0000 Subject: [PATCH] CI(lint): split Python lint into a multi-language Lint CI workflow Drop the python-lint job from studio-backend-ci.yml and move it into the dedicated `Lint CI` workflow. Two material changes: 1. License-header check now accepts BOTH header families The previous version only counted SPDX-License-Identifier, which warned on every Apache-2.0 file in unsloth/, unsloth_cli/, and scripts/ (e.g. unsloth/models/llama.py opens with the standard `# Copyright ... Daniel Han-Chen & the Unsloth team. All rights reserved. # Licensed under the Apache License, Version 2.0` block, which is correct, but my SPDX-only regex flagged it). New rule: a file is OK if either `SPDX-License-Identifier` or `Licensed under the Apache License` appears in the first 20 lines. Empty __init__.py files are skipped. Whole-repo coverage instead of just studio/backend. 2. Add shell / YAML / JSON parse gates - `bash -n` over every committed *.sh (14 today). Same idea as compileall: parse-only check. - `yaml.safe_load_all` over every *.yml / *.yaml (97 today), including .github/workflows/* so a typo in the workflow file itself shows up immediately. - `json.loads` over every *.json (18 today). Skips package-lock.json / bun.lock (huge, machine-generated) and tsconfig*.json (TypeScript JSONC convention -- already validated by `tsc --noEmit` in Frontend CI). TypeScript and Rust are NOT duplicated here: - Studio Frontend CI runs `npm run typecheck` + `npm run build` on every studio/frontend/** change, which is a full TS AST + type check. - Studio Tauri CI runs `tauri build --debug --no-bundle` on every studio/src-tauri/** or studio/frontend/** change, which is a full Rust compile. A duplicate fast-fail step here would burn cache for marginal value, and the dedicated workflows already block merges. Lint CI runs on every PR (no path filter): the whole job is under 30 s of CI time, so paying that on every PR is preferable to missing a regression on a path the focused workflows skip. --- .github/workflows/lint-ci.yml | 269 ++++++++++++++++++++++++ .github/workflows/studio-backend-ci.yml | 111 +--------- 2 files changed, 275 insertions(+), 105 deletions(-) create mode 100644 .github/workflows/lint-ci.yml diff --git a/.github/workflows/lint-ci.yml b/.github/workflows/lint-ci.yml new file mode 100644 index 0000000000..3896c07868 --- /dev/null +++ b/.github/workflows/lint-ci.yml @@ -0,0 +1,269 @@ +# SPDX-License-Identifier: AGPL-3.0-only +# Copyright 2026-present the Unsloth AI Inc. team. All rights reserved. + +# Whole-repo, multi-language source-lint gate. Runs on every PR +# (no path filter) because each step is sub-second to a few seconds +# and together they catch a class of breakage the focused build +# workflows would miss: +# +# - Python syntax + ruff + leftover debugger calls (across 350+ +# committed .py files, not just studio/backend). +# - Shell `bash -n` parse for every committed *.sh. +# - `yaml.safe_load` and `json.loads` round-trip for every +# committed YAML / JSON config. +# +# TypeScript and Rust are NOT duplicated here on purpose: +# - Studio Frontend CI runs `npm run typecheck` (= `tsc --noEmit`) +# and `npm run build` (vite/swc) on every studio/frontend/** +# change, which is a full TS AST + type check. +# - Studio Tauri CI runs `tauri build --debug --no-bundle` on +# every studio/src-tauri/** or studio/frontend/** change, which +# compiles the Rust crate (= cargo check + cargo build). +# Each is a stricter check than a parse-only step would be, so a +# fast-fail duplicate here would only burn cache; the dedicated +# workflows already block merges on Rust / TS regressions. + +name: Lint CI + +on: + pull_request: + push: + branches: [main, pip] + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +permissions: + contents: read + +jobs: + source-lint: + name: Source lint (Python + shell + YAML + JSON + safety nets) + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-python@v5 + with: + python-version: '3.12' + cache: 'pip' + + # Pin to match .pre-commit-config.yaml so a CI-only ruff bump + # cannot disagree with what pre-commit accepted. + - run: pip install 'ruff==0.15.12' 'pyyaml>=6' + + - name: Python AST/syntax check (every committed .py must compile) + # python -m compileall uses the same parser the interpreter + # uses, so anything broken here would also crash at + # `import X` on a user's machine. Sub-second across 350+ + # files. Hard gate. + run: | + python -m compileall -q -j 0 \ + unsloth unsloth_cli studio tests cli.py unsloth-cli.py + + - name: Python ruff check (whole repo) + # The narrow rule set in pyproject.toml [tool.ruff.lint] + # selects E9 / F63 / F7 / F82 -- syntax errors, broken + # comparisons, undefined names. The whole repo passes today, + # so this is a hard gate. + run: | + ruff check unsloth unsloth_cli studio tests cli.py unsloth-cli.py + + - name: No leftover debugger / pdb / breakpoint calls + # Catches the "I'll just stick a breakpoint() here" mistake + # before it ships. AST-based so commented-out debugger + # markers don't false-positive (a bare grep would; there + # are three commented `# breakpoint()` markers in + # unsloth/models/rl* today). Sub-second. + run: | + python <<'PY' + import ast, pathlib, sys + + SKIP_PARTS = {".venv", "venv", "build", "dist", ".git", + "unsloth_compiled_cache", "node_modules", + "unsloth.egg-info"} + + bad = [] + scanned = 0 + for path in sorted(pathlib.Path(".").rglob("*.py")): + if any(part in SKIP_PARTS for part in path.parts): + continue + scanned += 1 + try: + tree = ast.parse(path.read_text(encoding="utf-8", errors="replace")) + except SyntaxError: + continue # compileall step above already failed this + for node in ast.walk(tree): + if not isinstance(node, ast.Call): + continue + fn = node.func + if isinstance(fn, ast.Name) and fn.id == "breakpoint": + bad.append((path, node.lineno, "breakpoint()")) + elif (isinstance(fn, ast.Attribute) and fn.attr == "set_trace" + and isinstance(fn.value, ast.Name) + and fn.value.id in {"pdb", "ipdb"}): + bad.append((path, node.lineno, f"{fn.value.id}.set_trace()")) + + if bad: + for path, lineno, what in bad: + print(f"::error file={path},line={lineno}::leftover {what} -- remove before merging") + sys.exit(1) + print(f"no leftover debugger calls (scanned {scanned} files)") + PY + + - name: License-header drift (informational; whole repo) + # The repo currently uses two header families: + # 1. AGPL-3.0-only with `# SPDX-License-Identifier:` + # across studio/ (every committed .py opens with it). + # 2. Apache-2.0 with the + # `# Licensed under the Apache License, Version 2.0` + # preamble across unsloth/, unsloth_cli/, scripts/. + # Either is acceptable. Empty files (mainly empty + # __init__.py) are skipped. We surface the count without + # blocking; cleaning up the missing files is real work and + # belongs in its own PR. + continue-on-error: true + run: | + python <<'PY' + import pathlib + + SPDX = "SPDX-License-Identifier" + APACHE = "Licensed under the Apache License" + SKIP_PARTS = {".venv", "venv", "build", "dist", ".git", + "unsloth_compiled_cache", "node_modules", + "unsloth.egg-info"} + + studio_missing = [] + other_missing = [] + for path in sorted(pathlib.Path(".").rglob("*.py")): + if any(part in SKIP_PARTS for part in path.parts): + continue + text = path.read_text(encoding="utf-8", errors="replace") + if not text.strip(): + continue # empty __init__.py etc. + head = "\n".join(text.splitlines()[:20]) + if SPDX in head or APACHE in head: + continue + if "studio" in path.parts: + studio_missing.append(path) + else: + other_missing.append(path) + + total = len(studio_missing) + len(other_missing) + if total == 0: + print("every committed .py has a recognised license header") + else: + print(f"::warning::{total} Python files are missing both SPDX-License-Identifier " + f"and the Apache-2.0 preamble (studio={len(studio_missing)}, " + f"other={len(other_missing)})") + for path in (studio_missing + other_missing)[:30]: + print(f" {path}") + if total > 30: + print(f" ... and {total - 30} more") + PY + + - name: Shell scripts parse cleanly (`bash -n`) + # Same idea as Python's compileall: parse-only check that + # every committed *.sh would not blow up at `bash script.sh` + # invocation time on a release box. tests/sh/ is the largest + # cluster (the install.sh shape tests). + run: | + shopt -s globstar + fail=0 + for f in $(git ls-files '*.sh'); do + if ! bash -n "$f"; then + echo "::error file=$f::shell parse error" + fail=1 + fi + done + if [ "$fail" -ne 0 ]; then + exit 1 + fi + n=$(git ls-files '*.sh' | wc -l) + echo "$n shell scripts parse cleanly" + + - name: YAML files parse cleanly (yaml.safe_load) + # Catches truncated workflow files, broken indents in + # dependabot.yml / pre-commit configs, etc. Includes + # .github/workflows/*.yml so a typo in the file we just + # added shows up immediately. + run: | + python <<'PY' + import pathlib, sys, yaml + + SKIP_PARTS = {".venv", "venv", "build", "dist", ".git", + "node_modules", "unsloth_compiled_cache", + "unsloth.egg-info"} + + bad = [] + scanned = 0 + for path in sorted(list(pathlib.Path(".").rglob("*.yml")) + + list(pathlib.Path(".").rglob("*.yaml"))): + if any(part in SKIP_PARTS for part in path.parts): + continue + scanned += 1 + try: + with path.open("r", encoding="utf-8") as fh: + list(yaml.safe_load_all(fh)) + except Exception as exc: + bad.append((path, exc)) + + if bad: + for path, exc in bad: + print(f"::error file={path}::YAML parse failed: {exc}") + sys.exit(1) + print(f"{scanned} YAML files parse cleanly") + PY + + - name: JSON files parse cleanly (json.loads) + # Catches malformed package.json, biome.json, etc. Skips: + # - huge npm/bun lockfiles (machine-generated, slow to + # parse, no value). + # - tsconfig*.json: TypeScript convention is JSONC (JSON + # with `/* ... */` comments), which standard json.loads + # rejects. Strip-and-validate would need json5 or a + # hand-rolled comment scrubber for marginal value, since + # `tsc --noEmit` already validates these in Frontend CI. + run: | + python <<'PY' + import fnmatch, json, pathlib, sys + + SKIP_PARTS = {".venv", "venv", "build", "dist", ".git", + "node_modules", "unsloth_compiled_cache", + "unsloth.egg-info"} + SKIP_NAMES = {"package-lock.json", "bun.lock"} + SKIP_PATTERNS = ("tsconfig*.json",) + + bad = [] + scanned = 0 + for path in sorted(pathlib.Path(".").rglob("*.json")): + if any(part in SKIP_PARTS for part in path.parts): + continue + if path.name in SKIP_NAMES: + continue + if any(fnmatch.fnmatch(path.name, pat) for pat in SKIP_PATTERNS): + continue + scanned += 1 + try: + json.loads(path.read_text(encoding="utf-8")) + except Exception as exc: + bad.append((path, exc)) + + if bad: + for path, exc in bad: + print(f"::error file={path}::JSON parse failed: {exc}") + sys.exit(1) + print(f"{scanned} JSON files parse cleanly") + PY + + - name: ruff format drift (informational) + # The canonical formatter is scripts/run_ruff_format.py + # = ruff format + scripts/enforce_kwargs_spacing.py, so plain + # `ruff format --check` reports the kwarg-spacing diff as + # drift. Surface the count for visibility but keep + # non-blocking until the custom pipeline is wired in here. + continue-on-error: true + run: | + ruff format --check unsloth unsloth_cli studio tests cli.py unsloth-cli.py diff --git a/.github/workflows/studio-backend-ci.yml b/.github/workflows/studio-backend-ci.yml index ae10ff1d3c..a1f0cfb9c4 100644 --- a/.github/workflows/studio-backend-ci.yml +++ b/.github/workflows/studio-backend-ci.yml @@ -12,11 +12,14 @@ # - -k 'not llama_cpp_load_progress_live': spawns a real llama.cpp process, # not appropriate for CPU-only runners. # -# Three jobs: +# Two jobs: # - pytest matrix (3.10/3.11/3.12/3.13) over studio/backend/tests # - repo-cpu-tests: auto-discovered tests/ + state-isolated spoof files -# - python-lint: whole-repo Python gate (compileall + ruff + -# debugger-leftover scan) +# +# Whole-repo Python lint (syntax + ruff + debugger-leftover scan) +# moved to the dedicated `Lint CI` workflow (.github/workflows/lint-ci.yml) +# so it fires on every PR rather than only on studio/unsloth/tests +# path changes. name: Backend CI @@ -207,105 +210,3 @@ jobs: echo "::endgroup::" done - python-lint: - # Whole-repo Python gate. Fast (~10-12 s) so it fits the same - # short-feedback budget as the old studio/backend-only ruff job, - # but actually blocks merges on real breakage rather than just - # printing the lint count. - name: Python lint (syntax + ruff + safety nets) - runs-on: ubuntu-latest - timeout-minutes: 5 - steps: - - uses: actions/checkout@v4 - - uses: actions/setup-python@v5 - with: - python-version: '3.12' - cache: 'pip' - # Pin to match .pre-commit-config.yaml so a CI-only ruff bump - # cannot disagree with what pre-commit accepted. - - run: pip install 'ruff==0.15.12' - - - name: AST/syntax check (every committed .py must compile) - # python -m compileall uses the same parser the interpreter - # uses, so anything broken here would also crash at - # `import X` on a user's machine. Sub-second across 350+ - # files. Hard gate. - run: | - python -m compileall -q -j 0 \ - unsloth unsloth_cli studio tests cli.py unsloth-cli.py - - - name: ruff check (whole repo) - # The narrow rule set in pyproject.toml [tool.ruff.lint] - # selects E9 / F63 / F7 / F82 -- syntax errors, broken - # comparisons, undefined names. The whole repo passes today, - # so this is now a hard gate; the prior studio/backend-only - # `|| true` was masking real breakage on the wider tree. - run: | - ruff check unsloth unsloth_cli studio tests cli.py unsloth-cli.py - - - name: No leftover debugger / pdb / breakpoint calls - # Catches the "I'll just stick a breakpoint() here" mistake - # before it ships. AST-based so commented-out debugger - # markers don't false-positive (grep would). Sub-second. - run: | - python <<'PY' - import ast, pathlib, sys - - SKIP_PARTS = {".venv", "venv", "build", "dist", ".git", - "unsloth_compiled_cache", "node_modules"} - - bad = [] - scanned = 0 - for path in sorted(pathlib.Path(".").rglob("*.py")): - if any(part in SKIP_PARTS for part in path.parts): - continue - scanned += 1 - try: - tree = ast.parse(path.read_text(encoding="utf-8", errors="replace")) - except SyntaxError: - continue # compileall step above already fails this - for node in ast.walk(tree): - if not isinstance(node, ast.Call): - continue - fn = node.func - if isinstance(fn, ast.Name) and fn.id == "breakpoint": - bad.append((path, node.lineno, "breakpoint()")) - elif (isinstance(fn, ast.Attribute) and fn.attr == "set_trace" - and isinstance(fn.value, ast.Name) - and fn.value.id in {"pdb", "ipdb"}): - bad.append((path, node.lineno, f"{fn.value.id}.set_trace()")) - - if bad: - for path, lineno, what in bad: - print(f"::error file={path},line={lineno}::leftover {what} -- remove before merging") - sys.exit(1) - print(f"no leftover debugger calls (scanned {scanned} files)") - PY - - - name: SPDX-License-Identifier on every studio/backend .py (warning) - # studio/backend is the only tree where we have a strict - # SPDX policy right now (every committed .py opens with the - # AGPL-3.0-only line). Surface drift without blocking; the - # whole-repo rollout is a separate cleanup. - continue-on-error: true - run: | - missing=$(git ls-files 'studio/backend/*.py' \ - | xargs grep -L "SPDX-License-Identifier" 2>/dev/null || true) - if [ -n "$missing" ]; then - count=$(echo "$missing" | wc -l) - echo "::warning::$count studio/backend Python files are missing SPDX-License-Identifier" - echo "$missing" | head -20 - else - echo "all studio/backend .py files have SPDX-License-Identifier" - fi - - - name: ruff format drift (informational; whole-repo count) - # The repo's canonical formatter is scripts/run_ruff_format.py - # = `ruff format` + scripts/enforce_kwargs_spacing.py. Plain - # `ruff format --check` reports the kwarg-spacing diff as - # drift, which is expected. Surface the count so we can - # track it; keep non-blocking until the custom pipeline is - # wired in here. - continue-on-error: true - run: | - ruff format --check unsloth unsloth_cli studio tests cli.py unsloth-cli.py