timothy
85ecd352f4
Cold review (Opus, isolated worktree) cleared the mechanical reformat: 25 of 33 files AST-identical, the suite's collected node IDs identical to main, and the three percent-format -> f-string conversions byte-equivalent at both call sites. Everything below is config/CI/doc layer. M4: `S105` is no longer exempted directory-wide. A blanket over `scripts/tests/**` gives up hardcoded-credential coverage over the largest Python surface in the repo — permanently, and this is the only Python lint we run — to suppress 8 known lines. Those 8 now carry a per-site `# noqa: S105`; removing one is red (witnessed). Only `S101` stays directory-wide. M1: `ruff check .` on an empty population prints a stderr warning and exits 0, so a stray `exclude` would switch the gate off silently — the vacuity failure `testing.guard-derives-population-from-source` is about. The step now compares `--show-files` against `git ls-files '*.py'` and fails on drift. Mutation proof: a top-level `exclude = ["scripts"]` takes `ruff check` to green and the step to `::error::… 0 Python files but git tracks 33`. Note the same exclude under `[format]` does NOT reproduce it — it scopes to the formatter, and a mutation written that way passes while proving nothing. M3: lint moved ahead of the preflights. Behind `Preflight jq version` (a hard `--expect` tripwire) the lint would go dark for as long as the jq contract is broken, under a red that names jq. H3: ruff.toml named a `Script lint (ruff)` job that does not exist — wrong on the day it shipped. H1: the "72 errors" claim is now reproducible against the committed config (it was only true of the pre-M4 config), carries its reproduction command and base sha, and splits honestly: 54 fixed in code, 18 per-site `# noqa`s. H2: `docs/ci-cd.md` still said the suite is ~190 tests / ~10s next to a new "four-minute" figure. Measured: 773 tests, ~4.5 min. The no-path-filter rationale rested on the dead number; it now rests on the input set, which is the real reason. M2: the nine E501 `# noqa`s claimed splitting "would change the payload". Implicit string concatenation preserves the bytes exactly, so the stated reason was false nine times. They say what is actually true: kept one-line for greppability. Nits: the record no longer claims the config is identical to the operator's global one (per-file-ignores differ), and the unpinned pytest/pyyaml asymmetry is stated rather than left looking like an oversight. Refs: #780 Decisions-Edit: yes
4.8 KiB
key, title, status, since, supersedes, superseded-by, rule, signals, mechanics
| key | title | status | since | supersedes | superseded-by | rule | signals | mechanics |
|---|---|---|---|---|---|---|---|---|
| ci.python-lint-ruff-config-committed | 2026-08-21 — Python lint is a committed ruff.toml enforced in CI, not the operator machine's global config (#780) | active | 2026-08-21 | none | none | The repo commits `ruff.toml` and the `script-tests` job runs `ruff check .` + `ruff format --check .` against a PINNED ruff. Never rely on `~/.config/ruff/ruff.toml`, and never add a lint rule to the config without making the tree clean against it in the same PR. | ruff · pyright · python lint · `ruff format --check` · lint passes on my machine but not yours · no repo lint config · S105 on a test stub credential · paths: `ruff.toml`, `.gitea/workflows/pr-checks.yml` · issues: #780, #773, #648, #512 | Config at repo root; the job pins `ruff==0.12.11` via pip and runs both commands before pytest so a style red arrives in seconds. Bumping the pin is a deliberate PR because a new ruff release adds rules. Population is whatever `ruff check .` discovers from the root, so a new `.py` anywhere is covered without enumeration. |
The global instructions tell every session to run ruff check, ruff format --check and pyright
after touching Python. Before this, the repo enforced none of them and committed no config, so ruff
fell back to whichever ~/.config/ruff/ruff.toml the operator's machine happened to have — a second
machine lints this repo differently, or not at all. That is the same shape as #643/#647/#648 (a
shell gate whose behaviour was a function of an untested interpreter version) and #512 (a test that
passed on a fast laptop and flaked on a starved CI VM): the verdict was a property of the environment
rather than of the repo.
The committed config is the operator's global one apart from per-file-ignores, which is narrowed
to scripts/tests/**. That is what the tree was de-facto written against, so adopting it cost a
mechanical reformat rather than a rewrite: 72 findings against 706674272, of which 54 were fixed in
code (mostly by the format pass) and 18 carry a per-site # noqa with its reason inline.
One exemption is directory-wide, and it is the boring one. S101 for scripts/tests/**, because
a test suite asserts. S105 is deliberately NOT directory-wide, even though all eight of its
hits are stub credentials handed to the real hooks (env["ETV_GITEA_TOKEN"] = "stub") with no true
positive in the tree today. A directory blanket would give up hardcoded-credential coverage over the
largest Python surface in the repo, permanently, to suppress eight known lines — and this is the only
Python lint the repo runs, so nothing else would catch a real token pasted into a fixture next year.
Eight per-site # noqa: S105 cost the same and keep the rule live.
The population is checked, because an empty one is green. ruff check . on a tree with no Python
files prints warning: No Python files found to stderr and exits 0, so a stray exclude, a wrong
working directory, or a change to ruff's default excludes would switch this gate off without a red.
The CI step therefore compares ruff check . --show-files | wc -l against git ls-files '*.py' | wc -l
and fails on drift in either direction. Witnessed, not assumed: adding exclude = ["scripts"] takes
ruff check . to green-with-a-warning and the step to ::error::ruff would check 0 Python files but git tracks 33. Construct that mutation at the TOP level of ruff.toml: an exclude under the
[format] table scopes to the formatter only, so a mutation written there passes and proves nothing.
Lint runs first in the job, ahead of the preflights. Preflight jq version is a hard --expect
tripwire; a lint step behind it stops running for as long as the jq contract is broken, under a red
that names jq. Ordering is the difference between a gate that is skipped and one that is not.
pyright is deliberately NOT gated. Its only findings here are reportMissingImports for
etv_client in scripts/scripted-schedules/entrypoint.py, resolvable only inside that script's
deploy environment. Gating it would put a node toolchain on the git-only small lane to find nothing.
Revisit when this repo grows a typed Python surface — the reason is the cost/finding ratio today, not
a judgement that type checking does not belong.
The pin is the second half of the fix. An unpinned pip install ruff re-introduces exactly the
divergence the config closes, one layer up: the verdict becomes a function of when the job ran. Same
argument as the jq pin in the same job (ci.jq-version-contract), and the same consequence — a bump
is a PR someone reads. pytest and pyyaml in the same job stay unpinned, and the
asymmetry is the point rather than an oversight: a pytest release does not add assertions to your
suite, a ruff release adds rules to your lint.