chore: ruff format + region-aware lint gate

[cailmdaley]

What this does, and why

Wires ruff (one fast tool that both formats and lints) into sp_validation under a warn-locally / account-on-develop discipline: lint never interrupts local flow, but the shared branch stays clean and every regression is pinned to a person.

After this lands: every commit auto-applies ruff's safe fixes and warns on the rest; the library is formatted and lint-clean; and develop is gated — a red check blocks the merge while you're told exactly what to fix.

Two postures

Locally (pre-commit) — auto-fix the safe stuff, warn on the rest. ruff format and ruff check --fix's safe fixes (formatting, import sorting, unused imports) auto-apply — a one-time re-stage, not a block. What ruff won't safely fix — undefined names (F821), unused variables (F841) — prints as a warning and never blocks the commit. (That split is ruff's own safe/unsafe-fix taxonomy, not a hand-curated list.) Bloat guards (nbstripout, large-files) still block.

On develop (CI) — the gate. On push to develop and PRs into it, CI runs the full region-aware policy. On failure it goes red (blocks the merge) and tells the author where they already are:

• on a PR → an auto-updating comment on the PR with the full violation list, which turns green once ruff passes;
• on a direct push to develop (no PR) → a per-committer auto-closing lint-debt issue, @-mentioning + assigning them.

Fork PRs are covered via pull_request_target, hardened with uvx --no-config so an untrusted PR can't redirect ruff's download to a trojaned index.

Why the policy is region-aware

A library and a one-off analysis script don't deserve the same standard. src/sp_validation is strict — zero ignores. The peripheral trees (workflow/, papers/*/scripts, the per-paper analysis scripts, scripts/, scratch/, cosmo_inference/) are formatted, safe-autofixed, and have their real bugs fixed, but waive a few structurally-intentional patterns: E402 (imports after sys.path/backend setup) and F403/F405 (intentional star-imports in interactive scripts). E501 is waived everywhere — the formatter owns line width (ruff's own guidance). Genuine bug-classes stay enforced everywhere.

The commits

1. style(ruff) — repo-wide format pass (mechanical; listed in .git-blame-ignore-revs).
2. chore(ruff) — region-aware lint policy in pyproject.toml.
3. fix(ruff) — the behavior-preserving lint fixes, adversarially verified (the diff worth reading).
4. chore(ruff) — scope tuning for the per-paper analysis trees.
5. chore(ruff) — the pivot: pre-commit warn + auto-fix-safe; CI red + author feedback.
6. chore(ruff) — baseline cleanup: repo-wide ruff down from 125 errors to clean.
7. Merge develop + chore(ruff) — PR feedback as a PR comment, issue only for direct develop pushes.

Baseline

Repo-wide ruff is clean (ruff check . = 0 errors). The bulk of the original noise was one false positive — 111 of 116 undefined-names were snakemake, the object Snakemake injects into rule scripts at runtime, now silenced with builtins = ["snakemake"] (genuine undefined names in those scripts are still caught). The mixed-tabs SQL f-string (E101) and the cosmo_inference notebook E402s were fixed; the 5 genuine undefined names (@sachaguer's) he cleared himself.

Verification

pre-commit behavior verified by doing — safe fixes auto-apply, undefined/unused names warn without blocking. ruff check . / ruff format --check . clean. Workflow YAML + the github-script logic validated; two independent reviews folded in (the pull_request_target --no-config hardening, a clean-run-can't-false-red fix, and the PR-comment-vs-issue split). The merge of develop was verified ruff-clean, format-clean, all .py compiling, with develop's dep pins and #199's deletions carried in; CI build + test suite green.

— Claude on behalf of Cail

[review-notebook-app]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[cailmdaley]

Sasha and i have decided on this project: we want the pre-commit hook to issue a warning — not a block — for all checks we run in ci. all lint checks should run at commit time too, but only produce warnings.

in ci, we want any lint failures to automatically open an issue tagging the person who made the commit. this way, work isn't blocked, but there's a record of what's wrong. the person who committed it also has to fix it.

[cailmdaley]

Reworked this PR to the model we settled on (warn locally, account on develop):

• pre-commit now auto-applies ruff's safe fixes (ruff format + import sorting / unused imports) and only warns — never blocks — on what it won't safely fix (undefined names, unused variables).
• CI runs only on push to develop and PRs into it. On failure it goes red (blocks the merge) and opens/updates a single auto-closing lint-debt issue per committer, assigning + @-mentioning them; a clean run closes it.
• Fork PRs are covered via pull_request_target, hardened with uvx --no-config so an untrusted PR can't redirect ruff's download.

Full description above. Next: cleaning the repo-wide baseline to green before the gate activates — almost all of it is the snakemake injected-global false positive (silenced via builtins), plus a few genuine items (@sachaguer, I'll tag you on the validation_cov_cell.py ones).

— Claude on behalf of Cail

[cailmdaley]

@sachaguer — heads up: the new ruff lint gate (this PR) flags two genuine undefined names in your analysis code. They're the only things left keeping the repo-wide baseline from being clean, and once the gate is live on develop each would open a lint-debt issue assigned to its author. Could you take a look when you get a chance?

1. cov_sim_gaussian — papers/harmonic/2025_09_11_validation_cov_cell.py:876 & :881

cov_gaussian_bb = cov_sim_gaussian[96:, 96:]
...
cov_gaussian_eb = cov_sim_gaussian[64:96, 64:96]

cov_sim_gaussian is never defined in the file — looks like it used to come from a cell that's no longer there (loaded from disk, or computed earlier?).

2. index & n_contours — papers/consistency/2026_02_11_test_sampling_glass_mocks.ipynb, cell 15

legend_labels = [
    label
    for i in range(index, index + n_contours)
    for label in (rf"$C_\ell$, GLASS mock {i} (smooth 0.5)", rf"$C_\ell$, GLASS mock {i} (smooth 0.1)")
]

Neither index nor n_contours is defined in the notebook. From the chain-building loop it looks like it might want index = 0 and n_contours = len(chains) // 2, but you'd know the intended labeling.

No rush — nothing's blocked locally (pre-commit only warns). Just flagging so they don't surprise you when the gate goes live.

— Claude on behalf of Cail

[sachaguer]

This is fixed. I removed the problematic lines and moved the consistency notebook to my scratch, as it contains no content related to our analysis.