Skip to content

🔧 Unify the ruff and mypy pins, and pin the rule set that goes with them - #1805

Merged
chrisjsewell merged 3 commits into
masterfrom
ci/lint-versions
Sep 2, 2026
Merged

chrisjsewell merged 3 commits into
masterfrom
ci/lint-versions

Conversation

@chrisjsewell

Copy link
Copy Markdown
Member

ruff was pinned in two places that disagreed — v0.15.8 in .pre-commit-config.yaml, 0.14.11
in the ruff dependency group — so uv run prek run and tox -e ruff-check have been running
different linters. mypy is pinned in two places too. This moves ruff to 0.16.5 and mypy to
2.3.1 in both places each, and adds the one config line that makes a ruff version bump mean
"a new ruff", not "a new rule set".

ruff 0.16 replaced its default rule set. In an empty project, ruff check --isolated --show-settings reports 59 default rules on 0.15.8 and 413 on 0.16.5. This project only
ever set extend-select, never select, so its base set has always been whatever the installed
ruff happened to default to. On today's master that means:

ruff ruff check findings
0.14.11 (dependency group) 0
0.15.8 (hook) 0
0.16.5, unpinned base set 298
0.16.5, base set pinned 148
0.16.5, base set pinned, as merged here 0

The bump is only a version bump if select is pinned, so the first commit does that:

select = ["E4", "E7", "E9", "F"]   # ruff's pre-0.16 default

At the versions in force at that commit it is a byte-for-byte no-op (same 259 enabled rules, same
"All checks passed!", same 326 files already formatted). What it buys is that the enabled set is now
a property of this file rather than of the pinned version. Enabled-rule set diff, old config on
0.15.8 vs the config in this PR on 0.16.5 (ruff check --show-settings, counting
linter.rules.enabled): 259 -> 262, gained exactly RUF036, RUF063, RUF068, lost none —
all three were preview-only in 0.15.8 and stabilised in 0.16. ISC004 stabilised with them and is
added to extend-ignore next to the existing ISC001: all 35 occurrences here are deliberately
wrapped string literals and its only fix is unsafe.

So the entire lint surface of this bump is RUF036 none-not-at-end-of-union, 113 findings, all safe
autofixes, applied machine-wise with ruff check --select RUF036 --fix followed by ruff format.
Every one is None | X -> X | None: identical at runtime and to every type checker, already the
prevailing spelling here, and never evaluated at all, since these modules use from __future__ import annotations. git diff -w is the same as git diff — there is no reformatting hiding in
it.

Why #1693 goes red in all 13 Core cells and this does not. That same RUF036 autofix wave
rewrote constraints_results: None | Mapping[str, Mapping[str, bool]] to Mapping[...] | None in
sphinx_needs/data.py. tests/test_data.py::test_consistent cross-checks each NeedsCoreFields
entry against its TypedDict annotation by string-matching the raw ForwardRef text, and while its
scalar branches listed both union orders, its container branches only listed None | dict[ /
None | Mapping[ — so the reordered annotation fell into the non-optional branch and asserted
"object" against ["object", "null"]. One brittle test, no runtime behaviour involved; this PR
teaches the container branches both orders (optional case tested first, so it cannot be swallowed)
and the suite is green.

Why #1693's Lint job stays red as well. Of the 298 findings the unpinned bump produces, 151
are not autofixable
— BLE001 43, PLW1510 30, TRY004 20, DTZ005/TRY002 4 each, and so on.
The bot force-pushes a partially-fixed tree, and no amount of re-running clears the rest.

And one thing #1693 does silently. E402 (module-import-not-at-top-of-file) is in ruff's
pre-0.16 default set and not in 0.16's, along with 17 others (E401 E402 E7xx E711-E714 E721 E731 E741-E743 F403 F405 F406 F722). So on that branch E402 stopped being enforced and RUF100 unused-noqa then offered to delete all twelve # noqa: E402 suppressions in docs/conf.py as
unused — lint coverage lost, plus a twelve-site re-annotation for whoever wants it back. Pinning
select keeps both the rule and the suppressions:

ruff check --isolated --select RUF100 docs/conf.py        ->  12x "Unused `noqa` (non-enabled: E402)"
ruff check --isolated --select RUF100,E402 docs/conf.py   ->  All checks passed!

The ruleset expansion is deliberately deferred. Those 151 findings are a real backlog worth
reviewing — but rule by rule, in its own PR, not as a side effect of a version pin. The select pin
is precisely what turns that into a clean opt-in: adopting 0.16's defaults will then be a legible
one-line diff with its own reviewable consequences.

ruff format in 0.16 also formats python code blocks embedded in markdown, which brings AGENTS.md
and tests/conformance/needflow/README.md into its scope. [tool.ruff.format] exclude = ["*.md"]
keeps it out — same reasoning as the existing yamlfmt ^tests/conformance/ exclude, since that
corpus is shared byte-for-byte with ubCode and checksummed in its own manifest. Without the exclude,
ruff format rewrites 25 lines of AGENTS.md on this branch.

mypy 2.3.1 is folded in because its two pins have to move together: 1.19.1 requires the
# type: ignore[literal-required] in compile_validator and 2.3.1 rejects it as unused, so
splitting the change would force a bridging [literal-required, unused-ignore] into the tree and a
follow-up to take it out again. Moving both pins in one commit makes it a plain deletion. mypy 2.3.1
reports nothing else: no new error classes, no deprecation warnings, no config migration.

Two follow-ups this PR deliberately does not do:

  • The mypy hook's additional_dependencies and the mypy dependency group are maintained by hand
    and have already drifted apart — the hook carries minijinja~=2.15 and the dependency group does
    not, so uv run prek run mypy and tox -e mypy type-check against different environments. It is
    pre-existing and orthogonal to a version bump; the fix is one line (add minijinja~=2.15 to the
    group) and belongs in its own PR.
  • Routine hook autoupdates should be owned by a scheduled prek autoupdate job rather than by
    pre-commit.ci reopening a PR every release; that is a separate change. With select pinned, such
    updates are finally safe to take.

This supersedes #1693, which can be closed.

ruff 0.16 replaces its default `select` wholesale, and this project only ever
set `extend-select`, so the enabled rule set has been whatever the installed
ruff happened to default to. Pin `select` to ruff's pre-0.16 default so that
the set is a property of this file rather than of the pinned version, ignore
ISC004 (stabilised in 0.16, all occurrences here are deliberately wrapped
literals), and keep ruff-format away from markdown so that it cannot rewrite
the ubCode-shared conformance corpus.

At the currently pinned ruff this is a no-op: the enabled rule set is the same
259 rules, `ruff check` still reports nothing and `ruff format --check` still
reports 326 files already formatted.
The hook was pinned to v0.15.8 and the `ruff` dependency group to 0.14.11, so
`uv run prek run` and `tox -e ruff-check` have been running different linters.
Both now say 0.16.5.

With the rule set pinned, the whole lint surface of the move is RUF036
(none-not-at-end-of-union), stabilised out of preview in 0.16. Its 113 findings
are fixed machine-wise (`ruff check --select RUF036 --fix`, all safe fixes,
followed by `ruff format`): every one is `None | X` -> `X | None`, which is
already the prevailing spelling here, is identical at runtime and to every type
checker, and is never evaluated at all because these modules use
`from __future__ import annotations`.

`tests/test_data.py::test_consistent` cross-checks each `NeedsCoreFields` entry
against its `TypedDict` annotation by string-matching the raw `ForwardRef` text,
and its container branches only listed the `None | ...` order (the scalar
branches already listed both). It therefore read `Mapping[...] | None` as
non-optional and asserted "object" against ["object", "null"]. The container
branches now accept both orders, with the optional form tested first so it
cannot be swallowed by the non-optional branch.
The hook was pinned to v1.19.1 and the `mypy` dependency group to the same, so
this is only a version move; both now say 2.3.1.

The two pins have to move together. mypy 1.19.1 requires the
`# type: ignore[literal-required]` in `compile_validator` and mypy 2.3.1 rejects
it as unused, so no single spelling is clean under both unless a bridging
`[literal-required, unused-ignore]` is written and then removed again. Since
both pins move in this commit, the ignore is simply deleted.
@codecov

codecov Bot commented Sep 1, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.26%. Comparing base (4e10030) to head (c4cf0c4).
⚠️ Report is 342 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1805      +/-   ##
==========================================
+ Coverage   86.87%   91.26%   +4.38%     
==========================================
  Files          56       77      +21     
  Lines        6532    11761    +5229     
==========================================
+ Hits         5675    10734    +5059     
- Misses        857     1027     +170     
Flag Coverage Δ
pytests 91.26% <100.00%> (+4.38%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@chrisjsewell
chrisjsewell merged commit 3e0deb0 into master Sep 2, 2026
24 checks passed
@chrisjsewell
chrisjsewell deleted the ci/lint-versions branch September 2, 2026 07:34
chrisjsewell added a commit that referenced this pull request Sep 2, 2026
Adds `.github/workflows/prek-update.yaml`, a `Prek update` workflow that
keeps the hook
revisions in `.pre-commit-config.yaml` current and opens a pull request
with the result.
It replaces the weekly autoupdate PR that pre-commit.ci has been
raising.

### What it does

Runs monthly — `cron: "0 6 1 * *"`, 06:00 UTC on the 1st, the same
cadence dependabot
uses in this repo — and on `workflow_dispatch` for manual runs. Each
run:

- `uv run prek update --cooldown-days 7`, on the same
`astral-sh/setup-uv@v10.0.1 (enable-cache: true)` toolchain the `Lint`
job uses. The
seven-day cooldown keeps day-zero hook releases out: as of today it
selects ruff
  `v0.16.4` and holds back `v0.16.5`, which is younger than a week.
- If nothing moved, the job logs `no hook updates eligible` and ends
green having created
  nothing. No empty PRs.
- If something moved, it runs `prek run --all-files` over the updated
tree, so formatter
and `--fix` churn arrives already applied rather than landing on whoever
merges. Hooks
that still report findings do **not** fail the workflow — the PR is
opened anyway, red,
with `hooks clean after autofix: no` and a link to the run. A rev bump
that needs human
  attention should be visible, not silently withheld.
- Opens the PR on the fixed branch `prek-update`, so a re-run updates
the existing PR
instead of stacking a new one. The body lists each repository with its
old and new rev.

### Why a GitHub App token

A pull request created or pushed with the default `GITHUB_TOKEN` does
not trigger
`pull_request` workflows — GitHub's anti-recursion rule. `Lint` is a
required check on
master under strict mode, so such a PR would never become mergeable. The
workflow
therefore mints a token from the org bot app (`BOT_APP_ID` /
`BOT_PRIVATE_KEY`, both
repository secrets) and uses it for every write. Top-level permissions
stay
`contents: read`; the default token only ever performs the checkout.

One setup item to confirm: the app needs `Contents` and `Pull requests`
read/write, and
ideally `Workflows: Read & write` as well. `yamlfmt` is one of the hooks
being bumped and
it formats `.github/workflows/*.yaml`, so a future yamlfmt release that
changes its
formatting would put a workflow file in the PR, and GitHub rejects app
pushes that touch
workflows without that permission. Runs fail loudly if it is missing, so
it is safe to
add later.

### Why not keep pre-commit.ci

`prek update` is workspace-aware: one invocation updates every
`.pre-commit-config.yaml`
in the tree. pre-commit.ci reads only the root config with vanilla
pre-commit, so it
cannot follow the per-package configs the uv-workspace monorepo layout
will introduce.
This workflow is monorepo-ready; pre-commit.ci is not.

### Merge ordering

**Merge this after #1805.** #1805 carries the same ruff and mypy rev
bumps together with
the `select` pin that makes them tolerable. If this lands first, its
next run re-proposes
those bumps *without* that pin — a rehearsal on current master produced
151 remaining
ruff findings and a mypy `unused-ignore`, across 23 rewritten files.

### After merge

1. Smoke-test it by hand: `gh workflow run prek-update.yaml`. With #1805
in, expect the
no-op path — `no hook updates eligible`, green, no PR. That single run
also proves the
app token and its install, which cannot be checked any other way. The
first real PR
   appears once a hook release clears the seven-day cooldown.
2. Uninstall the pre-commit.ci app. Its autofix pushes and its weekly
autoupdate PR are
both superseded — autofix by the `Lint` gate, autoupdate by this
workflow. Nothing in
this PR touches `.pre-commit-config.yaml`, so the `ci:` block question
does not arise.
chrisjsewell added a commit that referenced this pull request Sep 2, 2026
`uv.lock` has been in `.gitignore` since uv was adopted. This commits
it, re-enables the
`uv-lock` hook that keeps it honest, and points CI and dependabot at it.

## Why commit the lock

A uv lock file is **universal**: one file records the resolution for
every operating
system, architecture and Python version the project supports, so there
is no per-platform
lock to maintain and no reason to regenerate it per CI job.

It is also **not package metadata**. It is not in the sdist or the
wheel, and pip and uv
both ignore it when installing `sphinx-needs` as a dependency. **Nothing
changes for anyone
who installs this package** — the version ranges in `pyproject.toml`
remain the contract.
What the lock pins is *contributors and CI*, which is exactly what has
been unpinned.

The practical effect is on the failure mode. Today, when any transitive
dependency of the
lint environment ships a breaking release, every open pull request goes
red at once and
somebody has to work out which upstream did it. With the lock committed,
the environment
stays fixed until it is deliberately changed, so that same release turns
one dependabot
pull request red instead, in isolation, with the offending bump named in
the diff.

Scope, to be clear: the lock governs the `Lint` and `Prek update` jobs,
which are the only
uv-based jobs here. The pytest matrix still installs with pip across
`sphinx~=7.4/8.2/9.1`
on purpose — testing the *range* is the point there, and that is
unchanged.

## What changed

**The lock.** `uv.lock` removed from `.gitignore` and committed. It was
generated with uv
0.12.9 — the same version the hook below pins, and the same version
`astral-sh/setup-uv@v10`
installs today — and re-running `uv lock` leaves it byte-identical.
Nothing needed a
`tool.uv.conflicts` table; it resolves clean at 113 packages.

**The `uv-lock` hook, re-enabled.** `.pre-commit-config.yaml` has
carried a commented-out
`astral-sh/uv-pre-commit` block at `rev: 0.5.5` with `# TODO this does
not work on
pre-commit.ci`. That blocker is gone — pre-commit.ci is no longer in use
and the `Lint` job
runs prek — so the block is now live at `rev: 0.12.9`, and the stale
comment is deleted.
The hook uses its default `files:
^(uv\.lock|pyproject\.toml|uv\.toml)$`, so it only fires
when the dependencies actually change; the rest of the time it is a
no-op. Change a
dependency without re-locking and it fails with the refreshed lock
already in the diff.

**`--frozen` on every CI uv command** — the `Lint` job's `uv run prek
run`, and both
`uv run` calls in the `Prek update` workflow. `--frozen` means *use the
committed lock
verbatim, never re-resolve, and error out if it is missing*. Without it,
uv silently
re-resolves and writes a new lock when one is absent — no warning, no
non-zero exit — which
is precisely the drift the lock is meant to remove.

Not `--locked`, which additionally errors when the lock is *stale*: that
would be a
redundant second check, because the `uv-lock` hook already runs in the
same `Lint` job via
prek and fails on a stale lock, with a more useful message and the fix
in the diff.

**Dependabot: `pip` → `uv`.** The `pip` ecosystem reads `pyproject.toml`
and proposes bumps
to the `~=` pins in the extras. Nobody merges those — #1431, #1415 and
#1377 have been open
for the best part of a year. The `uv` ecosystem instead refreshes
`uv.lock`, which is what
now needs refreshing. Same `directory`, same monthly schedule, same `⬆️`
prefix, plus one
group: **minor and patch updates are batched into a single monthly pull
request, while
major updates fall outside the group and arrive one per dependency.**
Breaking changes
concentrate in majors, so the bumps most likely to go red are the ones
that come isolated,
and the routine churn does not become one PR per transitive dependency
(the `uv` ecosystem
tracks indirect dependencies too). If a batch ever goes red on a 0.x
"minor" that is really
a major, the escape hatch is `exclude-patterns` on the group —
dependabot then rebuilds the
batch without it and that dependency gets its own pull request. The
`github-actions`
stanza is untouched.

> Please close #1431, #1415 and #1377 after this merges — they are
pip-ecosystem pull
> requests that nothing will update any more. (#1360 and #1197 are
`github-actions` pull
> requests and are *not* affected by this change; #1360 in particular is
worth merging on
> its own, since `codecov/codecov-action@v3` is old enough that
actionlint flags it.)

**Docs.** `docs/contributing.rst` gains a short paragraph next to `uv
sync`: the lock is
committed so `uv sync` installs exactly those versions, edit
`pyproject.toml` and let the
`uv-lock` hook (or `uv lock`) update it, and dependabot refreshes it
monthly. No minimum uv
version is called out because none is needed — uv 0.9.24 reads this lock
and agrees it is
up to date, so no one has to upgrade uv for this.

## What the lock resolves to

`sphinx` **7.4.7** and `docutils` **0.20** — one version each, no
per-marker split. That is
not uv being conservative: the `mypy` dependency-group pins
`sphinx==7.4.7` and
`docutils==0.20` exactly, and a universal resolution has to satisfy
every group and extra
at once, so those two `==` pins fix sphinx and docutils across the whole
lock. This is
existing behaviour rather than anything new — `uv sync` on `master`
resolves to the same
thing today — but committing the lock makes it visible, and it is worth
knowing before
anyone is surprised by a `uv sync` dev environment on sphinx 7.4.

## Out of scope

`requires-python` and the CI matrix are **untouched**; the lock is
generated for the
current `>=3.10,<4`. The move of the floor to 3.11 is user-visible
(matrix, classifiers,
changelog) and belongs in its own pull request. The tox configuration is
likewise left
alone. No changelog entry, matching #1804, #1805 and #1806.
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.

2 participants