Repository navigation
[CI] pin ruff and make its configuration explicit - #4403
Conversation
The style job runs `pip3 install ruff` unpinned against a repo with no ruff config, so the enforced rule set is whatever the latest release happens to default to. ruff 0.16.0 widened those defaults well past E4/E7/E9/F, which turned PRs red for reasons that had nothing to do with their contents. This pins the inputs; it does not change any verdict on existing code. - Pin ruff==0.16.0 in .github/workflows/pre-checks.yaml. Bumping it is now the single reviewable place where the rule set can move. - Add [tool.ruff] to pyproject.toml with target-version = "py310". There is no [project] table, so ruff cannot read requires-python and was falling back to its own default; py310 is what it already resolved to, so the UP0xx verdicts are unchanged. Deliberately no select/ignore override -- the rule set stays exactly ruff's default for the pinned version. - Restate the default exclude list with `dist/` instead of the bare `dist`. A pattern without a slash matches the basename at any depth, so the built-in entry was silently skipping aiter/dist/ -- 15 real source modules that had never been linted. They contribute 346 pre-existing findings, to be burned down with the rest of the backlog. The setuptools output directory stays excluded. Also exclude 3rdparty/, aiter/jit/build/ and aiter_logs/. - setup.py: python_requires ">=3.8" -> ">=3.10". 3.8/3.9 have not worked for a long time -- 81 modules use PEP 604 annotations without `from __future__ import annotations` and raise TypeError at import on <3.10. - bench_deepgemm_attention.py: the AOT name f-string reused its outer quotes, which is PEP 701 (3.12+) and left the module unparseable on 3.10/3.11. Use single quotes inside. Found by pinning target-version. Verified against main: no existing finding disappears (nothing is weakened) and no new finding appears outside aiter/dist/. black --check stays clean.
🏷️ CI GuideRuns automatically on every PR:
Extended tests (opt-in via labels):
|
generate_schema matched Tensor-bearing unions with `get_origin(t) is Union`,
but `X | None` has origin types.UnionType, not typing.Union. Every PEP 604
annotation therefore fell through to the "unsupported type" branch and the
parameter was silently dropped from the registered custom-op schema:
def f(a: torch.Tensor, b: Union[torch.Tensor, int] = None)
-> (Tensor(a0!) a, Tensor(a1!)? b=None) -> Tensor
def f(a: torch.Tensor, b: torch.Tensor | int = None)
-> (Tensor(a0!) a, * ) -> Tensor # b is gone
This is pre-existing: 449 annotations across 99 modules already use PEP 604.
aiter/jit/core.py got this right via its _is_union() helper; torch_guard.py is
the one place that did not. Match core.py's behaviour.
Safe autofixes only, for rules where every finding in the repo was
safe-fixable. No behaviour change:
RUF100 305 unused noqa directives
PIE808 87 range(0, n) -> range(n)
PLR0402 65 import x.y as y -> from x import y
PLR2044 59 empty comments
PIE790 46 unnecessary pass / ...
RUF010 42 f"{str(x)}" -> f"{x!s}"
C408 51 dict() -> {} (ruff marks this fix unsafe; every call site
uses identifier keys and `dict` is not shadowed anywhere in
the repo, both checked)
Applied with `--fixable`, not `--select`. That distinction matters: `--select`
narrows the *enabled* rule set, so RUF100 then treats every noqa referencing a
non-selected rule as unused and strips it. A first attempt that way deleted
live suppressions -- `# noqa: F821` on gfx950_a8w8_blockscale_asm (a name the
compile_ops decorator injects at runtime) and several `# noqa: E722` -- and
introduced 51 new findings. `--fixable` keeps the full rule set enabled for
analysis and only applies the listed fixes.
Verified: ruff total 6678 -> 6022, and the per-rule diff is reductions only --
no rule gains a single finding. black --check clean, all 261 files compile.
PEP 585 / PEP 604 annotations, now that python_requires is >=3.10: UP045 2165 Optional[X] -> X | None UP006 613 typing.Dict/List/Tuple -> dict/list/tuple UP007 109 Union[X, Y] -> X | Y UP035 166 Callable/Iterable/Iterator -> collections.abc F401 410 typing imports left unused by the above Applied with `--fixable` so the full rule set stays enabled during analysis (see batch 1 for why `--select` is wrong here). None of the unused imports were in an __init__.py, so no re-export was dropped. Two real bugs this surfaced, both fixed here: - Optional[torch.tensor] -- lowercase `tensor`, the factory function, not the `Tensor` type -- in fused_moe.py, fused_moe_dp_shared_expert.py, test_common.py and test_pa_prefill.py. typing.Optional accepts any callable and silently swallowed it; `torch.tensor | None` raises TypeError at import. Corrected to torch.Tensor. Without this, `import aiter.fused_moe` fails outright, which is how it was caught. - The generate_schema union handling, fixed in the preceding commit; it had to land first or every converted Tensor union would have produced a truncated custom-op schema. Verified: - Import sweep over every aiter submodule, before vs after: 249 ok / 1 fail both times, the one failure (pod_attention) pre-existing and unrelated. - generate_schema returns identical schemas for Optional[T], T|None, Union[T,int] and T|int. - ruff total 6022 -> 2936, per-rule diff is reductions only. - black --check clean, all 252 files compile.
Ten findings ruff left behind, now resolved so UP006/UP007/UP035/UP045 and the batch 1 rules are all at zero: - dist/utils.py: the recursive JSONTree alias -> PEP 604. Verified the runtime form actually builds (forward-ref strings inside `|` are fine). - rotary_embedding.py: List[float] -> list[float] and drop the typing.List import. The bare `# noqa` on that line was suppressing UP006 and is no longer needed -- bare noqa hides everything, so removing it restores coverage. - torch_guard.py: the three `param_type == Optional[X]` comparisons are runtime values rather than annotations; both spellings compare equal, so converting them is safe and the comment now says so. - torch_guard.py / triton/__init__.py: `# noqa: F401` with a reason on the `Library` availability probe and on the deliberate comms re-exports. - Two typing imports left unused by the above. Note for reviewers: check residues with the full rule set, never with `--select`. `--select` narrows the *enabled* rules, so RUF100 then reports every noqa referencing an unselected rule as unused -- it flagged 29 phantom findings here, the same trap that corrupted the first attempt at batch 1. Re-verified after the change: import sweep still 249 ok / 1 pre-existing fail, generate_schema identical across spellings, black clean, all files compile.
I001 across the 620 files whose module-level imports all sit at the top of the file with no sys.path manipulation. The other 43 I001 files -- code generators under csrc/ that insert into sys.path before importing, tests doing the same, and the runtime's deliberate late imports -- are left for a follow-up that has to read each one. Static "imports are all at the top" is necessary but not sufficient: it says nothing about whether their relative order matters. aiter/__init__.py proved that. Sorting its block moved `from . import mla` ahead of `from .utility import dtypes`, and mla.py does `from aiter import dtypes`, so `import aiter` died with a circular-import ImportError. That block is now fenced with `# isort: off` / `# isort: on` and a comment explaining why the order is load-bearing; the file's other two import blocks are sorted normally. Caught by importing every aiter submodule and diffing against the pre-change baseline -- 249 ok / 1 pre-existing failure both before and after, with the regression showing up as a hard failure of `import aiter` itself. ruff total 2926 -> 2262 (I001 754 -> 90), no other rule moves in either direction. black --check clean, all 620 files compile. Not covered: op_tests/ and csrc/ scripts are not exercised by the import sweep, so their import-order behaviour rests on CI.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit
ruff
__all__ is not sorted
aiter/aiter/ops/triton/_triton_kernels/gated_delta_rule/__init__.py
Lines 36 to 50 in 464dd84
__all__ is not sorted
__all__ is not sorted
aiter/aiter/ops/triton/comms/__init__.py
Lines 21 to 28 in 464dd84
Import block is un-sorted or un-formatted
aiter/aiter/ops/triton/fusions/fused_kv_cache.py
Lines 4 to 10 in 464dd84
__all__ is not sorted
aiter/aiter/ops/triton/gated_delta_net/causal_conv1d_prefill.py
Lines 35 to 39 in 464dd84
The file is executable but no shebang is present
The file is executable but no shebang is present
Use float instead of int | float
Use float instead of int | float
Use float instead of int | float
Use float instead of int | float
Use float instead of int | float
Use float instead of int | float
Import block is un-sorted or un-formatted
aiter/aiter/ops/triton/gluon/mla_gluon.py
Lines 46 to 53 in 464dd84
Do not catch blind exception: Exception
__all__ is not sorted
aiter/aiter/ops/triton/quant/__init__.py
Lines 36 to 67 in 464dd84
__all__ is not sorted
aiter/aiter/ops/triton/quant/fused_mxfp8_quant.py
Lines 13 to 17 in 464dd84
Invalid type for environment variable default; expected str or None
aiter/aiter/rotary_embedding.py
Line 42 in 464dd84
Invalid type for environment variable default; expected str or None
aiter/aiter/rotary_embedding.py
Line 43 in 464dd84
Invalid type for environment variable default; expected str or None
aiter/aiter/rotary_embedding.py
Line 44 in 464dd84
Use float instead of int | float
aiter/aiter/rotary_embedding.py
Line 118 in 464dd84
Use float instead of int | float
aiter/aiter/rotary_embedding.py
Line 938 in 464dd84
Use float instead of int | float
aiter/aiter/rotary_embedding.py
Line 1187 in 464dd84
Use float instead of int | float
aiter/aiter/rotary_embedding.py
Line 1542 in 464dd84
Import block is un-sorted or un-formatted
Lines 18 to 28 in 464dd84
Do not catch blind exception: Exception
aiter/aiter/utility/base_tuner.py
Line 1488 in 464dd84
Do not catch blind exception: Exception
aiter/aiter/utility/mp_tuner.py
Line 128 in 464dd84
try-except-pass detected, consider logging the exception
aiter/aiter/utility/pretune.py
Lines 86 to 87 in 464dd84
Do not catch blind exception: Exception
aiter/aiter/utility/pretune.py
Line 86 in 464dd84
Import block is un-sorted or un-formatted
aiter/csrc/ck_batched_gemm_a8w8/gen_instances.py
Lines 20 to 26 in 464dd84
Import block is un-sorted or un-formatted
aiter/csrc/ck_batched_gemm_bf16/gen_instances.py
Lines 20 to 26 in 464dd84
Import block is un-sorted or un-formatted
aiter/csrc/ck_deepgemm/gen_instances.py
Lines 19 to 25 in 464dd84
Import block is un-sorted or un-formatted
aiter/csrc/ck_gemm_a4w4_blockscale/gen_instances.py
Lines 20 to 31 in 464dd84
Import block is un-sorted or un-formatted
aiter/csrc/ck_gemm_a8w8/gen_instances.py
Lines 20 to 26 in 464dd84
Import block is un-sorted or un-formatted
aiter/csrc/ck_gemm_a8w8_blockscale/gen_instances.py
Lines 20 to 31 in 464dd84
Import block is un-sorted or un-formatted
aiter/csrc/ck_gemm_a8w8_blockscale/gen_instances_cktile.py
Lines 20 to 31 in 464dd84
Import block is un-sorted or un-formatted
aiter/csrc/ck_gemm_a8w8_blockscale_bpreshuffle/gen_instances.py
Lines 20 to 31 in 464dd84
Import block is un-sorted or un-formatted
aiter/csrc/ck_gemm_a8w8_bpreshuffle/gen_instances.py
Lines 20 to 26 in 464dd84
Import block is un-sorted or un-formatted
aiter/csrc/cktile_gemm_a8w8_bpreshuffle/gen_instances.py
Lines 19 to 26 in 464dd84
Unnecessary map() usage (rewrite using a list comprehension)
Unnecessary map() usage (rewrite using a list comprehension)
Prefer TypeError exception for invalid type
Line 344 in 464dd84
Use key in dict instead of key in dict.keys()
Use key in dict instead of key in dict.keys()
Import block is un-sorted or un-formatted
aiter/op_tests/cpp/mha/compile.py
Line 11 in 464dd84
Import block is un-sorted or un-formatted
aiter/op_tests/flydsl_tests/test_flydsl_fmha.py
Lines 6 to 11 in 464dd84
Import block is un-sorted or un-formatted
Do not catch blind exception: Exception
Do not catch blind exception: Exception
Import alias does not rename original package
Import alias does not rename original package
Unpacked variable kv_block_indices is never used
Unpacked variable lut_start is never used
Self-assignment of variable scale
Remove unnecessary True if ... else False
Shebang is present but file is not executable
Import block is un-sorted or un-formatted
warning() call on root logger
Do not perform function call Range in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
Invalid module name: 'test_aiter_addInp'
aiter/op_tests/test_aiter_addInp.py
Line 1 in 464dd84
Import block is un-sorted or un-formatted
aiter/op_tests/test_gemm_codegen.py
Lines 38 to 44 in 464dd84
Invalid module name: 'test_layernorm2dFusedAddQuant'
PEP 484 prohibits implicit Optional
aiter/op_tests/test_mla_v40_persistent.py
Line 337 in 464dd84
The file is executable but no shebang is present
Line 1 in 464dd84
Invalid module name: 'test_moeTopkSoftmax'
aiter/op_tests/test_moeTopkSoftmax.py
Line 1 in 464dd84
Import block is un-sorted or un-formatted
aiter/op_tests/test_opus_a16w16_gemm.py
Lines 10 to 15 in 464dd84
Import block is un-sorted or un-formatted
aiter/op_tests/test_opus_a16w16_gemm.py
Lines 24 to 25 in 464dd84
Import block is un-sorted or un-formatted
aiter/op_tests/test_pretune.py
Lines 25 to 31 in 464dd84
Unpacked variable d is never used
aiter/op_tests/test_rmsnorm2d.py
Line 91 in 464dd84
Unpacked variable res_d is never used
aiter/op_tests/test_rmsnorm2d.py
Line 91 in 464dd84
Unpacked variable avg_d is never used
aiter/op_tests/test_rmsnorm2d.py
Line 91 in 464dd84
Invalid module name: 'test_rmsnorm2dFusedAddQuant'
Import block is un-sorted or un-formatted
Lines 4 to 10 in 464dd84
Unpacked variable avg_a is never used
aiter/op_tests/test_smoothquant.py
Line 137 in 464dd84
Useless return statement at end of function
Do not use mutable data structures for argument defaults
Use float instead of int | float
CI's reviewdog runs with filter-mode=diff_context, so touching 756 files dragged ~117 findings that were already in the tree into review scope. None of them are introduced by this PR; they are simply adjacent to lines it changed. Clearing them is what it takes to get the PR green. Autofixed: PYI041, RUF022, RUF013, RUF059, SIM114, SIM118, SIM210, C417, FURB136, PLR1711, PLC0206, PLW1508, LOG015, I001. Real fixes: - cuda_wrapper.py: two mutable class attributes -> ClassVar (RUF012). - quick_all_reduce.py: dropped `from aiter import logger`, immediately shadowed by the module's own getLogger (F811). - rotary_embedding.py: os.environ.get(..., 0) -> "0" -- the int default was never reachable but is the wrong type (PLW1508). - base_tuner.py, test_pa_decode_gluon.py, bench_fav3_sage.py: unused unpacked values, a mutable default argument, and a dead self-assignment. - flydsl_dispatch_combine: bind expert_rsrcs/expert_vlds as closure defaults (B023), same pattern as the other FlyDSL kernels. - 13 files: cleared a stray executable bit; one file got the bit it needed. - 4 op_tests modules keep their camelCase names with a file-level noqa; renaming them would break the test-selection lists that reference those paths. `# noqa` with a stated reason where the finding is a deliberate choice: intentional blanket catches, framework defaults evaluated at import, PEP 484 redundant-alias re-exports, and a best-effort probe. Third instance of the same latent bug: RUF013 rewrote `x: callable = None` into `callable | None`, and `callable` is the builtin function, not a type -- so `import aiter` died again, exactly like Optional[torch.tensor] did in batch 2. Fixed in all 6 files to collections.abc.Callable. The annotation was always wrong; making unions real is what turns it into a hard failure. Verified: diff-context findings 117 -> 0 (measured locally against merge-base, since `gh pr diff` truncates a PR this size), import sweep back to the 249 ok / 1 pre-existing fail baseline, black clean, all touched files compile.
2076 -> ~830 findings. Everything ruff can rewrite mechanically, plus the executable-bit and shebang findings: - EXE001/EXE002: 34 files got the executable bit their shebang asks for, 15 lost a bit they had no shebang for. - EXE005: pa_mqa_logits.py's 66 `#!=----` separators are decorative comments, not shebangs -- file-level noqa rather than rewriting them all. hipify_python.py had a real shebang below the licence header; moved to line 1. - RUF013's 38 sites were pre-checked for the trap that bit batch 2 twice: every annotation involved is a genuine builtin type (int/float/str), no factory functions, so making the Optionals explicit is safe. One autofix had to be reverted: TC004 hoisted `from fla import __version__` out of its `if TYPE_CHECKING:` block in gated_delta_rule_utils.py. fla is an optional dependency, so that turned `import aiter.ops.triton.gated_delta_net` into a ModuleNotFoundError. The guard is restored with a noqa explaining why. (The runtime read of __version__ under a TYPE_CHECKING-only import is a pre-existing latent bug in this vendored file; not touched here.) Import sweep back at the 249 ok / 1 pre-existing fail baseline, black clean, all 357 touched files compile.
- LOG015 (145): five files logged through the root logger; each now has a module-level getLogger(__name__) and uses it. Only the logging calls were rewritten, not basicConfig/level constants. - RUF012 (30): mutable class attributes annotated ClassVar. - PLW1508 (57 total): os.environ.get/getenv integer defaults quoted; the value was always passed through int() so behaviour is unchanged. - RUF059 (27): unused unpacked values prefixed with an underscore. - PLW0602 (20): dropped 'global' statements that only ever read the name. - PLW1510 (17): explicit check=False on subprocess.run -- this is the existing default, so no behaviour change. Import sweep still 249 ok / 1 pre-existing fail; black clean; everything compiles.
Conflicts in 4 files: fused_compress_attn.py, quant.py, test_dsv4_rotate_quant.py, test_flydsl_compress_attn.py. Resolved by taking main's version wholesale and re-running the same ruff autofix passes on it, rather than hand-merging. For a repo-wide style branch that is the safer resolution: main's content is authoritative and the style transformation is reproducible, so there is no chance of a hand-merge silently keeping a stale half-transformed hunk. Checked that main's `if TYPE_CHECKING:` block in quant.py survived, given TC004 had to be reverted once already on this branch. Post-merge: ruff total still 566, import sweep 249 ok / 1 pre-existing fail, black clean.
- gated_delta_rule_utils.py: TC004 is reported on the import line, not on the `if TYPE_CHECKING:` header, so the noqa sat one line too high and RUF100 flagged it as unused while TC004 still fired. - bench_attn_models.py: the LOG015 noqa became dead once this file was switched to a module logger in the LOG015 pass. Both were self-inflicted in earlier commits on this branch.
All 186 BLE001 sites across 102 files. Sampling them shows one consistent shape rather than sloppiness, so suppressing beats rewriting: - tuners and benchmark sweeps that must record a failed shape and keep going instead of aborting the whole run - optional fast-path probes (fused kernels, ipex, iris, fla) that fall back to a reference path when the import or the call fails - best-effort cleanup and environment introspection Narrowing these to specific exception types would need to enumerate everything a GPU kernel launch, a JIT build or an optional dependency can raise, which is exactly the open set the blanket catch exists to absorb. Deliberately a bare `# noqa: BLE001` per site rather than a repo-level ignore: the rule keeps applying to new code, and a reviewer sees the suppression at the point of use. The rationale lives here rather than repeated 186 times. One site needed care: black had reflowed `except Exception as e:` in tuned_gemm.py into a parenthesised multi-line form once the comment pushed it past the line limit, which stranded the noqa on the wrong line. Rewritten as a single-line except with the explanation moved above it. Import sweep 249 ok / 1 pre-existing fail, black clean, 102 files compile. Repo total 566 -> 380; the remaining rules are still open for review.
All 106 sites across 30 files, 42 distinct closures (93 in def, 12 in lambda),
fixed properly rather than suppressed: the loop variable is now bound as a
default argument, so each closure captures its own iteration's value.
def _accum_loop(end, U): -> def _accum_loop(end, U, hdim_off=hdim_off):
lambda: bench(x_aiter) -> lambda x_aiter=x_aiter: bench(x_aiter)
This is the right fix here rather than a noqa. Most of these are FlyDSL/Triton
codegen closures that are invoked inside the same iteration, so late binding
happens to give the right answer today -- but nothing enforces that, and the
binding makes the intent explicit. Where a closure genuinely outlives its
iteration, B023 was flagging a real bug and this fixes it.
Verified as signature-only: stripping the newly added `name=name` defaults from
both sides makes the before and after ASTs compare equal across all 30 files,
so no function body changed. The closures have no *args/**kwargs/kw-only
parameters, so appending defaulted parameters cannot shift an existing binding.
Two bugs in my own transformation, caught before anything was written:
- zero-argument lambdas are spelled `lambda:` with no space, so inserting the
binding before the colon produced `lambdax=x:`. The script re-parses every
file it edits and skipped the three affected files instead of corrupting them.
- ruff reports B023 at the *use* site, so the enclosing closure has to be
resolved by innermost-enclosing-node rather than by line proximity.
Import sweep 249 ok / 1 pre-existing fail, black clean, 30 files compile.
Repo total 380 -> 274.
Both sites are the same line in a multi-GPU test:
# warmup and align all gpu
get_tp_group().device_group
device_group is a plain attribute assigned in GroupCoordinator.__init__, not a
lazy property, so the attribute access really is a no-op -- B018 is right about
that half. What the statement still does is call get_tp_group(), which raises if
the TP group was never initialised, so it is not dead code either.
Bound to `_` and documented rather than deleted, which keeps that check and
satisfies the rule. Both files are otherwise ruff-clean now.
There was a problem hiding this comment.
Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit
ruff
try-except-pass detected, consider logging the exception
aiter/op_tests/triton_tests/moe/test_moe_routing.py
Lines 912 to 913 in 64c4228
try-except-pass detected, consider logging the exception
aiter/op_tests/tuning_tests/test_run_config.py
Lines 73 to 74 in 64c4228
try-except-pass detected, consider logging the exception
Lines 88 to 89 in 64c4228
try-except-pass detected, consider logging the exception
Lines 356 to 357 in 64c4228
Real fixes: - TRY002 (33): `raise Exception(...)` -> `raise RuntimeError(...)`. RuntimeError is an Exception subclass, so every existing `except Exception` still catches. - SIM102 (38): nested ifs merged with `and`, which is exactly equivalent when neither branch has an else. 35 done by an AST-driven pass, two more by hand (a three-level nest, and one whose inner condition carried an inline comment). noqa where the finding is deliberate: - S110 (30): try/except/pass around optional imports and best-effort probes, already carrying `# noqa: BLE001` -- S110 merged into the same directive. - B008 (41): framework defaults such as fx.Stream(None) and Range(...) that are meant to be built once at import. - PLR0124 (21): `x == x` is the NaN test in Triton kernels. Rewriting it as ruff suggests would delete the check. - PLW0127 (13): `accumulator = accumulator` keeps a Triton if/elif chain structurally uniform. - TRY004 (12): ValueError is the established contract of these helpers. Three SIM102 sites were left nested on purpose, each with the reason inline. One matters: `# pyre-ignore[61]` sits between the two ifs in hstu_attention.py and applies to the inner one, so merging would have silently dropped a type-checker suppression. Placing noqa needed a convergence loop rather than a single pass: adding one can push a line past the limit, black then splits the statement, and the directive ends up on a line where the rule no longer fires -- RUF100 flags it while the original finding comes back. The loop adds, reformats, strips whatever RUF100 calls unused, and repeats. Import sweep 249 ok / 1 pre-existing fail; black clean; 112 files compile. Repo total 272 -> 84.
stream: fx.Stream = fx.Stream(None) -> stream: fx.Stream All 33 FlyDSL jit launchers. Replaces the noqa these carried a moment ago: the default really was being built once at import and shared, which is what B008 objects to. Checked every call site before removing it, since this makes the parameter required. 17 pass a stream (the `_run_compiled(fn, (..., torch.cuda .current_stream()))` form), and the four that looked like they did not: - two `launch()` calls in aot/flydsl/common.py are a different, local scheduling helper -- name collision, not these functions - flydsl_fp8_mqa_logits takes a plain `stream=None`, a different pattern that was never part of this set - launch_pa_decode_ps_reduce_flydsl in pa_decode_gluon.py genuinely relied on the default. That one call site now passes `stream=fx.Stream(None)` explicitly, which is byte-for-byte the previous behaviour (fx.Stream(None) is the default queue) and moves the construction out of the signature. `stream` is the last parameter in all 33, so removing its default cannot produce a non-default-after-default signature; verified via AST rather than by eye. Import sweep 249 ok / 1 pre-existing fail, black clean, 40 files compile. Caveat: the import sweep proves these modules still load, not that a kernel still launches. The launcher call paths need GPU CI.
F821 (12) -- three different causes, so three different answers: - IrisCommContext was referenced from a string annotation in all_gather.py and reduce_scatter.py but never imported. It does exist, in comms/iris.py, so both files now import it under `if TYPE_CHECKING:`. - test_pa_ragged_experimental.py is a near-copy of csrc/cpp_itfs/pa/ pa_ragged_test.py that lost STR_DTYPE_TO_TORCH_DTYPE along the way. Its get_kv_cache_torch_dtype would NameError on any string cache_dtype. Dict restored from the original. - The rest are unfixable here and got a noqa plus a file-level note: dist/ utils.py is vendored from vLLM and reaches for `envs`, `current_platform` and `enable_trace_function_call` from vllm modules that were never carried over, and mla_extend_ref.py calls vLLM's context_attention_fwd, which was never ported. Adding an import is not an option; these paths simply cannot run. SIM115 (8): - Three real leaks-by-omission converted to `with`: view-screen.py held a json handle across a 50-line loop, test_common.py opened a metafile inside a list comprehension, test_gemm_codegen.py read a file without closing it. - Five are `tempfile.NamedTemporaryFile(delete=False)`, where the path deliberately outlives the handle. noqa'd with that reason. UP031 (8): printf-style formatting to f-strings. Seven are the same "%d / %d mismatched elements" line duplicated across the moe tests. Import sweep 249 ok / 1 pre-existing fail, black clean, 18 files compile. Repo total 55 -> 27.
Four of these were real bugs, not style:
- PLE1205: logger.warning("Custom quick allreduce:", f"Invalid quantization
level: ...") passed the detail as a %-arg to a format string with no
placeholders, so it never reached the log. Joined into one message.
- F601: a8w8_gemm1_kernels_list_gfx950 keyed two entries `4`, so the first was
silently dropped at dict construction and key 3 did not exist. Renumbered the
first to 3. Lookups for 1/2/4/5/6 are unaffected -- 4 already resolved to the
second entry -- so this only restores the missing 3. Worth an author check
that 3 is indeed the intended key.
- RUF034: two `x.stride(0) if not flash_layout else x.stride(0)` in
fused_kv_cache.py, identical on both branches while the neighbouring lines
genuinely permute. Collapsed; flagging in case the layout swap was meant to
differ here too.
- B012: three `return` statements inside `finally` in test_communication.py,
which swallow anything the handler above re-raises. Moved out of the finally.
Behaviour-preserving cleanups: dead `run_aiter_hip` in test_pa_mtp.py (shadowed
by a later definition, so the removed one never ran) and its now-unused import;
the unused `remap_xcd` import in lean_atten.py that a local @triton.jit
definition overrode; SIM103/SIM113/PERF402/PLC0206/E722/DTZ005/UP031 rewrites.
Two renames needed scope checks before I trusted them. get_args_of_build's loop
rebinds its own `ops_name` parameter, and `ops_name` is read again at three
later lines -- but those sit in elif/else branches that the loop's branch
returns before reaching, so the shadowing is unreachable and the rename is
cosmetic. test_la_paged's `for h in range(h)` shadows the head-count argument;
every `h` in the body is therefore the index, so renaming the loop and its body
to `hi` is exact.
noqa with a reason for the two that should stay: functools.lru_cache on a
method of a process-lifetime singleton (B019), and the exec() the JIT loader
needs (S102).
`ruff check .` now passes on the whole repository. black clean, import sweep
249 ok / 1 pre-existing fail, all touched files compile.
CI caught this: `python setup.py` died with
File "aiter/jit/core.py", line 1145, in convert
val[idx] = eval(el)
NameError: name 'torch' is not defined
get_args_of_build eval()s strings out of optCompilerConfig.json, some of which
reference torch. The original code bound the name right before the eval:
if "torch" in el:
import torch as torch
val[idx] = eval(el)
The `as torch` spelling was almost certainly there to mark the import as
deliberate. F401 does not read eval() strings, saw an unused import, deleted it,
and left `pass` behind -- so every build that hit a torch-bearing config entry
crashed. Restored as a plain `import torch` with a noqa and a comment saying
what the binding is for.
Swept for the same failure mode across the branch: compared the set of imported
names before and after per file, kept only names that disappeared entirely (a
line that merely lost a `# noqa` is not a removal), intersected with files that
actually call eval/exec, and checked whether the name still appears in a string
literal. One candidate came back, `List` in core.py, and it is a false positive
-- the exec() namespace at line 1630 maps "List" explicitly, it does not read
the module scope. This was the only real one.
Verified by reproducing the CI command: `python setup.py --help` now completes,
and get_args_of_build("all") returns its 106 srcs. ruff and black still clean.
…orting
gen_instances.py had all its imports at the top of the file and relied on
an import-time side effect of a sibling: moe_cktile2stages_common does a
sys.path.insert() for aiter/jit/utils, and that is what made `chip_info`
resolvable. isort sorted the two local imports alphabetically, putting
chip_info first, so the blob_gen_cmd subprocess died with:
File "csrc/ck_tile_gemm_moe_2stages/gen_instances.py", line 11
from chip_info import get_gfx, get_gfx_list
ModuleNotFoundError: No module named 'chip_info'
The file was the odd one out. Every other chip_info consumer in csrc/
(10 of them) puts the sys.path.insert() above its local imports, which
both makes them self-sufficient and gives isort a barrier it will not
sort across. Give this file the same shape instead of fencing it with
`# isort: off`: the hidden cross-file coupling goes away, the import
block is free to be sorted, and the file now matches its siblings.
Its own sys.path.insert() -- previously sitting below the imports, where
it did nothing for them -- simply moves up.
Verified: the real blob_gen_cmd (`gen_instances.py --working_path ...`)
exits 0 and emits 224 files, byte-identical to the pre-regression output.
Swept for the same class repo-wide:
- top-level import order before/after across all 821 changed .py files
vs every module that mutates sys.path at import time (20): 1 hit,
this file.
- all 17 blob_gen_cmd codegen scripts from optCompilerConfig.json run
end to end: all pass (hsa/codegen.py for each of -m pa/mla/mla_v4/
i8gemm/bf16gemm, once AITER_GPU_ARCHS is set).
- the 300 reordered scripts outside aiter/, differentially probed
old-vs-new: 264 clean, 36 failures that reproduce identically on the
base revision, 0 regressions.
- repo-wide scan for the fragile shape itself (an injector imported
inside an unbarriered leading import block): 2 files, this one and
ck_gemm_moe_2stages_codegen/gen_instances.py, whose only other
imports are stdlib and is therefore unaffected.
check_args() rewrites a pybind op's __doc__ into a `def` and execs it to
recover the signature. The rewrite emits List[, Tuple[ and Optional[, but
only List and Optional were bound in the exec namespace -- Tuple happened
to be supplied by the `from aiter import *` on the first line of the
exec'd source, because some submodule under aiter did `from typing import
Tuple` at module scope and star-import leaked it into the aiter namespace.
UP035 rewrote every `from typing import Tuple` in the repo, so the leak
stopped and the first call to any op with a tuple in its signature died:
File "aiter/jit/core.py", line 1637, in check_args
exec( # noqa: S102
File "<string>", line 2, in <module>
NameError: name 'Tuple' is not defined
Bind "Tuple": tuple next to "List": list. Star-import was never a sound
source for these; the comment now says so.
Verified: `Tuple in dir(aiter)` is True on the base revision and False at
HEAD; replaying the exec with the old namespace reproduces the CI
NameError verbatim and with the new one it resolves.
Swept the whole class rather than this one name: dumped the star-import
surface of all 322 importable aiter modules on both revisions and diffed
them (aiter_export_surface_diff.py).
- top-level `aiter` lost exactly 3 names -- List, Optional, Tuple. The
first two were already bound explicitly, so Tuple was the only gap.
Nothing was gained, and no module became unimportable.
- across all modules, 210 names lost, 203 of them typing re-exports.
The other 7 (logging, get_hip_arch, torch, lru_cache, tl, triton)
belong to modules nothing star-imports; aiter/__init__.py is the only
star-importer in the repo and its result is the surface already
diffed above.
- enumerated every eval/exec/compile site in the repo and intersected
with the per-module lost names: only core.py (lost List) and
tuned_gemm.py (lost Optional) qualify. core.py's evals consume
optCompilerConfig.json, where "List" appears 0 times, and its exec
binds List explicitly; tuned_gemm.py's evals consume dtype strings
from configs/*.csv, where "Optional" appears 0 times. Both clear.
Conflicts came from #4230 (FLYDSL paged mqa logits fp4), which touched five files this branch had also reformatted. Resolved by taking main's content verbatim for every conflicted file and re-running the lint pass over it, rather than hand-merging formatting against functional changes: git checkout --theirs <files> && ruff check --fix && black Only mechanical rules applied -- UP045 (5), I001 (2), RUF022 (1), all safe fixes, no --unsafe-fixes needed. `git diff origin/main` on those five files now shows lint changes and nothing else, which is the proof that main's functional content survived intact.
Standard Tests shard 1 died with:
File "op_tests/test_pa_mtp.py", line 272, in run_aiter_hip
return aiter.paged_attn.PagedAttention.forward_decode(
AttributeError: module 'aiter' has no attribute 'paged_attn'
Two rules interacted. The file used to define run_aiter_hip twice; the
second shadowed the first, so the first was dead code and F811 removed
it. The dead copy was the only user of the `ops` alias from
`from aiter import dtypes, paged_attn as ops`, so F401 then removed that
import as unused -- correct in isolation, except the import was also
what bound `paged_attn` as an attribute of the aiter package, which is
how the surviving copy reached it. Neither rule can see that side effect.
Import paged_attn under its own name and call it directly instead of
going through the package attribute, so the dependency is visible to
static analysis and needs no noqa.
Verified: `aiter.paged_attn` does not resolve after a plain `import
aiter`, which is exactly why the attribute access failed; running
op_tests/test_pa_mtp.py now executes into aiter/paged_attn.py:272 and
only stops later at a JIT compile step, because git worktree does not
populate 3rdparty/composable_kernel.
Swept both rules across the branch:
- every import removed from the 821 changed files, checked for one
whose parent package is still reached by attribute access: this file
was the only true hit. test_sample.py looked similar but
aiter.greedy_sample / aiter.random_sample resolve on a plain import
anyway.
- widened to every `aiter.X` access in the changed files, resolved
against a real import: 11 do not resolve, and all 11 fail the same
way on the base revision, so none of them are new.
- definitions removed anywhere in the branch: exactly one, the
shadowed run_aiter_hip above.
The other failure in that shard, test_quant_mxfp4.py
test_gui_shuffle_scale_w2_k_pad inter_tp=640, is not ours: the same job
on main at ad2f051 fails on the same assertion at the same line.
test_gui_shuffle_scale_w2_k_pad has been failing on main at inter_tp=640:
File "op_tests/test_quant_mxfp4.py", line 334
assert torch.equal(
AssertionError: inter_tp=640
The kernel is fine; the reference model in the test is wrong. Both sides
of the assert call the same shuffle_scale -- the only difference is who
pads k_groups up to a multiple of 8. shuffle_scale does it internally
with 0x7F for 1-byte scales (aiter/ops/shuffle.py:372), which in e8m0 is
the exponent bias, i.e. 2^0. The test pre-padded with torch.zeros, and
0x00 in e8m0 is 2^-127, so the two layouts genuinely differ.
inter_tp=256 gives k_groups=8, already a multiple of 8, so no padding
happens and the fill value never matters -- which is why only the 640
case failed.
Pad the reference with the neutral scale instead, mirroring what the
implementation guarantees: k-groups added by padding describe weights
that do not exist, so their scale has to be 1.0.
Verified on gfx950: with torch.zeros the assert fails for both float16
and bfloat16 at inter_tp=640 and passes at 256; with the neutral fill
all four combinations are byte-identical. `python3
op_tests/test_quant_mxfp4.py --a16w4_shuffle` goes from exit 1 to exit
0 on the same tree, and the full file passes.
Not a regression -- the test was written this way. The internal padding
and its 0x7F fill both landed in c2b69b9 (#4130, 2026-07-09);
test_gui_shuffle_scale_w2_k_pad was added later in 3135022 (#4397,
2026-07-28) and did not mirror the fill value.
Swept for the same assumption elsewhere: no other test hand-pads a scale
tensor before calling shuffle_scale. Every other caller passes unpadded
production data and relies on the internal padding, which is correct.
… sweep
Running op_tests/test_moeTopkSoftmax.py with its default token list dies
immediately:
File "op_tests/test_moeTopkSoftmax.py", line 708, in <module>
assert not gating_output.is_contiguous()
AssertionError
The block builds a row-strided view of a fused router buffer:
backing = torch.randn((token, 6016), dtype=bf16)
gating_output = backing[:, 1536:1536 + 896]
and the default token list starts at 1. At token=1 the view is
(1, 896) with stride (6016, 1), and a row stride you can never take is
not a stride at all -- torch reports it as contiguous. The assert is
false for the first case it ever sees.
`assert gating_output.stride(0) == fused_front_width` is the check that
actually pins the layout under test, and it holds for every token. Keep
it and drop the other one.
Verified on gfx950: `-t 1` goes from exit 1 to exit 0, and the full
default sweep now completes with exit 0. Max err_aiter per table --
biased_grouped_topk 1.5e-05, kimi_k25 1.3e-05, kimi_k3_strided 3.7e-04,
grouped_topk 0.125 -- all as before, no AssertionError anywhere.
Not a regression: this branch never touched the block (git diff against
origin/main reports no change to it), and `-t 1` fails identically on
origin/main. It arrived with 3135022 (#4397), the same PR as the mxfp4
scale-pad reference bug fixed in the previous commit.
Swept the other `assert not X.is_contiguous()` sites -- 9 of them, in
test_fused_qk_norm_rope_cache_quant.py, triton_tests/attention/
test_mha.py and test_mha_v3.py. None of them slices a dimension that can
collapse to 1 the way this one does.
Six tracked files that nothing reads and nothing will read again, plus the workflow fix that stops two of them returning. CI scratch the bot committed. update-split-tests.yaml wrote split-tests-update-summary.md and then pr-body.md into the workspace and handed them to peter-evans/create-pull-request, which commits every new and modified file when add-paths is unset. Both reached the repo root that way in #5131, and the tracked copies were stale the moment they landed because the workflow regenerates them on every run. The job only ever means to change one file, so add-paths now names .github/scripts/split_tests.sh, and both scratch files move to the runner temp directory so they are never dirty workspace files in the first place. The .gitignore entries remain as a backstop for local runs, where --summary-path still defaults to the working directory. Merge and POC leftovers that ship in the wheel. hsa/gfx950/pa/pa_bf16_noquant_gqa8_1tg_4w.co.orig and .co.poc_kl_merg arrived with #3211 next to the real .co. Nothing references either by name, and the ASM loaders build .co paths explicitly rather than globbing the directory, so neither is reachable. They are not merely inert: setup.py copies all of hsa/ into aiter_meta and MANIFEST.in grafts it, so 48 KB of scratch binaries go out in every wheel. *.orig is now ignored so the next conflicted merge does not leave another one behind. One-off files their PRs forgot. probe_noqa_check.py is a single line, x = 1, added in #4403 to confirm the pinned ruff config flagged it. REPO_CLEANUP_PLAN.md is a pull request description from #2167 in March. Its Phase 1 shipped -- the .pt test data is gone from main and the size rules are in .gitignore -- and its Phase 2 history rewrite needs a coordinated force push that has not happened in the six months since. Neither file is referenced anywhere in the tree. Left alone: scripts/download_test_data.sh, a no-op stub from that same plan, because docs/tutorials/index.rst still points readers at it. Retiring the script and the doc together is a separate change.
Six tracked files that nothing reads and nothing will read again, plus the workflow fix that stops two of them returning. CI scratch the bot committed. update-split-tests.yaml wrote split-tests-update-summary.md and then pr-body.md into the workspace and handed them to peter-evans/create-pull-request, which commits every new and modified file when add-paths is unset. Both reached the repo root that way in #5131, and the tracked copies were stale the moment they landed because the workflow regenerates them on every run. The job only ever means to change one file, so add-paths now names .github/scripts/split_tests.sh, and both scratch files move to the runner temp directory so they are never dirty workspace files in the first place. The .gitignore entries remain as a backstop for local runs, where --summary-path still defaults to the working directory. Merge and POC leftovers that ship in the wheel. hsa/gfx950/pa/pa_bf16_noquant_gqa8_1tg_4w.co.orig and .co.poc_kl_merg arrived with #3211 next to the real .co. Nothing references either by name, and the ASM loaders build .co paths explicitly rather than globbing the directory, so neither is reachable. They are not merely inert: setup.py copies all of hsa/ into aiter_meta and MANIFEST.in grafts it, so 48 KB of scratch binaries go out in every wheel. *.orig is now ignored so the next conflicted merge does not leave another one behind. One-off files their PRs forgot. probe_noqa_check.py is a single line, x = 1, added in #4403 to confirm the pinned ruff config flagged it. REPO_CLEANUP_PLAN.md is a pull request description from #2167 in March. Its Phase 1 shipped -- the .pt test data is gone from main and the size rules are in .gitignore -- and its Phase 2 history rewrite needs a coordinated force push that has not happened in the six months since. Neither file is referenced anywhere in the tree. Left alone: scripts/download_test_data.sh, a no-op stub from that same plan, because docs/tutorials/index.rst still points readers at it. Retiring the script and the doc together is a separate change.
1 participant
Why
The style job runs
pip3 install ruffunpinned against a repo with no ruff config, so the enforced rule set is whatever the latest release happens to default to. ruff 0.16.0 widened those defaults well pastE4/E7/E9/F(413 rules across 40 prefixes), which turned PRs red for reasons that had nothing to do with their contents — #4397 hit this and needed two rounds of unrelated lint fixes to land.This PR pins the inputs. It changes no verdict on existing code.
What
ruff==0.16.0inpre-checks.yaml[tool.ruff]withtarget-version = "py310"[project]table exists, so ruff could not readrequires-pythonand fell back to its own default.py310is what it already resolved to →UP0xxverdicts unchangeddist/instead of the built-in baredistinexcludeaiter/dist/— 15 real source modules that had never been linted. The setuptools output dir stays excluded3rdparty/,aiter/jit/build/,aiter_logs/python_requires>=3.8→>=3.10X | None) withoutfrom __future__ import annotationsand raiseTypeErrorat import on <3.10bench_deepgemm_attention.py:401target-versionThere is deliberately no
select/ignoreoverride: the rule set stays exactly ruff's default for the pinned version, so nothing is relaxed for new code.Verification
Diffed the full
ruff check .output againstmain:aiter/dist/→ no verdict changedaiter/dist/invalid-syntaxunderpy310: 0 (was 2, both the f-string above)black --checkclean;pyproject.toml/pre-checks.yamlparseFollow-up
This leaves a backlog of ~6.7k pre-existing findings (
ruff check .is not green). CI is diff-filtered so that does not block anything, but it does mean a PR touching legacy code can be blocked by its neighbours. Plan is to burn it down per-module in follow-up PRs — ~4.5k are safe-autofixable, the rest (RUF059 426, BLE001 199, LOG015 146, C417 136, B023 107, …) need review. Not folded in here: a 754-file autofix diff is not reviewable, and much of it is FlyDSL/Triton kernel code.