diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md new file mode 100644 index 0000000..eb11471 --- /dev/null +++ b/.github/pull_request_template.md @@ -0,0 +1,18 @@ + + +## What & why + + + +## How it was verified + + + +## Checklist + +- [ ] `python tools/thaw_lint.py` is clean +- [ ] `ruff check --select F,E9` passes on the Python I touched +- [ ] `cargo clippy --workspace --exclude thaw-py --all-targets -- -D warnings` is clean (if Rust changed) +- [ ] Tests pass (`pytest tests/ -q` and/or `cargo test`), and new behavior has a test +- [ ] Diff is scoped — no unrelated files +- [ ] **Perf claims ship their receipt** (`site/receipts/*.json` from the run), state what was actually measured, and use a re-validated number — no retracted multipliers, no local paths in receipts diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 68354c1..9a6f7db 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -10,6 +10,69 @@ permissions: contents: read jobs: + lint: + name: lint (clippy, ruff, thaw-lint) + runs-on: ubuntu-22.04 + timeout-minutes: 15 + steps: + # fetch-depth: 0 so the changed-files ruff step can diff against the + # PR base (or the previous commit on a push). + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - uses: actions/setup-python@v5 + with: + python-version: '3.12' + cache: pip + + - name: Install ruff + run: pip install ruff + + # ruff runs on CHANGED Python only. The legacy tree has known debt + # (tracked in the ratchet plan in CONTRIBUTING.md); gating the whole + # tree on it would block every PR. Scoping to the diff means new and + # touched code must be clean without forcing a one-shot tree cleanup + # that would collide with in-flight PRs. --select F,E9 = real bugs + # (undefined names, unused imports, syntax), not style. + - name: ruff (real-bug rules) on changed Python + run: | + set -euo pipefail + BASE="${{ github.event.pull_request.base.sha }}" + if [ -z "$BASE" ]; then BASE="${{ github.event.before }}"; fi + if ! git rev-parse --verify "$BASE" >/dev/null 2>&1; then + BASE="$(git rev-parse HEAD^ 2>/dev/null || true)" + fi + if [ -n "$BASE" ]; then + FILES="$(git diff --name-only --diff-filter=ACMR "$BASE" HEAD -- '*.py' || true)" + else + FILES="$(git ls-files '*.py')" + fi + if [ -z "$FILES" ]; then + echo "No changed Python files — skipping ruff." + exit 0 + fi + echo "Linting changed Python files:" + echo "$FILES" + echo "$FILES" | xargs ruff check --select F,E9 + + # thaw-lint runs on the WHOLE tree: every rule is a regression guard on + # something currently clean (or a non-blocking WARN), so it stays green. + - name: thaw-lint (project footguns) + run: python tools/thaw_lint.py + + - uses: dtolnay/rust-toolchain@stable + with: + components: clippy + + - uses: Swatinem/rust-cache@v2 + + # clippy runs tree-wide with -D warnings: the workspace is clean today, + # so this is a regression guard. thaw-py is excluded (it needs the + # PyO3/CUDA surface that only builds in the wheel workflow). + - name: cargo clippy (deny warnings) + run: cargo clippy --workspace --exclude thaw-py --all-targets -- -D warnings + rust: name: cargo test (${{ matrix.os }}) runs-on: ${{ matrix.os }} diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000..00d120b --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,126 @@ +# Contributing to thaw + +thaw is a small team moving fast. These are the rails that keep PRs flowing +without regressions or hours lost to footguns we've already paid for once. + +## The loop + +1. Branch off `main` (`git checkout -b area/short-description`). +2. Make the change. Keep the diff scoped — one concern per PR. +3. Run the checks below locally. +4. Open a PR. CI runs `lint`, `cargo test`, and `pytest`. Fill in the template. +5. Get a review. Address blocking + high items; land it. + +Never push to `main` directly, and never tag a release without a GPU +validation pass — see [Release](#release). + +## Checks before you push + +```bash +# Project footgun linter (whole tree, fast, no deps) +python tools/thaw_lint.py + +# Real-bug lint on the Python you touched +ruff check --select F,E9 + +# Rust: format + lint + tests +cargo fmt --all +cargo clippy --workspace --exclude thaw-py --all-targets -- -D warnings +cargo test --workspace --exclude thaw-py + +# Python tests (torch/vllm/sglang are mocked — no GPU needed) +pytest tests/ -q +``` + +### What CI gates + +| Gate | Scope | Why scoped this way | +|------|-------|---------------------| +| `cargo clippy -D warnings` | whole workspace (minus `thaw-py`) | clean today — a regression guard | +| `python tools/thaw_lint.py` | whole tree | every ERROR rule is currently green | +| `ruff check --select F,E9` | **changed Python only** | the legacy tree has known debt; see the ratchet below | + +**The ruff ratchet.** We do not gate the whole tree on ruff yet — there are +~36 pre-existing real-bug findings (mostly unused imports) that a one-shot +cleanup would land right on top of in-flight PRs. So ruff runs only on the +files your PR changes: new and touched code must be clean, legacy debt does +not block you. Once the open PRs land, a dedicated `ruff --fix` sweep will +clear the backlog and the gate widens to the whole tree. If you're already in +a legacy file, fixing its findings while you're there is welcome. + +`cargo fmt --check` is **not** gated yet (a full `cargo fmt` touches ~16 files +and would conflict with open Rust PRs). Run `cargo fmt --all` before pushing +anyway; we'll turn the gate on once the tree is normalized. + +## thaw-lint — the project-specific rules + +`tools/thaw_lint.py` encodes the mistakes that have actually cost us time. +`ERROR` fails CI; `WARN` just surfaces. Run `python tools/thaw_lint.py --list` +to see them all. Current ERROR rules: + +- **`import thaw_native`** → the `thaw-native` wheel installs the module + `thaw`. Write `import thaw`. (This one cost real fresh-pod debugging hours.) +- **Personal `@icloud.com` email** in tracked files → the public contact is + `nils@thaw.sh`. +- **`matteso1/thaw` URLs** → the repo lives at `thaw-ai/thaw`; the old one is + archived and 404s clone scripts. +- **Leftover `breakpoint()` / `pdb.set_trace()`** in shipped Python. +- **Retracted speedup multipliers** (`17.2x`, `12.6x`, `9.7x`, `5.9x`) in the + README or `site/` — see [receipt hygiene](#receipt--benchmark-hygiene). +- **Hard-setting `VLLM_ALLOW_INSECURE_SERIALIZATION`** → use + `os.environ.setdefault` so a user can override a security-sensitive flag. + +Sure a line is a false positive? Add `# thaw-lint: allow` to it. New footgun +worth guarding? Add a `Rule` to `tools/thaw_lint.py` with a test — keep ERROR +rules ones the tree is currently clean on, so they're regression guards, not a +backlog. + +## Receipt & benchmark hygiene + +Performance is the pitch, so the bar on numbers is high. + +- **Only claim re-validated numbers.** Default to the most conservative + defensible figure, not the most flattering one. +- **A perf claim ships with its receipt** — a JSON under `site/receipts/` from + the run that produced it, in the same PR. Prose-only numbers rot. +- **Say what the benchmark actually measured.** If it injects synthetic + latency, it measured latency-hiding, not bandwidth — don't let the README + round that up to a throughput claim. +- **Scrub local paths** (`C:\Users\...`, `/home/you/...`) out of committed + receipts. +- Retired numbers live in `docs/ARCHITECTURE.md` so they don't resurface. + +## GPU vs. no-GPU + +A lot of thaw runs without a GPU — the whole inspect/diff/log/rewind surface, +the format readers, the Rust core against `MockCuda`. Those are unit-tested in +CI. The GPU paths (real freeze/restore/fork, TP, KV cache) are validated on a +pod per release; `tests/gpu/` and `benchmarks/` hold those. Don't put a +GPU-requiring test in the default `pytest` path — mock the engine +(`tests/conftest.py` shows how) or gate it under `tests/gpu/`. + +## Ownership boundary (format vs. transport) + +To keep two people out of each other's way, work splits at the file format +line: + +- **Above the line — format, semantics, the verb surface.** The `.thaw` / + `.thawkv` schema, `handle.json`, cross-engine restore, `verify`, lineage, + the fork API contract. Changes here define *what the bytes mean*. +- **Below the line — byte transport & throughput.** Restore DMA, freeze + pipelining, multi-NVMe/TP write paths, parallel cloud fetch. Changes here + move *the same bytes faster*. + +`python/thaw_common/cloud.py` is the one file both sides touch — partition it: +manifest parse/resolve above the line, the transfer executor below it, with a +typed boundary between. When a change spans the line (e.g. shard-at-freeze), +freeze the on-disk schema + golden fixtures **first**, then build transport +against bytes-on-disk rather than against in-flight code. + +## Release + +The `thaw-native` version string lives in **two** independent places that must +match: `Cargo.toml` `[workspace.package]` and **`crates/thaw-py/pyproject.toml`** +(the authoritative file maturin reads — the other crate `Cargo.toml`s inherit +via `version.workspace = true`). Desyncing them has failed a release before. +Bump both, validate on a GPU pod, then tag. diff --git a/Cargo.toml b/Cargo.toml index a109723..3a9faaf 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -36,7 +36,7 @@ version = "0.3.2" edition = "2021" rust-version = "1.75" license = "Apache-2.0" -repository = "https://github.com/matteso1/thaw" +repository = "https://github.com/thaw-ai/thaw" authors = ["Nils Matteson "] # Release profile tuning. `opt-level = 3` is the Rust default for release, diff --git a/demos/bench_cold_cache.sh b/demos/bench_cold_cache.sh index 73bfb08..c68194d 100755 --- a/demos/bench_cold_cache.sh +++ b/demos/bench_cold_cache.sh @@ -8,7 +8,7 @@ # Usage (fresh pod, one line): # export GH_PAT=ghp_xxx HF_TOKEN=hf_xxx # curl -sSL -H "Authorization: token $GH_PAT" \ -# https://raw.githubusercontent.com/matteso1/thaw/main/demos/bench_cold_cache.sh \ +# https://raw.githubusercontent.com/thaw-ai/thaw/main/demos/bench_cold_cache.sh \ # | GH_PAT=$GH_PAT HF_TOKEN=$HF_TOKEN bash # # Or after the repo is cloned: @@ -76,11 +76,11 @@ if [ "$SKIP_SETUP" != "1" ]; then log " thaw/ already exists — pulling latest" cd thaw && git pull else - git clone "https://${GH_PAT}@github.com/matteso1/thaw.git" + git clone "https://${GH_PAT}@github.com/thaw-ai/thaw.git" cd thaw fi # Scrub PAT from remote so it doesn't linger in git config on the pod - git remote set-url origin "https://github.com/matteso1/thaw.git" + git remote set-url origin "https://github.com/thaw-ai/thaw.git" log "=== Installing vLLM + maturin ===" pip install -q vllm "maturin[patchelf]" huggingface_hub 2>&1 | tail -3 | tee -a "$MAIN_LOG" diff --git a/demos/run_quick_test.sh b/demos/run_quick_test.sh index b34139f..3481be3 100755 --- a/demos/run_quick_test.sh +++ b/demos/run_quick_test.sh @@ -10,7 +10,7 @@ # Usage on a fresh RunPod/Lambda pod: # export HF_TOKEN=hf_xxxxx # export GITHUB_PAT=ghp_xxxxx -# curl -sSL https://raw.githubusercontent.com/matteso1/thaw/main/demos/run_quick_test.sh | bash -s -- --setup +# curl -sSL https://raw.githubusercontent.com/thaw-ai/thaw/main/demos/run_quick_test.sh | bash -s -- --setup set -euo pipefail @@ -37,7 +37,7 @@ if $SETUP; then # Clone repo if [ ! -d /workspace/thaw ]; then if [ -n "${GITHUB_PAT:-}" ]; then - git clone "https://${GITHUB_PAT}@github.com/matteso1/thaw.git" /workspace/thaw + git clone "https://${GITHUB_PAT}@github.com/thaw-ai/thaw.git" /workspace/thaw else git clone https://github.com/thaw-ai/thaw.git /workspace/thaw fi diff --git a/demos/run_stress_test.sh b/demos/run_stress_test.sh index b7628ca..5c4d6e0 100755 --- a/demos/run_stress_test.sh +++ b/demos/run_stress_test.sh @@ -61,7 +61,7 @@ if [ -d thaw ]; then cd thaw && git pull else if [ -n "${GITHUB_PAT:-}" ]; then - git clone "https://${GITHUB_PAT}@github.com/matteso1/thaw.git" + git clone "https://${GITHUB_PAT}@github.com/thaw-ai/thaw.git" else git clone https://github.com/thaw-ai/thaw.git fi diff --git a/demos/setup_runpod.sh b/demos/setup_runpod.sh index 9d27bff..c55ff7a 100755 --- a/demos/setup_runpod.sh +++ b/demos/setup_runpod.sh @@ -20,7 +20,7 @@ pip install vllm echo "[2/5] Cloning thaw..." cd /root -[ -d thaw ] && cd thaw && git pull || (git clone https://github.com/matteso1/thaw.git && cd thaw) +[ -d thaw ] && cd thaw && git pull || (git clone https://github.com/thaw-ai/thaw.git && cd thaw) cd /root/thaw pip install -e . diff --git a/docs/BENCHMARKS.md b/docs/BENCHMARKS.md index b4936eb..7d3c471 100644 --- a/docs/BENCHMARKS.md +++ b/docs/BENCHMARKS.md @@ -231,7 +231,7 @@ For why whole-flow speedup numbers depend heavily on whether HF download is in t ```bash # On any CUDA 12+ machine with enough VRAM for the model: -git clone https://github.com/matteso1/thaw.git && cd thaw +git clone https://github.com/thaw-ai/thaw.git && cd thaw pip install "maturin[patchelf]" vllm cd crates/thaw-py && maturin develop --release --features cuda && cd ../.. diff --git a/docs/SETUP.md b/docs/SETUP.md index fe28267..a75fd9e 100644 --- a/docs/SETUP.md +++ b/docs/SETUP.md @@ -91,7 +91,7 @@ Colab doesn't persist state across sessions, so you re-run setup each time. Budg ```python # Cell 1: clone the repo -!git clone https://github.com/matteso1/thaw.git +!git clone https://github.com/thaw-ai/thaw.git %cd thaw # Cell 2: install Rust (cached across sessions if you use a persistent disk, diff --git a/notebooks/gpu_benchmark.ipynb b/notebooks/gpu_benchmark.ipynb index dfea67e..02420d6 100644 --- a/notebooks/gpu_benchmark.ipynb +++ b/notebooks/gpu_benchmark.ipynb @@ -31,7 +31,7 @@ "# [3] Clone repo\n", "from google.colab import userdata\n", "pat = userdata.get('GITHUB_PAT')\n", - "!git clone https://{pat}@github.com/matteso1/thaw.git /content/thaw 2>/dev/null || (cd /content/thaw && git pull)\n", + "!git clone https://{pat}@github.com/thaw-ai/thaw.git /content/thaw 2>/dev/null || (cd /content/thaw && git pull)\n", "!cd /content/thaw && git log --oneline -3" ] }, diff --git a/notebooks/quickstart.ipynb b/notebooks/quickstart.ipynb index c1c48f6..561a5f8 100644 --- a/notebooks/quickstart.ipynb +++ b/notebooks/quickstart.ipynb @@ -22,7 +22,7 @@ "outputs": [], "source": [ "# Install thaw-vllm + fix Colab compatibility\n", - "!pip install git+https://github.com/matteso1/thaw.git -q\n", + "!pip install git+https://github.com/thaw-ai/thaw.git -q\n", "!pip install vllm fastapi uvicorn -q\n", "!pip install \"sympy==1.13.3\" -q" ] diff --git a/scripts/pod-bench-freeze.sh b/scripts/pod-bench-freeze.sh index e77307e..5fe1531 100755 --- a/scripts/pod-bench-freeze.sh +++ b/scripts/pod-bench-freeze.sh @@ -12,7 +12,7 @@ # pattern, and measures wall-clock time for each path. # # Required env: -# GH_PAT GitHub PAT with repo access to matteso1/thaw +# GH_PAT GitHub PAT with repo access to thaw-ai/thaw # # Optional env: # SIZE_MB default 16384 (16 GiB — approx Llama-3-8B weights) @@ -22,7 +22,7 @@ # # Usage on vllm/vllm-openai:latest pod: # export GH_PAT=... -# git clone https://$GH_PAT@github.com/matteso1/thaw.git /workspace/thaw +# git clone https://$GH_PAT@github.com/thaw-ai/thaw.git /workspace/thaw # bash /workspace/thaw/scripts/pod-bench-freeze.sh # # If the checkout already exists, the script will `git fetch + reset --hard` @@ -63,7 +63,7 @@ fi banner "[1/3] Clone + update thaw" mkdir -p "$(dirname "$THAW_DIR")" if [ ! -d "$THAW_DIR/.git" ]; then - git clone "https://$GH_PAT@github.com/matteso1/thaw.git" "$THAW_DIR" + git clone "https://$GH_PAT@github.com/thaw-ai/thaw.git" "$THAW_DIR" else cd "$THAW_DIR" git fetch --quiet origin main diff --git a/scripts/pod-validate-kv.sh b/scripts/pod-validate-kv.sh index 171c42a..2133567 100755 --- a/scripts/pod-validate-kv.sh +++ b/scripts/pod-validate-kv.sh @@ -17,7 +17,7 @@ # overlay the git checkout for the Python code. # # Required env: -# GH_PAT GitHub PAT with repo scope (matteso1/thaw) +# GH_PAT GitHub PAT with repo scope (thaw-ai/thaw) # # Optional env: # HF_TOKEN HF token for gated Llama weights @@ -29,7 +29,7 @@ # Usage (pod: vllm/vllm-openai:latest): # export GH_PAT=... # export HF_TOKEN=... # for gated Llama -# git clone https://$GH_PAT@github.com/matteso1/thaw.git /workspace/thaw +# git clone https://$GH_PAT@github.com/thaw-ai/thaw.git /workspace/thaw # bash /workspace/thaw/scripts/pod-validate-kv.sh # # Re-run to pull latest code: same command. Script fetch+resets origin/main. @@ -93,7 +93,7 @@ fi banner "[2/4] Clone + install thaw (Python, --no-deps)" mkdir -p "$(dirname "$THAW_DIR")" if [ ! -d "$THAW_DIR/.git" ]; then - git clone "https://$GH_PAT@github.com/matteso1/thaw.git" "$THAW_DIR" + git clone "https://$GH_PAT@github.com/thaw-ai/thaw.git" "$THAW_DIR" else cd "$THAW_DIR" git fetch --quiet origin main diff --git a/scripts/pod-validate-s3.sh b/scripts/pod-validate-s3.sh index 8725e36..5b47040 100755 --- a/scripts/pod-validate-s3.sh +++ b/scripts/pod-validate-s3.sh @@ -20,7 +20,7 @@ # Usage (pod template: vllm-latest → vllm/vllm-openai:latest): # export GH_PAT=... AWS_ACCESS_KEY_ID=... AWS_SECRET_ACCESS_KEY=... # export AWS_DEFAULT_REGION=us-east-2 THAW_BUCKET=thaw-test-nils-2026 -# git clone https://$GH_PAT@github.com/matteso1/thaw.git /workspace/thaw +# git clone https://$GH_PAT@github.com/thaw-ai/thaw.git /workspace/thaw # bash /workspace/thaw/scripts/pod-validate-s3.sh # # The script fetches + hard-resets origin/main itself, so to re-run with @@ -69,7 +69,7 @@ fi banner "[2/3] Clone + install thaw (Python, --no-deps)" mkdir -p "$(dirname "$THAW_DIR")" if [ ! -d "$THAW_DIR/.git" ]; then - git clone "https://$GH_PAT@github.com/matteso1/thaw.git" "$THAW_DIR" + git clone "https://$GH_PAT@github.com/thaw-ai/thaw.git" "$THAW_DIR" else cd "$THAW_DIR" git fetch --quiet origin main diff --git a/tests/test_thaw_lint.py b/tests/test_thaw_lint.py new file mode 100644 index 0000000..14425b1 --- /dev/null +++ b/tests/test_thaw_lint.py @@ -0,0 +1,139 @@ +"""Tests for tools/thaw_lint.py — the project-specific footgun linter. + +These exercise the pure core (`lint_text`) on synthetic content, so they never +touch the real tree. The emphasis is on behavior: each rule fires on the thing +it should catch, and — just as important — stays quiet on the look-alikes the +linter critic flagged as false-positive risks (dependency specs, dict keys, +rustup's `curl | sh`, the retracted numbers where docs legitimately retract +them). +""" + +import os +import sys + +REPO_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) +sys.path.insert(0, os.path.join(REPO_ROOT, "tools")) + +from thaw_lint import ERROR, WARN, RULES, lint_text # noqa: E402 + + +def _ids(findings): + return {f.rule.id for f in findings} + + +# --- no-thaw-native-import ------------------------------------------------- + +def test_flags_import_thaw_native(): + findings = lint_text("python/x.py", ["import thaw_native"], RULES) + assert "no-thaw-native-import" in _ids(findings) + + +def test_flags_from_thaw_native_import(): + findings = lint_text("python/x.py", ["from thaw_native import freeze"], RULES) + assert "no-thaw-native-import" in _ids(findings) + + +def test_does_not_flag_thaw_native_dict_key_or_dep_spec(): + # `thaw_native_version` key and the hyphenated dep name are legitimate. + lines = [ + ' "thaw_native_version": meta.version,', + ' native = ["thaw-native>=0.3.2"]', + " backend = thaw_native_slot_pinned", + ] + findings = lint_text("python/x.py", lines, RULES) + assert "no-thaw-native-import" not in _ids(findings) + + +def test_thaw_native_rule_is_python_only(): + # A prose mention of the wheel name in markdown must not trip the rule. + findings = lint_text("README.md", ["pip install thaw-native, then import thaw_native"], RULES) + assert "no-thaw-native-import" not in _ids(findings) + + +# --- no-personal-email ----------------------------------------------------- + +def test_flags_icloud_email(): + findings = lint_text("README.md", ["contact: nilsmatteson@icloud.com"], RULES) + assert "no-personal-email" in _ids(findings) + + +def test_does_not_flag_public_email(): + findings = lint_text("README.md", ["contact: nils@thaw.sh"], RULES) + assert "no-personal-email" not in _ids(findings) + + +# --- no-archived-repo-url -------------------------------------------------- + +def test_flags_archived_repo_url(): + findings = lint_text("docs/SETUP.md", ["git clone https://github.com/matteso1/thaw.git"], RULES) + assert "no-archived-repo-url" in _ids(findings) + + +def test_does_not_flag_current_repo_url(): + findings = lint_text("docs/SETUP.md", ["git clone https://github.com/thaw-ai/thaw.git"], RULES) + assert "no-archived-repo-url" not in _ids(findings) + + +# --- no-debugger ----------------------------------------------------------- + +def test_flags_breakpoint_and_pdb(): + assert "no-debugger" in _ids(lint_text("python/x.py", [" breakpoint()"], RULES)) + assert "no-debugger" in _ids(lint_text("python/x.py", [" import pdb; pdb.set_trace()"], RULES)) + assert "no-debugger" in _ids(lint_text("python/x.py", ["import pdb"], RULES)) + + +# --- no-curl-pipe-bash (WARN) --------------------------------------------- + +def test_curl_pipe_bash_is_a_warning(): + findings = lint_text("demos/x.sh", ["curl -sSL https://example.com/install.sh | bash"], RULES) + hit = [f for f in findings if f.rule.id == "no-curl-pipe-bash"] + assert hit and hit[0].rule.severity == WARN + + +def test_rustup_curl_pipe_sh_is_not_flagged(): + # Upstream `curl … | sh` (rustup) is standard and must stay quiet. + findings = lint_text("demos/x.sh", ["curl --proto '=https' https://sh.rustup.rs | sh -s -- -y"], RULES) + assert "no-curl-pipe-bash" not in _ids(findings) + + +# --- no-retracted-multiplier (scoped to README + site) -------------------- + +def test_flags_retracted_multiplier_in_readme(): + findings = lint_text("README.md", ["70B 2xA100: 546s -> 31.8s (17.2x)"], RULES) + assert "no-retracted-multiplier" in _ids(findings) + + +def test_retracted_multiplier_not_flagged_in_docs(): + # docs/ legitimately references the numbers to retract them. + findings = lint_text("docs/ARCHITECTURE.md", ["Retired numbers that must never resurface: 17.2x"], RULES) + assert "no-retracted-multiplier" not in _ids(findings) + + +# --- no-insecure-serialization-hardset ------------------------------------ + +def test_flags_hardset_serialization_env(): + findings = lint_text("python/x.py", ['os.environ["VLLM_ALLOW_INSECURE_SERIALIZATION"] = "1"'], RULES) + assert "no-insecure-serialization-hardset" in _ids(findings) + + +def test_setdefault_serialization_is_allowed(): + findings = lint_text( + "python/x.py", + ['os.environ.setdefault("VLLM_ALLOW_INSECURE_SERIALIZATION", "1")'], + RULES, + ) + assert "no-insecure-serialization-hardset" not in _ids(findings) + + +# --- suppression + severity wiring ---------------------------------------- + +def test_inline_suppression_marker_skips_line(): + line = "import thaw_native # thaw-lint: allow" + assert lint_text("python/x.py", [line], RULES) == [] + + +def test_all_error_rules_have_nonempty_messages(): + # Guard against a rule shipped with an empty operator-facing message. + for rule in RULES: + assert rule.severity in (ERROR, WARN) + assert rule.message.strip() diff --git a/tools/thaw_lint.py b/tools/thaw_lint.py new file mode 100644 index 0000000..01987c8 --- /dev/null +++ b/tools/thaw_lint.py @@ -0,0 +1,285 @@ +#!/usr/bin/env python3 +"""thaw-lint — the project-specific footgun linter. + +ruff and clippy catch generic style and bug classes. This catches the things +that are specific to *thaw* — the mistakes that have actually cost this team +hours, the ones a general linter has no way to know about. It encodes the +"Critical constraints" section of CLAUDE.md as executable rules so the +knowledge survives in CI instead of in one person's head. + +It is deliberately small and dependency-free: it walks the git-tracked files +(so it respects .gitignore and never wanders into build output), scans each +line against a short list of rules, and prints findings grouped by severity. + +Severity: + ERROR fails CI (exit 1). Reserved for things that are *currently* clean and + must stay clean — a regression guard, not a backlog. + WARN surfaces the finding but never fails the build. For preferences and + soft guidance where existing violations are acceptable. + +Inline suppression: + Put `thaw-lint: allow` anywhere on a line to skip every rule for that line. + (This file uses it on the lines that *define* the banned patterns, so the + linter doesn't flag its own source.) + +Usage: + python tools/thaw_lint.py # lint the whole tracked tree + python tools/thaw_lint.py path ... # lint specific files/dirs + python tools/thaw_lint.py --list # show the rules and exit +""" + +from __future__ import annotations + +import argparse +import os +import re +import subprocess +import sys +from dataclasses import dataclass, field +from typing import Iterable, List, Optional, Sequence + +# Marker a line can carry to opt out of all rules, the way a linter ignore +# comment does. +SUPPRESS_MARKER = "thaw-lint: allow" + +ERROR = "ERROR" +WARN = "WARN" + + +@dataclass(frozen=True) +class Rule: + """One lint rule. + + `pattern` is matched against each line. A rule only applies to files whose + suffix is in `suffixes` (empty means "any text file"). `skip_paths` are + repo-relative paths the rule never looks at — used for fixtures and for + this linter's own source, which necessarily contains the banned strings. + """ + + id: str + severity: str + message: str + pattern: "re.Pattern[str]" + suffixes: frozenset = field(default_factory=frozenset) + skip_paths: frozenset = field(default_factory=frozenset) + only_prefixes: frozenset = field(default_factory=frozenset) + + def applies_to(self, rel_path: str) -> bool: + if rel_path in self.skip_paths: + return False + if self.only_prefixes and not any( + rel_path == p or rel_path.startswith(p) for p in self.only_prefixes + ): + return False + if not self.suffixes: + return True + return any(rel_path.endswith(suffix) for suffix in self.suffixes) + + +@dataclass(frozen=True) +class Finding: + rule: Rule + path: str + line_no: int + line: str + + +# The linter's own source and its test fixtures contain the banned patterns by +# necessity. Skip them globally rather than peppering every line with markers. +_SELF = frozenset({"tools/thaw_lint.py", "tests/test_thaw_lint.py"}) + +PY = frozenset({".py"}) +SHELL_DOC = frozenset({".sh", ".md"}) + + +RULES: List[Rule] = [ + # The PyPI package `thaw-native` installs a module literally named `thaw`. + # Writing `import thaw_native` fails with ModuleNotFoundError and has cost + # this team hours of fresh-pod debugging. The dependency spec + # `thaw-native>=...` (hyphen, in pyproject) is a different string and is + # not matched here. + Rule( + id="no-thaw-native-import", + severity=ERROR, + message="Import the module as `import thaw`, not `import thaw_native` " + "(the thaw-native wheel installs the module `thaw`).", + pattern=re.compile(r"^\s*(?:from|import)\s+thaw_native\b"), # thaw-lint: allow + suffixes=PY, + skip_paths=_SELF, + ), + # The public contact is nils@thaw.sh. A personal @icloud.com address must + # never ship in README, site, Cargo.toml, or outreach assets. + Rule( + id="no-personal-email", + severity=ERROR, + message="Use the public contact nils@thaw.sh, not a personal @icloud.com address.", + pattern=re.compile(r"[\w.+-]+@icloud\.com", re.IGNORECASE), # thaw-lint: allow + skip_paths=_SELF, + ), + # The repo lives at github.com/thaw-ai/thaw. The old matteso1/thaw is + # archived; any link there sends users (and clone scripts) to a dead repo. + Rule( + id="no-archived-repo-url", + severity=ERROR, + message="Point at github.com/thaw-ai/thaw — matteso1/thaw is archived.", + pattern=re.compile(r"matteso1/thaw"), # thaw-lint: allow + skip_paths=_SELF, + ), + # A stray debugger breakpoint in shipped Python hangs CI or a user's + # process with no output. ruff's T100 covers this too, but we are not + # gating full ruff on the legacy tree yet, so guard it here. + Rule( + id="no-debugger", + severity=ERROR, + message="Remove the leftover debugger (breakpoint()/pdb.set_trace()/import pdb).", + pattern=re.compile(r"\bbreakpoint\s*\(\s*\)|\bpdb\.set_trace\b|^\s*import\s+pdb\b"), # thaw-lint: allow + suffixes=PY, + skip_paths=_SELF, + ), + # Piping curl into bash is the install pattern this project deliberately + # avoids for its OWN scripts (prefer `git clone` + run from the checkout). + # Require a literal `bash` after the pipe: upstream `curl … | sh` installers + # (rustup, etc.) are standard and are not what we object to. + Rule( + id="no-curl-pipe-bash", + severity=WARN, + message="Prefer `git clone` + run from the checkout over curl-pipe-bash.", + pattern=re.compile(r"curl\b[^\n|]*\|\s*(?:sudo\s+)?bash\b"), # thaw-lint: allow + suffixes=SHELL_DOC, + skip_paths=_SELF, + ), + # Retracted speedup multipliers must never resurface on investor-facing + # surfaces. CLAUDE.md and docs/ARCHITECTURE.md list these as pod-specific + # or never-reproduced; docs/ may discuss them to retract them, but the + # README and the marketing site must stay clean. Anchored to a trailing + # x/× so it cannot fire on bare digits in data/log files. + Rule( + id="no-retracted-multiplier", + severity=ERROR, + message="Retracted speedup (17.2x / 12.6x / 9.7x / 5.9x) must not appear " + "in the README or site — see CLAUDE.md 'Claims NOT validated'.", + pattern=re.compile(r"(?:17\.2|12\.6|9\.7|5\.9) ?[x×]"), # thaw-lint: allow + only_prefixes=frozenset({"README.md", "site/"}), + skip_paths=_SELF, + ), + # VLLM_ALLOW_INSECURE_SERIALIZATION is set on import so collective_rpc can + # ship a function object. It MUST go through os.environ.setdefault so a + # user's explicit choice wins — a hard assignment silently overrides a + # security-sensitive flag. CLAUDE.md: "Setdefault means users can override." + Rule( + id="no-insecure-serialization-hardset", + severity=ERROR, + message="Set VLLM_ALLOW_INSECURE_SERIALIZATION via os.environ.setdefault, " + "never a hard assignment — let the user override.", + pattern=re.compile( # thaw-lint: allow + r"os\.environ\[[\"']VLLM_ALLOW_INSECURE_SERIALIZATION[\"']\]\s*=" + ), + suffixes=PY, + skip_paths=_SELF, + ), +] + + +def repo_root() -> str: + try: + out = subprocess.check_output( + ["git", "rev-parse", "--show-toplevel"], stderr=subprocess.DEVNULL + ) + return out.decode().strip() + except (subprocess.CalledProcessError, FileNotFoundError): + return os.getcwd() + + +def tracked_files(root: str) -> List[str]: + """Repo-relative paths of all git-tracked files.""" + out = subprocess.check_output(["git", "-C", root, "ls-files"]) + return out.decode().splitlines() + + +def _read_lines(abs_path: str) -> Optional[List[str]]: + try: + with open(abs_path, "r", encoding="utf-8", errors="strict") as fh: + return fh.read().splitlines() + except (UnicodeDecodeError, FileNotFoundError, IsADirectoryError, PermissionError): + # Binary or unreadable file — nothing textual to lint. + return None + + +def lint_text(rel_path: str, lines: Sequence[str], rules: Sequence[Rule]) -> List[Finding]: + """Pure core: scan already-read lines. Used directly by the tests.""" + findings: List[Finding] = [] + applicable = [r for r in rules if r.applies_to(rel_path)] + if not applicable: + return findings + for i, line in enumerate(lines, start=1): + if SUPPRESS_MARKER in line: + continue + for rule in applicable: + if rule.pattern.search(line): + findings.append(Finding(rule, rel_path, i, line.rstrip())) + return findings + + +def lint_paths(root: str, rel_paths: Iterable[str], rules: Sequence[Rule]) -> List[Finding]: + findings: List[Finding] = [] + for rel in rel_paths: + lines = _read_lines(os.path.join(root, rel)) + if lines is None: + continue + findings.extend(lint_text(rel, lines, rules)) + return findings + + +def _expand_args(root: str, args: Sequence[str]) -> List[str]: + """Turn CLI path args into a set of repo-relative tracked files.""" + tracked = tracked_files(root) + if not args: + return tracked + tracked_set = set(tracked) + selected: List[str] = [] + for arg in args: + rel = os.path.relpath(os.path.abspath(arg), root) + if rel in tracked_set: + selected.append(rel) + else: + # Treat as a directory prefix. + prefix = rel.rstrip("/") + "/" + selected.extend(t for t in tracked if t.startswith(prefix)) + return selected + + +def main(argv: Optional[Sequence[str]] = None) -> int: + parser = argparse.ArgumentParser(description="thaw project-specific footgun linter") + parser.add_argument("paths", nargs="*", help="files or dirs to lint (default: whole tree)") + parser.add_argument("--list", action="store_true", help="list rules and exit") + ns = parser.parse_args(argv) + + if ns.list: + for rule in RULES: + scope = ",".join(sorted(rule.suffixes)) or "all text" + print(f"[{rule.severity:5}] {rule.id:24} ({scope}) — {rule.message}") + return 0 + + root = repo_root() + rel_paths = _expand_args(root, ns.paths) + findings = lint_paths(root, rel_paths, RULES) + + errors = [f for f in findings if f.rule.severity == ERROR] + warns = [f for f in findings if f.rule.severity == WARN] + + for f in warns: + print(f"WARN {f.path}:{f.line_no}: [{f.rule.id}] {f.rule.message}") + for f in errors: + print(f"ERROR {f.path}:{f.line_no}: [{f.rule.id}] {f.rule.message}") + print(f" {f.line.strip()}") + + if errors: + print(f"\nthaw-lint: {len(errors)} error(s), {len(warns)} warning(s). " + f"Fix the errors, or add `{SUPPRESS_MARKER}` to a line you are sure about.") + return 1 + print(f"thaw-lint: clean ({len(warns)} warning(s)).") + return 0 + + +if __name__ == "__main__": + sys.exit(main())