Skip to content

ci: make ruff actually lint the Python it claimed to lint - #437

Merged
JasonYeYuhe merged 1 commit into
mainfrom
ci-ruff-repo-wide-coverage
Aug 18, 2026
Merged

ci: make ruff actually lint the Python it claimed to lint#437
JasonYeYuhe merged 1 commit into
mainfrom
ci-ruff-repo-wide-coverage

Conversation

@JasonYeYuhe

Copy link
Copy Markdown
Collaborator

The gap

The repo-wide ruff check . was neither repo-wide nor reachable. Two independent defects in helper-ci.yml, either one sufficient on its own to hide every non-helper Python file:

  1. paths: filtered to helper/**, three named scripts and one Swift file — a change to scripts/*.py never started the workflow;
  2. the ruff step ran under working-directory: helper, so even when the workflow did start, ruff check . resolved to helper/.

Worth stating plainly because it changes the fix: widening the path filter alone — the obvious reading — would have changed nothing. Both had to go.

Measured cost: 91 ruff errors no CI run had ever reported. PR #433 was the demonstration case, landing two F541s on main. Those two are already gone (overwritten by #436) — the errors were transient, the hole they came through was not.

What's here

ruff.toml (new, repo root) — excludes archive/ only; AGENTS.md lists it under Archived or historical and it carried 11 of the 91. select is written out explicitly as ruff 0.15.11's current defaults, verified rule-for-rule identical (59 rules), so this PR changes no rule selection today but a future pin bump changes rule behaviour without silently changing the rule set.

The remaining 80 errors fixed — zero suppressed. No # noqa was needed.

rule n treatment
E701/E702 38 statements split
F541 18 dropped the f prefix
F401 12 import removed
F841 8 dead binding removed
E401 8 imports split
E722 2 except Exception:
E731 1 lambda → def

python-lint.yml (new) — runs ruff check . from the repo root on **/*.py, registered in ci-gate.yml's GATES so it blocks. helper-ci.yml's ruff step and pin are removed rather than duplicated: **/*.py is a strict superset of helper/, and two pinned ruff versions in two workflows is the copied-constant drift this repo has been bitten by before.

The ruff==0.15.11 pin carries over verbatim, 2026-07-30 incident note included. Bumping it stays its own task.

Behaviour is proven unchanged, not assumed

The 38 statement splits were driven off ruff's own reported offsets, with an assertion that each offset really was the ; or : claimed — then checked against a real oracle:

  • pet_golden_oracle.py → 32KB PetGoldenVectors.json, byte-identical before/after
  • pet_art_gen.py → 36 SVG frames, byte-identical before/after
  • ci_check_android_strings_parity.pyexecuted, not just compiled (it's live in android-ci.yml): passes
  • pet_normalize_assets.py --selftest — passes, and a negative control confirms the if got != expect: ok = False line I split still flips the result and exits 1
  • every removed import confirmed unreferenced: math in pet_art_gen.py is re-imported locally as _m; json in appstore_metadata.py survived only as r.json() method calls
  • resubmit.py's discarded probe keeps its network call, drops only the unused binding

Not runtime-verified: the ASC operational scripts (resubmit.py, submit_v1_*.py, appstore_metadata.py) need live App Store Connect credentials. They are byte-compiled only. Their changes are import removals, dead-binding removals and except:except Exception:.

The gate was verified by making it fail

Per this repo's rule about guards that never run — a green check is not evidence. On a throwaway branch, two deliberately-broken files:

##[error]scripts/_ruff_gate_probe.py:3:8: F401 `os` imported but unused
##[error]CLI Pulse Bar/scripts/_ruff_gate_probe_spaces.py:4:12: F821 Undefined name
Process completed with exit code 1.

Python Lint | completed | failure — and the log shows ruff 0.15.11 actually executing.

The second probe is deliberately under a directory with spaces: CLI Pulse Bar/scripts/ holds ~24 of the repo's Python files, and a glob that silently skipped it would have recreated this exact bug. It was flagged.

Exactly 2 error lines were emitted and 0 files under archive/ — which also proves ruff.toml was read on the runner, since without it archive/'s 11 errors would have appeared.

Scratch branch deleted. On this branch: Python Lint green (log confirms ruff ran), Helper CI green with 971 passed, 1 skipped — pytest intact after losing its neighbour step.

🤖 Generated with Claude Code

The repo-wide `ruff check .` was neither repo-wide nor reachable. Two
independent defects in helper-ci.yml, either one sufficient on its own to
hide every non-helper Python file:

  1. `paths:` filtered to helper/**, three named scripts and one Swift
     file, so a change to scripts/*.py never started the workflow;
  2. the ruff step ran under `working-directory: helper`, so even when the
     workflow DID start, `ruff check .` resolved to helper/.

Fixing only the path filter — the obvious reading — would have changed
nothing. Measured cost of the gap: 91 ruff errors that no CI run had ever
reported. PR #433 was the demonstration case, landing two F541s on main;
they have since been overwritten by #436, so the errors are gone but the
hole they came through was not.

Three parts, in the order they have to happen:

* ruff.toml at the repo root. Excludes `archive/` only — AGENTS.md lists it
  under "Archived or historical" and it carried 11 of the 91. `select` is
  written out explicitly as ruff 0.15.11's current defaults (verified rule-
  for-rule identical, 59 rules), so raising the pin later changes rule
  BEHAVIOUR without silently changing the rule SET.

* The remaining 80 errors fixed, none suppressed — no `# noqa` was needed.
  38 were compound-statement splits (E701/E702) driven off ruff's own
  reported offsets with an assertion that each offset really was the `;`
  or `:` claimed. Behaviour is proven unchanged, not assumed: both pet
  generators were run before and after and their output is byte-identical
  (32KB PetGoldenVectors.json; 36 SVG frames). Every removed import was
  confirmed to have no remaining reference — `math` in pet_art_gen.py is
  re-imported locally as `_m`, and `json` in appstore_metadata.py survives
  only as `r.json()` method calls. The one bare-except pair became
  `except Exception:`, and the one discarded network probe in resubmit.py
  keeps its call and drops only the unused binding.

* python-lint.yml runs ruff from the REPO ROOT on `**/*.py`, and is
  registered in ci-gate.yml's GATES so it blocks. helper-ci.yml's ruff step
  and pin are removed rather than duplicated: `**/*.py` is a strict superset
  of helper/, and two pinned ruff versions in two workflows is exactly the
  copied-constant drift this repo has been bitten by before.

The ruff==0.15.11 pin is carried over verbatim, including the 2026-07-30
incident note. Bumping it stays its own task.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 17, 2026 14:23

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@JasonYeYuhe
JasonYeYuhe merged commit 05ac51b into main Aug 18, 2026
11 of 13 checks passed
@JasonYeYuhe
JasonYeYuhe deleted the ci-ruff-repo-wide-coverage branch August 18, 2026 02:37
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