From b306df68411f3be0df017dc0c66a171ce84c0cb4 Mon Sep 17 00:00:00 2001 From: Konrad Rokicki Date: Thu, 15 Oct 2026 12:00:32 -0400 Subject: [PATCH] ruff: enable the complexity rules (C901, PLR0911/12/13/15) and RUF100 Defaults except max-args 6 and max-returns 8; tests are exempt from C901 and PLR0913 (fake dispatchers are one switch each). The nine functions over the line keep a noqa with the reason on the same line. RUF100 catches a noqa that no longer suppresses anything (one stale BLE001 removed). The guide says what the limits are and that a new noqa needs a reason. Co-Authored-By: Claude Fable 5.1 --- autodev/agent.py | 2 +- autodev/github.py | 2 +- autodev/workflows.py | 2 +- docs/development.md | 7 ++++++- mia_data/activities.py | 2 +- mia_data/binding.py | 2 +- mia_data/cli.py | 2 +- mia_data/crop_test.py | 2 +- mia_data/jobspec.py | 2 +- mia_data/publish.py | 2 +- pyproject.toml | 10 +++++++++- tests/test_mia_publish.py | 2 +- 12 files changed, 25 insertions(+), 12 deletions(-) diff --git a/autodev/agent.py b/autodev/agent.py index d7f248d..8c72e21 100644 --- a/autodev/agent.py +++ b/autodev/agent.py @@ -41,7 +41,7 @@ """ -def render_prompt( +def render_prompt( # noqa: PLR0913 the template's seven inputs; a PromptInput would be this signature as a class cfg: Config, issue: dict, comments: list[dict], branch: str, base_sha: str, pr: int | None, feedback: str | None ) -> str: """Fill the prompt template with the issue, the human discussion on it, and any reviewer feedback. diff --git a/autodev/github.py b/autodev/github.py index 0e3afd1..534a146 100644 --- a/autodev/github.py +++ b/autodev/github.py @@ -147,7 +147,7 @@ async def request( conditional: bool = False, ) -> httpx.Response | None: ... - async def request( + async def request( # noqa: C901, PLR0913 one retry loop over every request option; splitting it hides the order of checks self, method: str, path: str, diff --git a/autodev/workflows.py b/autodev/workflows.py index 705e5e1..be08358 100644 --- a/autodev/workflows.py +++ b/autodev/workflows.py @@ -169,7 +169,7 @@ async def _run_round(self, feedback: Feedback | None) -> PrRef: self.workspace = None return result - async def _watch_pr(self) -> str: + async def _watch_pr(self) -> str: # noqa: C901 the review state machine, one branch per action; kept whole so replay changes are visible """Watch the PR until it is merged or closed, running revision rounds and escalations as needed. The wait between polls is observe_pr's retry interval (the activity fails while there is nothing to diff --git a/docs/development.md b/docs/development.md index b709867..b3a781a 100644 --- a/docs/development.md +++ b/docs/development.md @@ -21,7 +21,12 @@ Run all of these before pushing. CI runs the last three in this order on every p | `pixi run test` | pytest over `tests/`, including the Temporal replay tests | Configuration for all four is in `pyproject.toml`. Keep them clean rather than suppressing: there are no -`type: ignore` or `noqa` comments in the source today, and a new one needs a reason next to it. +`type: ignore` comments in the source today, and a `noqa` needs its reason on the same line. + +ruff also enforces size limits: complexity 10 (`C901`), 12 branches, 50 statements, 8 returns and 6 parameters +per function (`PLR09xx`). More than six parameters wants a parameter object (`JobArgs`, `Reviewers`). The nine +functions over the line are marked with a `noqa` saying why they are kept whole; do not add to that list +without the same. ## Layout diff --git a/mia_data/activities.py b/mia_data/activities.py index 06251c9..10b799b 100644 --- a/mia_data/activities.py +++ b/mia_data/activities.py @@ -196,7 +196,7 @@ class Activities: `bot` is the GitHub login mia-data posts as. `probe` returns the csub policy's allowed_hosts patterns; it is injectable for tests.""" - def __init__( + def __init__( # noqa: PLR0913 the clients the activities need, injected here so tests can fake them self, cfg: Config, gh_data: GitHub, diff --git a/mia_data/binding.py b/mia_data/binding.py index 522c86f..7d77bf2 100644 --- a/mia_data/binding.py +++ b/mia_data/binding.py @@ -46,7 +46,7 @@ def record_urls(record: dict) -> list[str]: ] -def check_binding(binding: dict, record: dict, catalog_names: set[str], *, pins: Pins) -> list[str]: +def check_binding(binding: dict, record: dict, catalog_names: set[str], *, pins: Pins) -> list[str]: # noqa: C901 a validator: one branch per rule """Errors in an agent-written binding (empty = ok). Rewrites s3:// URLs to https in place. The binding must name the pinned record and commit, use only snippets from the catalog, and only URLs diff --git a/mia_data/cli.py b/mia_data/cli.py index e9d0b62..f04c1a6 100644 --- a/mia_data/cli.py +++ b/mia_data/cli.py @@ -234,7 +234,7 @@ async def cmd_init_labels(cfg: Config, args: argparse.Namespace) -> int: return 0 -async def run_test_pr(activities: Activities, pr: int, binding_file: Path, record_id: str) -> int: +async def run_test_pr(activities: Activities, pr: int, binding_file: Path, record_id: str) -> int: # noqa: C901, PLR0912, PLR0915 long but linear: the PR path through the same stages the workflow runs """Run the test crops on a snippets PR's head with a binding from disk, synchronously, and post the `mia-data/crops` status and a result comment on the PR. Returns 0 if the crops passed. diff --git a/mia_data/crop_test.py b/mia_data/crop_test.py index 1da1f96..a1b87db 100644 --- a/mia_data/crop_test.py +++ b/mia_data/crop_test.py @@ -34,7 +34,7 @@ def judge(verify: VerifyResult, jobs: list[JobDone]) -> tuple[bool, list[str]]: return not failed and verify.ok_count > 0 and not verify.errors, failed -async def run_crop_test( +async def run_crop_test( # noqa: PLR0913 keyword-only; the workflow passes the round's state through activities: Activities, *, issue: int, diff --git a/mia_data/jobspec.py b/mia_data/jobspec.py index e377f22..4c476d8 100644 --- a/mia_data/jobspec.py +++ b/mia_data/jobspec.py @@ -238,7 +238,7 @@ def _parent(file: Path | None) -> Path | None: return Path(file).parent if file else None -def _spec( +def _spec( # noqa: PLR0913 keyword-only; every job builder is a thin wrapper that fills these in cfg: Config, kind: str, workspace: Path, diff --git a/mia_data/publish.py b/mia_data/publish.py index 92cfbc1..cf373c7 100644 --- a/mia_data/publish.py +++ b/mia_data/publish.py @@ -48,7 +48,7 @@ def _assert_same(dest: Path, files: dict, crop_id: str) -> None: raise Conflict(f"{crop_id} is already published with different checksums") -def publish( +def publish( # noqa: C901, PLR0912 one pass over the crops with a branch per file outcome; the tests cover each dataset_dir: Path, src_dir: Path, verified: list[dict], withdrawn: dict[str, str], *, tmp_max_age_s: float ) -> dict: """Copy the verified files of each crop from `src_dir` (a crop job's crops/) into `dataset_dir`. diff --git a/pyproject.toml b/pyproject.toml index 67504fe..becdc81 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -70,9 +70,17 @@ line-length = 120 extend-exclude = ["notes"] [tool.ruff.lint] -select = ["E", "F", "W", "I", "UP", "B"] +select = ["E", "F", "W", "I", "UP", "B", "C901", "PLR0911", "PLR0912", "PLR0913", "PLR0915", "RUF100"] ignore = ["E501"] # long lines are reviewed by eye; the renames pushed a few past 120 +[tool.ruff.lint.per-file-ignores] +# fake activity dispatchers and fake services are one big switch each; that is the test, not a smell +"tests/*" = ["C901", "PLR0913"] + +[tool.ruff.lint.pylint] +max-args = 6 # more than this wants a parameter object (JobArgs, Reviewers) +max-returns = 8 # decide() and _round() return once per outcome + [tool.mypy] python_version = "3.12" check_untyped_defs = true diff --git a/tests/test_mia_publish.py b/tests/test_mia_publish.py index 7df8368..5b76d12 100644 --- a/tests/test_mia_publish.py +++ b/tests/test_mia_publish.py @@ -82,7 +82,7 @@ def lockstep(a, b): # both attempts have copied the crop; both now rename it def run(src, verified): try: results.append(publish(ds, src, verified, {}, tmp_max_age_s=AGE)) - except Exception as e: # noqa: BLE001 + except Exception as e: errors.append(e) ts = [threading.Thread(target=run, args=s) for s in srcs]