Skip to content

ci(ruff): pin ruff to the lockfile and stop CI auto-fixing - #704

Merged
thomaspinder merged 2 commits into
mainfrom
fix/ruff-ci-drift
Jul 26, 2026
Merged

ci(ruff): pin ruff to the lockfile and stop CI auto-fixing#704
thomaspinder merged 2 commits into
mainfrom
fix/ruff-ci-drift

Conversation

@thomaspinder

Copy link
Copy Markdown
Owner

Checklist

  • I've formatted the new code by running uv run poe format before committing.
  • I've added tests for new code.
  • I've added docstrings for the new code.

Description

main is currently red on ruff through no change of its own, and #702 / #703 inherit it. Fixing that surfaced a second, quieter problem in the same workflow.

1. Ruff was floating, not pinned

chartboost/ruff-action@v1 installs the latest ruff on every run. So a ruff release can turn every open PR red without anyone having committed anything — which is exactly what happened. Ruff 0.16.0 stabilised PLR0917 (too-many-positional-arguments) out of preview, and since pyproject.toml selects the whole "PLR" category, it was adopted the moment it shipped.

Reproduced on an unmodified main:

ruff 0.14.14 (uv.lock, what `poe lint` uses) → All checks passed
ruff 0.16.0  (latest, what CI installed)     → 10 errors

The workflow now installs uv and runs the locked ruff, mirroring tests.yml. uv.lock becomes the single source of truth, so local and CI cannot disagree again.

2. CI could have silently auto-fixed violations

The obvious fix is to point CI at poe lint — but that runs ruff check --fix, which mutates the checkout and then reports only what's left. Every auto-fixable violation would be quietly repaired in the ephemeral runner and pass.

Not theoretical. Planting an unsorted import block:

poe lint     → exit 0   (silently fixed it)
poe lint-ci  → exit 1   (I001 reported, file untouched)

So this adds a lint-ci task using --no-fix for CI. poe lint keeps --fix for local ergonomics and is unchanged, as is the all-tests sequence.

3. PLR0917 ignored alongside PLR0913

PLR0913 (too many arguments) was already ignored; PLR0917 is its positional-only sibling and wasn't. They flag the same constructors:

rule counts ignores
PLR0913 all parameters, including keyword-only self/cls
PLR0917 only parameters passable positionally self/cls, anything after *

Kernels, likelihoods and variational families are deliberately constructed positionally with defaults — RBF(active_dims, lengthscale, variance, n_dims, compute_engine) is 6, over the limit of 5. Satisfying the rule properly means inserting * into ten public constructors, which breaks every positional caller. Ignoring it is consistent with the existing PLR0913 decision.

Verification

uv run poe lint-ci                                     → All checks passed  (ruff 0.14.14)
uvx ruff@0.16.0 check --no-fix ./gpjax ./tests ./benchmarks → All checks passed

The second line is the one that matters: the version that broke CI now passes.

Note on job runtime

The lint job goes from ~11s to a uv sync --frozen (cached via setup-uv). That's the price of resolving ruff from the lockfile instead of the network. If it proves too slow, the alternative is astral-sh/ruff-action@v3 with an explicit version: — faster, but reintroduces a second place to keep the ruff version in sync.

Merging this unblocks #702 and #703.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Bj9k5fnAZ8JzD4Rg3HMDMj

Two independent problems, both of which let CI diverge from local.

1. Floating ruff. `chartboost/ruff-action@v1` installs the latest ruff on
   every run, so a release can turn every open PR red with no commit having
   been made. That is what happened: ruff 0.16.0 stabilised PLR0917
   (too-many-positional-arguments) out of preview, and since pyproject
   selects the whole "PLR" category it was picked up immediately. Ten
   pre-existing violations on an unmodified main; the pinned 0.14.14 from
   uv.lock that `poe lint` uses reports none.

   The workflow now installs uv and runs the locked ruff, mirroring
   tests.yml. uv.lock becomes the single source of truth.

2. CI could auto-fix. `poe lint` runs `ruff check --fix`, which mutates the
   checkout and then reports only what is left. Pointing CI at it would mask
   every auto-fixable violation. Demonstrated with an unsorted import block:
   `poe lint` exits 0 (having silently fixed it) where `poe lint-ci` exits 1.

   Adds a `lint-ci` task using `--no-fix`. `poe lint` keeps `--fix` for local
   ergonomics and is unchanged.

Also ignores PLR0917 alongside the PLR0913 already ignored. Kernels,
likelihoods and variational families are constructed positionally with
defaults; making those arguments keyword-only to satisfy the rule would be a
breaking change to public constructors.

Verified: ruff 0.16.0 now reports no errors on gpjax, tests and benchmarks.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bj9k5fnAZ8JzD4Rg3HMDMj
@thomaspinder
thomaspinder enabled auto-merge (squash) July 26, 2026 14:16
@thomaspinder
thomaspinder merged commit b378847 into main Jul 26, 2026
19 checks passed
@thomaspinder thomaspinder mentioned this pull request Jul 26, 2026
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant