Skip to content

Add ruff as a Python lint gate in CI #239

Description

@alex-clickhouse

Proposal

Add ruff as a Python lint gate in CI. Land the lint rules now; leave the tree-wide formatting rewrite as an open question, since that is the part with a real cost.

The repo has no Python lint or format tooling today: pyproject.toml has no [tool.*] sections and there is no .pre-commit-config.yaml. CI runs pytest and the Vite build only.

Ruff is the default choice for this: one binary in place of flake8 + isort + pyupgrade + much of pylint, with a black-compatible formatter included. CI already uses uv, so uvx ruff adds no new dependency management.

Baseline

Measured with ruff 0.16.1 on main (2008b70), 311 Python files under nerve/, tests/, scripts/:

Scope Findings
Proposed gate — default rules (E4, E7, E9, F) plus I 160 (135 auto-fixable)
Broader opt-in families (B, SIM, UP, RUF, S, BLE, PERF, …) ~950
ruff format 221 of 311 files rewritten

Phase 1 — lint gate, now

ruff check --fix clears 135 of the 160, touching 84 files and 372 lines (+170/−202), nearly all of it import blocks from I. The remaining 25 need hands and are all small and local: 13 F841 unused variable, 4 E741 ambiguous name l, 4 E702 semicolon statements, 3 E731 lambda assignment, 1 F821 undefined name.

Open branches may hit small mechanical conflicts in import order. That is a different scale from the formatter's 221-file rewrite, and it is why I is in this phase and the formatter is not.

The F821 is a fair illustration of what the gate buys. nerve/cli.py:1820 annotates list[tuple[str, Any]] without importing Any. It does not fail today, because from __future__ import annotations makes the annotation a string that is never evaluated — but it breaks under typing.get_type_hints(), and it means that annotation has never been checked by anything.

Config:

[tool.ruff]
target-version = "py313"

[tool.ruff.lint]
select = ["E4", "E7", "E9", "F", "I"]

[tool.ruff.lint.per-file-ignores]
# Re-exports in package __init__ files are intentional, not dead imports.
"__init__.py" = ["F401"]

The per-file-ignores entry is a guard rather than a fix for anything currently broken: the autofix reorders imports in seven __init__.py files today without removing any re-export, but without that line a future re-export would be silently stripped.

CI, as a separate job rather than a step in backend-tests, so a lint failure does not mask test results — same reasoning as the existing split between the fast and slow test steps:

  lint:
    name: Lint (ruff)
    runs-on: ubuntu-latest
    timeout-minutes: 5
    steps:
      - uses: actions/checkout@v5
      - uses: astral-sh/setup-uv@v6
        with:
          enable-cache: true
          cache-dependency-glob: "pyproject.toml"
      - run: uvx ruff@0.16.1 check nerve tests scripts

The ruff version is pinned deliberately. Rule sets change between releases, and an unpinned linter turns unrelated PRs red on a ruff release day. This matches the existing upper bounds on the test extra.

Open question — ruff format and the broader rule families

Undecided, and worth deciding separately from phase 1.

The formatter would rewrite 221 of 311 files. There are currently 30 open PRs, including the 14-PR config-refactor stack (#204#216, #224) and branches from several contributors. A rewrite that size landing on main forces a conflict pass through all of them, and in a stack that deep each rebase cascades into the ones above it.

Declining the formatter permanently is a legitimate answer. The gate in phase 1 works without it, and skipping it means none of the cost below is ever paid. If we do want it, it should be one commit that does nothing but reformat, scheduled for a quiet moment rather than dropped on a busy tree, and git blame handled the standard way:

  1. Record the reformat commit in .git-blame-ignore-revs:
    # ruff format, <date>
    <sha>
    
  2. git config blame.ignoreRevsFile .git-blame-ignore-revs for local blame, documented in the contributor setup notes.

GitHub's blame view reads .git-blame-ignore-revs from the repo root automatically, so the web UI needs no per-clone setup. Blame is the cheap part of this; the rebase pass across 30 branches is the expensive part.

Possible follow-up — pre-commit hooks

Once CI is the enforcement point, a .pre-commit-config.yaml running the same pinned ruff would catch these before push rather than in review. Purely a convenience, additive, and can be added later without changing anything above.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions