ci: lint gates (clippy, ruff, thaw-lint) + contributor rails#94
Conversation
Two engineers are now shipping PRs, so add deterministic gates and a project-specific linter that keep the pipeline clean without a one-shot tree cleanup that would collide with in-flight work. lint CI job (.github/workflows/test.yml): - cargo clippy -D warnings, tree-wide (the workspace is clean today, so this is a regression guard) - thaw-lint, tree-wide (every ERROR rule is currently green) - ruff --select F,E9 on CHANGED Python only — new/touched code must be clean; the ~36 legacy real-bug findings don't block the open PRs. The ratchet to a tree-wide gate is documented in CONTRIBUTING. tools/thaw_lint.py: a small, dependency-free regex linter over `git ls-files` with ERROR (fails CI) / WARN tiers and `# thaw-lint: allow` inline suppression. Rules encode CLAUDE.md's "cost me hours" constraints: import thaw_native, personal @iCloud email, archived matteso1/thaw URL, leftover debugger, retracted speedup multipliers in README/site, and hard-set VLLM_ALLOW_INSECURE_SERIALIZATION. 17 unit tests cover detection plus the false-positive guards (dep specs, dict keys, rustup curl|sh, docs that legitimately retract numbers). Fix the linter's first real catch: 20 stale matteso1/thaw URLs (the repo moved to thaw-ai/thaw; the old one is archived) across Cargo.toml, demo + pod scripts, docs, and notebooks. CONTRIBUTING.md + PR template: the branch/PR loop, the gates and how to run them locally, the ruff ratchet, receipt/benchmark hygiene, the format-vs-transport ownership boundary, and the release version-file gotcha.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
📝 WalkthroughWalkthroughThe PR adds a new thaw-lint script, wires it into tests and CI, expands contribution guidance, and updates repository references in docs, notebooks, demos, scripts, and metadata to ChangesPR guidance and lint enforcement
Repository reference migration
Sequence Diagram(s)sequenceDiagram
participant Runner as GitHub Actions runner
participant ThawLint as tools/thaw_lint.py
participant Git as git
participant Files as git-tracked files
Runner->>ThawLint: python tools/thaw_lint.py
ThawLint->>Git: git rev-parse --show-toplevel
Git-->>ThawLint: repo root
ThawLint->>Git: git ls-files
Git-->>ThawLint: tracked paths
loop each tracked file
ThawLint->>Files: read and scan lines
end
ThawLint-->>Runner: WARN/ERROR findings and exit code
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
demos/setup_runpod.sh (1)
23-23: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value
A && B || Cis not if-then-else here.If the directory exists and
cd thawsucceeds butgit pullfails (e.g., transient network error), the||branch still runsgit cloneinto the existing checkout, which then fails. Consider an explicitif.♻️ Explicit if/else
-[ -d thaw ] && cd thaw && git pull || (git clone https://github.com/thaw-ai/thaw.git && cd thaw) +if [ -d thaw ]; then + cd thaw && git pull +else + git clone https://github.com/thaw-ai/thaw.git && cd thaw +fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@demos/setup_runpod.sh` at line 23, The current shell one-liner in the setup logic uses an unsafe A && B || C pattern, so a failed git pull after cd thaw can incorrectly fall through to git clone inside an existing checkout. Refactor the setup flow around the thaw checkout into an explicit if/else that handles the “directory exists” and “clone vs update” cases separately, preserving the existing thaw path and git pull behavior without attempting to clone into an existing repository.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/test.yml:
- Around line 20-24: The workflow uses floating action versions, so update every
action reference in the test workflow to a pinned commit SHA instead of tags
like actions/checkout@v4, actions/setup-python@v5,
dtolnay/rust-toolchain@stable, and Swatinem/rust-cache@v2. Apply the same fix to
all repeated workflow sections in this file so every uses entry is locked to a
specific commit hash.
- Around line 29-31: The Ruff installation step in the test workflow is unpinned
and can drift as new releases are published, changing lint behavior
unexpectedly. Update the install command in the workflow to use an explicit Ruff
version so the CI environment is stable; the relevant step is the “Install ruff”
entry in the test workflow.
In `@CONTRIBUTING.md`:
- Around line 65-66: The CONTRIBUTING.md reference is using the archived repo
slug that triggers the no-archived-repo-url lint gate. Update the wording in the
affected sentence to avoid the literal deprecated token, or add the appropriate
inline suppression if this doc must intentionally mention it; make the change in
the CONTRIBUTING.md guidance text that references the repo slug so thaw-lint no
longer flags it.
In `@demos/run_quick_test.sh`:
- Line 13: The bootstrap curl-pipe-bash line in run_quick_test.sh is
intentionally allowed, so suppress the thaw-lint no-curl-pipe-bash finding
inline on that exact command. Update the commented bootstrap invocation in
run_quick_test.sh using the linter’s supported suppression syntax so the PR’s
lint job stays clean while preserving the documented fresh-pod setup behavior.
---
Nitpick comments:
In `@demos/setup_runpod.sh`:
- Line 23: The current shell one-liner in the setup logic uses an unsafe A && B
|| C pattern, so a failed git pull after cd thaw can incorrectly fall through to
git clone inside an existing checkout. Refactor the setup flow around the thaw
checkout into an explicit if/else that handles the “directory exists” and “clone
vs update” cases separately, preserving the existing thaw path and git pull
behavior without attempting to clone into an existing repository.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3a20efa7-34e5-4d0b-ab43-2cbecbdabbd1
📒 Files selected for processing (17)
.github/pull_request_template.md.github/workflows/test.ymlCONTRIBUTING.mdCargo.tomldemos/bench_cold_cache.shdemos/run_quick_test.shdemos/run_stress_test.shdemos/setup_runpod.shdocs/BENCHMARKS.mddocs/SETUP.mdnotebooks/gpu_benchmark.ipynbnotebooks/quickstart.ipynbscripts/pod-bench-freeze.shscripts/pod-validate-kv.shscripts/pod-validate-s3.shtests/test_thaw_lint.pytools/thaw_lint.py
| - uses: actions/checkout@v4 | ||
| with: | ||
| fetch-depth: 0 | ||
|
|
||
| - uses: actions/setup-python@v5 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify non-SHA-pinned actions in workflows
rg -n 'uses:\s+[[:alnum:]_.-]+/[[:alnum:]_.-]+@[^[:space:]]+' .github/workflows \
| rg -v '@[0-9a-fA-F]{40}$'Repository: thaw-ai/thaw
Length of output: 1642
Pin all workflow actions to commit SHAs. actions/checkout@v4, actions/setup-python@v5, dtolnay/rust-toolchain@stable, and Swatinem/rust-cache@v2 are all still floating refs in this workflow; the same pattern also appears at lines 64-68, 90-94, and 111-113.
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 20-22: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 20-20: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 24-24: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/test.yml around lines 20 - 24, The workflow uses floating
action versions, so update every action reference in the test workflow to a
pinned commit SHA instead of tags like actions/checkout@v4,
actions/setup-python@v5, dtolnay/rust-toolchain@stable, and
Swatinem/rust-cache@v2. Apply the same fix to all repeated workflow sections in
this file so every uses entry is locked to a specific commit hash.
Source: Linters/SAST tools
| - name: Install ruff | ||
| run: pip install ruff | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for unpinned ruff installs across workflows/docs/scripts
rg -n "pip install .*ruff(\\s|$)" .github/workflows CONTRIBUTING.md .github/pull_request_template.mdRepository: thaw-ai/thaw
Length of output: 210
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== .github/workflows/test.yml (around install step) ==\n'
cat -n .github/workflows/test.yml | sed -n '1,120p'
printf '\n== ruff references in repo ==\n'
rg -n --hidden --glob '!.git' --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' '\bruff\b|ruff==' .Repository: thaw-ai/thaw
Length of output: 6580
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== .github/workflows/test.yml (around install step) ==\n'
cat -n .github/workflows/test.yml | sed -n '1,120p'
printf '\n== ruff references in repo ==\n'
rg -n --hidden --glob '!.git' --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' '\bruff\b|ruff==' .Repository: thaw-ai/thaw
Length of output: 6580
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== .github/workflows/test.yml (around install step) ==\n'
cat -n .github/workflows/test.yml | sed -n '1,120p'
printf '\n== ruff references in repo ==\n'
rg -n --hidden --glob '!.git' --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' '\bruff\b|ruff==' .Repository: thaw-ai/thaw
Length of output: 6580
Pin Ruff in .github/workflows/test.yml:29-30. pip install ruff resolves the latest release at runtime, so the lint gate can change behavior over time. Use an explicit version instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/test.yml around lines 29 - 31, The Ruff installation step
in the test workflow is unpinned and can drift as new releases are published,
changing lint behavior unexpectedly. Update the install command in the workflow
to use an explicit Ruff version so the CI environment is stable; the relevant
step is the “Install ruff” entry in the test workflow.
| - **`matteso1/thaw` URLs** → the repo lives at `thaw-ai/thaw`; the old one is | ||
| archived and 404s clone scripts. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
CONTRIBUTING.md currently violates the new no-archived-repo-url gate.
This exact line is tripping CI (thaw-lint), so the PR cannot pass as-is. Reword to avoid the literal token or add inline suppression where you intentionally document the deprecated slug.
Suggested fix
-- **`matteso1/thaw` URLs** → the repo lives at `thaw-ai/thaw`; the old one is
+- **Archived repo URLs** (old owner/repo slug) → the repo lives at `thaw-ai/thaw`; the old one is
archived and 404s clone scripts.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - **`matteso1/thaw` URLs** → the repo lives at `thaw-ai/thaw`; the old one is | |
| archived and 404s clone scripts. | |
| - **Archived repo URLs** (old owner/repo slug) → the repo lives at `thaw-ai/thaw`; the old one is | |
| archived and 404s clone scripts. |
🧰 Tools
🪛 GitHub Actions: test / 0_lint (clippy, ruff, thaw-lint).txt
[error] 65-65: thaw-lint (no-archived-repo-url): Point at github.com/thaw-ai/thaw — matteso1/thaw is archived. 'matteso1/thaw' URLs should be updated to 'thaw-ai/thaw'.
🪛 GitHub Actions: test / lint (clippy, ruff, thaw-lint)
[error] 65-65: thaw-lint failed ([no-archived-repo-url]). Point at github.com/thaw-ai/thaw — matteso1/thaw is archived.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@CONTRIBUTING.md` around lines 65 - 66, The CONTRIBUTING.md reference is using
the archived repo slug that triggers the no-archived-repo-url lint gate. Update
the wording in the affected sentence to avoid the literal deprecated token, or
add the appropriate inline suppression if this doc must intentionally mention
it; make the change in the CONTRIBUTING.md guidance text that references the
repo slug so thaw-lint no longer flags it.
Source: Pipeline failures
| # 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 |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
thaw-lint flags this curl-pipe-bash bootstrap line.
The new thaw-lint gate emits [no-curl-pipe-bash] here. This is the documented fresh-pod bootstrap, so the pattern is intentional. To keep the PR's own lint job clean, add the inline suppression the linter supports.
🔧 Add inline suppression
-# curl -sSL https://raw.githubusercontent.com/thaw-ai/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 # thaw-lint: allow📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # curl -sSL https://raw.githubusercontent.com/thaw-ai/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 # thaw-lint: allow |
🧰 Tools
🪛 GitHub Actions: test / 0_lint (clippy, ruff, thaw-lint).txt
[warning] 13-13: thaw-lint: [no-curl-pipe-bash] Prefer git clone + run from the checkout over curl-pipe-bash.
🪛 GitHub Actions: test / lint (clippy, ruff, thaw-lint)
[warning] 13-13: thaw-lint warning ([no-curl-pipe-bash]): Prefer git clone + run from the checkout over curl-pipe-bash.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@demos/run_quick_test.sh` at line 13, The bootstrap curl-pipe-bash line in
run_quick_test.sh is intentionally allowed, so suppress the thaw-lint
no-curl-pipe-bash finding inline on that exact command. Update the commented
bootstrap invocation in run_quick_test.sh using the linter’s supported
suppression syntax so the PR’s lint job stays clean while preserving the
documented fresh-pod setup behavior.
Source: Pipeline failures
1 participant
Now that two of us are shipping PRs, this adds deterministic CI gates + a project-specific linter so the pipeline stays clean — without a one-shot tree cleanup that would collide with the open PRs (#87, #93).
lintCI jobcargo clippy -D warningstree-wide — the workspace is clean today, so this is a pure regression guard.thaw-linttree-wide — every ERROR rule is currently green.ruff --select F,E9on changed Python only — new/touched code must be clean; the ~36 pre-existing real-bug findings (mostly unused imports) don't block anyone. The ratchet to a tree-wide gate is documented inCONTRIBUTING.md. (cargo fmt --checkis intentionally deferred — a fullcargo fmttouches ~16 files and would conflict with open Rust PRs.)tools/thaw_lint.py— the project-specific footgun linterSmall, dependency-free, scans
git ls-files(so it never wanders into gitignored build output). Two tiers — ERROR fails CI, WARN just surfaces — plus# thaw-lint: allowinline suppression. Rules encode the "cost me hours" constraints fromCLAUDE.md:import thaw_native(the wheel installs the modulethaw)@icloud.comemail in tracked filesmatteso1/thawURLbreakpoint()/pdb.set_trace()17.2x/12.6x/9.7x/5.9x) in README/siteVLLM_ALLOW_INSECURE_SERIALIZATION(must besetdefault)curl … | bashin our own scripts17 unit tests cover detection and the false-positive guards (dependency specs,
thaw_native_versiondict keys, rustup'scurl … | sh, docs that legitimately retract the numbers).The linter's first real catch
Fixed 20 stale
matteso1/thawURLs (the repo moved tothaw-ai/thaw; the old one is archived and 404s clone scripts) acrossCargo.toml, demo + pod scripts, docs, and notebooks.Contributor rails
CONTRIBUTING.md+ a PR template: the branch/PR loop, the gates and how to run them locally, the ruff ratchet, receipt/benchmark hygiene, the format-vs-transport ownership boundary, and the release version-file gotcha.Verification
python tools/thaw_lint.py→ clean (2 expected WARNs)pytest tests/test_thaw_lint.py→ 17 passedruff check --select F,E9on changed files → cleancargo clippy --workspace --exclude thaw-py --all-targets→ zero warnings locallySummary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests